AMBARI-9167. Enable Running All Service Smoke Tests at various check points in upgrade pack (ncole)

ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java

@@ -28,6 +28,7 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.api.services.AmbariMetaInfo;
 import org.apache.ambari.server.controller.internal.RequestResourceProvider;
 import org.apache.ambari.server.controller.internal.StageResourceProvider;
 import org.apache.ambari.server.controller.predicate.AndPredicate;
@@ -50,6 +51,7 @@ import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
 import org.apache.ambari.server.state.stack.upgrade.ClusterGrouping;
 import org.apache.ambari.server.state.stack.upgrade.Grouping;
 import org.apache.ambari.server.state.stack.upgrade.ManualTask;
+import org.apache.ambari.server.state.stack.upgrade.ServiceCheckGrouping;
 import org.apache.ambari.server.state.stack.upgrade.StageWrapper;
 import org.apache.ambari.server.state.stack.upgrade.StageWrapperBuilder;
 import org.apache.ambari.server.state.stack.upgrade.Task;
@@ -111,6 +113,9 @@ public class UpgradeHelper {
   @Inject
   private Provider<ConfigHelper> m_configHelper;
 
+  @Inject
+  private Provider<AmbariMetaInfo> m_ambariMetaInfo;
+
   /**
    * Generates a list of UpgradeGroupHolder items that are used to execute a
    * downgrade
@@ -171,21 +176,40 @@ public class UpgradeHelper {
   private List<UpgradeGroupHolder> createSequence(MasterHostResolver mhr,
       UpgradePack upgradePack, String version, boolean forUpgrade)
       throws AmbariException {
+
     Cluster cluster = mhr.getCluster();
     Map<String, Map<String, ProcessingComponent>> allTasks = upgradePack.getTasks();
     List<UpgradeGroupHolder> groups = new ArrayList<UpgradeGroupHolder>();
 
-    int idx = 0;
 
-    for (Grouping group : upgradePack.getGroups()) {
+    for (Grouping group : upgradePack.getGroups(forUpgrade)) {
       if (ClusterGrouping.class.isInstance(group)) {
         UpgradeGroupHolder groupHolder = getClusterGroupHolder(
             cluster, (ClusterGrouping) group, forUpgrade ? null : version);
+
         if (null != groupHolder) {
           groups.add(groupHolder);
-          idx++;
-          continue;
         }
+
+        continue;
+      } else if (ServiceCheckGrouping.class.isInstance(group)) {
+        ServiceCheckGrouping scg = (ServiceCheckGrouping) group;
+
+        scg.getBuilder().setHelpers(cluster, m_ambariMetaInfo.get());
+
+        List<StageWrapper> wrappers = scg.getBuilder().build();
+
+        if (!wrappers.isEmpty()) {
+          UpgradeGroupHolder groupHolder = new UpgradeGroupHolder();
+          groupHolder.name = group.name;
+          groupHolder.title = group.title;
+          groupHolder.skippable = group.skippable;
+          groupHolder.allowRetry = group.allowRetry;
+          groupHolder.items = wrappers;
+          groups.add(groupHolder);
+        }
+
+        continue;
       }
 
       UpgradeGroupHolder groupHolder = new UpgradeGroupHolder();
@@ -257,11 +281,7 @@ public class UpgradeHelper {
 
       if (!proxies.isEmpty()) {
         groupHolder.items = proxies;
-        if (forUpgrade) {
-          groups.add(groupHolder);
-        } else {
-          groups.add(idx, groupHolder);
-        }
+        groups.add(groupHolder);
       }
     }
 

ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java

@@ -17,6 +17,8 @@
  */
 package org.apache.ambari.server.state.stack;
 
+import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -29,7 +31,9 @@ import javax.xml.bind.annotation.XmlElementWrapper;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.XmlTransient;
 
+import org.apache.ambari.server.state.stack.upgrade.ClusterGrouping;
 import org.apache.ambari.server.state.stack.upgrade.Grouping;
+import org.apache.ambari.server.state.stack.upgrade.ServiceCheckGrouping;
 import org.apache.ambari.server.state.stack.upgrade.Task;
 
 /**
@@ -55,6 +59,8 @@ public class UpgradePack {
   private Map<String, List<String>> m_orders = null;
   @XmlTransient
   private Map<String, Map<String, ProcessingComponent>> m_process = null;
+  @XmlTransient
+  private boolean m_resolvedGroups = false;
 
   /**
    * @return the target version for the upgrade pack
@@ -63,8 +69,75 @@ public class UpgradePack {
     return target;
   }
 
-  public List<Grouping> getGroups() {
-    return groups;
+  /**
+   * Gets the groups defined for the upgrade pack
+   * @param upgrade {@code true} if returning the groups in proper order for an upgrade
+   * @return the list of groups
+   */
+  public List<Grouping> getGroups(boolean upgrade) {
+    return upgrade ? groups : getDowngradeGroups();
+  }
+
+  /**
+   * Calculates the group orders when performing a downgrade
+   * <ul>
+   *   <li>ClusterGroupings must remain at the same positions (first/last).</li>
+   *   <li>When there is a ServiceCheck group, it must ALWAYS follow the same</li>
+   *       preceding group, whether for an upgrade or a downgrade.</li>
+   *   <li>All other groups must follow the reverse order.</li>
+   * </ul>
+   * For example, give the following order of groups:
+   * <ol>
+   *   <li>PRE_CLUSTER</li>
+   *   <li>ZK</li>
+   *   <li>CORE_MASTER</li>
+   *   <li>SERVICE_CHECK_1</li>
+   *   <li>CLIENTS</li>
+   *   <li>FLUME</li>
+   *   <li>SERVICE_CHECK_2</li>
+   *   <li>POST_CLUSTER</li>
+   * </ol>
+   * The reverse would be:
+   * <ol>
+   *   <li>PRE_CLUSTER</li>
+   *   <li>FLUME</li>
+   *   <li>SERVICE_CHECK_2</li>
+   *   <li>CLIENTS</li>
+   *   <li>CORE_MASTER</li>
+   *   <li>SERVICE_CHECK_1</li>
+   *   <li>ZK</li>
+   *   <li>POST_CLUSTER</li>
+   * </ol>
+   * @return the list of groups, reversed appropriately for a downgrade.
+   */
+  private List<Grouping> getDowngradeGroups() {
+    List<Grouping> reverse = new ArrayList<Grouping>();
+
+    int idx = 0;
+    int iter = 0;
+    Iterator<Grouping> it = groups.iterator();
+
+    while (it.hasNext()) {
+      Grouping g = it.next();
+      if (ClusterGrouping.class.isInstance(g)) {
+        reverse.add(g);
+        idx++;
+      } else {
+        if (iter+1 < groups.size()) {
+          Grouping peek = groups.get(iter+1);
+          if (ServiceCheckGrouping.class.isInstance(peek)) {
+            reverse.add(idx, it.next());
+            reverse.add(idx, g);
+            iter++;
+          } else {
+            reverse.add(idx, g);
+          }
+        }
+      }
+      iter++;
+    }
+
+    return reverse;
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java

@@ -49,20 +49,22 @@ public class ColocatedGrouping extends Grouping {
 
   @Override
   public StageWrapperBuilder getBuilder() {
-    return new MultiHomedBuilder(batch);
+    return new MultiHomedBuilder(batch, performServiceCheck);
   }
 
   private static class MultiHomedBuilder extends StageWrapperBuilder {
 
-    private Batch batch;
+    private Batch m_batch;
+    private boolean m_serviceCheck = true;
 
     // !!! host -> list of tasks
     private Map<String, List<TaskProxy>> initialBatch = new LinkedHashMap<String, List<TaskProxy>>();
     private Map<String, List<TaskProxy>> finalBatches = new LinkedHashMap<String, List<TaskProxy>>();
 
 
-    private MultiHomedBuilder(Batch batch) {
-      this.batch = batch;
+    private MultiHomedBuilder(Batch batch, boolean serviceCheck) {
+      this.m_batch = batch;
+      m_serviceCheck = serviceCheck;
     }
 
     @Override
@@ -70,7 +72,7 @@ public class ColocatedGrouping extends Grouping {
         boolean forUpgrade, boolean clientOnly, ProcessingComponent pc) {
 
       int count = Double.valueOf(Math.ceil(
-          (double) batch.percent / 100 * hostsType.hosts.size())).intValue();
+          (double) m_batch.percent / 100 * hostsType.hosts.size())).intValue();
 
       int i = 0;
       for (String host : hostsType.hosts) {
@@ -143,8 +145,8 @@ public class ColocatedGrouping extends Grouping {
 
       // !!! TODO when manual tasks are ready
       ManualTask task = new ManualTask();
-      task.summary = batch.summary;
-      task.message = batch.message;
+      task.summary = m_batch.summary;
+      task.message = m_batch.message;
 
       StageWrapper wrapper = new StageWrapper(
           StageWrapper.Type.SERVER_SIDE_ACTION,
@@ -192,7 +194,7 @@ public class ColocatedGrouping extends Grouping {
 
       }
 
-      if (serviceChecks.size() > 0) {
+      if (m_serviceCheck && serviceChecks.size() > 0) {
         // !!! add the service check task
         List<TaskWrapper> tasks = new ArrayList<TaskWrapper>();
         for (String service : serviceChecks) {

ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java

@@ -35,7 +35,7 @@ import org.apache.commons.lang.StringUtils;
 /**
  *
  */
-@XmlSeeAlso(value = { ColocatedGrouping.class, ClusterGrouping.class })
+@XmlSeeAlso(value = { ColocatedGrouping.class, ClusterGrouping.class, ServiceCheckGrouping.class })
 public class Grouping {
 
   @XmlAttribute(name="name")
@@ -53,18 +53,27 @@ public class Grouping {
   @XmlElement(name="service")
   public List<UpgradePack.OrderService> services = new ArrayList<UpgradePack.OrderService>();
 
+  @XmlElement(name="service-check", defaultValue="true")
+  public boolean performServiceCheck = true;
+
+
   /**
    * Gets the default builder.
    */
   public StageWrapperBuilder getBuilder() {
-    return new DefaultBuilder();
+    return new DefaultBuilder(performServiceCheck);
   }
 
 
   private static class DefaultBuilder extends StageWrapperBuilder {
 
-    private List<StageWrapper> stages = new ArrayList<StageWrapper>();
-    private Set<String> serviceChecks = new HashSet<String>();
+    private List<StageWrapper> m_stages = new ArrayList<StageWrapper>();
+    private Set<String> m_servicesToCheck = new HashSet<String>();
+    private boolean m_serviceCheck = true;
+
+    private DefaultBuilder(boolean serviceCheck) {
+      m_serviceCheck = serviceCheck;
+    }
 
     /**
      * Add stages where the restart stages are ordered
@@ -86,7 +95,7 @@ public class Grouping {
             getStageText("Preparing", pc.name, preTasksEffectiveHosts),
             preTasks
             );
-        stages.add(stage);
+        m_stages.add(stage);
       }
 
       // !!! FIXME upgrade definition have only one step, and it better be a restart
@@ -98,7 +107,7 @@ public class Grouping {
                 StageWrapper.Type.RESTART,
                 getStageText("Restarting", pc.name, Collections.singleton(hostName)),
                 new TaskWrapper(service, pc.name, Collections.singleton(hostName), t));
-            stages.add(stage);
+            m_stages.add(stage);
           }
         }
       }
@@ -112,11 +121,11 @@ public class Grouping {
             getStageText("Completing", pc.name, postTasksEffectiveHosts),
             postTasks
             );
-        stages.add(stage);
+        m_stages.add(stage);
       }
 
       if (!clientOnly) {
-        serviceChecks.add(service);
+        m_servicesToCheck.add(service);
       }
     }
 
@@ -124,22 +133,22 @@ public class Grouping {
     public List<StageWrapper> build() {
 
       List<TaskWrapper> tasks = new ArrayList<TaskWrapper>();
-      for (String service : serviceChecks) {
+      for (String service : m_servicesToCheck) {
         tasks.add(new TaskWrapper(
             service, "", Collections.<String>emptySet(), new ServiceCheckTask()));
       }
 
-      if (serviceChecks.size() > 0) {
+      if (m_serviceCheck && m_servicesToCheck.size() > 0) {
         StageWrapper wrapper = new StageWrapper(
             StageWrapper.Type.SERVICE_CHECK,
-            "Service Check " + StringUtils.join(serviceChecks, ", "),
+            "Service Check " + StringUtils.join(m_servicesToCheck, ", "),
             tasks.toArray(new TaskWrapper[0])
             );
 
-        stages.add(wrapper);
+        m_stages.add(wrapper);
       }
 
-      return stages;
+      return m_stages;
     }
   }
 

ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java

@@ -0,0 +1,153 @@
+/**
+ * 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.state.stack.upgrade;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlElementWrapper;
+import javax.xml.bind.annotation.XmlType;
+
+import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.api.services.AmbariMetaInfo;
+import org.apache.ambari.server.stack.HostsType;
+import org.apache.ambari.server.state.Cluster;
+import org.apache.ambari.server.state.Service;
+import org.apache.ambari.server.state.ServiceInfo;
+import org.apache.ambari.server.state.StackId;
+import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
+
+/**
+ * Grouping that is used to create stages that are service checks for a cluster.
+ */
+@XmlType(name="service-check")
+public class ServiceCheckGrouping extends Grouping {
+
+  @XmlElementWrapper(name="priority")
+  @XmlElement(name="service")
+  private Set<String> priorityServices = new HashSet<String>();
+
+  private ServiceCheckBuilder m_builder = new ServiceCheckBuilder();
+
+  @Override
+  public ServiceCheckBuilder getBuilder() {
+    return m_builder;
+  }
+
+  /**
+   * Used to build stages for service check groupings.
+   */
+  public class ServiceCheckBuilder extends StageWrapperBuilder {
+    private Cluster m_cluster;
+    private AmbariMetaInfo m_metaInfo;
+
+    /**
+     * Sets the helpers needed to determine service check validity
+     * @param cluster   the cluster
+     * @param metaInfo  the metainfo instance to determine command script availability
+     */
+    public void setHelpers(Cluster cluster, AmbariMetaInfo metaInfo) {
+      m_cluster = cluster;
+      m_metaInfo = metaInfo;
+    }
+
+    @Override
+    public void add(HostsType hostsType, String service, boolean forUpgrade, boolean clientOnly,
+        ProcessingComponent pc) {
+      // !!! nothing to do here
+    }
+
+    @Override
+    public List<StageWrapper> build() {
+      List<StageWrapper> result = new ArrayList<StageWrapper>();
+
+      Map<String, Service> serviceMap = m_cluster.getServices();
+
+      Set<String> clusterServices = new LinkedHashSet<String>(serviceMap.keySet());
+
+      // create stages for the priorities
+      for (String service : ServiceCheckGrouping.this.priorityServices) {
+        if (checkServiceValidity(service, serviceMap)) {
+          StageWrapper wrapper = new StageWrapper(
+              StageWrapper.Type.SERVICE_CHECK,
+              "Service Check " + service,
+              new TaskWrapper(service, "", Collections.<String>emptySet(),
+                  new ServiceCheckTask()));
+          result.add(wrapper);
+
+          clusterServices.remove(service);
+        }
+      }
+
+      // create stages for everything else
+      for (String service : clusterServices) {
+        if (checkServiceValidity(service, serviceMap)) {
+          StageWrapper wrapper = new StageWrapper(
+              StageWrapper.Type.SERVICE_CHECK,
+              "Service Check " + service,
+              new TaskWrapper(service, "", Collections.<String>emptySet(),
+                  new ServiceCheckTask()));
+          result.add(wrapper);
+        }
+      }
+      return result;
+    }
+
+    /**
+     * Checks if the service is valid for a service check
+     *
+     * @param service         the name of the service to check
+     * @param clusterServices the map of available services for a cluster
+     * @return {@code true} if the service is valid and can execute a service check
+     */
+    private boolean checkServiceValidity(String service, Map<String, Service> clusterServices) {
+      if (!clusterServices.containsKey(service)) {
+        return false;
+      } else {
+        Service svc = clusterServices.get(service);
+        if (null == svc) {
+          return false;
+        } else {
+          if (svc.isClientOnlyService()) {
+            return false;
+          } else {
+            StackId stackId = m_cluster.getDesiredStackVersion();
+            try {
+              ServiceInfo si = m_metaInfo.getService(stackId.getStackName(),
+                  stackId.getStackVersion(), service);
+              if (null == si.getCommandScript()) {
+                return false;
+              }
+            } catch (AmbariException e) {
+              return false;
+            }
+          }
+        }
+      }
+
+      return true;
+    }
+  }
+
+}

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

@@ -29,6 +29,7 @@
     </group>
 
     <group name="CORE_MASTER" title="Core Masters">
+      <service-check>false</service-check>
       <service name="HDFS">
         <component>JOURNALNODE</component>
         <component>NAMENODE</component>
@@ -46,9 +47,20 @@
       <service name="HBASE">
         <component>HBASE_MASTER</component>
       </service>
+      
     </group>
-
+    
+    <group name="SERVICE_CHECK" title="Full Service Check" xsi:type="service-check">
+      <priority>
+        <service>HDFS</service>
+        <service>MAPREDUCE2</service>
+        <service>YARN</service>
+        <service>HBASE</service>
+      </priority>
+    </group>
+    
     <group name="CORE_SLAVES" title="Core Slaves" xsi:type="colocated">
+      <service-check>false</service-check>
       <service name="HDFS">
         <component>DATANODE</component>
       </service>
@@ -57,7 +69,7 @@
         <component>HBASE_REGIONSERVER</component>
       </service>
 
-     <service name="YARN">
+      <service name="YARN">
         <component>NODEMANAGER</component>
       </service>
 
@@ -67,6 +79,14 @@
         <message>Core Services have been upgraded. You are advised to perform tests against your cluster to ensure proper operation before proceeding with upgrade of remaining services.</message>
       </batch>
     </group>
+    
+    <group name="SERVICE_CHECK" title="Full Service Check" xsi:type="service-check">
+      <priority>
+        <service>HDFS</service>
+        <service>YARN</service>
+        <service>HBASE</service>
+      </priority>
+    </group>
 
     <group name="HIVE" title="Hive">
       <skippable>true</skippable>
@@ -93,6 +113,45 @@
       </service>
     </group>
 
+    <group name="CLIENTS" title="Client Components">
+      <service name="HDFS">
+        <component>HDFS_CLIENT</component>
+      </service>
+
+      <service name="YARN">
+        <component>YARN_CLIENT</component>
+      </service>
+
+      <service name="MAPREDUCE2">
+        <component>MAPREDUCE2_CLIENT</component>
+      </service>
+
+      <service name="TEZ">
+        <component>TEZ_CLIENT</component>
+      </service>
+
+      <service name="HBASE">
+        <component>HBASE_CLIENT</component>
+      </service>
+
+      <service name="PIG">
+        <component>PIG</component>
+      </service>
+
+      <service name="HIVE">
+        <component>HIVE_CLIENT</component>
+        <component>HCAT</component>
+      </service>
+    </group>
+
+    <group name="SERVICE_CHECK" title="Full Service Check" xsi:type="service-check">
+      <priority>
+        <service>HDFS</service>
+        <service>YARN</service>
+        <service>HBASE</service>
+      </priority>
+    </group>
+
     <group name="KAFKA" title="Kafka">
       <skippable>true</skippable>
       <service name="KAFKA">
@@ -132,36 +191,6 @@
       </service>
     </group>
 
-    <group name="CLIENTS" title="Client Components">
-      <service name="HDFS">
-        <component>HDFS_CLIENT</component>
-      </service>
-
-      <service name="YARN">
-        <component>YARN_CLIENT</component>
-      </service>
-
-      <service name="MAPREDUCE2">
-        <component>MAPREDUCE2_CLIENT</component>
-      </service>
-
-      <service name="TEZ">
-        <component>TEZ_CLIENT</component>
-      </service>
-
-      <service name="HBASE">
-        <component>HBASE_CLIENT</component>
-      </service>
-
-      <service name="PIG">
-        <component>PIG</component>
-      </service>
-
-      <service name="HIVE">
-        <component>HIVE_CLIENT</component>
-        <component>HCAT</component>
-      </service>
-    </group>
 
     <group xsi:type="cluster" name="POST_CLUSTER" title="Finalize Upgrade">
       <skippable>true</skippable>

ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java

@@ -24,6 +24,7 @@ import static org.easymock.EasyMock.createStrictMock;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
+import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -32,7 +33,6 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.hamcrest.CoreMatchers.*;
 
 import java.io.File;
 import java.io.IOException;
@@ -6973,15 +6973,15 @@ public class AmbariManagementControllerTest {
     } catch (StackAccessException e) {
       // do nothing
     }
-    
+
     // test that a stack response has upgrade packs
     requestWithParams = new StackVersionRequest(STACK_NAME, "2.1.1");
     responsesWithParams = controller.getStackVersions(Collections.singleton(requestWithParams));
-    
+
     Assert.assertEquals(1, responsesWithParams.size());
     StackVersionResponse resp = responsesWithParams.iterator().next();
     assertNotNull(resp.getUpgradePacks());
-    assertEquals(2, resp.getUpgradePacks().size());
+    assertEquals(3, resp.getUpgradePacks().size());
     assertTrue(resp.getUpgradePacks().contains("upgrade_test"));
   }
 
@@ -9106,7 +9106,7 @@ public class AmbariManagementControllerTest {
       componentHostRequests.add(new ServiceComponentHostRequest("c1", null, "DATANODE", "host2", "DISABLED"));
       updateHostComponents(amc, componentHostRequests, mapRequestProps, false);
       org.junit.Assert.assertEquals(State.DISABLED, sch.getState());
-      
+
       // State should not be changed if componentHostRequests are empty
       componentHostRequests.clear();
       mapRequestProps.put(RequestOperationLevel.OPERATION_CLUSTER_ID,"c1");

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

@@ -108,7 +108,7 @@ public class UpgradeHelperTest {
     UpgradePack upgrade = upgrades.get("upgrade_test");
     assertNotNull(upgrade);
 
-    Cluster cluster = makeCluster();
+    makeCluster();
 
     List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
         m_masterHostResolver, upgrade, UPGRADE_VERSION);
@@ -136,15 +136,12 @@ public class UpgradeHelperTest {
 
   @Test
   public void testDowngradeOrchestration() throws Exception {
-    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("foo", "bar");
-    assertTrue(upgrades.isEmpty());
-
-    upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
+    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
     assertTrue(upgrades.containsKey("upgrade_test"));
     UpgradePack upgrade = upgrades.get("upgrade_test");
     assertNotNull(upgrade);
 
-    Cluster cluster = makeCluster();
+    makeCluster();
 
     List<UpgradeGroupHolder> groups = m_upgradeHelper.createDowngrade(
         m_masterHostResolver, upgrade, DOWNGRADE_VERSION);
@@ -174,15 +171,12 @@ public class UpgradeHelperTest {
 
   @Test
   public void testBuckets() throws Exception {
-    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("foo", "bar");
-    assertTrue(upgrades.isEmpty());
-
-    upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
+    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
     assertTrue(upgrades.containsKey("upgrade_bucket_test"));
     UpgradePack upgrade = upgrades.get("upgrade_bucket_test");
     assertNotNull(upgrade);
 
-    Cluster cluster = makeCluster();
+    makeCluster();
 
     List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
         m_masterHostResolver, upgrade, UPGRADE_VERSION);
@@ -195,15 +189,12 @@ public class UpgradeHelperTest {
 
   @Test
   public void testManualTaskPostProcessing() throws Exception {
-    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("foo", "bar");
-    assertTrue(upgrades.isEmpty());
-
-    upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
+    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
     assertTrue(upgrades.containsKey("upgrade_test"));
     UpgradePack upgrade = upgrades.get("upgrade_test");
     assertNotNull(upgrade);
 
-    Cluster cluster = makeCluster();
+    makeCluster();
 
     List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
         m_masterHostResolver, upgrade, UPGRADE_VERSION);
@@ -221,6 +212,34 @@ public class UpgradeHelperTest {
         manualTask.message);
   }
 
+
+  @Test
+  public void testServiceCheckStages() throws Exception {
+
+    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
+    assertTrue(upgrades.containsKey("upgrade_test_checks"));
+    UpgradePack upgrade = upgrades.get("upgrade_test_checks");
+    assertNotNull(upgrade);
+
+    makeCluster();
+
+    List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
+        m_masterHostResolver, upgrade, UPGRADE_VERSION);
+
+    assertEquals(7, groups.size());
+
+    // grab the manual task out of ZK which has placeholder text
+    UpgradeGroupHolder zookeeperGroup = groups.get(1);
+    assertEquals("ZOOKEEPER", zookeeperGroup.name);
+    ManualTask manualTask = (ManualTask) zookeeperGroup.items.get(0).getTasks().get(
+        0).getTasks().get(0);
+
+    assertEquals(
+        "This is a manual task with a placeholder of placeholder-rendered-properly",
+        manualTask.message);
+  }
+
+
   /**
    * Create an HA cluster
    * @throws AmbariException

ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java

@@ -32,6 +32,7 @@ import org.apache.ambari.server.api.services.AmbariMetaInfo;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
+import org.apache.ambari.server.state.stack.upgrade.Grouping;
 import org.apache.ambari.server.state.stack.upgrade.RestartTask;
 import org.apache.ambari.server.state.stack.upgrade.Task;
 import org.junit.After;
@@ -139,6 +140,47 @@ public class UpgradePackTest {
     assertEquals(1, pc.tasks.size());
   }
 
+  @Test
+  public void testGroupOrders() {
+    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
+    assertTrue(upgrades.size() > 0);
+    assertTrue(upgrades.containsKey("upgrade_test_checks"));
+
+    UpgradePack up = upgrades.get("upgrade_test_checks");
+
+    List<String> expected_up = Arrays.asList(
+        "PRE_CLUSTER",
+        "ZOOKEEPER",
+        "CORE_MASTER",
+        "SERVICE_CHECK_1",
+        "CORE_SLAVES",
+        "SERVICE_CHECK_2",
+        "POST_CLUSTER");
+
+    List<String> expected_down = Arrays.asList(
+        "PRE_CLUSTER",
+        "CORE_SLAVES",
+        "SERVICE_CHECK_2",
+        "CORE_MASTER",
+        "SERVICE_CHECK_1",
+        "ZOOKEEPER",
+        "POST_CLUSTER");
+
+    int i = 0;
+    List<Grouping> groups = up.getGroups(true);
+    for (Grouping g : groups) {
+      assertEquals(expected_up.get(i), g.name);
+      i++;
+    }
+
+    i = 0;
+    groups = up.getGroups(false);
+    for (Grouping g : groups) {
+      assertEquals(expected_down.get(i), g.name);
+      i++;
+    }
+  }
+
 
   private int indexOf(Map<String, ?> map, String keyToFind) {
     int result = -1;

ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml

@@ -0,0 +1,182 @@
+<?xml version="1.0"?>
+<!--
+   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.
+-->
+<upgrade xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <target>2.2.*</target>
+  
+  <order>
+    <group xsi:type="cluster" name="PRE_CLUSTER" title="Pre Upgrade" stage="pre">
+      <execute-stage title="Confirm 1">
+        <task xsi:type="manual">
+          <message>Foo</message>
+        </task>
+      </execute-stage>
+      <execute-stage service="HDFS" component="NAMENODE" title="Finalize HDFS">
+        <task xsi:type="execute">
+          <command>ls</command>
+        </task>
+      </execute-stage>
+      <execute-stage title="Confirm 2">
+        <task xsi:type="manual">
+          <message>Foo</message>
+        </task>
+      </execute-stage>
+    </group>
+  
+    <group name="ZOOKEEPER" title="Zookeeper">
+      <service name="ZOOKEEPER">
+        <component>ZOOKEEPER_SERVER</component>
+        <component>ZOOKEEPER_CLIENT</component>
+      </service>
+    </group>
+    <group name="CORE_MASTER" title="Core Masters">
+      <service name="HDFS">
+        <component>JOURNALNODE</component>
+        <component>NAMENODE</component>
+      </service>
+      <service name="YARN">
+        <component>RESOURCEMANAGER</component>
+      </service>
+      <service-check>false</service-check>
+    </group>
+    
+    <group name="SERVICE_CHECK_1" title="Post-Master Service Checks" xsi:type="service-check">
+      <priority>
+        <service>HDFS</service>
+        <service>YARN</service>
+      </priority>
+    </group>
+    
+    <group name="CORE_SLAVES" title="Core Slaves" xsi:type="colocated">
+      <skippable>true</skippable>      <!-- set skippable for test -->
+      <allow-retry>false</allow-retry> <!-- set no retry for test -->
+      <service name="HDFS">
+        <component>DATANODE</component>
+      </service>
+      <service name="HBASE">
+        <component>REGIONSERVER</component>
+      </service>
+      <service name="YARN">
+        <component>NODEMANAGER</component>
+      </service>
+      
+      <batch>
+        <percent>20</percent>
+        <message>Please run additional tests</message>
+      </batch>
+    </group>
+    
+    <group name="SERVICE_CHECK_2" title="Post-Slave Service Checks" xsi:type="service-check">
+      <priority>
+        <service>HDFS</service>
+        <service>YARN</service>
+      </priority>
+    </group>
+    
+    
+    <group xsi:type="cluster" name="POST_CLUSTER" title="Finalize Upgrade" stage="post">
+      <execute-stage title="Confirm Finalize">
+        <task xsi:type="manual">
+          <message>Please confirm you are ready to finalize</message>
+        </task>
+      </execute-stage>
+      <execute-stage service="HDFS" component="NAMENODE" title="Execute HDFS Finalize">
+        <task xsi:type="execute">
+          <command>ls</command>
+        </task>
+      </execute-stage>
+      <execute-stage title="Save Cluster State" service="" component="">
+        <task xsi:type="server_action" class="org.apache.ambari.server.serveraction.upgrades.FinalizeUpgradeAction">
+        </task>
+      </execute-stage>
+    </group>
+        
+  </order>
+  
+
+  <processing>
+  
+    <service name="ZOOKEEPER">
+      <component name="ZOOKEEPER_SERVER">
+        <pre-upgrade>
+          <task xsi:type="manual">
+            <summary>SUMMARY OF PREPARE</summary>
+            <message>This is a manual task with a placeholder of {{foo/bar}}</message>
+          </task>
+        </pre-upgrade>
+        <upgrade>
+          <task xsi:type="restart" />
+        </upgrade>
+        <post-upgrade>
+          <task xsi:type="configure" />
+        </post-upgrade>
+      </component>
+    </service>
+    <service name="HDFS">
+      <component name="NAMENODE">
+        <pre-upgrade>
+          <task xsi:type="execute" hosts="master">
+            <command>su - {hdfs-user} -c 'dosomething'</command>
+          </task>
+          <task xsi:type="configure">
+            <type>hdfs-site</type>
+            <key>myproperty</key>
+            <value>mynewvalue</value>
+          </task>
+          <task xsi:type="manual">
+            <message>Update your database</message>
+          </task>
+        </pre-upgrade>
+        <upgrade>
+          <task xsi:type="restart" />
+        </upgrade>
+        <post-upgrade>
+          <task xsi:type="execute">
+            <command>ls</command>
+          </task>
+        </post-upgrade>
+      </component>
+      <component name="DATANODE">
+        <pre-downgrade />
+        <upgrade>
+          <task xsi:type="restart" />
+        </upgrade>
+        <post-downgrade>
+          <task xsi:type="manual">
+            <message>Manual Downgrade</message>
+          </task>
+        </post-downgrade>
+      </component>
+    </service>
+    <service name="YARN">
+      <component name="RESOURCEMANAGER">
+        <pre-upgrade>
+          <task xsi:type="execute">
+            <command>ls</command>
+          </task>
+        </pre-upgrade>
+      </component>
+      <component name="NODEMANAGER">
+        <pre-upgrade>
+          <task xsi:type="execute">
+            <command>ls</command>
+          </task>
+        </pre-upgrade>
+      </component>
+    </service>
+  </processing>
+</upgrade>