This is an automated email from the ASF dual-hosted git repository.
pbacsko pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new 414d401 YARN-10958. Use correct configuration for Group service init
in CSMappingPlacementRule (#3560)
414d401 is described below
commit 414d40155cea40ecceab3c927dd9ebfd899ba7a0
Author: Szilard Nemeth <954799+szilard-nem...@users.noreply.github.com>
AuthorDate: Wed Oct 20 10:48:42 2021 +0200
YARN-10958. Use correct configuration for Group service init in
CSMappingPlacementRule (#3560)
* YARN-10958. Initial commit
* Fix javadoc + behaviour
* Fix review comments
* fix checkstyle + blanks
* fix checkstyle + blanks
* Fix checkstyle + blanks
---
.../java/org/apache/hadoop/security/Groups.java | 5 +
.../placement/CSMappingPlacementRule.java | 7 +-
.../csmappingrule/TestCSMappingPlacementRule.java | 137 +++++++++++++++++++++
3 files changed, 148 insertions(+), 1 deletion(-)
diff --git
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
index bc2ea45..70c633c 100644
---
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
+++
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
@@ -493,4 +493,9 @@ public class Groups {
GROUPS = new Groups(conf);
return GROUPS;
}
+
+ @VisibleForTesting
+ public static void reset() {
+ GROUPS = null;
+ }
}
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 ef98c1a..91e0138 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
@@ -133,7 +133,7 @@ public class CSMappingPlacementRule extends PlacementRule {
overrideWithQueueMappings = conf.getOverrideWithQueueMappings();
if (groups == null) {
- groups = Groups.getUserToGroupsMappingService(conf);
+ groups = Groups.getUserToGroupsMappingService(csContext.getConf());
}
MappingRuleValidationContext validationContext = buildValidationContext();
@@ -535,4 +535,9 @@ public class CSMappingPlacementRule extends PlacementRule {
return name;
}
}
+
+ @VisibleForTesting
+ public Groups getGroups() {
+ return groups;
+ }
}
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 0cf1059..3e614bc 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
@@ -18,6 +18,8 @@
package org.apache.hadoop.yarn.server.resourcemanager.placement.csmappingrule;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.security.GroupMappingServiceProvider;
import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableMap;
import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableSet;
import org.apache.hadoop.security.Groups;
@@ -51,6 +53,7 @@ import static junit.framework.TestCase.assertNotNull;
import static junit.framework.TestCase.assertNull;
import static junit.framework.TestCase.assertTrue;
import static junit.framework.TestCase.fail;
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_GROUP_MAPPING;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -771,4 +774,138 @@ public class TestCSMappingPlacementRule {
"Application should have been placed to
root.groups.sec_dot_test_dot_grp",
engine, app, "test.user", "root.groups.sec_dot_test_dot_grp");
}
+
+ /**
+ * 1. Invoke Groups.reset(). This make sure that the underlying singleton
{@link Groups#GROUPS}
+ * is set to null.<br>
+ * 2. Create a Configuration object in which the property
"hadoop.security.group.mapping"
+ * refers to an existing a test implementation.<br>
+ * 3. Create a mock CapacityScheduler where getConf() and getConfiguration()
contain different
+ * settings for "hadoop.security.group.mapping". Since getConf() is the
service config, this
+ * should return the config object created in step #2.<br>
+ * 4. Create an instance of CSMappingPlacementRule with a single primary
group rule.<br>
+ * 5. Run the placement evaluation.<br>
+ * 6. Expected: The returned queue matches what is supposed to be coming
from the test group
+ * mapping service ("testuser" --> "testGroup1").<br>
+ * 7. Modify "hadoop.security.group.mapping" in the config object created in
step #2.
+ * This step is required to guarantee that the CSMappingPlacementRule
doesn't try to recreate
+ * the group mapping implementation and uses the one that was previously
created.<br>
+ * 8. Call Groups.refresh() which changes the group mapping ("testuser" -->
"testGroup0"). This
+ * requires that the test group mapping service implement
GroupMappingServiceProvider
+ * .cacheGroupsRefresh().<br>
+ * 9. Create a new instance of CSMappingPlacementRule. This is important as
we want to test
+ * that even this new {@link CSMappingPlacementRule} instance uses the same
group mapping
+ * instance.<br>
+ * 10. Run the placement evaluation again<br>
+ * 11. Expected: with the same user, the target queue has changed to
'testGroup0'.<br>
+ * <p>
+ * These all looks convoluted, but the steps above make sure all the
following conditions are met:
+ * <p>
+ * 1. CSMappingPlacementRule will force the initialization of groups.<br>
+ * 2. We select the correct configuration for group service init.<br>
+ * 3. We don't create a new Groups instance if the singleton is initialized,
so we cover the
+ * original problem described in YARN-10597.<br>
+ */
+ @Test
+ public void testPlacementEngineSelectsCorrectConfigurationForGroupMapping()
throws IOException {
+ Groups.reset();
+ final String user = "testuser";
+
+ //Create service-wide configuration object
+ Configuration yarnConf = new Configuration();
+ yarnConf.setClass(HADOOP_SECURITY_GROUP_MAPPING,
MockUnixGroupsMapping.class,
+ GroupMappingServiceProvider.class);
+
+ //Create CS configuration object with a single, primary group mapping rule
+ List<MappingRule> mappingRules = new ArrayList<>();
+ mappingRules.add(
+ new MappingRule(
+ MappingRuleMatchers.createUserMatcher(user),
+ (new MappingRuleActions.PlaceToQueueAction(
+ "root.man.%primary_group", true))
+ .setFallbackReject()));
+ CapacitySchedulerConfiguration csConf = new
CapacitySchedulerConfiguration() {
+ @Override
+ public List<MappingRule> getMappingRules() {
+ return mappingRules;
+ }
+ };
+ csConf.setOverrideWithQueueMappings(true);
+ //Intentionally add a dummy implementation class -
+ // The "HADOOP_SECURITY_GROUP_MAPPING" should not be read from the
+ // CapacitySchedulerConfiguration instance!
+ csConf.setClass(HADOOP_SECURITY_GROUP_MAPPING, String.class, Object.class);
+
+ CapacityScheduler cs = createMockCS(yarnConf, csConf);
+
+ //Create app, submit to placement engine, expecting queue=testGroup1
+ CSMappingPlacementRule engine = initPlacementEngine(cs);
+ ApplicationSubmissionContext app = createApp("app");
+ assertPlace(engine, app, user, "root.man.testGroup1");
+
+ //Intentionally add a dummy implementation class!
+ // The "HADOOP_SECURITY_GROUP_MAPPING" should not be read from the
+ // CapacitySchedulerConfiguration instance!
+ //This makes sure that the Groups instance is not recreated by
CSMappingPlacementRule
+ yarnConf.setClass(HADOOP_SECURITY_GROUP_MAPPING, String.class,
Object.class);
+
+ //Refresh the groups, this makes testGroup0 as primary group for "testUser"
+ engine.getGroups().refresh();
+
+ //Create app, submit to placement engine, expecting queue=testGroup0 (the
new primary group)
+ engine = initPlacementEngine(cs);
+ assertPlace(engine, app, user, "root.man.testGroup0");
+ }
+
+ private CSMappingPlacementRule initPlacementEngine(CapacityScheduler cs)
throws IOException {
+ CSMappingPlacementRule engine = new CSMappingPlacementRule();
+ engine.setFailOnConfigError(true);
+ engine.initialize(cs);
+ return engine;
+ }
+
+ private CapacityScheduler createMockCS(Configuration conf,
+ CapacitySchedulerConfiguration csConf) {
+ CapacitySchedulerQueueManager qm =
+ mock(CapacitySchedulerQueueManager.class);
+ createQueueHierarchy(qm);
+
+ CapacityScheduler cs = mock(CapacityScheduler.class);
+ when(cs.getConfiguration()).thenReturn(csConf);
+ when(cs.getConf()).thenReturn(conf);
+ when(cs.getCapacitySchedulerQueueManager()).thenReturn(qm);
+ return cs;
+ }
+
+ public static class MockUnixGroupsMapping implements
GroupMappingServiceProvider {
+
+ public MockUnixGroupsMapping() {
+ GROUP.clear();
+ GROUP.add("testGroup1");
+ GROUP.add("testGroup2");
+ GROUP.add("testGroup3");
+ }
+
+ private static final List<String> GROUP = new ArrayList<>();
+
+ @Override
+ public List<String> getGroups(String user) throws IOException {
+ return GROUP;
+ }
+
+ @Override
+ public void cacheGroupsRefresh() {
+ GROUP.add(0, "testGroup0");
+ }
+
+ @Override
+ public void cacheGroupsAdd(List<String> groups) {
+ // Do nothing
+ }
+
+ @Override
+ public Set<String> getGroupsSet(String user) {
+ return ImmutableSet.copyOf(GROUP);
+ }
+ }
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
