From 7f6cc196f8396651d2d794a20c801086e641b107 Mon Sep 17 00:00:00 2001 From: Szilard Nemeth <954799+szilard-nemeth@users.noreply.github.com> Date: Wed, 11 Jan 2023 13:23:04 +0100 Subject: [PATCH] YARN-11190. CS Mapping rule bug: User matcher does not work correctly for usernames with dot (#4471) --- .../placement/CSMappingPlacementRule.java | 8 +++- .../placement/VariableContext.java | 9 ++++ .../csmappingrule/MappingRuleMatchers.java | 6 +++ .../TestCSMappingPlacementRule.java | 43 ++++++++++++++++++- 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java index cefed1dd9fd..e0fab39b053 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java +++ b/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 @@ public class CSMappingPlacementRule extends PlacementRule { 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 @@ public class CSMappingPlacementRule extends PlacementRule { //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", ""); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/VariableContext.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/VariableContext.java index d60e7b5630a..e8e419c64eb 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/VariableContext.java +++ b/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 variables = new HashMap<>(); + private Map originalVariables = new HashMap<>(); /** * This is our conditional variable store. @@ -124,6 +125,10 @@ public class VariableContext { 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 @@ -150,6 +155,10 @@ public class VariableContext { return ret == null ? "" : ret; } + public String getOriginal(String name) { + return originalVariables.get(name); + } + /** * Adds a set to the context, each name can only be added once. The extra * dataset is different from the regular variables because it cannot be diff --git a/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 b/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 index 9d56e89121c..0466dcffe97 100644 --- a/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 +++ b/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 class MappingRuleMatchers { } String substituted = variables.replaceVariables(value); + + String originalVariableValue = variables.getOriginal(variable); + if (originalVariableValue != null) { + return substituted.equals(originalVariableValue); + } + return substituted.equals(variables.get(variable)); } diff --git a/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 b/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 index 3e614bcbc96..41ce2b56eab 100644 --- a/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 +++ b/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 @@ -65,7 +65,7 @@ public class TestCSMappingPlacementRule { @Rule public TemporaryFolder folder = new TemporaryFolder(); - + private Map> userGroups = ImmutableMap.>builder() .put("alice", ImmutableSet.of("p_alice", "unique", "user")) @@ -85,6 +85,7 @@ public class TestCSMappingPlacementRule { .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") @@ -857,6 +858,46 @@ public class TestCSMappingPlacementRule { assertPlace(engine, app, user, "root.man.testGroup0"); } + @Test + public void testOriginalUserNameWithDotCanBeUsedInMatchExpression() throws IOException { + List rules = new ArrayList<>(); + rules.add( + new MappingRule( + MappingRuleMatchers.createUserMatcher("test.user"), + (MappingRuleActions.createUpdateDefaultAction("root.user.testuser")) + .setFallbackSkip())); + rules.add(new MappingRule( + 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", + engine, app, "test.user", "root.user.testuser"); + } + + @Test + public void testOriginalGroupNameWithDotCanBeUsedInMatchExpression() throws IOException { + List rules = new ArrayList<>(); + rules.add( + new MappingRule( + MappingRuleMatchers.createUserGroupMatcher("sec.test.grp"), + (MappingRuleActions.createUpdateDefaultAction("root.user.testuser")) + .setFallbackSkip())); + rules.add(new MappingRule( + 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", + engine, app, "test.user", "root.user.testuser"); + } + private CSMappingPlacementRule initPlacementEngine(CapacityScheduler cs) throws IOException { CSMappingPlacementRule engine = new CSMappingPlacementRule(); engine.setFailOnConfigError(true);