From 4d1f3d9020b8a8bf1d2a81e4d6ad20418ed9bcc2 Mon Sep 17 00:00:00 2001 From: Subru Krishnan Date: Tue, 17 Jan 2017 14:47:27 -0800 Subject: [PATCH] YARN-6016. Fix minor bugs in handling of local AMRMToken in AMRMProxy. (Botong Huang via Subru). --- .../hadoop/yarn/client/api/impl/TestAMRMProxy.java | 11 ++++++----- .../amrmproxy/AMRMProxyApplicationContextImpl.java | 2 +- .../nodemanager/amrmproxy/AMRMProxyService.java | 5 +++++ .../amrmproxy/MockResourceManagerFacade.java | 7 ++++++- .../nodemanager/amrmproxy/TestAMRMProxyService.java | 13 +++++++++++++ 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMProxy.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMProxy.java index 9eef9a076b..14df94abf3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMProxy.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMProxy.java @@ -139,11 +139,11 @@ public void testAMRMProxyE2E() throws Exception { /* * This test validates the token renewal from the AMRMPRoxy. The test verifies - * that the received token it is different from the previous one within 5 - * requests. + * that the received token from AMRMProxy is different from the previous one + * within 5 requests. */ @Test(timeout = 120000) - public void testE2ETokenRenewal() throws Exception { + public void testAMRMProxyTokenRenewal() throws Exception { ApplicationMasterProtocol client; try (MiniYARNCluster cluster = @@ -176,7 +176,8 @@ public void testE2ETokenRenewal() throws Exception { client.registerApplicationMaster(RegisterApplicationMasterRequest .newInstance(NetUtils.getHostname(), 1024, "")); - LOG.info("testAMRMPRoxy - Allocate Resources Application Master"); + LOG.info( + "testAMRMProxyTokenRenewal - Allocate Resources Application Master"); AllocateRequest request = createAllocateRequest(rmClient.getNodeReports(NodeState.RUNNING)); @@ -196,7 +197,7 @@ public void testE2ETokenRenewal() throws Exception { lastToken = response.getAMRMToken(); - // Time slot to be sure the RM renew the token + // Time slot to be sure the AMRMProxy renew the token Thread.sleep(1500); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyApplicationContextImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyApplicationContextImpl.java index 2e5aa94128..6d4fdfc3dc 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyApplicationContextImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyApplicationContextImpl.java @@ -115,7 +115,7 @@ public synchronized int getLocalAMRMTokenKeyId() { throw new YarnRuntimeException("Missing AMRM token for " + this.applicationAttemptId); } - keyId = this.amrmToken.decodeIdentifier().getKeyId(); + keyId = this.localToken.decodeIdentifier().getKeyId(); this.localTokenKeyId = keyId; } catch (IOException e) { throw new YarnRuntimeException("AMRM token decode error for " diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java index dc56090afd..5e91a20498 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java @@ -342,9 +342,14 @@ private void updateAMRMTokens(AMRMTokenIdentifier amrmTokenIdentifier, // check to see if the RM has issued a new AMRMToken & accordingly update // the real ARMRMToken in the current context if (allocateResponse.getAMRMToken() != null) { + LOG.info("RM rolled master-key for amrm-tokens"); + org.apache.hadoop.yarn.api.records.Token token = allocateResponse.getAMRMToken(); + // Do not propagate this info back to AM + allocateResponse.setAMRMToken(null); + org.apache.hadoop.security.token.Token newTokenId = new org.apache.hadoop.security.token.Token( token.getIdentifier().array(), token.getPassword().array(), diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/MockResourceManagerFacade.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/MockResourceManagerFacade.java index c69313f66a..f584c94f7f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/MockResourceManagerFacade.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/MockResourceManagerFacade.java @@ -108,6 +108,7 @@ import org.apache.hadoop.yarn.api.records.NodeId; import org.apache.hadoop.yarn.api.records.NodeReport; import org.apache.hadoop.yarn.api.records.ResourceRequest; +import org.apache.hadoop.yarn.api.records.Token; import org.apache.hadoop.yarn.api.records.UpdatedContainer; import org.apache.hadoop.yarn.api.records.YarnApplicationAttemptState; import org.apache.hadoop.yarn.api.records.YarnApplicationState; @@ -297,10 +298,14 @@ public AllocateResponse allocate(AllocateRequest request) Log.getLog().info("Allocating containers: " + containerList.size() + " for application attempt: " + conf.get("AMRMTOKEN")); + + // Always issue a new AMRMToken as if RM rolled master key + Token newAMRMToken = Token.newInstance(new byte[0], "", new byte[0], ""); + return AllocateResponse.newInstance(0, new ArrayList(), containerList, new ArrayList(), null, AMCommand.AM_RESYNC, 1, null, - new ArrayList(), + new ArrayList(), newAMRMToken, new ArrayList()); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyService.java index 69b913a7c5..7fffddf683 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyService.java @@ -401,6 +401,9 @@ private List getContainersAndAssert(int appId, AllocateResponse allocateResponse = allocate(appId, allocateRequest); Assert.assertNotNull("allocate() returned null response", allocateResponse); + Assert.assertNull( + "new AMRMToken from RM should have been nulled by AMRMProxyService", + allocateResponse.getAMRMToken()); containers.addAll(allocateResponse.getAllocatedContainers()); @@ -412,6 +415,9 @@ private List getContainersAndAssert(int appId, allocate(appId, Records.newRecord(AllocateRequest.class)); Assert.assertNotNull("allocate() returned null response", allocateResponse); + Assert.assertNull( + "new AMRMToken from RM should have been nulled by AMRMProxyService", + allocateResponse.getAMRMToken()); containers.addAll(allocateResponse.getAllocatedContainers()); @@ -447,6 +453,9 @@ private void releaseContainersAndAssert(int appId, AllocateResponse allocateResponse = allocate(appId, allocateRequest); Assert.assertNotNull(allocateResponse); + Assert.assertNull( + "new AMRMToken from RM should have been nulled by AMRMProxyService", + allocateResponse.getAMRMToken()); // The way the mock resource manager is setup, it will return the containers // that were released in the response. This is done because the UAMs run @@ -467,6 +476,10 @@ private void releaseContainersAndAssert(int appId, allocateResponse = allocate(appId, Records.newRecord(AllocateRequest.class)); Assert.assertNotNull(allocateResponse); + Assert.assertNull( + "new AMRMToken from RM should have been nulled by AMRMProxyService", + allocateResponse.getAMRMToken()); + containersForReleasedContainerIds.addAll(allocateResponse .getAllocatedContainers());