YARN-4465. SchedulerUtils#validateRequest for Label check should happen only when nodelabel enabled. (Bibin A Chundatt via wangda)

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerUtils.java

@@ -24,6 +24,7 @@ import java.util.Set;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceStability.Unstable;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.yarn.api.records.ContainerExitStatus;
 import org.apache.hadoop.yarn.api.records.ContainerId;
 import org.apache.hadoop.yarn.api.records.ContainerState;
@@ -32,6 +33,7 @@ import org.apache.hadoop.yarn.api.records.QueueACL;
 import org.apache.hadoop.yarn.api.records.QueueInfo;
 import org.apache.hadoop.yarn.api.records.Resource;
 import org.apache.hadoop.yarn.api.records.ResourceRequest;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.exceptions.InvalidLabelResourceRequestException;
 import org.apache.hadoop.yarn.exceptions.InvalidResourceRequestException;
 import org.apache.hadoop.yarn.factories.RecordFactory;
@@ -222,6 +224,17 @@ public class SchedulerUtils {
       Resource maximumResource, String queueName, YarnScheduler scheduler,
       boolean isRecovery, RMContext rmContext, QueueInfo queueInfo)
       throws InvalidResourceRequestException {
+    Configuration conf = rmContext.getYarnConfiguration();
+    // If Node label is not enabled throw exception
+    if (null != conf && !YarnConfiguration.areNodeLabelsEnabled(conf)) {
+      String labelExp = resReq.getNodeLabelExpression();
+      if (!(RMNodeLabelsManager.NO_LABEL.equals(labelExp)
+          || null == labelExp)) {
+        throw new InvalidLabelResourceRequestException(
+            "Invalid resource request, node label not enabled "
+                + "but request contains label expression");
+      }
+    }
     if (null == queueInfo) {
       try {
         queueInfo = scheduler.getQueueInfo(queueName, false, false);
@@ -283,8 +296,8 @@ public class SchedulerUtils {
     // we don't allow specify label expression other than resourceName=ANY now
     if (!ResourceRequest.ANY.equals(resReq.getResourceName())
         && labelExp != null && !labelExp.trim().isEmpty()) {
-      throw new InvalidResourceRequestException(
+      throw new InvalidLabelResourceRequestException(
-          "Invailid resource request, queue=" + queueInfo.getQueueName()
+          "Invalid resource request, queue=" + queueInfo.getQueueName()
               + " specified node label expression in a "
               + "resource request has resource name = "
               + resReq.getResourceName());
@@ -303,15 +316,28 @@ public class SchedulerUtils {
       if (!checkQueueLabelExpression(queueInfo.getAccessibleNodeLabels(),
           labelExp, rmContext)) {
         throw new InvalidLabelResourceRequestException(
-            "Invalid resource request"
+            "Invalid resource request" + ", queue=" + queueInfo.getQueueName()
-            + ", queue="
+                + " doesn't have permission to access all labels "
-            + queueInfo.getQueueName()
+                + "in resource request. labelExpression of resource request="
-            + " doesn't have permission to access all labels "
+                + labelExp + ". Queue labels="
-            + "in resource request. labelExpression of resource request="
+                + (queueInfo.getAccessibleNodeLabels() == null ? ""
-            + labelExp
+                    : StringUtils.join(
-            + ". Queue labels="
+                        queueInfo.getAccessibleNodeLabels().iterator(), ',')));
-            + (queueInfo.getAccessibleNodeLabels() == null ? "" : StringUtils.join(queueInfo
+      } else {
-                .getAccessibleNodeLabels().iterator(), ',')));
+        checkQueueLabelInLabelManager(labelExp, rmContext);
+      }
+    }
+  }
+
+  private static void checkQueueLabelInLabelManager(String labelExpression,
+      RMContext rmContext) throws InvalidLabelResourceRequestException {
+    // check node label manager contains this label
+    if (null != rmContext) {
+      RMNodeLabelsManager nlm = rmContext.getNodeLabelManager();
+      if (nlm != null && !nlm.containsNodeLabel(labelExpression)) {
+        throw new InvalidLabelResourceRequestException(
+            "Invalid label resource request, cluster do not contain "
+                + ", label= " + labelExpression);
       }
     }
   }
@@ -338,14 +364,6 @@ public class SchedulerUtils {
             return false;
           }
         }
-        
-        // check node label manager contains this label
-        if (null != rmContext) {
-          RMNodeLabelsManager nlm = rmContext.getNodeLabelManager();
-          if (nlm != null && !nlm.containsNodeLabel(str)) {
-            return false;
-          }
-        }
       }
     }
     return true;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/TestSchedulerUtils.java

@@ -95,7 +95,8 @@ public class TestSchedulerUtils {
   private static final Log LOG = LogFactory.getLog(TestSchedulerUtils.class);
   
   private RMContext rmContext = getMockRMContext();
-  
+  private static YarnConfiguration conf = new YarnConfiguration();
+
   @Test (timeout = 30000)
   public void testNormalizeRequest() {
     ResourceCalculator resourceCalculator = new DefaultResourceCalculator();
@@ -464,6 +465,34 @@ public class TestSchedulerUtils {
       rmContext.getNodeLabelManager().removeFromClusterNodeLabels(
           Arrays.asList("x"));
     }
+    try {
+      Resource resource = Resources.createResource(0,
+          YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_VCORES);
+      ResourceRequest resReq1 = BuilderUtils
+          .newResourceRequest(mock(Priority.class), "*", resource, 1, "x");
+      SchedulerUtils.normalizeAndvalidateRequest(resReq1, maxResource, "queue",
+          scheduler, rmContext);
+      fail("Should fail");
+    } catch (InvalidResourceRequestException e) {
+      assertEquals("Invalid label resource request, cluster do not contain , "
+          + "label= x", e.getMessage());
+    }
+
+    try {
+      rmContext.getYarnConfiguration()
+          .set(YarnConfiguration.NODE_LABELS_ENABLED, "false");
+      Resource resource = Resources.createResource(0,
+          YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_VCORES);
+      ResourceRequest resReq1 = BuilderUtils
+          .newResourceRequest(mock(Priority.class), "*", resource, 1, "x");
+      SchedulerUtils.normalizeAndvalidateRequest(resReq1, maxResource, "queue",
+          scheduler, rmContext);
+      Assert.assertEquals(RMNodeLabelsManager.NO_LABEL,
+          resReq1.getNodeLabelExpression());
+    } catch (InvalidResourceRequestException e) {
+      assertEquals("Invalid resource request, node label not enabled but "
+          + "request contains label expression", e.getMessage());
+    }
   }
 
   @Test (timeout = 30000)
@@ -773,6 +802,9 @@ public class TestSchedulerUtils {
     RMContext rmContext = mock(RMContext.class);
     RMNodeLabelsManager nlm = new NullRMNodeLabelsManager();
     nlm.init(new Configuration(false));
+    when(rmContext.getYarnConfiguration()).thenReturn(conf);
+    rmContext.getYarnConfiguration().set(YarnConfiguration.NODE_LABELS_ENABLED,
+        "true");
     when(rmContext.getNodeLabelManager()).thenReturn(nlm);
     return rmContext;
   }