AMBARI-14734. RU/EU, add PreCheck to prevent starting upgrade if a previous upgrade skipped the step Save DB State (alejandro)

ambari-server/src/main/java/org/apache/ambari/server/checks/AbstractCheckDescriptor.java

@@ -29,6 +29,7 @@ import org.apache.ambari.server.configuration.Configuration;
 import org.apache.ambari.server.orm.dao.ClusterVersionDAO;
 import org.apache.ambari.server.orm.dao.HostVersionDAO;
 import org.apache.ambari.server.orm.dao.RepositoryVersionDAO;
+import org.apache.ambari.server.orm.dao.UpgradeDAO;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.Config;
@@ -66,6 +67,9 @@ public abstract class AbstractCheckDescriptor {
   @Inject
   Provider<RepositoryVersionDAO> repositoryVersionDaoProvider;
 
+  @Inject
+  Provider<UpgradeDAO> upgradeDaoProvider;
+
   @Inject
   Provider<RepositoryVersionHelper> repositoryVersionHelper;
 

ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java

@@ -152,6 +152,13 @@ public enum CheckDescription {
             "The following Services must be reinstalled: {{fails}}. Try to reinstall the service components in INSTALL_FAILED state.");
       }}),
 
+  PREVIOUS_UPGRADE_COMPLETED(PrereqCheckType.CLUSTER,
+      "A previous upgrade did not complete.",
+      new HashMap<String, String>() {{
+        put(AbstractCheckDescriptor.DEFAULT,
+            "The last upgrade attempt did not complete. {{fails}}");
+      }}),
+
   INSTALL_PACKAGES_CHECK(PrereqCheckType.CLUSTER,
       "Install packages must be re-run",
       new HashMap<String, String>() {{

ambari-server/src/main/java/org/apache/ambari/server/checks/HiveDynamicServiceDiscoveryCheck.java

@@ -39,7 +39,7 @@ import com.google.inject.Singleton;
  * is properly configured for dynamic discovery.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 1.0f, required = true)
+@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 20.0f, required = true)
 public class HiveDynamicServiceDiscoveryCheck extends AbstractCheckDescriptor {
 
   static final String HIVE_DYNAMIC_SERVICE_DISCOVERY_ENABLED_KEY = "hive.dynamic-service.discovery.enabled.key";

ambari-server/src/main/java/org/apache/ambari/server/checks/HiveMultipleMetastoreCheck.java

@@ -37,7 +37,7 @@ import com.google.inject.Singleton;
  * Metastore instances in the cluster.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.MULTIPLE_COMPONENT_WARNING, order = 1.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.MULTIPLE_COMPONENT_WARNING, order = 20.1f)
 public class HiveMultipleMetastoreCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/HostMaintenanceModeCheck.java

@@ -39,7 +39,7 @@ import com.google.inject.Singleton;
  * @see HostsHeartbeatCheck
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.MAINTENANCE_MODE, order = 3.0f, required = true)
+@UpgradeCheck(group = UpgradeCheckGroup.MAINTENANCE_MODE, order = 7.0f, required = true)
 public class HostMaintenanceModeCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/HostsMasterMaintenanceCheck.java

@@ -40,7 +40,7 @@ import com.google.inject.Singleton;
  * Checks that all hosts in maintenance state do not have master components.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.MAINTENANCE_MODE, order = 1.0f, required = true)
+@UpgradeCheck(group = UpgradeCheckGroup.MAINTENANCE_MODE, order = 5.0f, required = true)
 public class HostsMasterMaintenanceCheck extends AbstractCheckDescriptor {
 
   static final String KEY_NO_UPGRADE_NAME = "no_upgrade_name";

ambari-server/src/main/java/org/apache/ambari/server/checks/InstallPackagesCheck.java

@@ -42,7 +42,7 @@ import com.google.inject.Singleton;
  * Checks if Install Packages needs to be re-run
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 2.0f, required = true)
+@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 3.0f, required = true)
 public class InstallPackagesCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/MapReduce2JobHistoryStatePreservingCheck.java

