You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ji...@apache.org on 2014/12/16 20:01:31 UTC
hadoop git commit: YARN-2762. Fixed RMAdminCLI to trim and check
node-label related arguments before sending to RM. Contributed by Rohith
Sharmaks (cherry picked from commit c65f1b382ec5ec93dccf459dbf8b2c93c3e150ab)
Repository: hadoop
Updated Branches:
refs/heads/branch-2 b70c0c295 -> 878350a0e
YARN-2762. Fixed RMAdminCLI to trim and check node-label related arguments before sending to RM. Contributed by Rohith Sharmaks
(cherry picked from commit c65f1b382ec5ec93dccf459dbf8b2c93c3e150ab)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/878350a0
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/878350a0
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/878350a0
Branch: refs/heads/branch-2
Commit: 878350a0e8b1e63a5ab28986208e59daa6444b09
Parents: b70c0c2
Author: Jian He <ji...@apache.org>
Authored: Tue Dec 16 11:00:25 2014 -0800
Committer: Jian He <ji...@apache.org>
Committed: Tue Dec 16 11:01:24 2014 -0800
----------------------------------------------------------------------
hadoop-yarn-project/CHANGES.txt | 3 ++
.../hadoop/yarn/client/cli/RMAdminCLI.java | 38 ++++++++++++--------
.../hadoop/yarn/client/cli/TestRMAdminCLI.java | 36 +++++++++++++++++--
3 files changed, 61 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/878350a0/hadoop-yarn-project/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt
index 77de70a..f719e57 100644
--- a/hadoop-yarn-project/CHANGES.txt
+++ b/hadoop-yarn-project/CHANGES.txt
@@ -101,6 +101,9 @@ Release 2.7.0 - UNRELEASED
YARN-2056. Disable preemption at Queue level (Eric Payne via jlowe)
+ YARN-2762. Fixed RMAdminCLI to trim and check node-label related arguments
+ before sending to RM. (Rohith Sharmaks via jianhe)
+
OPTIMIZATIONS
BUG FIXES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/878350a0/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java
index c7cc4d2..af2321e 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java
@@ -69,6 +69,10 @@ public class RMAdminCLI extends HAAdmin {
RecordFactoryProvider.getRecordFactory(null);
private boolean directlyAccessNodeLabelStore = false;
static CommonNodeLabelsManager localNodeLabelsManager = null;
+ private static final String NO_LABEL_ERR_MSG =
+ "No cluster node-labels are specified";
+ private static final String NO_MAPPING_ERR_MSG =
+ "No node-to-labels mappings are specified";
protected final static Map<String, UsageInfo> ADMIN_USAGE =
ImmutableMap.<String, UsageInfo>builder()
@@ -332,18 +336,24 @@ public class RMAdminCLI extends HAAdmin {
return localNodeLabelsManager;
}
- private int addToClusterNodeLabels(String args) throws IOException,
- YarnException {
+ private Set<String> buildNodeLabelsSetFromStr(String args) {
Set<String> labels = new HashSet<String>();
for (String p : args.split(",")) {
- labels.add(p);
+ if (!p.trim().isEmpty()) {
+ labels.add(p.trim());
+ }
}
- return addToClusterNodeLabels(labels);
+ if (labels.isEmpty()) {
+ throw new IllegalArgumentException(NO_LABEL_ERR_MSG);
+ }
+ return labels;
}
- private int addToClusterNodeLabels(Set<String> labels) throws IOException,
+ private int addToClusterNodeLabels(String args) throws IOException,
YarnException {
+ Set<String> labels = buildNodeLabelsSetFromStr(args);
+
if (directlyAccessNodeLabelStore) {
getNodeLabelManagerInstance(getConf()).addToCluserNodeLabels(labels);
} else {
@@ -358,10 +368,7 @@ public class RMAdminCLI extends HAAdmin {
private int removeFromClusterNodeLabels(String args) throws IOException,
YarnException {
- Set<String> labels = new HashSet<String>();
- for (String p : args.split(",")) {
- labels.add(p);
- }
+ Set<String> labels = buildNodeLabelsSetFromStr(args);
if (directlyAccessNodeLabelStore) {
getNodeLabelManagerInstance(getConf()).removeFromClusterNodeLabels(
@@ -377,7 +384,7 @@ public class RMAdminCLI extends HAAdmin {
return 0;
}
- private Map<NodeId, Set<String>> buildNodeLabelsFromStr(String args)
+ private Map<NodeId, Set<String>> buildNodeLabelsMapFromStr(String args)
throws IOException {
Map<NodeId, Set<String>> map = new HashMap<NodeId, Set<String>>();
@@ -404,12 +411,15 @@ public class RMAdminCLI extends HAAdmin {
}
}
+ if (map.isEmpty()) {
+ throw new IllegalArgumentException(NO_MAPPING_ERR_MSG);
+ }
return map;
}
private int replaceLabelsOnNodes(String args) throws IOException,
YarnException {
- Map<NodeId, Set<String>> map = buildNodeLabelsFromStr(args);
+ Map<NodeId, Set<String>> map = buildNodeLabelsMapFromStr(args);
return replaceLabelsOnNodes(map);
}
@@ -507,21 +517,21 @@ public class RMAdminCLI extends HAAdmin {
exitCode = getGroups(usernames);
} else if ("-addToClusterNodeLabels".equals(cmd)) {
if (i >= args.length) {
- System.err.println("No cluster node-labels are specified");
+ System.err.println(NO_LABEL_ERR_MSG);
exitCode = -1;
} else {
exitCode = addToClusterNodeLabels(args[i]);
}
} else if ("-removeFromClusterNodeLabels".equals(cmd)) {
if (i >= args.length) {
- System.err.println("No cluster node-labels are specified");
+ System.err.println(NO_LABEL_ERR_MSG);
exitCode = -1;
} else {
exitCode = removeFromClusterNodeLabels(args[i]);
}
} else if ("-replaceLabelsOnNode".equals(cmd)) {
if (i >= args.length) {
- System.err.println("No cluster node-labels are specified");
+ System.err.println(NO_MAPPING_ERR_MSG);
exitCode = -1;
} else {
exitCode = replaceLabelsOnNodes(args[i]);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/878350a0/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
index bee114b..69b79da 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
@@ -443,14 +443,31 @@ public class TestRMAdminCLI {
new String[] { "-addToClusterNodeLabels",
"-directlyAccessNodeLabelStore" };
assertTrue(0 != rmAdminCLI.run(args));
+
+ // no labels, should fail at client validation
+ args = new String[] { "-addToClusterNodeLabels", " " };
+ assertTrue(0 != rmAdminCLI.run(args));
+
+ // no labels, should fail at client validation
+ args = new String[] { "-addToClusterNodeLabels", " , " };
+ assertTrue(0 != rmAdminCLI.run(args));
+
+ // successfully add labels
+ args =
+ new String[] { "-addToClusterNodeLabels", ",x,,",
+ "-directlyAccessNodeLabelStore" };
+ assertEquals(0, rmAdminCLI.run(args));
+ assertTrue(dummyNodeLabelsManager.getClusterNodeLabels().containsAll(
+ ImmutableSet.of("x")));
}
@Test
public void testRemoveFromClusterNodeLabels() throws Exception {
// Successfully remove labels
- dummyNodeLabelsManager.addToCluserNodeLabels(ImmutableSet.of("x"));
+ dummyNodeLabelsManager.addToCluserNodeLabels(ImmutableSet.of("x", "y"));
String[] args =
- { "-removeFromClusterNodeLabels", "x", "-directlyAccessNodeLabelStore" };
+ { "-removeFromClusterNodeLabels", "x,,y",
+ "-directlyAccessNodeLabelStore" };
assertEquals(0, rmAdminCLI.run(args));
assertTrue(dummyNodeLabelsManager.getClusterNodeLabels().isEmpty());
@@ -463,6 +480,14 @@ public class TestRMAdminCLI {
new String[] { "-removeFromClusterNodeLabels",
"-directlyAccessNodeLabelStore" };
assertTrue(0 != rmAdminCLI.run(args));
+
+ // no labels, should fail at client validation
+ args = new String[] { "-removeFromClusterNodeLabels", " " };
+ assertTrue(0 != rmAdminCLI.run(args));
+
+ // no labels, should fail at client validation
+ args = new String[] { "-removeFromClusterNodeLabels", ", " };
+ assertTrue(0 != rmAdminCLI.run(args));
}
@Test
@@ -487,6 +512,13 @@ public class TestRMAdminCLI {
new String[] { "-replaceLabelsOnNode",
"-directlyAccessNodeLabelStore" };
assertTrue(0 != rmAdminCLI.run(args));
+
+ // no labels, should fail
+ args = new String[] { "-replaceLabelsOnNode", " " };
+ assertTrue(0 != rmAdminCLI.run(args));
+
+ args = new String[] { "-replaceLabelsOnNode", ", " };
+ assertTrue(0 != rmAdminCLI.run(args));
}
@Test