Repository: hadoop Updated Branches: refs/heads/trunk b7923a356 -> c65f1b382
YARN-2762. Fixed RMAdminCLI to trim and check node-label related arguments before sending to RM. Contributed by Rohith Sharmaks Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/c65f1b38 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/c65f1b38 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/c65f1b38 Branch: refs/heads/trunk Commit: c65f1b382ec5ec93dccf459dbf8b2c93c3e150ab Parents: b7923a3 Author: Jian He <[email protected]> Authored: Tue Dec 16 11:00:25 2014 -0800 Committer: Jian He <[email protected]> Committed: Tue Dec 16 11:00:25 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/c65f1b38/hadoop-yarn-project/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index e235f78..37147d7 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -133,6 +133,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/c65f1b38/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/c65f1b38/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
