YARN-5685. RM configuration allows all failover methods to disabled when automatic failover is enabled

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/HAUtil.java

@@ -34,6 +34,7 @@ import java.util.Collection;
 public class HAUtil {
   private static Log LOG = LogFactory.getLog(HAUtil.class);
 
+  @VisibleForTesting
   public static final String BAD_CONFIG_MESSAGE_PREFIX =
     "Invalid configuration! ";
 
@@ -79,6 +80,7 @@ public class HAUtil {
     throws YarnRuntimeException {
     verifyAndSetRMHAIdsList(conf);
     verifyAndSetCurrentRMHAId(conf);
+    verifyLeaderElection(conf);
     verifyAndSetAllServiceAddresses(conf);
   }
 
@@ -117,7 +119,7 @@ public class HAUtil {
       msg.append("Can not find valid RM_HA_ID. None of ");
       for (String id : conf
           .getTrimmedStringCollection(YarnConfiguration.RM_HA_IDS)) {
-        msg.append(addSuffix(YarnConfiguration.RM_ADDRESS, id) + " ");
+        msg.append(addSuffix(YarnConfiguration.RM_ADDRESS, id)).append(" ");
       }
       msg.append(" are matching" +
           " the local address OR " + YarnConfiguration.RM_HA_ID + " is not" +
@@ -133,6 +135,32 @@ public class HAUtil {
     conf.set(YarnConfiguration.RM_HA_ID, rmId);
   }
 
+  /**
+   * This method validates that some leader election service is enabled. YARN
+   * allows leadership election to be disabled in the configuration, which
+   * breaks automatic failover. If leadership election is disabled, this
+   * method will throw an exception via
+   * {@link #throwBadConfigurationException(java.lang.String)}.
+   *
+   * @param conf the {@link Configuration} to validate
+   */
+  private static void verifyLeaderElection(Configuration conf) {
+    if (isAutomaticFailoverEnabled(conf) &&
+        !conf.getBoolean(YarnConfiguration.CURATOR_LEADER_ELECTOR,
+          YarnConfiguration.DEFAULT_CURATOR_LEADER_ELECTOR_ENABLED) &&
+        !isAutomaticFailoverEmbedded(conf)) {
+      throwBadConfigurationException(NO_LEADER_ELECTION_MESSAGE);
+    }
+  }
+
+  @VisibleForTesting
+  static final String NO_LEADER_ELECTION_MESSAGE =
+      "The yarn.resourcemanager.ha.automatic-failover.embedded "
+      + "and yarn.resourcemanager.ha.curator-leader-elector.enabled "
+      + "properties are both false. One of these two properties must "
+      + "be true when yarn.resourcemanager.ha.automatic-failover.enabled "
+      + "is true";
+
   private static void verifyAndSetConfValue(String prefix, Configuration conf) {
     String confKey = null;
     String confValue = null;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java

@@ -627,8 +627,22 @@ public class YarnConfiguration extends Configuration {
       AUTO_FAILOVER_PREFIX + "enabled";
   public static final boolean DEFAULT_AUTO_FAILOVER_ENABLED = true;
 
+  /**
+   * This property controls whether {@link ActiveStandbyElector} leader
+   * election should be used when {@link #CURATOR_LEADER_ELECTOR} is
+   * {@code false}.
+   *
+   * @deprecated This property should never be set to {@code false}.
+   */
+  @Deprecated
   public static final String AUTO_FAILOVER_EMBEDDED =
       AUTO_FAILOVER_PREFIX + "embedded";
+  /**
+   * The default value for {@link #AUTO_FAILOVER_EMBEDDED}.
+   *
+   * @deprecated The {@link #AUTO_FAILOVER_EMBEDDED} property is deprecated.
+   */
+  @Deprecated
   public static final boolean DEFAULT_AUTO_FAILOVER_EMBEDDED = true;
 
   public static final String AUTO_FAILOVER_ZK_BASE_PATH =
@@ -667,7 +681,7 @@ public class YarnConfiguration extends Configuration {
 
 
   /**
-   * Whether to use curator-based elector for leader election.
+   * Whether to use the Curator-based elector for leader election.
    *
    * @deprecated Eventually, we want to default to the curator-based
    * implementation and remove the {@link ActiveStandbyElector} based

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/conf/TestHAUtil.java

@@ -85,44 +85,47 @@ public class TestHAUtil {
 
   @Test
   public void testVerifyAndSetConfiguration() throws Exception {
+    Configuration myConf = new Configuration(conf);
+
     try {
-      HAUtil.verifyAndSetConfiguration(conf);
+      HAUtil.verifyAndSetConfiguration(myConf);
     } catch (YarnRuntimeException e) {
       fail("Should not throw any exceptions.");
     }
 
     assertEquals("Should be saved as Trimmed collection",
-      StringUtils.getStringCollection(RM_NODE_IDS), HAUtil.getRMHAIds(conf));
+        StringUtils.getStringCollection(RM_NODE_IDS),
+        HAUtil.getRMHAIds(myConf));
     assertEquals("Should be saved as Trimmed string",
-      RM1_NODE_ID, HAUtil.getRMHAId(conf));
+        RM1_NODE_ID, HAUtil.getRMHAId(myConf));
-    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(conf)) {
+    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(myConf)) {
       assertEquals("RPC address not set for " + confKey,
-        RM1_ADDRESS, conf.get(confKey));
+          RM1_ADDRESS, myConf.get(confKey));
     }
 
-    conf.clear();
+    myConf = new Configuration(conf);
-    conf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID);
+    myConf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID);
     try {
-      HAUtil.verifyAndSetConfiguration(conf);
+      HAUtil.verifyAndSetConfiguration(myConf);
     } catch (YarnRuntimeException e) {
       assertEquals("YarnRuntimeException by verifyAndSetRMHAIds()",
         HAUtil.BAD_CONFIG_MESSAGE_PREFIX +
           HAUtil.getInvalidValueMessage(YarnConfiguration.RM_HA_IDS,
-              conf.get(YarnConfiguration.RM_HA_IDS) +
+              myConf.get(YarnConfiguration.RM_HA_IDS) +
               "\nHA mode requires atleast two RMs"),
         e.getMessage());
     }
 
-    conf.clear();
+    myConf = new Configuration(conf);
     // simulate the case YarnConfiguration.RM_HA_ID is not set
-    conf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID + ","
+    myConf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID + ","
         + RM2_NODE_ID);
-    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(conf)) {
+    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(myConf)) {
-      conf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS);
+      myConf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS);
-      conf.set(HAUtil.addSuffix(confKey, RM2_NODE_ID), RM2_ADDRESS);
+      myConf.set(HAUtil.addSuffix(confKey, RM2_NODE_ID), RM2_ADDRESS);
     }
     try {
-      HAUtil.verifyAndSetConfiguration(conf);
+      HAUtil.verifyAndSetConfiguration(myConf);
     } catch (YarnRuntimeException e) {
       assertEquals("YarnRuntimeException by getRMId()",
         HAUtil.BAD_CONFIG_MESSAGE_PREFIX +
@@ -130,16 +133,16 @@ public class TestHAUtil {
         e.getMessage());
     }
 
-    conf.clear();
+    myConf = new Configuration(conf);
-    conf.set(YarnConfiguration.RM_HA_ID, RM_INVALID_NODE_ID);
+    myConf.set(YarnConfiguration.RM_HA_ID, RM_INVALID_NODE_ID);
-    conf.set(YarnConfiguration.RM_HA_IDS, RM_INVALID_NODE_ID + ","
+    myConf.set(YarnConfiguration.RM_HA_IDS, RM_INVALID_NODE_ID + ","
         + RM1_NODE_ID);
-    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(conf)) {
+    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(myConf)) {
       // simulate xml with invalid node id
-      conf.set(confKey + RM_INVALID_NODE_ID, RM_INVALID_NODE_ID);
+      myConf.set(confKey + RM_INVALID_NODE_ID, RM_INVALID_NODE_ID);
     }
     try {
-      HAUtil.verifyAndSetConfiguration(conf);
+      HAUtil.verifyAndSetConfiguration(myConf);
     } catch (YarnRuntimeException e) {
       assertEquals("YarnRuntimeException by addSuffix()",
         HAUtil.BAD_CONFIG_MESSAGE_PREFIX +
@@ -148,12 +151,12 @@ public class TestHAUtil {
         e.getMessage());
     }
 
-    conf.clear();
+    myConf = new Configuration();
     // simulate the case HAUtil.RM_RPC_ADDRESS_CONF_KEYS are not set
-    conf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID);
+    myConf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID);
-    conf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID + "," + RM2_NODE_ID);
+    myConf.set(YarnConfiguration.RM_HA_IDS, RM1_NODE_ID + "," + RM2_NODE_ID);
     try {
-      HAUtil.verifyAndSetConfiguration(conf);
+      HAUtil.verifyAndSetConfiguration(myConf);
       fail("Should throw YarnRuntimeException. by Configuration#set()");
     } catch (YarnRuntimeException e) {
       String confKey =
@@ -166,21 +169,36 @@ public class TestHAUtil {
 
     // simulate the case YarnConfiguration.RM_HA_IDS doesn't contain
     // the value of YarnConfiguration.RM_HA_ID
-    conf.clear();
+    myConf = new Configuration(conf);
-    conf.set(YarnConfiguration.RM_HA_IDS, RM2_NODE_ID + "," + RM3_NODE_ID);
+    myConf.set(YarnConfiguration.RM_HA_IDS, RM2_NODE_ID + "," + RM3_NODE_ID);
-    conf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID_UNTRIMMED);
+    myConf.set(YarnConfiguration.RM_HA_ID, RM1_NODE_ID_UNTRIMMED);
-    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(conf)) {
+    for (String confKey : YarnConfiguration.getServiceAddressConfKeys(myConf)) {
-      conf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS_UNTRIMMED);
+      myConf.set(HAUtil.addSuffix(confKey, RM1_NODE_ID), RM1_ADDRESS_UNTRIMMED);
-      conf.set(HAUtil.addSuffix(confKey, RM2_NODE_ID), RM2_ADDRESS);
+      myConf.set(HAUtil.addSuffix(confKey, RM2_NODE_ID), RM2_ADDRESS);
-      conf.set(HAUtil.addSuffix(confKey, RM3_NODE_ID), RM3_ADDRESS);
+      myConf.set(HAUtil.addSuffix(confKey, RM3_NODE_ID), RM3_ADDRESS);
     }
     try {
-      HAUtil.verifyAndSetConfiguration(conf);
+      HAUtil.verifyAndSetConfiguration(myConf);
     } catch (YarnRuntimeException e) {
       assertEquals("YarnRuntimeException by getRMId()'s validation",
         HAUtil.BAD_CONFIG_MESSAGE_PREFIX +
         HAUtil.getRMHAIdNeedToBeIncludedMessage("[rm2, rm3]", RM1_NODE_ID),
-        e.getMessage());
+          e.getMessage());
+    }
+
+    // simulate the case that no leader election is enabled
+    myConf = new Configuration(conf);
+    myConf.setBoolean(YarnConfiguration.RM_HA_ENABLED, true);
+    myConf.setBoolean(YarnConfiguration.AUTO_FAILOVER_ENABLED, true);
+    myConf.setBoolean(YarnConfiguration.AUTO_FAILOVER_EMBEDDED, false);
+    myConf.setBoolean(YarnConfiguration.CURATOR_LEADER_ELECTOR, false);
+
+    try {
+      HAUtil.verifyAndSetConfiguration(myConf);
+    } catch (YarnRuntimeException e) {
+      assertEquals("YarnRuntimeException by getRMId()'s validation",
+          HAUtil.BAD_CONFIG_MESSAGE_PREFIX + HAUtil.NO_LEADER_ELECTION_MESSAGE,
+          e.getMessage());
     }
   }
 

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

@@ -78,6 +78,8 @@ import org.junit.Test;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import org.apache.hadoop.yarn.exceptions.YarnRuntimeException;
+import static org.junit.Assert.assertTrue;
 
 public class TestRMAdminService {
 
@@ -841,6 +843,78 @@ public class TestRMAdminService {
     }
   }
 
+  /**
+   * Test that a configuration with no leader election configured fails.
+   */
+  @Test
+  public void testHAConfWithoutLeaderElection() {
+    Configuration conf = new Configuration(configuration);
+
+    conf.setBoolean(YarnConfiguration.RM_HA_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_EMBEDDED, false);
+    conf.setBoolean(YarnConfiguration.CURATOR_LEADER_ELECTOR, false);
+    conf.set(YarnConfiguration.RM_HA_IDS, "rm1,rm2");
+
+    int base = 100;
+
+    for (String confKey :
+        YarnConfiguration.getServiceAddressConfKeys(configuration)) {
+      conf.set(HAUtil.addSuffix(confKey, "rm1"), "0.0.0.0:"
+          + (base + 20));
+      conf.set(HAUtil.addSuffix(confKey, "rm2"), "0.0.0.0:"
+          + (base + 40));
+      base = base * 2;
+    }
+
+    conf.set(YarnConfiguration.RM_HA_ID, "rm1");
+
+    checkBadConfiguration(conf);
+  }
+
+  /**
+   * Test that a configuration with a single RM fails.
+   */
+  @Test
+  public void testHAConfWithSingleRMID() {
+    Configuration conf = new Configuration(configuration);
+
+    conf.setBoolean(YarnConfiguration.RM_HA_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_EMBEDDED, true);
+    conf.set(YarnConfiguration.RM_HA_IDS, "rm1");
+
+    int base = 100;
+
+    for (String confKey :
+        YarnConfiguration.getServiceAddressConfKeys(configuration)) {
+      conf.set(HAUtil.addSuffix(confKey, "rm1"), "0.0.0.0:"
+          + (base + 20));
+      base = base * 2;
+    }
+
+    conf.set(YarnConfiguration.RM_HA_ID, "rm1");
+
+    checkBadConfiguration(conf);
+  }
+
+  /**
+   * Test that a configuration with no service information fails.
+   */
+  @Test
+  public void testHAConfWithoutServiceInfo() {
+    Configuration conf = new Configuration(configuration);
+
+    conf.setBoolean(YarnConfiguration.RM_HA_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_ENABLED, true);
+    conf.setBoolean(YarnConfiguration.AUTO_FAILOVER_EMBEDDED, true);
+    conf.set(YarnConfiguration.RM_HA_IDS, "rm1,rm2");
+
+    conf.set(YarnConfiguration.RM_HA_ID, "rm1");
+
+    checkBadConfiguration(conf);
+  }
+
   @Test
   public void testRMStartsWithoutConfigurationFilesProvided() {
     // enable FileSystemBasedConfigurationProvider without uploading
@@ -1368,4 +1442,25 @@ public class TestRMAdminService {
       base = base * 2;
     }
   }
+
+  /**
+   * This method initializes an RM with the given configuration and expects it
+   * to fail with a configuration error.
+   *
+   * @param conf the {@link Configuration} to use
+   */
+  private void checkBadConfiguration(Configuration conf) {
+    MockRM rm1 = null;
+
+    conf.set(YarnConfiguration.RM_HA_ID, "rm1");
+
+    try {
+      rm1 = new MockRM(conf);
+      rm1.init(conf);
+      fail("The RM allowed an invalid configuration");
+    } catch (YarnRuntimeException e) {
+      assertTrue("The RM initialization threw an unexpected exception",
+          e.getMessage().startsWith(HAUtil.BAD_CONFIG_MESSAGE_PREFIX));
+    }
+  }
 }