AMBARI-14377 - HiveServer start fails after enabling security post EU from 2.1 to 2.3 on non-HA cluster (jonathanhurley)

ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveKerberosConfigAction.java → ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java

@@ -31,25 +31,25 @@ import org.apache.ambari.server.state.Config;
 import com.google.inject.Inject;
 
 /**
- * The {@link HiveKerberosConfigAction} is used to ensure that the following
- * settings are correctly set when upgrading a Keberized Hive Server:
+ * The {@link HiveZKQuorumConfigAction} is used to ensure that the following
+ * settings are correctly set when upgrading a Hive Server:
  * <ul>
  * <li>hive.zookeeper.quorum</li>
  * <li>hive.cluster.delegation.token.store.zookeeper.connectString</li>
  * </ul>
- *
+ * <p/>
  * This is typically only needed when upgrading from a version which does not
- * have these properties. The upgrade merge logic can't do complex calculations,
- * such as the ZK quorum.
+ * have these properties but where the Hive server is already Kerberized. The
+ * upgrade merge logic can't do complex calculations, such as the ZK quorum.
  * <p/>
- * The above properties will only be set if {@code cluster-env/security_enabled}
- * is {@code true}.
+ * The above properties will be set regardless of whether
+ * {@code cluster-env/security_enabled} is {@code true}. This is because the
+ * Kerberization wizard doesn't know to set these when Kerberizing a version of
+ * Hive that was upgraded previously. They are actually set (incorrectly) on a
+ * non-Kerberized Hive installation by the installation wizard.
  */
