YARN-3028. Better syntax for replaceLabelsOnNode in RMAdmin CLI. Contributed by Rohith Sharmaks

hadoop-yarn-project/CHANGES.txt

@@ -212,6 +212,9 @@ Release 2.7.0 - UNRELEASED
     YARN-2897. CrossOriginFilter needs more log statements (Mit Desai via
     jeagles)
 
+    YARN-3028. Better syntax for replaceLabelsOnNode in RMAdmin CLI
+    (Rohith Sharmaks via wangda)
+
   OPTIMIZATIONS
 
   BUG FIXES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java

@@ -100,7 +100,8 @@ public class RMAdminCLI extends HAAdmin {
               new UsageInfo("[label1,label2,label3] (label splitted by \",\")",
                   "remove from cluster node labels"))
           .put("-replaceLabelsOnNode",
-              new UsageInfo("[node1:port,label1,label2 node2:port,label1,label2]",
+              new UsageInfo(
+                  "[node1[:port]=label1,label2 node2[:port]=label1,label2]",
                   "replace labels on nodes"))
           .put("-directlyAccessNodeLabelStore",
               new UsageInfo("", "Directly access node label store, "
@@ -199,7 +200,7 @@ private static void printHelp(String cmd, boolean isHAEnabled) {
       " [-getGroup [username]]" +
       " [[-addToClusterNodeLabels [label1,label2,label3]]" +
       " [-removeFromClusterNodeLabels [label1,label2,label3]]" +
-      " [-replaceLabelsOnNode [node1:port,label1,label2 node2:port,label1]" +
+      " [-replaceLabelsOnNode [node1[:port]=label1,label2 node2[:port]=label1]" +
       " [-directlyAccessNodeLabelStore]]");
     if (isHAEnabled) {
       appendHAUsage(summary);
@@ -398,8 +399,18 @@ private Map<NodeId, Set<String>> buildNodeLabelsMapFromStr(String args)
         continue;
       }
 
-      String[] splits = nodeToLabels.split(",");
+      // "," also supported for compatibility
+      String[] splits = nodeToLabels.split("=");
+      int index = 0;
+      if (splits.length != 2) {
+        splits = nodeToLabels.split(",");
+        index = 1;
+      }
+
       String nodeIdStr = splits[0];
+      if (index == 0) {
+        splits = splits[1].split(",");
+      }
 
       if (nodeIdStr.trim().isEmpty()) {
         throw new IOException("node name cannot be empty");
@@ -408,7 +419,7 @@ private Map<NodeId, Set<String>> buildNodeLabelsMapFromStr(String args)
       NodeId nodeId = ConverterUtils.toNodeIdWithDefaultPort(nodeIdStr);
       map.put(nodeId, new HashSet<String>());
 
-      for (int i = 1; i < splits.length; i++) {
+      for (int i = index; i < splits.length; i++) {
         if (!splits[i].trim().isEmpty()) {
           map.get(nodeId).add(splits[i].trim());
         }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java

@@ -73,7 +73,6 @@ public class TestRMAdminCLI {
   @Before
   public void configure() throws IOException, YarnException {
     remoteAdminServiceAccessed = false;
-    dummyNodeLabelsManager = new DummyCommonNodeLabelsManager();
     admin = mock(ResourceManagerAdministrationProtocol.class);
     when(admin.addToClusterNodeLabels(any(AddToClusterNodeLabelsRequest.class)))
         .thenAnswer(new Answer<AddToClusterNodeLabelsResponse>() {
@@ -105,6 +104,7 @@ protected HAServiceTarget resolveTarget(String rmId) {
         return haServiceTarget;
       }
     };
+    initDummyNodeLabelsManager();
     rmAdminCLI.localNodeLabelsManager = dummyNodeLabelsManager;
 
     YarnConfiguration conf = new YarnConfiguration();
@@ -124,6 +124,13 @@ protected HAServiceTarget resolveTarget(String rmId) {
     };
   }
   
+  private void initDummyNodeLabelsManager() {
+    Configuration conf = new YarnConfiguration();
+    conf.setBoolean(YarnConfiguration.NODE_LABELS_ENABLED, true);
+    dummyNodeLabelsManager = new DummyCommonNodeLabelsManager();
+    dummyNodeLabelsManager.init(conf);
+  }
+  
   @Test(timeout=500)
   public void testRefreshQueues() throws Exception {
     String[] args = { "-refreshQueues" };
@@ -281,7 +288,7 @@ public void testHelp() throws Exception {
               "[-refreshAdminAcls] [-refreshServiceAcl] [-getGroup" +
               " [username]] [[-addToClusterNodeLabels [label1,label2,label3]]" +
               " [-removeFromClusterNodeLabels [label1,label2,label3]] [-replaceLabelsOnNode " +
-              "[node1:port,label1,label2 node2:port,label1] [-directlyAccessNodeLabelStore]] " +
+              "[node1[:port]=label1,label2 node2[:port]=label1] [-directlyAccessNodeLabelStore]] " +
               "[-help [cmd]]"));
       assertTrue(dataOut
           .toString()
@@ -361,7 +368,7 @@ public void testHelp() throws Exception {
               "[-refreshAdminAcls] [-refreshServiceAcl] [-getGroup" +
               " [username]] [[-addToClusterNodeLabels [label1,label2,label3]]" +
               " [-removeFromClusterNodeLabels [label1,label2,label3]] [-replaceLabelsOnNode " +
-              "[node1:port,label1,label2 node2:port,label1] [-directlyAccessNodeLabelStore]] " +
+              "[node1[:port]=label1,label2 node2[:port]=label1] [-directlyAccessNodeLabelStore]] " +
               "[-transitionToActive [--forceactive] <serviceId>] " + 
               "[-transitionToStandby <serviceId>] [-failover" +
               " [--forcefence] [--forceactive] <serviceId> <serviceId>] " +
@@ -501,15 +508,21 @@ public void testRemoveFromClusterNodeLabels() throws Exception {
   @Test
   public void testReplaceLabelsOnNode() throws Exception {
     // Successfully replace labels
-    dummyNodeLabelsManager.addToCluserNodeLabels(ImmutableSet.of("x", "Y"));
+    dummyNodeLabelsManager
+        .addToCluserNodeLabels(ImmutableSet.of("x", "y", "Y"));
     String[] args =
-        { "-replaceLabelsOnNode", "node1,x,Y node2,Y",
+        { "-replaceLabelsOnNode",
+            "node1:8000,x,y node2:8000=y node3,x,Y node4=Y",
             "-directlyAccessNodeLabelStore" };
     assertEquals(0, rmAdminCLI.run(args));
     assertTrue(dummyNodeLabelsManager.getNodeLabels().containsKey(
-        NodeId.newInstance("node1", 0)));
+        NodeId.newInstance("node1", 8000)));
     assertTrue(dummyNodeLabelsManager.getNodeLabels().containsKey(
-        NodeId.newInstance("node2", 0)));
+        NodeId.newInstance("node2", 8000)));
+    assertTrue(dummyNodeLabelsManager.getNodeLabels().containsKey(
+        NodeId.newInstance("node3", 0)));
+    assertTrue(dummyNodeLabelsManager.getNodeLabels().containsKey(
+        NodeId.newInstance("node4", 0)));
 
     // no labels, should fail
     args = new String[] { "-replaceLabelsOnNode" };
@@ -517,8 +530,7 @@ public void testReplaceLabelsOnNode() throws Exception {
 
     // no labels, should fail
     args =
-        new String[] { "-replaceLabelsOnNode",
+        new String[] { "-replaceLabelsOnNode", "-directlyAccessNodeLabelStore" };
-            "-directlyAccessNodeLabelStore" };
     assertTrue(0 != rmAdminCLI.run(args));
 
     // no labels, should fail
@@ -529,20 +541,6 @@ public void testReplaceLabelsOnNode() throws Exception {
     assertTrue(0 != rmAdminCLI.run(args));
   }
 
-  @Test
-  public void testReplaceLabelsOnNodeWithPort() throws Exception {
-    // Successfully replace labels
-    dummyNodeLabelsManager.addToCluserNodeLabels(ImmutableSet.of("x", "y"));
-    String[] args =
-      { "-replaceLabelsOnNode", "node1:8000,x,y node2:8000,y",
-      "-directlyAccessNodeLabelStore" };
-    assertEquals(0, rmAdminCLI.run(args));
-    assertTrue(dummyNodeLabelsManager.getNodeLabels().containsKey(
-        NodeId.newInstance("node1", 8000)));
-    assertTrue(dummyNodeLabelsManager.getNodeLabels().containsKey(
-        NodeId.newInstance("node2", 8000)));
-  }
-
   private void testError(String[] args, String template,
       ByteArrayOutputStream data, int resultCode) throws Exception {
     int actualResultCode = rmAdminCLI.run(args);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java

@@ -333,23 +333,32 @@ private void assertNodeLabelsDisabledErrorMessage(IOException e) {
   public void testNodeLabelsDisabled() throws IOException {
     DummyCommonNodeLabelsManager mgr = new DummyCommonNodeLabelsManager();
     Configuration conf = new YarnConfiguration();
-    conf.setBoolean(YarnConfiguration.NODE_LABELS_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.NODE_LABELS_ENABLED, false);
     mgr.init(conf);
     mgr.start();
+    boolean caught = false;
     
     // add labels
     try {
       mgr.addToCluserNodeLabels(ImmutableSet.of("x"));
     } catch (IOException e) {
       assertNodeLabelsDisabledErrorMessage(e);
+      caught = true;
     }
+    // check exception caught
+    Assert.assertTrue(caught);
+    caught = false;
     
     // remove labels
     try {
       mgr.removeFromClusterNodeLabels(ImmutableSet.of("x"));
     } catch (IOException e) {
       assertNodeLabelsDisabledErrorMessage(e);
+      caught = true;
     }
+    // check exception caught
+    Assert.assertTrue(caught);
+    caught = false;
     
     // add labels to node
     try {
@@ -357,7 +366,11 @@ public void testNodeLabelsDisabled() throws IOException {
           CommonNodeLabelsManager.EMPTY_STRING_SET));
     } catch (IOException e) {
       assertNodeLabelsDisabledErrorMessage(e);
+      caught = true;
     }
+    // check exception caught
+    Assert.assertTrue(caught);
+    caught = false;
     
     // remove labels from node
     try {
@@ -365,7 +378,11 @@ public void testNodeLabelsDisabled() throws IOException {
           CommonNodeLabelsManager.EMPTY_STRING_SET));
     } catch (IOException e) {
       assertNodeLabelsDisabledErrorMessage(e);
+      caught = true;
     }
+    // check exception caught
+    Assert.assertTrue(caught);
+    caught = false;
     
     // replace labels on node
     try {
@@ -373,7 +390,11 @@ public void testNodeLabelsDisabled() throws IOException {
           CommonNodeLabelsManager.EMPTY_STRING_SET));
     } catch (IOException e) {
       assertNodeLabelsDisabledErrorMessage(e);
+      caught = true;
     }
+    // check exception caught
+    Assert.assertTrue(caught);
+    caught = false;
     
     mgr.close();
   }