Fix bug

Author: szilard-nemeth

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java

@@ -228,7 +228,11 @@ private VariableContext createVariableContext(
       ApplicationSubmissionContext asc, String user) {
     VariableContext vctx = new VariableContext();
 
-    vctx.put("%user", cleanName(user));
+    String cleanedName = cleanName(user);
+    if (!user.equals(cleanedName)) {
+      vctx.putOriginal("%user", user);
+    }
+    vctx.put("%user", cleanedName);
     //If the specified matches the default it means NO queue have been specified
     //as per ClientRMService#submitApplication which sets the queue to default
     //when no queue is provided.
@@ -239,7 +243,7 @@ private VariableContext createVariableContext(
       //Adding specified as empty will prevent it to be undefined and it won't
       //try to place the application to a queue named '%specified', queue path
       //validation will reject the empty path or the path with empty parts,
-      //so we sill still hit the fallback action of this rule if no queue
+      //so we still hit the fallback action of this rule if no queue
       //is specified
       vctx.put("%specified", "");
     }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/VariableContext.java

@@ -39,6 +39,7 @@ public class VariableContext {
    * This is our actual variable store.
    */
   private Map<String, String> variables = new HashMap<>();
+  private Map<String, String> originalVariables = new HashMap<>();
 
   /**
    * This is our conditional variable store.
@@ -124,6 +125,10 @@ public VariableContext put(String name, String value) {
     return this;
   }
 
+  public void putOriginal(String name, String value) {
+    originalVariables.put(name, value);
+  }
+
   /**
    * This method is used to add a conditional variable to the variable context.
    * @param name Name of the variable
@@ -149,6 +154,11 @@ public String get(String name) {
     String ret = variables.get(name);
     return ret == null ? "" : ret;
   }
+  
+  public String getOriginal(String name) {
+    String ret = originalVariables.get(name);
+    return ret;
+  }
 
   /**
    * Adds a set to the context, each name can only be added once. The extra

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/csmappingrule/MappingRuleMatchers.java

@@ -87,6 +87,12 @@ public boolean match(VariableContext variables) {
       }
 
       String substituted = variables.replaceVariables(value);
+      
+      String originalVariableValue = variables.getOriginal(variable);
+      if (originalVariableValue != null) {
+        return substituted.equals(originalVariableValue);
+      }
+      
       return substituted.equals(variables.get(variable));
     }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/csmappingrule/TestCSMappingPlacementRule.java

@@ -37,6 +37,7 @@
 import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerQueueManager;
 import org.apache.hadoop.yarn.util.Records;
 import org.apache.log4j.Level;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -69,6 +70,11 @@ public class TestCSMappingPlacementRule {
 
   @Rule
   public TemporaryFolder folder = new TemporaryFolder();
+  
+  @Before
+  public void setUp() {
+    setLoggerToDebug();
+  }
 
   private Map<String, Set<String>> userGroups =
       ImmutableMap.<String, Set<String>>builder()
@@ -89,6 +95,7 @@ private void createQueueHierarchy(CapacitySchedulerQueueManager queueManager) {
         .withQueue("root.user.alice")
         .withQueue("root.user.bob")
         .withQueue("root.user.test_dot_user")
+        .withQueue("root.user.testuser")
         .withQueue("root.groups.main_dot_grp")
         .withQueue("root.groups.sec_dot_test_dot_grp")
         .withQueue("root.secondaryTests.unique")
@@ -862,18 +869,12 @@ public List<MappingRule> getMappingRules() {
   }
 
   @Test
-  public void testUserNameSetDefaultAndPlaceWith2Rules() throws IOException {
-    Log log = LogFactory.getLog("org.apache.hadoop.yarn.server.resourcemanager.placement");
-    if (log instanceof Log4JLogger) {
-      org.apache.log4j.Logger logger = ((Log4JLogger) log).getLogger();
-      logger.setLevel(Level.DEBUG);
-    }
-
+  public void testOriginalUserNameWithDotCanBeUsedInMatchExpression() throws IOException {
     List<MappingRule> rules = new ArrayList<>();
     rules.add(
             new MappingRule(
                     MappingRuleMatchers.createUserMatcher("test.user"),
-                    (MappingRuleActions.createUpdateDefaultAction("root.user"))
+                    (MappingRuleActions.createUpdateDefaultAction("root.user.testuser"))
                             .setFallbackSkip()));
     rules.add(new MappingRule(
             MappingRuleMatchers.createUserMatcher("test.user"),
@@ -884,34 +885,35 @@ public void testUserNameSetDefaultAndPlaceWith2Rules() throws IOException {
     ApplicationSubmissionContext app = createApp("app");
     assertPlace(
             "test.user should be placed to root.user",
-            engine, app, "test.user", "root.user");
+            engine, app, "test.user", "root.user.testuser");
   }
 
   @Test
-  public void testUserNameSetDefaultAndPlaceWith2RulesUsernameReplacedWithDot() throws IOException {
-    Log log = LogFactory.getLog("org.apache.hadoop.yarn.server.resourcemanager.placement");
-    if (log instanceof Log4JLogger) {
-      org.apache.log4j.Logger logger = ((Log4JLogger) log).getLogger();
-      logger.setLevel(Level.DEBUG);
-    }
-
-
-    ArrayList<MappingRule> rules = new ArrayList<>();
+  public void testOriginalGroupNameWithDotCanBeUsedInMatchExpression() throws IOException {
+    List<MappingRule> rules = new ArrayList<>();
     rules.add(
-            new MappingRule(
-                    MappingRuleMatchers.createUserMatcher("test_dot_user"),
-                    (MappingRuleActions.createUpdateDefaultAction("root.user.bob"))
-                            .setFallbackSkip()));
+        new MappingRule(
+            MappingRuleMatchers.createUserGroupMatcher("sec.test.grp"),
+            (MappingRuleActions.createUpdateDefaultAction("root.user.testuser"))
+                .setFallbackSkip()));
     rules.add(new MappingRule(
-            MappingRuleMatchers.createUserMatcher("test_dot_user"),
-            (MappingRuleActions.createPlaceToDefaultAction())
-                    .setFallbackReject()));
+        MappingRuleMatchers.createUserMatcher("test.user"),
+        (MappingRuleActions.createPlaceToDefaultAction())
+            .setFallbackReject()));
 
     CSMappingPlacementRule engine = setupEngine(true, rules);
     ApplicationSubmissionContext app = createApp("app");
     assertPlace(
-            "test.user should be placed to root.user.bob",
-            engine, app, "test.user", "root.user.bob");
+        "test.user should be placed to root.user",
+        engine, app, "test.user", "root.user.testuser");
+  }
+
+  private static void setLoggerToDebug() {
+    Log log = LogFactory.getLog("org.apache.hadoop.yarn.server.resourcemanager.placement");
+    if (log instanceof Log4JLogger) {
+      org.apache.log4j.Logger logger = ((Log4JLogger) log).getLogger();
+      logger.setLevel(Level.DEBUG);
+    }
   }
 
   private CSMappingPlacementRule initPlacementEngine(CapacityScheduler cs) throws IOException {