From fd93e5387b554a78413bc0f14b729e58fea604ea Mon Sep 17 00:00:00 2001 From: Wangda Tan Date: Tue, 27 Jan 2015 15:23:45 -0800 Subject: [PATCH] YARN-3028. Better syntax for replaceLabelsOnNode in RMAdmin CLI. Contributed by Rohith Sharmaks --- hadoop-yarn-project/CHANGES.txt | 3 ++ .../hadoop/yarn/client/cli/RMAdminCLI.java | 19 ++++++-- .../yarn/client/cli/TestRMAdminCLI.java | 48 +++++++++---------- .../TestCommonNodeLabelsManager.java | 23 ++++++++- 4 files changed, 63 insertions(+), 30 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 56ffe979ed..2752824736 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/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 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 9ea333cab4..6f1bbd09d8 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 @@ -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> 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> buildNodeLabelsMapFromStr(String args) NodeId nodeId = ConverterUtils.toNodeIdWithDefaultPort(nodeIdStr); map.put(nodeId, new HashSet()); - 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()); } 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 92af27dc69..1dfeac21d4 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 @@ -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() { @@ -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] ] " + "[-transitionToStandby ] [-failover" + " [--forcefence] [--forceactive] ] " + @@ -501,24 +508,29 @@ 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" }; assertTrue(0 != rmAdminCLI.run(args)); - + // no labels, should fail args = - new String[] { "-replaceLabelsOnNode", - "-directlyAccessNodeLabelStore" }; + new String[] { "-replaceLabelsOnNode", "-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); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java index 242f59caf2..0ab1115491 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java +++ b/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(); }