YARN-5272. Handle queue names consistently in FairScheduler. (Wilfred Spiegelenburg via rchiang)

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

@@ -53,6 +53,7 @@
 import org.w3c.dom.Text;
 import org.xml.sax.SAXException;
 
+import com.google.common.base.CharMatcher;
 import com.google.common.annotations.VisibleForTesting;
 
 @Public
@@ -445,7 +446,8 @@ private void loadQueue(String parentName, Element element,
       Set<String> reservableQueues,
       Set<String> nonPreemptableQueues)
       throws AllocationConfigurationException {
-    String queueName = element.getAttribute("name").trim();
+    String queueName = CharMatcher.WHITESPACE.trimFrom(
+        element.getAttribute("name"));
 
     if (queueName.contains(".")) {
       throw new AllocationConfigurationException("Bad fair scheduler config "

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

@@ -37,6 +37,7 @@
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.xml.sax.SAXException;
 
+import com.google.common.base.CharMatcher;
 import com.google.common.annotations.VisibleForTesting;
 /**
  * Maintains a list of queues as well as scheduling parameters for each queue,
@@ -457,6 +458,9 @@ public void updateAllocationConfiguration(AllocationConfiguration queueConf) {
    */
   @VisibleForTesting
   boolean isQueueNameValid(String node) {
-    return !node.isEmpty() && node.equals(node.trim());
+    // use the same white space trim as in QueueMetrics() otherwise things fail
+    // guava uses a different definition for whitespace than java.
+    return !node.isEmpty() &&
+        node.equals(CharMatcher.WHITESPACE.trimFrom(node));
   }
 }

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

@@ -20,8 +20,11 @@
 import static org.junit.Assert.*;
 
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.FileWriter;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 
 import org.apache.hadoop.conf.Configuration;
@@ -580,6 +583,31 @@ public void testQueueNameContainingOnlyWhitespace() throws Exception {
     allocLoader.reloadAllocations();
   }
 
+  /**
+   * Verify that you can't have the queue name with just a non breaking
+   * whitespace in the allocations file.
+   */
+  @Test (expected = AllocationConfigurationException.class)
+  public void testQueueNameContainingNBWhitespace() throws Exception {
+    Configuration conf = new Configuration();
+    conf.set(FairSchedulerConfiguration.ALLOCATION_FILE, ALLOC_FILE);
+
+    PrintWriter out = new PrintWriter(new OutputStreamWriter(
+        new FileOutputStream(ALLOC_FILE), StandardCharsets.UTF_8));
+    out.println("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
+    out.println("<allocations>");
+    out.println("<queue name=\"\u00a0\">");
+    out.println("</queue>");
+    out.println("</allocations>");
+    out.close();
+
+    AllocationFileLoaderService allocLoader = new AllocationFileLoaderService();
+    allocLoader.init(conf);
+    ReloadListener confHolder = new ReloadListener();
+    allocLoader.setReloadListener(confHolder);
+    allocLoader.reloadAllocations();
+  }
+
   /**
    * Verify that defaultQueueSchedulingMode can't accept FIFO as a value.
    */

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

@@ -4362,6 +4362,17 @@ public void testQueueNameWithTrailingSpace() throws Exception {
     assertEquals(1, scheduler.getQueueManager().getLeafQueue("B.C", true)
         .getNumRunnableApps());
     assertNotNull(scheduler.getSchedulerApp(appAttemptId3));
+
+    // submit app with queue name "A\u00a0" (non-breaking space)
+    ApplicationAttemptId appAttemptId4 = createAppAttemptId(4, 1);
+    AppAddedSchedulerEvent appAddedEvent4 = new AppAddedSchedulerEvent(
+        appAttemptId4.getApplicationId(), "A\u00a0", "user1");
+    scheduler.handle(appAddedEvent4);
+    // submission rejected
+    assertEquals(3, scheduler.getQueueManager().getLeafQueues().size());
+    assertNull(scheduler.getSchedulerApplications().get(appAttemptId4.
+        getApplicationId()));
+    assertNull(scheduler.getSchedulerApp(appAttemptId4));
   }
 
   @Test

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

@@ -131,6 +131,9 @@ public void testCheckQueueNodeName() {
     assertFalse(queueManager.isQueueNameValid(" a"));
     assertFalse(queueManager.isQueueNameValid("a "));
     assertFalse(queueManager.isQueueNameValid(" a "));
+    assertFalse(queueManager.isQueueNameValid("\u00a0"));
+    assertFalse(queueManager.isQueueNameValid("a\u00a0"));
+    assertFalse(queueManager.isQueueNameValid("\u00a0a\u00a0"));
     assertTrue(queueManager.isQueueNameValid("a b"));
     assertTrue(queueManager.isQueueNameValid("a"));
   }