@@ -38,7 +38,7 @@ import java.util.Map;
  * is used to check that the MR2 History server has state preserving mode enabled.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.CONFIGURATION_WARNING, order = 1.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.CONFIGURATION_WARNING, order = 17.0f)
 public class MapReduce2JobHistoryStatePreservingCheck extends AbstractCheckDescriptor {
 
   final static String MAPREDUCE2_JOBHISTORY_RECOVERY_ENABLE_KEY =

ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java

@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.checks;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.actionmanager.HostRoleStatus;
+import org.apache.ambari.server.controller.PrereqCheckRequest;
+import org.apache.ambari.server.orm.dao.HostRoleCommandDAO;
+import org.apache.ambari.server.orm.dao.RequestDAO;
+import org.apache.ambari.server.orm.entities.ClusterVersionEntity;
+import org.apache.ambari.server.orm.entities.HostRoleCommandEntity;
+import org.apache.ambari.server.orm.entities.HostVersionEntity;
+import org.apache.ambari.server.orm.entities.RepositoryVersionEntity;
+import org.apache.ambari.server.orm.entities.RequestEntity;
+import org.apache.ambari.server.orm.entities.UpgradeEntity;
+import org.apache.ambari.server.state.Cluster;
+import org.apache.ambari.server.state.Host;
+import org.apache.ambari.server.state.MaintenanceState;
+import org.apache.ambari.server.state.RepositoryVersionState;
+import org.apache.ambari.server.state.StackId;
+import org.apache.ambari.server.state.stack.PrereqCheckStatus;
+import org.apache.ambari.server.state.stack.PrerequisiteCheck;
+import org.apache.ambari.server.state.stack.upgrade.Direction;
+import org.apache.commons.lang.StringUtils;
+
+import javax.inject.Provider;
+import java.text.MessageFormat;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Checks if Install Packages needs to be re-run
+ */
+@Singleton
+@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 4.0f, required = true)
+public class PreviousUpgradeCompleted extends AbstractCheckDescriptor {
+
+  /**
+   * If this ever changes, we will need to keep the historic name.
+   */
+  public static final String FINALIZE_ACTION_CLASS_NAME = "org.apache.ambari.server.serveraction.upgrades.FinalizeUpgradeAction";
+  public static final String SET_CURRENT_COMMAND = "ambari-server set-current --cluster-name=$CLUSTERNAME --version-display-name=$VERSION_NAME";
+
+  @Inject
+  Provider<RequestDAO> requestDaoProvider;
+
+  @Inject
+  Provider<HostRoleCommandDAO> hostRoleCommandDaoProvider;
+
+  /**
+   * Constructor.
+   */
+  public PreviousUpgradeCompleted() {
+    super(CheckDescription.PREVIOUS_UPGRADE_COMPLETED);
+  }
+
+  @Override
+  public void perform(PrerequisiteCheck prerequisiteCheck, PrereqCheckRequest request) throws AmbariException {
+    final String clusterName = request.getClusterName();
+    final Cluster cluster = clustersProvider.get().getCluster(clusterName);
+
+    String errorMessage = null;
+
+    List<UpgradeEntity> upgrades= upgradeDaoProvider.get().findAll();
+    if (upgrades != null) {
+      Long lastStartTime = 0L;
+      UpgradeEntity mostRecentUpgrade = null;
+      UpgradeEntity correspondingDowngrade = null;
+      for (UpgradeEntity upgrade : upgrades) {
+        // Find the most recent upgrade for this cluster
+        if (upgrade.getClusterId() == cluster.getClusterId() && upgrade.getDirection() == Direction.UPGRADE) {
+          Long requestId = upgrade.getRequestId();
+          RequestEntity upgradeRequest = requestDaoProvider.get().findByPK(requestId);
+          if (upgradeRequest != null && upgradeRequest.getStartTime() > lastStartTime) {
+            mostRecentUpgrade = upgrade;
+            lastStartTime = upgradeRequest.getStartTime();
+          }
+        }
+      }
+
+      // Check for the corresponding downgrade.
+      if (mostRecentUpgrade != null) {
+        for (UpgradeEntity downgrade : upgrades) {
+          // Surprisingly, a Downgrade's from and to version are identical.
+          if (downgrade.getClusterId() == cluster.getClusterId() && downgrade.getDirection() == Direction.DOWNGRADE &&
+              downgrade.getFromVersion().equals(mostRecentUpgrade.getToVersion())) {
+            correspondingDowngrade = downgrade;
+            break;
+          }
+        }
+      }
+
+      // If it has no downgrade, then the "Save Cluster State" step should have COMPLETED.
+      if (correspondingDowngrade == null) {
+        // Should have only 1 element.
+        List<HostRoleCommandEntity> finalizeCommandList = hostRoleCommandDaoProvider.get().
+            findSortedCommandsByRequestIdAndCustomCommandName(mostRecentUpgrade.getRequestId(), FINALIZE_ACTION_CLASS_NAME);
+
+        // If the action is not COMPLETED, then something went wrong.
+        if (finalizeCommandList != null) {
+          for (HostRoleCommandEntity command : finalizeCommandList) {
+            if (command.getStatus() != HostRoleStatus.COMPLETED) {
+              errorMessage = MessageFormat.format("Upgrade attempt (id: {0}, request id: {1}, from version: {2}, " +
+                  "to version: {3}) did not complete task with id {4} since its state is {5} instead of COMPLETED.",
+                  mostRecentUpgrade.getId(), mostRecentUpgrade.getRequestId(), mostRecentUpgrade.getFromVersion(),
+                  mostRecentUpgrade.getToVersion(), command.getTaskId(), command.getStatus());
+              errorMessage += " Please ensure that you called:\n" + SET_CURRENT_COMMAND;
+              errorMessage += MessageFormat.format("\nFurther, change the status of host_role_command with " +
+                  "id {0} to COMPLETED", mostRecentUpgrade.getId());
+              break;
+            }
+          }
+        }
+      }
+    }
+
+    if (null != errorMessage) {
+      LinkedHashSet<String> failedOn = new LinkedHashSet<String>();
+      failedOn.add(cluster.getClusterName());
+      prerequisiteCheck.setFailedOn(failedOn);
+      prerequisiteCheck.setStatus(PrereqCheckStatus.FAIL);
+      prerequisiteCheck.setFailReason(errorMessage);
+    }
+  }
+}

