luoyuan3471 commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4511376204
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException,
IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void
testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
+ ContainerId containerId, boolean keepContainersAcrossAttempts,
+ boolean includeAttempt, boolean includeContainer) {
+ ApplicationAttemptId attemptId = containerId.getApplicationAttemptId();
+ ApplicationId applicationId = attemptId.getApplicationId();
+ ResourceScheduler scheduler = mock(ResourceScheduler.class);
+ RMContext rmContext = mock(RMContext.class);
+ RMApp application = mock(RMApp.class);
+
+ ConcurrentHashMap<ApplicationId, RMApp> applications =
+ new ConcurrentHashMap<>();
+ applications.put(applicationId, application);
+ when(rmContext.getRMApps()).thenReturn(applications);
+ when(rmContext.getScheduler()).thenReturn(scheduler);
+ when(application.getApplicationId()).thenReturn(applicationId);
+ when(application.getUser()).thenReturn("test-user");
+
+ ApplicationSubmissionContext submissionContext =
+ mock(ApplicationSubmissionContext.class);
+ when(submissionContext.getKeepContainersAcrossApplicationAttempts())
+ .thenReturn(keepContainersAcrossAttempts);
+ when(application.getApplicationSubmissionContext())
+ .thenReturn(submissionContext);
+
+ Map<ApplicationAttemptId, RMAppAttempt> attempts = new HashMap<>();
+ if (includeAttempt) {
+ attempts.put(attemptId, mock(RMAppAttempt.class));
+ }
+ when(application.getAppAttempts()).thenReturn(attempts);
+
+ if (includeContainer) {
+ RMContainer rmContainer = mock(RMContainer.class);
+ ContainerReport containerReport = mock(ContainerReport.class);
+ when(containerReport.getContainerId()).thenReturn(containerId);
+ when(rmContainer.createContainerReport()).thenReturn(containerReport);
+ when(scheduler.getRMContainer(containerId)).thenReturn(rmContainer);
+ }
+
+ ApplicationACLsManager applicationACLsManager =
+ mock(ApplicationACLsManager.class);
+ QueueACLsManager queueACLsManager = mock(QueueACLsManager.class);
+ when(queueACLsManager.checkAccess(any(UserGroupInformation.class),
Review Comment:
Agreed, we should also verify that access is still denied on this path. I've
added an access flag to the helper and a keepContainers test that checks the
request is rejected with the “does not have privilege” message.
--
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]
