YARN-6016. Fix minor bugs in handling of local AMRMToken in AMRMProxy. (Botong Huang via Subru).

(cherry picked from commit 4d1f3d9020)

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
+   * that the received token from AMRMProxy is different from the previous one
-   * requests.
+   * 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);
 
       }

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 "

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<AMRMTokenIdentifier> newTokenId =
           new org.apache.hadoop.security.token.Token<AMRMTokenIdentifier>(
               token.getIdentifier().array(), token.getPassword().array(),

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<ContainerStatus>(), containerList,
         new ArrayList<NodeReport>(), null, AMCommand.AM_RESYNC, 1, null,
-        new ArrayList<NMToken>(),
+        new ArrayList<NMToken>(), newAMRMToken,
         new ArrayList<UpdatedContainer>());
   }
 

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());