ambari-server/src/main/java/org/apache/ambari/server/checks/RangerPasswordCheck.java

@@ -50,7 +50,7 @@ import com.google.inject.Singleton;
  * user had changed the password using the Ranger UI.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.CONFIGURATION_WARNING, order = 1.1f, required=true)
+@UpgradeCheck(group = UpgradeCheckGroup.CONFIGURATION_WARNING, order = 23.0f, required=true)
 public class RangerPasswordCheck extends AbstractCheckDescriptor {
 
   private static final Logger LOG = LoggerFactory.getLogger(RangerPasswordCheck.class);

ambari-server/src/main/java/org/apache/ambari/server/checks/SecondaryNamenodeDeletedCheck.java

@@ -41,7 +41,7 @@ import com.google.inject.Singleton;
  * Checks that the Secondary NameNode is not present on any of the hosts.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 2.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 16.0f)
 public class SecondaryNamenodeDeletedCheck extends AbstractCheckDescriptor {
   @Inject
   HostComponentStateDAO hostComponentStateDao;

ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesMaintenanceModeCheck.java

@@ -33,7 +33,7 @@ import com.google.inject.Singleton;
  * Checks that services are in the maintenance mode.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.MAINTENANCE_MODE, order = 2.0f, required = true)
+@UpgradeCheck(group = UpgradeCheckGroup.MAINTENANCE_MODE, order = 6.0f, required = true)
 public class ServicesMaintenanceModeCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesMapReduceDistributedCacheCheck.java

@@ -39,7 +39,7 @@ import com.google.inject.Singleton;
  * Checks that MR jobs reference hadoop libraries from the distributed cache.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 3.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 17.1f)
 public class ServicesMapReduceDistributedCacheCheck extends AbstractCheckDescriptor {
 
   static final String KEY_APP_CLASSPATH = "app_classpath";

ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesNamenodeHighAvailabilityCheck.java

@@ -35,7 +35,7 @@ import com.google.inject.Singleton;
  * Checks that namenode high availability is enabled.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 1.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 16.1f)
 public class ServicesNamenodeHighAvailabilityCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesNamenodeTruncateCheck.java

@@ -33,7 +33,7 @@ import java.util.Map;
  * Checks that namenode high availability is enabled.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 1.1f)
+@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 16.2f)
 public class ServicesNamenodeTruncateCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesTezDistributedCacheCheck.java

@@ -39,7 +39,7 @@ import com.google.inject.Singleton;
  * Checks that Tez jobs reference hadoop libraries from the distributed cache.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 4.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.NAMENODE_HA, order = 21.0f)
 public class ServicesTezDistributedCacheCheck extends AbstractCheckDescriptor {
 
   static final String KEY_LIB_URI_MISSING = "tez_lib_uri_missing";

ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesYarnWorkPreservingCheck.java

@@ -33,7 +33,7 @@ import java.util.Arrays;
  * Checks that YARN has work-preserving restart enabled.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 1.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 17.1f)
 public class ServicesYarnWorkPreservingCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/StormRestAPIDeletedCheck.java

@@ -40,7 +40,7 @@ import java.util.Set;
  * checks that STORM_REST_API Component is deleted when upgrading from HDP 2.1 to 2.2 or higher.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.TOPOLOGY, order = 1.0f, required= false)
