[AMBARI-24026] - Service deletion requests get queued up in Background Ops during Upgrade (#1468)

5 files changed, 68 insertions, 4 deletions

diff --git a/ambari-server/src/main/java/org/apache/ambari/annotations/ExperimentalFeature.java b/ambari-server/src/main/java/org/apache/ambari/annotations/ExperimentalFeature.java
index 55e09e0cd5..bc399ffd11 100644
--- a/ambari-server/src/main/java/org/apache/ambari/annotations/ExperimentalFeature.java
+++ b/ambari-server/src/main/java/org/apache/ambari/annotations/ExperimentalFeature.java
@@ -42,5 +42,11 @@ public enum ExperimentalFeature {
   /**
    * Support for service-specific repos for custom services
    */
-  CUSTOM_SERVICE_REPOS
+  CUSTOM_SERVICE_REPOS,
+
+  /**
+   * Automatically removing Kerberos identities when a service or component is
+   * removed.
+   */
+  ORPHAN_KERBEROS_IDENTITY_REMOVAL;
 }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleaner.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleaner.java
index 7ec4a6ebb8..985a2b3fe7 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleaner.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleaner.java
@@ -17,11 +17,14 @@
  */
 package org.apache.ambari.server.controller.utilities;
 
+import org.apache.ambari.annotations.Experimental;
+import org.apache.ambari.annotations.ExperimentalFeature;
 import org.apache.ambari.server.controller.KerberosHelper;
 import org.apache.ambari.server.events.ServiceComponentUninstalledEvent;
 import org.apache.ambari.server.events.ServiceRemovedEvent;
 import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
 import org.apache.ambari.server.serveraction.kerberos.KerberosMissingAdminCredentialsException;
+import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,6 +34,9 @@ import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
 @Singleton
+@Experimental(
+    feature = ExperimentalFeature.ORPHAN_KERBEROS_IDENTITY_REMOVAL,
+    comment = "This might need to have a switch so that it can be turned off if it is found to be desctructive to certain clusters")
 public class KerberosIdentityCleaner {
   private final static Logger LOG = LoggerFactory.getLogger(KerberosIdentityCleaner.class);
   private final AmbariEventPublisher eventPublisher;
@@ -45,7 +51,7 @@ public class KerberosIdentityCleaner {
   }
 
   public void register() {
-    this.eventPublisher.register(this);
+    eventPublisher.register(this);
   }
 
   /**
@@ -55,6 +61,14 @@ public class KerberosIdentityCleaner {
   @Subscribe
   public void componentRemoved(ServiceComponentUninstalledEvent event) throws KerberosMissingAdminCredentialsException {
     try {
+      Cluster cluster = clusters.getCluster(event.getClusterId());
+      if (null != cluster.getUpgradeInProgress()) {
+        LOG.info("Skipping removal of identities for {} since there is an upgrade in progress",
+            event.getComponentName());
+
+        return;
+      }
+
       LOG.info("Removing identities after {}", event);
       RemovableIdentities
         .ofComponent(clusters.getCluster(event.getClusterId()), event, kerberosHelper)
@@ -71,6 +85,14 @@ public class KerberosIdentityCleaner {
   @Subscribe
   public void serviceRemoved(ServiceRemovedEvent event) {
     try {
+      Cluster cluster = clusters.getCluster(event.getClusterId());
+      if (null != cluster.getUpgradeInProgress()) {
+        LOG.info("Skipping removal of identities for {} since there is an upgrade in progress",
+            event.getServiceName());
+
+        return;
+      }
+
       LOG.info("Removing identities after {}", event);
       RemovableIdentities
         .ofService(clusters.getCluster(event.getClusterId()), event, kerberosHelper)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/RemovableIdentities.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/RemovableIdentities.java
index cd23e834b1..c7787818bc 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/RemovableIdentities.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/RemovableIdentities.java
@@ -27,6 +27,8 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 
+import org.apache.ambari.annotations.Experimental;
+import org.apache.ambari.annotations.ExperimentalFeature;
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.controller.KerberosHelper;
 import org.apache.ambari.server.controller.utilities.UsedIdentities.ComponentExclude;
@@ -45,6 +47,7 @@ import org.apache.ambari.server.state.kerberos.KerberosServiceDescriptor;
  * I represent a group of kerberos identities which are to be deleted after a service or a component was removed.
  * My instances provide methods for removing the candidates, excluding those that are still used by other components or services.
  */
+@Experimental(feature = ExperimentalFeature.ORPHAN_KERBEROS_IDENTITY_REMOVAL)
 public class RemovableIdentities {
   private final List<KerberosIdentityDescriptor> candidateIdentities;
   private final UsedIdentities usedIdentities;
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/DeleteUnsupportedServicesAndComponents.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/DeleteUnsupportedServicesAndComponents.java
index c7edc276a5..c7e24bb9e9 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/DeleteUnsupportedServicesAndComponents.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/DeleteUnsupportedServicesAndComponents.java
@@ -29,6 +29,8 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Predicate;
 
+import org.apache.ambari.annotations.Experimental;
+import org.apache.ambari.annotations.ExperimentalFeature;
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.actionmanager.HostRoleStatus;
 import org.apache.ambari.server.agent.CommandReport;
@@ -46,9 +48,18 @@ import org.apache.commons.lang.StringUtils;
 import com.google.inject.Inject;
 
 /**
- * Upgrade Server Action that deletes the components and services which are no longer supported in the target stack.
- * The deletable component or service should be in deletable state (stopped) before executing this.
+ * Upgrade Server Action that deletes the components and services which are no
+ * longer supported in the target stack. The deletable component or service
+ * should be in deletable state (stopped) before executing this.
+ * <p/>
+ * This will orphan Kerberos keytabs and identities that belonged to the removed
+ * services and components. Since removing these automatically is a new and
+ * untested feature, its best to leave this type of cleanup to a future
+ * implementation.
  */
+@Experimental(
+    feature = ExperimentalFeature.ORPHAN_KERBEROS_IDENTITY_REMOVAL,
+    comment = "Not removing identities yet")
 public class DeleteUnsupportedServicesAndComponents extends AbstractUpgradeServerAction {
   @Inject
   private ServiceComponentSupport serviceComponentSupport;
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleanerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleanerTest.java
index e7b5deb00b..5783426cea 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleanerTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/KerberosIdentityCleanerTest.java
@@ -33,6 +33,7 @@ import org.apache.ambari.server.controller.KerberosHelper;
 import org.apache.ambari.server.events.ServiceComponentUninstalledEvent;
 import org.apache.ambari.server.events.ServiceRemovedEvent;
 import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
+import org.apache.ambari.server.orm.entities.UpgradeEntity;
 import org.apache.ambari.server.serveraction.kerberos.Component;
 import org.apache.ambari.server.serveraction.kerberos.KerberosMissingAdminCredentialsException;
 import org.apache.ambari.server.state.Cluster;
@@ -117,6 +118,8 @@ public class KerberosIdentityCleanerTest extends EasyMockSupport {
   public void skipsRemovingIdentityWhenClusterIsNotKerberized() throws Exception {
     reset(cluster);
     expect(cluster.getSecurityType()).andReturn(SecurityType.NONE).anyTimes();
+    expect(cluster.getUpgradeInProgress()).andReturn(null).once();
+
     replayAll();
     uninstallComponent(OOZIE, OOZIE_SERVER, HOST);
     verifyAll();
@@ -140,6 +143,24 @@ public class KerberosIdentityCleanerTest extends EasyMockSupport {
     verifyAll();
   }
 
+  /**
+   * Ensures that when an upgrade is in progress, new requests are not created
+   * to remove identities since it would interfere with the long running
+   * upgrade.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void skipsRemovingIdentityWhenClusterIsUpgrading() throws Exception {
+    installComponent(OOZIE, OOZIE_SERVER, HOST);
+    reset(cluster);
+    expect(cluster.getUpgradeInProgress()).andReturn(createNiceMock(UpgradeEntity.class)).once();
+
+    replayAll();
+    uninstallComponent(OOZIE, OOZIE_SERVER, HOST);
+    verifyAll();
+  }
+
   private ArrayList<Component> hdfsComponents() {
     return newArrayList(new Component(HOST, HDFS, NAMENODE, 0l), new Component(HOST, HDFS, DATANODE, 0l));
   }
@@ -280,5 +301,6 @@ public class KerberosIdentityCleanerTest extends EasyMockSupport {
     expect(cluster.getSecurityType()).andReturn(SecurityType.KERBEROS).anyTimes();
     expect(kerberosHelper.getKerberosDescriptor(cluster, false)).andReturn(kerberosDescriptor).anyTimes();
     expect(cluster.getServices()).andReturn(installedServices).anyTimes();
+    expect(cluster.getUpgradeInProgress()).andReturn(null).once();
   }
 }
\ No newline at end of file