-public class HiveKerberosConfigAction extends AbstractServerAction {
+public class HiveZKQuorumConfigAction extends AbstractServerAction {
   protected static final String HIVE_SITE_CONFIG_TYPE = "hive-site";
-  protected static final String CLUSTER_ENV_CONFIG_TYPE = "cluster-env";
-
-  protected static final String CLUSTER_ENV_SECURITY_ENABLED = "security_enabled";
   protected static final String HIVE_SITE_ZK_QUORUM = "hive.zookeeper.quorum";
   protected static final String HIVE_SITE_ZK_CONNECT_STRING = "hive.cluster.delegation.token.store.zookeeper.connectString";
 
@@ -69,32 +69,12 @@ public class HiveKerberosConfigAction extends AbstractServerAction {
     String clusterName = getExecutionCommand().getClusterName();
     Cluster cluster = m_clusters.getCluster(clusterName);
 
-    Config clusterEnv = cluster.getDesiredConfigByType(CLUSTER_ENV_CONFIG_TYPE);
-
-    if (null == clusterEnv) {
-      return createCommandReport(0, HostRoleStatus.COMPLETED, "{}",
-          String.format(
-              "The %s configuration type was not found; unable to determine whether Hive is Kerberized",
-              CLUSTER_ENV_CONFIG_TYPE),
-          "");
-    }
-
-    // gets the security_enabled property; if it doesn't exist or is blank,
-    // Boolean will be false (no need for extra null check)
-    Map<String, String> clusterEnvProperties = clusterEnv.getProperties();
-    boolean securityEnabled = Boolean.parseBoolean(clusterEnvProperties.get(CLUSTER_ENV_SECURITY_ENABLED));
-
-    if (!securityEnabled) {
-      return createCommandReport(0, HostRoleStatus.COMPLETED, "{}",
-          "Hive is not Kerberized, skipping Kerberos-specific configuration properties", "");
-    }
-
     Config hiveSite = cluster.getDesiredConfigByType(HIVE_SITE_CONFIG_TYPE);
     if (hiveSite == null) {
       return createCommandReport(0, HostRoleStatus.COMPLETED, "{}",
           String.format(
               "The %s configuration type was not found; unable to set Hive configuration properties",
-              CLUSTER_ENV_CONFIG_TYPE),
+              HIVE_SITE_CONFIG_TYPE),
           "");
     }
 

ambari-server/src/main/resources/stacks/HDP/2.1/upgrades/nonrolling-upgrade-2.3.xml

@@ -293,11 +293,11 @@
 
       <!--HIVE-->
       <execute-stage service="HIVE" component="HIVE_SERVER" title="Apply config changes for Hive Server">
-        <task xsi:type="server_action" summary="Calculating Kerberos Properties for Hive" class="org.apache.ambari.server.serveraction.upgrades.HiveKerberosConfigAction" />
+        <task xsi:type="server_action" summary="Calculating ZooKeeper Quorum Properties for Hive" class="org.apache.ambari.server.serveraction.upgrades.HiveZKQuorumConfigAction" />
       </execute-stage>
 
       <execute-stage service="HIVE" component="HIVE_SERVER" title="Apply config changes for Hive Server">
-        <task xsi:type="configure" id="hdp_2_2_0_0_hive_server_kerberized_token_store_class"/>
+        <task xsi:type="configure" id="hdp_2_2_0_0_hive_server_delegation_token_store_class"/>
       </execute-stage>
 
       <execute-stage service="HIVE" component="HIVE_SERVER" title="Apply config changes for Hive Server">

ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml

@@ -70,12 +70,9 @@
             </condition>
           </definition>
 
-          <definition xsi:type="configure" id="hdp_2_2_0_0_hive_server_kerberized_token_store_class">
-            <condition type="cluster-env" key="security_enabled" value="true">
-              <type>hive-site</type>
-              <key>hive.cluster.delegation.token.store.class</key>
-              <value>org.apache.hadoop.hive.thrift.ZooKeeperTokenStore</value>
-            </condition>
+          <definition xsi:type="configure" id="hdp_2_2_0_0_hive_server_delegation_token_store_class">
+            <type>hive-site</type>
+            <set key="hive.cluster.delegation.token.store.class" value="org.apache.hadoop.hive.thrift.ZooKeeperTokenStore"/>
           </definition>
         </changes>
       </component>

ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/HiveKerberosConfigActionTest.java → ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigActionTest.java

@@ -35,19 +35,18 @@ import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 /**
- * Tests {@link HiveKerberosConfigAction} to ensure that the correct properties
+ * Tests {@link HiveZKQuorumConfigAction} to ensure that the correct properties
  * are set.
  */
 @RunWith(PowerMockRunner.class)
 @PrepareForTest(ZooKeeperQuorumCalculator.class)
-public class HiveKerberosConfigActionTest {
+public class HiveZKQuorumConfigActionTest {
 
   private static final String CLUSTER_NAME = "c1";
-  private HiveKerberosConfigAction m_action = null;
+  private HiveZKQuorumConfigAction m_action = null;
 
   private Clusters m_clusters = EasyMock.createStrictMock(Clusters.class);
   private Cluster m_cluster = EasyMock.createStrictMock(Cluster.class);
-  private Config m_clusterEnvConfig = EasyMock.createStrictMock(Config.class);
   private Config m_hiveSiteConfig = EasyMock.createStrictMock(Config.class);
   private ExecutionCommand m_executionCommand = EasyMock.createNiceMock(ExecutionCommand.class);
 
@@ -58,14 +57,14 @@ public class HiveKerberosConfigActionTest {
    */
   @Before
   public void before() throws Exception {
-    m_action = new HiveKerberosConfigAction();
+    m_action = new HiveZKQuorumConfigAction();
 
     // setup clusters->cluster mock
     EasyMock.expect(m_executionCommand.getClusterName()).andReturn(CLUSTER_NAME).atLeastOnce();
     EasyMock.expect(m_clusters.getCluster(CLUSTER_NAME)).andReturn(m_cluster).atLeastOnce();
 
     // set the mock objects on the class under test
-    Field m_clusterField = HiveKerberosConfigAction.class.getDeclaredField("m_clusters");
+    Field m_clusterField = HiveZKQuorumConfigAction.class.getDeclaredField("m_clusters");
     m_clusterField.setAccessible(true);
     m_clusterField.set(m_action, m_clusters);
     m_action.setExecutionCommand(m_executionCommand);
@@ -73,75 +72,20 @@ public class HiveKerberosConfigActionTest {
   }
 
   /**
-   * Tests that nothing is set if Kerberos is not enabled.
+   * Tests that the correct properties are set.
    *
    * @throws Exception
    */
   @Test
-  public void testKerberosNotEnabled() throws Exception {
-    Map<String, String> clusterEnvProperties = new HashMap<>();
-    clusterEnvProperties.put(HiveKerberosConfigAction.CLUSTER_ENV_SECURITY_ENABLED, "false");
-
-    EasyMock.expect(m_clusterEnvConfig.getProperties()).andReturn(clusterEnvProperties).atLeastOnce();
-    EasyMock.expect(m_cluster.getDesiredConfigByType(HiveKerberosConfigAction.CLUSTER_ENV_CONFIG_TYPE)).andReturn(m_clusterEnvConfig).atLeastOnce();
-    EasyMock.replay(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
-
-    m_action.execute(null);
-
-    EasyMock.verify(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
-  }
-
-  /**
-   * Tests that nothing is set if Kerberos is not enabled.
-   *
-   * @throws Exception
-   */
-  @Test
-  public void testKerberosNotEnabledMissingClusterEnv() throws Exception {
-    EasyMock.expect(m_cluster.getDesiredConfigByType(HiveKerberosConfigAction.CLUSTER_ENV_CONFIG_TYPE)).andReturn(null).atLeastOnce();
-    EasyMock.replay(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
-
-    m_action.execute(null);
-
-    EasyMock.verify(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
-  }
-  
-  /**
-   * Tests that nothing is set if Kerberos is not enabled.
-   *
-   * @throws Exception
-   */
-  @Test
-  public void testKerberosNotEnabledMissingSecurityInformation() throws Exception {
-    Map<String, String> clusterEnvProperties = new HashMap<>();
-
-    EasyMock.expect(m_clusterEnvConfig.getProperties()).andReturn(clusterEnvProperties).atLeastOnce();
-    EasyMock.expect(m_cluster.getDesiredConfigByType(HiveKerberosConfigAction.CLUSTER_ENV_CONFIG_TYPE)).andReturn(m_clusterEnvConfig).atLeastOnce();
-    EasyMock.replay(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
-
-    m_action.execute(null);
-
-    EasyMock.verify(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
-  }  
-
-  /**
-   * Tests that the correct properties are set when Kerberos is enabled.
-   *
-   * @throws Exception
-   */
-  @Test
-  public void testKerberosEnabled() throws Exception {
+  public void testZKQuorumPropertiesSetCorrectly() throws Exception {
     final String zookeeperQuorum = "c6401.ambari.apache.org:2181,c6402.ambari.apache.org:2181";
 
     PowerMockito.mockStatic(ZooKeeperQuorumCalculator.class);
     PowerMockito.when(ZooKeeperQuorumCalculator.getZooKeeperQuorumString(m_cluster)).thenReturn(
         zookeeperQuorum);
 
-    Map<String, String> clusterEnvProperties = new HashMap<>();
     Map<String, String> hiveSiteProperties = new HashMap<>();
-    clusterEnvProperties.put(HiveKerberosConfigAction.CLUSTER_ENV_SECURITY_ENABLED, "true");
 
-    EasyMock.expect(m_clusterEnvConfig.getProperties()).andReturn(clusterEnvProperties).atLeastOnce();
     EasyMock.expect(m_hiveSiteConfig.getProperties()).andReturn(hiveSiteProperties).atLeastOnce();
 
     m_hiveSiteConfig.setProperties(EasyMock.anyObject(Map.class));
@@ -150,17 +94,16 @@ public class HiveKerberosConfigActionTest {
     m_hiveSiteConfig.persist(false);
     EasyMock.expectLastCall().once();
 
-    EasyMock.expect(m_cluster.getDesiredConfigByType(HiveKerberosConfigAction.CLUSTER_ENV_CONFIG_TYPE)).andReturn(m_clusterEnvConfig).atLeastOnce();
-    EasyMock.expect(m_cluster.getDesiredConfigByType(HiveKerberosConfigAction.HIVE_SITE_CONFIG_TYPE)).andReturn(m_hiveSiteConfig).atLeastOnce();
+    EasyMock.expect(m_cluster.getDesiredConfigByType(HiveZKQuorumConfigAction.HIVE_SITE_CONFIG_TYPE)).andReturn(m_hiveSiteConfig).atLeastOnce();
 
-    EasyMock.replay(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
+    EasyMock.replay(m_executionCommand, m_clusters, m_cluster, m_hiveSiteConfig);
 
     m_action.execute(null);
 
-    EasyMock.verify(m_executionCommand, m_clusters, m_cluster, m_clusterEnvConfig, m_hiveSiteConfig);
+    EasyMock.verify(m_executionCommand, m_clusters, m_cluster, m_hiveSiteConfig);
 
-    Assert.assertEquals(zookeeperQuorum, hiveSiteProperties.get(HiveKerberosConfigAction.HIVE_SITE_ZK_QUORUM));
-    Assert.assertEquals(zookeeperQuorum, hiveSiteProperties.get(HiveKerberosConfigAction.HIVE_SITE_ZK_CONNECT_STRING));
+    Assert.assertEquals(zookeeperQuorum, hiveSiteProperties.get(HiveZKQuorumConfigAction.HIVE_SITE_ZK_QUORUM));
+    Assert.assertEquals(zookeeperQuorum, hiveSiteProperties.get(HiveZKQuorumConfigAction.HIVE_SITE_ZK_CONNECT_STRING));
   }
 
 }