+@UpgradeCheck(group = UpgradeCheckGroup.TOPOLOGY, order = 15.0f, required= false)
 public class StormRestAPIDeletedCheck extends AbstractCheckDescriptor {
 
   @Inject

ambari-server/src/main/java/org/apache/ambari/server/checks/YarnRMHighAvailabilityCheck.java

@@ -35,7 +35,7 @@ import com.google.inject.Singleton;
  * for ResourceManager..
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.MULTIPLE_COMPONENT_WARNING, order = 2.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.MULTIPLE_COMPONENT_WARNING, order = 17.2f)
 public class YarnRMHighAvailabilityCheck extends AbstractCheckDescriptor {
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/checks/YarnTimelineServerStatePreservingCheck.java

@@ -38,7 +38,7 @@ import com.google.inject.Singleton;
  * present in HDP 2.2.4.2+.
  */
 @Singleton
-@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 1.0f)
+@UpgradeCheck(group = UpgradeCheckGroup.DEFAULT, order = 17.3f)
 public class YarnTimelineServerStatePreservingCheck extends AbstractCheckDescriptor {
 
   private final static String YARN_TIMELINE_STATE_RECOVERY_ENABLED_KEY = "yarn.timeline-service.recovery.enabled";

ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java

@@ -208,6 +208,15 @@ public class HostRoleCommandDAO {
     return daoUtils.selectList(query, role, status);
   }
 
+  @RequiresSession
+  public List<HostRoleCommandEntity> findSortedCommandsByRequestIdAndCustomCommandName(Long requestId, String customCommandName) {
+    TypedQuery<HostRoleCommandEntity> query = entityManagerProvider.get().createQuery("SELECT hostRoleCommand " +
+        "FROM HostRoleCommandEntity hostRoleCommand " +
+        "WHERE hostRoleCommand.requestId=?1 AND hostRoleCommand.customCommandName=?2 " +
+        "ORDER BY hostRoleCommand.taskId", HostRoleCommandEntity.class);
+    return daoUtils.selectList(query, requestId, customCommandName);
+  }
+
 
   @RequiresSession
   public List<HostRoleCommandEntity> findSortedCommandsByStageAndHost(StageEntity stageEntity, HostEntity hostEntity) {

ambari-server/src/test/java/org/apache/ambari/server/checks/UpgradeCheckOrderTest.java

@@ -76,28 +76,20 @@ public class UpgradeCheckOrderTest {
     for (AbstractCheckDescriptor check : checks) {
       UpgradeCheckGroup group = UpgradeCheckGroup.DEFAULT;
       UpgradeCheckGroup lastGroup = UpgradeCheckGroup.DEFAULT;
-      Float order = 1.0f;
-      Float lastOrder = 1.0f;
 
-      if (null == lastCheck) {
-        lastCheck = check;
-      }
-
-      UpgradeCheck annotation = check.getClass().getAnnotation(UpgradeCheck.class);
-      UpgradeCheck lastAnnotation = lastCheck.getClass().getAnnotation(UpgradeCheck.class);
+      if (null != lastCheck) {
 
-      if (null != annotation) {
-        group = annotation.group();
-        order = annotation.order();
-      }
+        UpgradeCheck annotation = check.getClass().getAnnotation(UpgradeCheck.class);
+        UpgradeCheck lastAnnotation = lastCheck.getClass().getAnnotation(UpgradeCheck.class);
 
-      if (null != lastAnnotation) {
-        lastGroup = lastAnnotation.group();
-        lastOrder = lastAnnotation.order();
+        if (null != annotation && null != lastAnnotation) {
+          group = annotation.group();
+          lastGroup = lastAnnotation.group();
+          Assert.assertTrue(lastGroup.getOrder().compareTo(group.getOrder()) <= 0);
+        }
       }
 
-      Assert.assertTrue(lastGroup.getOrder().compareTo(group.getOrder()) <= 0);
-      Assert.assertTrue(lastOrder.compareTo(order) <= 0);
+      lastCheck = check;
     }
   }
 }

ambari-server/src/test/java/org/apache/ambari/server/state/CheckHelperTest.java

@@ -34,6 +34,7 @@ import org.apache.ambari.server.controller.PrereqCheckRequest;
 import org.apache.ambari.server.orm.dao.ClusterVersionDAO;
 import org.apache.ambari.server.orm.dao.HostVersionDAO;
 import org.apache.ambari.server.orm.dao.RepositoryVersionDAO;
+import org.apache.ambari.server.orm.dao.UpgradeDAO;
 import org.apache.ambari.server.state.stack.OsFamily;
 import org.apache.ambari.server.state.stack.PrereqCheckStatus;
 import org.apache.ambari.server.state.stack.PrerequisiteCheck;
@@ -128,6 +129,7 @@ public class CheckHelperTest {
         bind(Clusters.class).toInstance(clusters);
         bind(ClusterVersionDAO.class).toProvider(Providers.<ClusterVersionDAO>of(null));
         bind(HostVersionDAO.class).toProvider(Providers.<HostVersionDAO>of(null));
+        bind(UpgradeDAO.class).toProvider(Providers.<UpgradeDAO>of(null));
         bind(RepositoryVersionDAO.class).toProvider(Providers.<RepositoryVersionDAO>of(null));
         bind(RepositoryVersionHelper.class).toProvider(Providers.<RepositoryVersionHelper>of(null));
         bind(AmbariMetaInfo.class).toProvider(Providers.<AmbariMetaInfo>of(null));