brumi1024 commented on a change in pull request #3857:
URL: https://github.com/apache/hadoop/pull/3857#discussion_r786638932
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java
##########
@@ -331,25 +338,23 @@ public void
testQueueSubmitWithACLsEnabledWithQueueMapping()
csConf.set(PREFIX + "root.test.acl_submit_applications", "test");
csConf.set(PREFIX + "root.test.acl_administer_queue", "test");
- asContext.setQueue("test");
-
MockRM newMockRM = new MockRM(csConf);
RMContext newMockRMContext = newMockRM.getRMContext();
-
newMockRMContext.setQueuePlacementManager(createMockPlacementManager("test",
"test", null));
+ newMockRMContext.setQueuePlacementManager(
+ createMockPlacementManager("test", "root.test", null));
TestRMAppManager newAppMonitor = createAppManager(newMockRMContext, conf);
- newAppMonitor.submitApplication(asContext, "test");
- RMApp app = newMockRMContext.getRMApps().get(appId);
- Assert.assertNotNull("app should not be null", app);
- Assert.assertEquals("the queue should be placed on 'test' queue", "test",
app.getQueue());
-
- try {
- asContext.setApplicationId(appId = MockApps.newAppID(2));
- newAppMonitor.submitApplication(asContext, "test1");
- Assert.fail("should fail since test1 does not have permission to submit
to queue");
- } catch(YarnException e) {
- assertTrue(e.getCause() instanceof AccessControlException);
- }
+ ApplicationSubmissionContext submission =
createAppSubmissionContext(MockApps.newAppID(1));
+ submission.setQueue("oldQueue");
+ verifyAppSubmission(submission,
+ newAppMonitor,
+ newMockRMContext,
+ "test",
+ "root.test");
+
+ verifyAppSubmissionFailure(newAppMonitor,
+ createAppSubmissionContext(MockApps.newAppID(2)),
+ "test1");
}
@Test
Review comment:
Nit: maybe the test name could mention that this is for the legacy AQC.
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/CapacityScheduler.md
##########
@@ -640,11 +640,11 @@ support other pre-configured queues to co-exist along
with auto-created queues.
* Configuring **legacy** `Auto-Created Leaf Queues` with `CapacityScheduler`
-The parent queue which has been enabled for auto leaf queue creation,supports
- the configuration of template parameters for automatic configuration of the
auto-created leaf queues. The auto-created queues support all of the
- leaf queue configuration parameters except for **Queue ACL**, **Absolute
- Resource** configurations. Queue ACLs are
- currently inherited from the parent queue i.e they are not configurable on
the leaf queue template
+The parent queue which has been enabled for auto leaf queue creation, supports
+ the configuration of template parameters for automatic configuration of the
auto-created leaf queues. The auto-created queues support all the
+ leaf queue configuration parameters. There is one caveat to the QueueACLs: at
auto queue creation the Queue ACLs in the leaf queue templates are not in
effect yet,
+ the parent's QueueACLs determines whether the queue can be created by the
user. When the auto-created leaf queue already exists then the QueueACLs in the
leaf queue template
Review comment:
the parent's QueueACLs determines -> hence the... or so the... sounds
better to me, what do you think?
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/CapacityScheduler.md
##########
@@ -640,11 +640,11 @@ support other pre-configured queues to co-exist along
with auto-created queues.
* Configuring **legacy** `Auto-Created Leaf Queues` with `CapacityScheduler`
-The parent queue which has been enabled for auto leaf queue creation,supports
- the configuration of template parameters for automatic configuration of the
auto-created leaf queues. The auto-created queues support all of the
- leaf queue configuration parameters except for **Queue ACL**, **Absolute
- Resource** configurations. Queue ACLs are
- currently inherited from the parent queue i.e they are not configurable on
the leaf queue template
+The parent queue which has been enabled for auto leaf queue creation, supports
+ the configuration of template parameters for automatic configuration of the
auto-created leaf queues. The auto-created queues support all the
+ leaf queue configuration parameters. There is one caveat to the QueueACLs: at
auto queue creation the Queue ACLs in the leaf queue templates are not in
effect yet,
Review comment:
at auto queue creation -> I would change this to "at the time of queue
auto creation" so that it refers more to the point (in time) when the queue is
being created and not the topic of auto queue creation.
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java
##########
@@ -395,25 +398,209 @@ public void
testQueueSubmitWithACLsEnabledWithQueueMappingForAutoCreatedQueue()
RMContext newMockRMContext = newMockRM.getRMContext();
newMockRMContext.setQueuePlacementManager(createMockPlacementManager(
- "user1|user2", "user1", "managedparent"));
+ "user1|user2", "user1", "root.managedparent"));
TestRMAppManager newAppMonitor = createAppManager(newMockRMContext, conf);
- newAppMonitor.submitApplication(asContext, "user1");
- RMApp app = newMockRMContext.getRMApps().get(appId);
- Assert.assertNotNull("app should not be null", app);
- Assert.assertEquals("the queue should be placed on 'managedparent.user1'
queue",
- "managedparent.user1",
- app.getQueue());
+ ApplicationSubmissionContext submission =
createAppSubmissionContext(MockApps.newAppID(1));
+ submission.setQueue("oldQueue");
+ verifyAppSubmission(submission,
+ newAppMonitor,
+ newMockRMContext,
+ "user1",
+ "root.managedparent.user1");
+
+ verifyAppSubmissionFailure(newAppMonitor,
+ createAppSubmissionContext(MockApps.newAppID(2)),
+ "user2");
+ }
+
+ @Test
+ public void testLegacyAutoCreatedQueuesWithACLTemplates()
+ throws IOException, YarnException {
+ YarnConfiguration conf = new YarnConfiguration(new Configuration(false));
+ conf.set(YarnConfiguration.YARN_ACL_ENABLE, "true");
+ conf.setClass(YarnConfiguration.RM_SCHEDULER, CapacityScheduler.class,
+ ResourceScheduler.class);
+ CapacitySchedulerConfiguration csConf = new CapacitySchedulerConfiguration(
+ conf, false);
+ csConf.set(PREFIX + "root.queues", "parent");
+ csConf.set(PREFIX + "root.acl_submit_applications", " ");
+ csConf.set(PREFIX + "root.acl_administer_queue", " ");
+
+ csConf.setCapacity("root.parent", 100.0f);
+ csConf.set(PREFIX + "root.parent.acl_administer_queue", "user1,user4");
+ csConf.set(PREFIX + "root.parent.acl_submit_applications", "user1,user4");
+ conf.set("yarn.scheduler.capacity.queue-mappings", "u:%user:parent.%user");
+
+ csConf.setAutoCreateChildQueueEnabled("root.parent", true);
+ csConf.setAutoCreatedLeafQueueConfigCapacity("root.parent", 50f);
+ csConf.setAutoCreatedLeafQueueConfigMaxCapacity("root.parent", 100f);
+
csConf.set(getQueuePrefix(csConf.getAutoCreatedQueueTemplateConfPrefix("root.parent"))
+
+ "acl_administer_queue", "user2,user4");
+
csConf.set(getQueuePrefix(csConf.getAutoCreatedQueueTemplateConfPrefix("root.parent"))
+
+ "acl_submit_applications", "user2,user4");
+
+ MockRM newMockRM = new MockRM(csConf);
+
+ RMContext newMockRMContext = newMockRM.getRMContext();
+ TestRMAppManager newAppMonitor = createAppManager(newMockRMContext, conf);
+
+ // user1 has permission on root.parent so a queue would be created
+ newMockRMContext.setQueuePlacementManager(createMockPlacementManager(
+ "user1", "user1", "root.parent"));
+ verifyAppSubmission(createAppSubmissionContext(MockApps.newAppID(1)),
+ newAppMonitor,
+ newMockRMContext,
+ "user1",
+ "root.parent.user1");
+
+ newMockRMContext.setQueuePlacementManager(createMockPlacementManager(
+ "user1|user2|user3|user4", "user2", "root.parent"));
+
+ // user2 doesn't have permission on root.parent so no queue will be created
+ verifyAppSubmissionFailure(newAppMonitor,
+ createAppSubmissionContext(MockApps.newAppID(2)),
+ "user2");
+
+ // user4 has permission on root.parent so a queue would be created
+ verifyAppSubmission(createAppSubmissionContext(MockApps.newAppID(3)),
+ newAppMonitor,
+ newMockRMContext,
+ "user4",
+ "root.parent.user2");
+
+ // create the root.parent.user2 manually, because
+ // TestAppManager.submitApplication doesn't lead to queue creation
+ CapacityScheduler cs =
+ ((CapacityScheduler) newMockRM.getResourceScheduler());
+ cs.getCapacitySchedulerQueueManager().createQueue(new
QueuePath("root.parent.user2"));
+ AutoCreatedLeafQueue autoCreatedLeafQueue = (AutoCreatedLeafQueue)
cs.getQueue("user2");
+ Assert.assertNotNull("Auto Creation of Queue failed",
autoCreatedLeafQueue);
+ ManagedParentQueue parentQueue = (ManagedParentQueue)
cs.getQueue("parent");
+ assertEquals(parentQueue, autoCreatedLeafQueue.getParent());
+ // reinitialize to load the ACLs for the queue
+ cs.reinitialize(csConf, newMockRMContext);
+
+ // since the root.parent.user2 queue already exists the app can be
submitted with user2
+ verifyAppSubmission(createAppSubmissionContext(MockApps.newAppID(4)),
+ newAppMonitor,
+ newMockRMContext,
+ "user2",
+ "root.parent.user2");
+
+ // user3 doesn't have permission for root.parent.user2 queue
+ verifyAppSubmissionFailure(newAppMonitor,
+ createAppSubmissionContext(MockApps.newAppID(5)),
+ "user3");
+
+ // user1 doesn't have permission for root.parent.user2 queue, but it has
for root.parent
+ verifyAppSubmission(createAppSubmissionContext(MockApps.newAppID(6)),
+ newAppMonitor,
+ newMockRMContext,
+ "user1",
+ "root.parent.user2");
+ }
+
+ @Test
+ public void testFlexibleAutoCreatedQueuesWithACLTemplates()
Review comment:
Did you maybe check the parent auto creation feature?
##########
File path:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/CapacityScheduler.md
##########
@@ -640,11 +640,11 @@ support other pre-configured queues to co-exist along
with auto-created queues.
* Configuring **legacy** `Auto-Created Leaf Queues` with `CapacityScheduler`
-The parent queue which has been enabled for auto leaf queue creation,supports
- the configuration of template parameters for automatic configuration of the
auto-created leaf queues. The auto-created queues support all of the
- leaf queue configuration parameters except for **Queue ACL**, **Absolute
- Resource** configurations. Queue ACLs are
- currently inherited from the parent queue i.e they are not configurable on
the leaf queue template
+The parent queue which has been enabled for auto leaf queue creation, supports
+ the configuration of template parameters for automatic configuration of the
auto-created leaf queues. The auto-created queues support all the
+ leaf queue configuration parameters. There is one caveat to the QueueACLs: at
auto queue creation the Queue ACLs in the leaf queue templates are not in
effect yet,
+ the parent's QueueACLs determines whether the queue can be created by the
user. When the auto-created leaf queue already exists then the QueueACLs in the
leaf queue template
Review comment:
Technically the queue is automatically created by CS during the app
submission, so this "whether the queue can be created by the user" might sound
a bit confusing. I think we should mention the connection between app
submission+queue acls+queue creation here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
