YARN-3963. AddNodeLabel on duplicate label addition shows success. (Bibin A Chundatt via wangda)

hadoop-yarn-project/CHANGES.txt

@@ -710,6 +710,9 @@ Release 2.8.0 - UNRELEASED
     YARN-3919. NPEs' while stopping service after exception during
     CommonNodeLabelsManager#start. (varun saxane via rohithsharmaks)
 
+    YARN-3963. AddNodeLabel on duplicate label addition shows success.
+    (Bibin A Chundatt via wangda)
+
 Release 2.7.2 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java

@@ -288,7 +288,8 @@ public class CommonNodeLabelsManager extends AbstractService {
     }
     List<NodeLabel> newLabels = new ArrayList<NodeLabel>();
     normalizeNodeLabels(labels);
-
+    // check any mismatch in exclusivity no mismatch with skip
+    checkExclusivityMatch(labels);
     // do a check before actual adding them, will throw exception if any of them
     // doesn't meet label name requirement
     for (NodeLabel label : labels) {
@@ -931,6 +932,23 @@ public class CommonNodeLabelsManager extends AbstractService {
     }
   }
 
+  private void checkExclusivityMatch(Collection<NodeLabel> labels)
+      throws IOException {
+    ArrayList<NodeLabel> mismatchlabels = new ArrayList<NodeLabel>();
+    for (NodeLabel label : labels) {
+      RMNodeLabel rmNodeLabel = this.labelCollections.get(label.getName());
+      if (rmNodeLabel != null
+          && rmNodeLabel.getIsExclusive() != label.isExclusive()) {
+        mismatchlabels.add(label);
+      }
+    }
+    if (mismatchlabels.size() > 0) {
+      throw new IOException(
+          "Exclusivity cannot be modified for an existing label with : "
+              + StringUtils.join(mismatchlabels.iterator(), ","));
+    }
+  }
+
   protected String normalizeLabel(String label) {
     if (label != null) {
       return label.trim();

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

@@ -70,7 +70,22 @@ public class TestCommonNodeLabelsManager extends NodeLabelTestBase {
 
     Assert.assertTrue(mgr.getClusterNodeLabelNames().containsAll(
         Sets.newHashSet("hello", "world", "hello1", "world1")));
-
+    try {
+      mgr.addToCluserNodeLabels(Arrays.asList(NodeLabel.newInstance("hello1",
+          false)));
+      Assert.fail("IOException not thrown on exclusivity change of labels");
+    } catch (Exception e) {
+      Assert.assertTrue("IOException is expected when exclusivity is modified",
+          e instanceof IOException);
+    }
+    try {
+      mgr.addToCluserNodeLabels(Arrays.asList(NodeLabel.newInstance("hello1",
+          true)));
+    } catch (Exception e) {
+      Assert.assertFalse(
+          "IOException not expected when no change in exclusivity",
+          e instanceof IOException);
+    }
     // try to remove null, empty and non-existed label, should fail
     for (String p : Arrays.asList(null, CommonNodeLabelsManager.NO_LABEL, "xx")) {
       boolean caught = false;