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 89d87cf..53b2854 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 @@ 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 ADMIN_USAGE = ImmutableMap.builder() @@ -336,7 +340,13 @@ private int addToClusterNodeLabels(String args) throws IOException, YarnException { Set labels = new HashSet(); for (String p : args.split(",")) { - labels.add(p); + if (!p.trim().isEmpty()) { + labels.add(p.trim()); + } + } + + if (labels.isEmpty()) { + throw new IllegalArgumentException(NO_LABEL_ERR_MSG); } return addToClusterNodeLabels(labels); @@ -360,7 +370,13 @@ private int removeFromClusterNodeLabels(String args) throws IOException, YarnException { Set labels = new HashSet(); for (String p : args.split(",")) { - labels.add(p); + if (!p.trim().isEmpty()) { + labels.add(p.trim()); + } + } + + if (labels.isEmpty()) { + throw new IllegalArgumentException(NO_LABEL_ERR_MSG); } if (directlyAccessNodeLabelStore) { @@ -404,6 +420,9 @@ private int removeFromClusterNodeLabels(String args) throws IOException, } } + if (map.isEmpty()) { + throw new IllegalArgumentException(NO_MAPPING_ERR_MSG); + } return map; } @@ -507,21 +526,21 @@ public int run(String[] args) throws Exception { 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]); 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 6176a3e..5a89d38 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 void testAddToClusterNodeLabels() throws Exception { 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 void testRemoveFromClusterNodeLabels() throws Exception { 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 void testReplaceLabelsOnNode() throws Exception { 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 -- 1.8.4