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 c4fb404 YARN-10415. Create a group matcher which checks ALL groups of
the user. Contributed by Gergely Pollak.
c4fb404 is described below
commit c4fb4044b2caf211210c955d832146c8294aea69
Author: Peter Bacsko <pbac...@cloudera.com>
AuthorDate: Tue Sep 8 10:57:00 2020 +0200
YARN-10415. Create a group matcher which checks ALL groups of the user.
Contributed by Gergely Pollak.
---
.../placement/CSMappingPlacementRule.java | 29 +++++-----
.../resourcemanager/placement/MappingRule.java | 2 +-
.../placement/MappingRuleMatchers.java | 55 ++++++++++++++++---
.../resourcemanager/placement/VariableContext.java | 32 ++++++++++-
.../placement/TestCSMappingPlacementRule.java | 33 ++++++++++++
.../resourcemanager/placement/TestMappingRule.java | 2 +
.../placement/TestMappingRuleMatchers.java | 62 ++++++++++++++++++++--
.../placement/TestVariableContext.java | 29 ++++++++++
8 files changed, 220 insertions(+), 24 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 2929ae0..9983ebc 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
@@ -156,23 +156,24 @@ public class CSMappingPlacementRule extends PlacementRule
{
return mappingRules.size() > 0;
}
- private String getPrimaryGroup(String user) throws IOException {
- return groups.getGroupsSet(user).iterator().next();
- }
-
/**
- * Traverse all secondary groups (as there could be more than one
- * and position is not guaranteed) and ensure there is queue with
- * the same name.
+ * Sets group related data for the provided variable context.
+ * Primary group is the first group returned by getGroups.
+ * To determine secondary group we traverse all groups
+ * (as there could be more than one and position is not guaranteed) and
+ * ensure there is queue with the same name.
+ * This method also sets the groups set for the variable context for group
+ * matching.
+ * @param vctx Variable context to be updated
* @param user Name of the user
- * @return Name of the secondary group if found, null otherwise
* @throws IOException
*/
- private String getSecondaryGroup(String user) throws IOException {
+ private void setupGroupsForVariableContext(VariableContext vctx, String user)
+ throws IOException {
Set<String> groupsSet = groups.getGroupsSet(user);
String secondaryGroup = null;
Iterator<String> it = groupsSet.iterator();
- it.next();
+ String primaryGroup = it.next();
while (it.hasNext()) {
String group = it.next();
if (this.queueManager.getQueue(group) != null) {
@@ -185,7 +186,10 @@ public class CSMappingPlacementRule extends PlacementRule {
LOG.debug("User {} is not associated with any Secondary " +
"Group. Hence it may use the 'default' queue", user);
}
- return secondaryGroup;
+
+ vctx.put("%primary_group", primaryGroup);
+ vctx.put("%secondary_group", secondaryGroup);
+ vctx.putExtraDataset("groups", groupsSet);
}
private VariableContext createVariableContext(
@@ -195,9 +199,8 @@ public class CSMappingPlacementRule extends PlacementRule {
vctx.put("%user", user);
vctx.put("%specified", asc.getQueue());
vctx.put("%application", asc.getApplicationName());
- vctx.put("%primary_group", getPrimaryGroup(user));
- vctx.put("%secondary_group", getSecondaryGroup(user));
vctx.put("%default", "root.default");
+ setupGroupsForVariableContext(vctx, user);
vctx.setImmutables(immutableVariables);
return vctx;
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/MappingRule.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingRule.java
index 50fb18f..e61ad95 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingRule.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingRule.java
@@ -109,7 +109,7 @@ public class MappingRule {
matcher = MappingRuleMatchers.createUserMatcher(source);
break;
case GROUP_MAPPING:
- matcher = MappingRuleMatchers.createGroupMatcher(source);
+ matcher = MappingRuleMatchers.createUserGroupMatcher(source);
break;
case APPLICATION_MAPPING:
matcher = MappingRuleMatchers.createApplicationNameMatcher(source);
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/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/MappingRuleMatchers.java
index fdc239f..24f147b 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/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/MappingRuleMatchers.java
@@ -19,6 +19,7 @@
package org.apache.hadoop.yarn.server.resourcemanager.placement;
import java.util.Arrays;
+import java.util.Set;
/**
* This class contains all the matcher and some helper methods to generate
them.
@@ -97,6 +98,48 @@ public class MappingRuleMatchers {
}
/**
+ * The GroupMatcher will check if any of the user's groups match the provided
+ * group name. It does not care if it's primary or secondary group, it just
+ * checks if the user is member of the expected group.
+ */
+ public static class UserGroupMatcher implements MappingRuleMatcher {
+ /**
+ * The group which should match the users's groups.
+ */
+ private String group;
+
+ UserGroupMatcher(String value) {
+ this.group = value;
+ }
+
+ /**
+ * The method will match (return true) if the user is in the provided
group.
+ * This matcher expect an extraVariableSet to be present in the variable
+ * context, if it's not present, we return false.
+ * If the expected group is null we always return false.
+ * @param variables The variable context, which contains all the variables
+ * @return true if user is member of the group
+ */
+ @Override
+ public boolean match(VariableContext variables) {
+ Set<String> groups = variables.getExtraDataset("groups");
+
+ if (group == null || groups == null) {
+ return false;
+ }
+
+ String substituted = variables.replaceVariables(group);
+ return groups.contains(substituted);
+ }
+
+ @Override
+ public String toString() {
+ return "GroupMatcher{" +
+ "group='" + group + '\'' +
+ '}';
+ }
+ }
+ /**
* AndMatcher is a basic boolean matcher which takes multiple other
* matcher as it's arguments, and on match it checks if all of them are true.
*/
@@ -193,13 +236,13 @@ public class MappingRuleMatchers {
}
/**
- * Convenience method to create a variable matcher which matches against the
- * user's primary group.
+ * Convenience method to create a group matcher which matches against the
+ * groups of the user.
* @param groupName The groupName to be matched
- * @return VariableMatcher with %primary_group as the variable
+ * @return UserGroupMatcher
*/
- public static MappingRuleMatcher createGroupMatcher(String groupName) {
- return new VariableMatcher("%primary_group", groupName);
+ public static MappingRuleMatcher createUserGroupMatcher(String groupName) {
+ return new UserGroupMatcher(groupName);
}
/**
@@ -215,7 +258,7 @@ public class MappingRuleMatchers {
String userName, String groupName) {
return new AndMatcher(
createUserMatcher(userName),
- createGroupMatcher(groupName));
+ createUserGroupMatcher(groupName));
}
/**
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 12adde2..f9bb426 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
@@ -45,6 +45,12 @@ public class VariableContext {
private Set<String> immutableNames;
/**
+ * Some matchers may need to find a data in a set, which is not usable
+ * as a variable in substitutions, this store is for those sets.
+ */
+ private Map<String, Set<String>> extraDataset = new HashMap<>();
+
+ /**
* Checks if the provided variable is immutable.
* @param name Name of the variable to check
* @return true if the variable is immutable
@@ -115,6 +121,31 @@ public class VariableContext {
}
/**
+ * 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
+ * referenced via tokens in the paths or any other input. However matchers
+ * and actions can explicitly access these datasets and can make decisions
+ * based on them.
+ * @param name Name which can be used to reference the collection
+ * @param set The dataset to be stored
+ */
+ public void putExtraDataset(String name, Set<String> set) {
+ if (extraDataset.containsKey(name)) {
+ throw new IllegalStateException(
+ "Dataset '" + name + "' is already set!");
+ }
+ extraDataset.put(name, set);
+ }
+
+ /**
+ * Returns the dataset referenced by the name.
+ * @param name Name of the set to be returned.
+ */
+ public Set<String> getExtraDataset(String name) {
+ return extraDataset.get(name);
+ }
+
+ /**
* Check if a variable is part of the context.
* @param name Name of the variable to be checked
* @return True if the variable is added to the context, false otherwise
@@ -195,5 +226,4 @@ public class VariableContext {
return String.join(".", parts);
}
-
}
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/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/TestCSMappingPlacementRule.java
index 5b47d34..288f006 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/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/TestCSMappingPlacementRule.java
@@ -411,4 +411,37 @@ public class TestCSMappingPlacementRule {
engine, app, "charlie", "root.man.create");
}
+
+ private MappingRule createGroupMapping(String group, String queue) {
+ MappingRuleMatcher matcher =
MappingRuleMatchers.createUserGroupMatcher(group);
+ MappingRuleAction action =
+ (new MappingRuleActions.PlaceToQueueAction(queue, true))
+ .setFallbackReject();
+ return new MappingRule(matcher, action);
+ }
+
+ @Test
+ public void testGroupMatching() throws IOException {
+ ArrayList<MappingRule> rules = new ArrayList<>();
+
+ rules.add(createGroupMapping("p_alice", "root.man.p_alice"));
+ rules.add(createGroupMapping("developer", "root.man.developer"));
+
+ //everybody is in the user group, this should catch all
+ rules.add(createGroupMapping("user", "root.man.user"));
+
+ CSMappingPlacementRule engine = setupEngine(true, rules);
+ ApplicationSubmissionContext app = createApp("app");
+
+ assertPlace(
+ "Alice should be placed to root.man.p_alice based on her primary
group",
+ engine, app, "alice", "root.man.p_alice");
+ assertPlace(
+ "Bob should be placed to root.man.developer based on his developer " +
+ "group", engine, app, "bob", "root.man.developer");
+ assertPlace(
+ "Charlie should be placed to root.man.user because he is not a " +
+ "developer nor in the p_alice group", engine, app, "charlie",
+ "root.man.user");
+ }
}
\ No newline at end of file
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/TestMappingRule.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestMappingRule.java
index c215c5b..e19cd89 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestMappingRule.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestMappingRule.java
@@ -21,6 +21,7 @@ package
org.apache.hadoop.yarn.server.resourcemanager.placement;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import com.google.common.collect.Sets;
import org.apache.hadoop.util.StringUtils;
import org.junit.Test;
@@ -107,6 +108,7 @@ public class TestMappingRule {
public void testLegacyEvaluation() {
VariableContext matching = setupVariables(
"bob", "developer", "users", "MR");
+ matching.putExtraDataset("groups", Sets.newHashSet("developer"));
VariableContext mismatching = setupVariables(
"joe", "tester", "admins", "Spark");
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/TestMappingRuleMatchers.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestMappingRuleMatchers.java
index d384f93..c0efebb 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestMappingRuleMatchers.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestMappingRuleMatchers.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.yarn.server.resourcemanager.placement;
+import com.google.common.collect.Sets;
import junit.framework.TestCase;
import org.junit.Test;
@@ -41,19 +42,21 @@ public class TestMappingRuleMatchers extends TestCase {
matchingContext.put("%primary_group", "developers");
matchingContext.put("%application", "TurboMR");
matchingContext.put("%custom", "Matching string");
+ matchingContext.putExtraDataset("groups", Sets.newHashSet("developers"));
VariableContext mismatchingContext = new VariableContext();
mismatchingContext.put("%user", "dave");
mismatchingContext.put("%primary_group", "testers");
mismatchingContext.put("%application", "Tester APP");
mismatchingContext.put("%custom", "Not matching string");
+ mismatchingContext.putExtraDataset("groups", Sets.newHashSet("testers"));
VariableContext emptyContext = new VariableContext();
Map<String, MappingRuleMatcher> matchers = new HashMap<>();
matchers.put("User matcher", MappingRuleMatchers.createUserMatcher("bob"));
matchers.put("Group matcher",
- MappingRuleMatchers.createGroupMatcher("developers"));
+ MappingRuleMatchers.createUserGroupMatcher("developers"));
matchers.put("Application name matcher",
MappingRuleMatchers.createApplicationNameMatcher("TurboMR"));
matchers.put("Custom matcher",
@@ -184,16 +187,17 @@ public class TestMappingRuleMatchers extends TestCase {
VariableContext developerBob = new VariableContext();
developerBob.put("%user", "bob");
developerBob.put("%primary_group", "developers");
-
+ developerBob.putExtraDataset("groups", Sets.newHashSet("developers"));
VariableContext testerBob = new VariableContext();
testerBob.put("%user", "bob");
testerBob.put("%primary_group", "testers");
+ testerBob.putExtraDataset("groups", Sets.newHashSet("testers"));
VariableContext testerDave = new VariableContext();
testerDave.put("%user", "dave");
testerDave.put("%primary_group", "testers");
-
+ testerDave.putExtraDataset("groups", Sets.newHashSet("testers"));
VariableContext accountantDave = new VariableContext();
accountantDave.put("%user", "dave");
@@ -252,4 +256,56 @@ public class TestMappingRuleMatchers extends TestCase {
", " + var.toString() + "]}", or.toString());
}
+ @Test
+ public void testGroupMatching() {
+ VariableContext letterGroups = new VariableContext();
+ letterGroups.putExtraDataset("groups", Sets.newHashSet("a", "b", "c"));
+
+ VariableContext numberGroups = new VariableContext();
+ numberGroups.putExtraDataset("groups", Sets.newHashSet("1", "2", "3"));
+
+ VariableContext noGroups = new VariableContext();
+
+ MappingRuleMatcher matchA =
+ MappingRuleMatchers.createUserGroupMatcher("a");
+ MappingRuleMatcher matchB =
+ MappingRuleMatchers.createUserGroupMatcher("b");
+ MappingRuleMatcher matchC =
+ MappingRuleMatchers.createUserGroupMatcher("c");
+ MappingRuleMatcher match1 =
+ MappingRuleMatchers.createUserGroupMatcher("1");
+ MappingRuleMatcher match2 =
+ MappingRuleMatchers.createUserGroupMatcher("2");
+ MappingRuleMatcher match3 =
+ MappingRuleMatchers.createUserGroupMatcher("3");
+ MappingRuleMatcher matchNull =
+ MappingRuleMatchers.createUserGroupMatcher(null);
+
+ //letter groups submission should match only the letters
+ assertTrue(matchA.match(letterGroups));
+ assertTrue(matchB.match(letterGroups));
+ assertTrue(matchC.match(letterGroups));
+ assertFalse(match1.match(letterGroups));
+ assertFalse(match2.match(letterGroups));
+ assertFalse(match3.match(letterGroups));
+ assertFalse(matchNull.match(letterGroups));
+
+ //numeric groups submission should match only the numbers
+ assertFalse(matchA.match(numberGroups));
+ assertFalse(matchB.match(numberGroups));
+ assertFalse(matchC.match(numberGroups));
+ assertTrue(match1.match(numberGroups));
+ assertTrue(match2.match(numberGroups));
+ assertTrue(match3.match(numberGroups));
+ assertFalse(matchNull.match(numberGroups));
+
+ //noGroups submission should not match anything
+ assertFalse(matchA.match(noGroups));
+ assertFalse(matchB.match(noGroups));
+ assertFalse(matchC.match(noGroups));
+ assertFalse(match1.match(noGroups));
+ assertFalse(match2.match(noGroups));
+ assertFalse(match3.match(noGroups));
+ assertFalse(matchNull.match(noGroups));
+ }
}
\ No newline at end of file
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/TestVariableContext.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestVariableContext.java
index d04e649..7cbfdd4 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestVariableContext.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestVariableContext.java
@@ -22,6 +22,8 @@ import com.google.common.collect.ImmutableSet;
import org.junit.Test;
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Set;
import static org.junit.Assert.*;
@@ -199,4 +201,31 @@ public class TestVariableContext {
assertEquals(expected, variables.replaceVariables(pattern)));
}
+ @Test
+ public void testCollectionStore() {
+ VariableContext variables = new VariableContext();
+ Set<String> coll1 = new HashSet<>();
+ Set<String> coll2 = new HashSet<>();
+
+ coll1.add("Bob");
+ coll1.add("Roger");
+ coll2.add("Bob");
+
+ variables.putExtraDataset("set", coll1);
+ variables.putExtraDataset("sameset", coll1);
+ variables.putExtraDataset("list", coll2);
+
+ try {
+ variables.putExtraDataset("set", coll1);
+ fail("Same name cannot be used multiple times to add collections");
+ } catch (IllegalStateException e) {
+ //Exception expected
+ }
+
+ assertSame(coll1, variables.getExtraDataset("set"));
+ assertSame(coll1, variables.getExtraDataset("sameset"));
+ assertSame(coll2, variables.getExtraDataset("list"));
+ assertNull(variables.getExtraDataset("Nothing"));
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
