From fb639aed90cb25ec3ba5b743a7db898c097cfafc 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). (cherry picked from commit 4d1f3d9020b8a8bf1d2a81e4d6ad20418ed9bcc2) --- .../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 9eef9a076b9..14df94abf35 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 class TestAMRMProxy extends BaseAMRMProxyE2ETest { /* * 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 class TestAMRMProxy extends BaseAMRMProxyE2ETest { 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 class TestAMRMProxy extends BaseAMRMProxyE2ETest { 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 2e5aa94128b..6d4fdfc3dc9 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 class AMRMProxyApplicationContextImpl implements 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 dc56090afd2..5e91a204988 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 @@ public class AMRMProxyService extends AbstractService implements // 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 b530fb48c31..499a5cbc37b 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.NMToken; 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; @@ -296,10 +297,14 @@ public class MockResourceManagerFacade implements Log.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 69b913a7c5c..7fffddf683b 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 @@ public class TestAMRMProxyService extends BaseAMRMProxyTest { 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 @@ public class TestAMRMProxyService extends BaseAMRMProxyTest { 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 @@ public class TestAMRMProxyService extends BaseAMRMProxyTest { 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 @@ public class TestAMRMProxyService extends BaseAMRMProxyTest { 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());