YARN-2065 AM cannot create new containers after restart

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1607440 13f79535-47bb-0310-9956-ffa450edef68

hadoop-yarn-project/CHANGES.txt

@@ -303,6 +303,9 @@ Release 2.5.0 - UNRELEASED
     YARN-2216 TestRMApplicationHistoryWriter sometimes fails in trunk.
     (Zhijie Shen via xgong)
 
+    YARN-2216 YARN-2065 AM cannot create new containers after restart
+    (Jian He via stevel)
+
 Release 2.4.1 - 2014-06-23 
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java

@@ -475,8 +475,8 @@ public class ContainerManagerImpl extends CompositeService implements
     boolean unauthorized = false;
     StringBuilder messageBuilder =
         new StringBuilder("Unauthorized request to start container. ");
-    if (!nmTokenIdentifier.getApplicationAttemptId().equals(
-        containerId.getApplicationAttemptId())) {
+    if (!nmTokenIdentifier.getApplicationAttemptId().getApplicationId().equals(
+        containerId.getApplicationAttemptId().getApplicationId())) {
       unauthorized = true;
       messageBuilder.append("\nNMToken for application attempt : ")
         .append(nmTokenIdentifier.getApplicationAttemptId())
@@ -810,26 +810,24 @@ public class ContainerManagerImpl extends CompositeService implements
      * belongs to the same application attempt (NMToken) which was used. (Note:-
      * This will prevent user in knowing another application's containers).
      */
-
-    if ((!identifier.getApplicationAttemptId().equals(
-      containerId.getApplicationAttemptId()))
-        || (container != null && !identifier.getApplicationAttemptId().equals(
-          container.getContainerId().getApplicationAttemptId()))) {
+    ApplicationId nmTokenAppId =
+        identifier.getApplicationAttemptId().getApplicationId();
+    if ((!nmTokenAppId.equals(containerId.getApplicationAttemptId().getApplicationId()))
+        || (container != null && !nmTokenAppId.equals(container
+          .getContainerId().getApplicationAttemptId().getApplicationId()))) {
       if (stopRequest) {
         LOG.warn(identifier.getApplicationAttemptId()
             + " attempted to stop non-application container : "
-            + container.getContainerId().toString());
+            + container.getContainerId());
         NMAuditLogger.logFailure("UnknownUser", AuditConstants.STOP_CONTAINER,
           "ContainerManagerImpl", "Trying to stop unknown container!",
-          identifier.getApplicationAttemptId().getApplicationId(),
-          container.getContainerId());
+          nmTokenAppId, container.getContainerId());
       } else {
         LOG.warn(identifier.getApplicationAttemptId()
             + " attempted to get status for non-application container : "
-            + container.getContainerId().toString());
+            + container.getContainerId());
       }
     }
-
   }
 
   class ContainerEventDispatcher implements EventHandler<ContainerEvent> {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests/src/test/java/org/apache/hadoop/yarn/server/TestContainerManagerSecurity.java

@@ -202,8 +202,6 @@ public class TestContainerManagerSecurity extends KerberosSecurityTestcase {
     ApplicationId appId = ApplicationId.newInstance(1, 1);
     ApplicationAttemptId validAppAttemptId =
         ApplicationAttemptId.newInstance(appId, 1);
-    ApplicationAttemptId invalidAppAttemptId =
-        ApplicationAttemptId.newInstance(appId, 2);
     
     ContainerId validContainerId =
         ContainerId.newInstance(validAppAttemptId, 0);
@@ -269,26 +267,14 @@ public class TestContainerManagerSecurity extends KerberosSecurityTestcase {
         testStartContainer(rpc, validAppAttemptId, validNode,
             validContainerToken, invalidNMToken, true)));
     
-    // using appAttempt-2 token for launching container for appAttempt-1.
-    invalidNMToken =
-        nmTokenSecretManagerRM.createNMToken(invalidAppAttemptId, validNode,
-            user);
-    sb = new StringBuilder("\nNMToken for application attempt : ");
-    sb.append(invalidAppAttemptId.toString())
-      .append(" was used for starting container with container token")
-      .append(" issued for application attempt : ")
-      .append(validAppAttemptId.toString());
-    Assert.assertTrue(testStartContainer(rpc, validAppAttemptId, validNode,
-        validContainerToken, invalidNMToken, true).contains(sb.toString()));
-    
     // using correct tokens. nmtoken for app attempt should get saved.
     conf.setInt(YarnConfiguration.RM_CONTAINER_ALLOC_EXPIRY_INTERVAL_MS,
         4 * 60 * 1000);
     validContainerToken =
         containerTokenSecretManager.createContainerToken(validContainerId,
             validNode, user, r, Priority.newInstance(0), 0);
-    testStartContainer(rpc, validAppAttemptId, validNode, validContainerToken,
-        validNMToken, false);
+    Assert.assertTrue(testStartContainer(rpc, validAppAttemptId, validNode,
+      validContainerToken, validNMToken, false).isEmpty());
     Assert.assertTrue(nmTokenSecretManagerNM
         .isAppAttemptNMTokenKeyPresent(validAppAttemptId));
     
@@ -330,6 +316,18 @@ public class TestContainerManagerSecurity extends KerberosSecurityTestcase {
     Assert.assertTrue(testGetContainer(rpc, validAppAttemptId, validNode,
         validContainerId, validNMToken, false).contains(sb.toString()));
 
+    // using appAttempt-1 NMtoken for launching container for appAttempt-2 should
+    // succeed.
+    ApplicationAttemptId attempt2 = ApplicationAttemptId.newInstance(appId, 2);
+    Token attempt1NMToken =
+        nmTokenSecretManagerRM
+          .createNMToken(validAppAttemptId, validNode, user);
+    org.apache.hadoop.yarn.api.records.Token newContainerToken =
+        containerTokenSecretManager.createContainerToken(
+          ContainerId.newInstance(attempt2, 1), validNode, user, r,
+            Priority.newInstance(0), 0);
+    Assert.assertTrue(testStartContainer(rpc, attempt2, validNode,
+      newContainerToken, attempt1NMToken, false).isEmpty());
   }
 
   private void waitForContainerToFinishOnNM(ContainerId containerId) {