YARN-4127. RM fail with noAuth error if switched from failover to non-failover. Contributed by Varun Saxena

hadoop-yarn-project/CHANGES.txt

@@ -1039,6 +1039,9 @@ Release 2.8.0 - UNRELEASED
     YARN-4288. Fixed RMProxy to retry on IOException from local host.
     (Junping Du via jianhe)
 
+    YARN-4127. RM fail with noAuth error if switched from failover to non-failover.
+    (Varun Saxena via jianhe)
+
 Release 2.7.2 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/recovery/ZKRMStateStore.java

@@ -282,8 +282,9 @@ public class ZKRMStateStore extends RMStateStore {
     // ensure root dirs exist
     createRootDirRecursively(znodeWorkingPath);
     create(zkRootNodePath);
+    setRootNodeAcls();
+    delete(fencingNodePath);
     if (HAUtil.isHAEnabled(getConfig())) {
-      fence();
       verifyActiveStatusThread = new VerifyActiveStatusThread();
       verifyActiveStatusThread.start();
     }
@@ -309,16 +310,19 @@ public class ZKRMStateStore extends RMStateStore {
     LOG.debug(builder.toString());
   }
 
-  private synchronized void fence() throws Exception {
-    if (LOG.isTraceEnabled()) {
-      logRootNodeAcls("Before fencing\n");
+  private void setRootNodeAcls() throws Exception {
+    if (LOG.isDebugEnabled()) {
+      logRootNodeAcls("Before setting ACLs'\n");
     }
 
+    if (HAUtil.isHAEnabled(getConfig())) {
       curatorFramework.setACL().withACL(zkRootNodeAcl).forPath(zkRootNodePath);
-    delete(fencingNodePath);
+    } else {
+      curatorFramework.setACL().withACL(zkAcl).forPath(zkRootNodePath);
+    }
 
-    if (LOG.isTraceEnabled()) {
-      logRootNodeAcls("After fencing\n");
+    if (LOG.isDebugEnabled()) {
+      logRootNodeAcls("After setting ACLs'\n");
     }
   }
 
@@ -933,7 +937,8 @@ public class ZKRMStateStore extends RMStateStore {
     return curatorFramework.getData().forPath(path);
   }
 
-  private List<ACL> getACL(final String path) throws Exception {
+  @VisibleForTesting
+  List<ACL> getACL(final String path) throws Exception {
     return curatorFramework.getACL().forPath(path);
   }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/recovery/TestZKRMStateStore.java

@@ -54,6 +54,8 @@ import org.apache.hadoop.yarn.server.resourcemanager.rmapp.attempt.RMAppAttemptS
 import org.apache.hadoop.yarn.server.resourcemanager.security.ClientToAMTokenSecretManagerInRM;
 import org.apache.hadoop.yarn.util.ConverterUtils;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Perms;
+import org.apache.zookeeper.data.ACL;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -65,6 +67,8 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.io.IOException;
+import java.util.List;
+
 import javax.crypto.SecretKey;
 
 public class TestZKRMStateStore extends RMStateStoreTestBase {
@@ -248,6 +252,70 @@ public class TestZKRMStateStore extends RMStateStoreTestBase {
     return conf;
   }
 
+  private static boolean verifyZKACL(String id, String scheme, int perm,
+      List<ACL> acls) {
+    for (ACL acl : acls) {
+      if (acl.getId().getScheme().equals(scheme) &&
+          acl.getId().getId().startsWith(id) &&
+          acl.getPerms() == perm) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Test if RM can successfully start in HA disabled mode if it was previously
+   * running in HA enabled mode. And then start it in HA mode after running it
+   * with HA disabled. NoAuth Exception should not be sent by zookeeper and RM
+   * should start successfully.
+   */
+  @Test
+  public void testZKRootPathAcls() throws Exception {
+    StateChangeRequestInfo req = new StateChangeRequestInfo(
+        HAServiceProtocol.RequestSource.REQUEST_BY_USER);
+    String rootPath =
+        YarnConfiguration.DEFAULT_ZK_RM_STATE_STORE_PARENT_PATH + "/" +
+            ZKRMStateStore.ROOT_ZNODE_NAME;
+
+    // Start RM with HA enabled
+    Configuration conf = createHARMConf("rm1,rm2", "rm1", 1234);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_ENABLED, false);
+    ResourceManager rm = new MockRM(conf);
+    rm.start();
+    rm.getRMContext().getRMAdminService().transitionToActive(req);
+    List<ACL> acls =
+        ((ZKRMStateStore)rm.getRMContext().getStateStore()).getACL(rootPath);
+    assertEquals(acls.size(), 2);
+    // CREATE and DELETE permissions for root node based on RM ID
+    verifyZKACL("digest", "localhost", Perms.CREATE | Perms.DELETE, acls);
+    verifyZKACL(
+        "world", "anyone", Perms.ALL ^ (Perms.CREATE | Perms.DELETE), acls);
+    rm.close();
+
+    // Now start RM with HA disabled. NoAuth Exception should not be thrown.
+    conf.setBoolean(YarnConfiguration.RM_HA_ENABLED, false);
+    rm = new MockRM(conf);
+    rm.start();
+    rm.getRMContext().getRMAdminService().transitionToActive(req);
+    acls = ((ZKRMStateStore)rm.getRMContext().getStateStore()).getACL(rootPath);
+    assertEquals(acls.size(), 1);
+    verifyZKACL("world", "anyone", Perms.ALL, acls);
+    rm.close();
+
+    // Start RM with HA enabled.
+    conf.setBoolean(YarnConfiguration.RM_HA_ENABLED, true);
+    rm = new MockRM(conf);
+    rm.start();
+    rm.getRMContext().getRMAdminService().transitionToActive(req);
+    acls = ((ZKRMStateStore)rm.getRMContext().getStateStore()).getACL(rootPath);
+    assertEquals(acls.size(), 2);
+    verifyZKACL("digest", "localhost", Perms.CREATE | Perms.DELETE, acls);
+    verifyZKACL(
+        "world", "anyone", Perms.ALL ^ (Perms.CREATE | Perms.DELETE), acls);
+    rm.close();
+  }
+
   @SuppressWarnings("unchecked")
   @Test
   public void testFencing() throws Exception {