Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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", "");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

package org.apache.hadoop.yarn.server.resourcemanager.placement.csmappingrule;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.commons.logging.impl.Log4JLogger;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.security.GroupMappingServiceProvider;
import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableMap;
Expand All @@ -33,6 +36,8 @@
import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
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;
Expand Down Expand Up @@ -65,6 +70,11 @@ public class TestCSMappingPlacementRule {

@Rule
public TemporaryFolder folder = new TemporaryFolder();

@Before
public void setUp() {
setLoggerToDebug();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you purposely left this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, I wanted to have a more verbouse output and I think it's not bad to keep the debug logs for later.
Is there any better way to increase verbosity of the logger? :)

}

private Map<String, Set<String>> userGroups =
ImmutableMap.<String, Set<String>>builder()
Expand All @@ -85,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")
Expand Down Expand Up @@ -857,6 +868,54 @@ public List<MappingRule> getMappingRules() {
assertPlace(engine, app, user, "root.man.testGroup0");
}

@Test
public void testOriginalUserNameWithDotCanBeUsedInMatchExpression() throws IOException {
List<MappingRule> 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<MappingRule> 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 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 {
CSMappingPlacementRule engine = new CSMappingPlacementRule();
engine.setFailOnConfigError(true);
Expand Down