AMBARI-9939. RU - Service Check group to include all services with a service_check script (alejandro)

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

@@ -145,7 +145,7 @@ public class UpgradeContext {
    * @param displayName the display name for the service
    */
   public void setServiceDisplay(String service, String displayName) {
-    m_serviceNames.put(service, displayName);
+    m_serviceNames.put(service, (displayName == null) ? service : displayName);
   }
 
   /**

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

@@ -33,11 +33,14 @@ 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.CommandScriptDefinition;
 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.UpgradeContext;
 import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Grouping that is used to create stages that are service checks for a cluster.
@@ -45,6 +48,8 @@ import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
 @XmlType(name="service-check")
 public class ServiceCheckGrouping extends Grouping {
 
+  private static Logger LOG = LoggerFactory.getLogger(ServiceCheckGrouping.class);
+
   @XmlElementWrapper(name="priority")
   @XmlElement(name="service")
   private Set<String> priorityServices = new HashSet<String>();
@@ -98,7 +103,7 @@ public class ServiceCheckGrouping extends Grouping {
         }
       }
 
-      // create stages for everything else
+      // create stages for everything else, as long it is valid
       for (String service : clusterServices) {
         if (checkServiceValidity(ctx, service, serviceMap)) {
           StageWrapper wrapper = new StageWrapper(
@@ -120,36 +125,24 @@ public class ServiceCheckGrouping extends Grouping {
      * @return {@code true} if the service is valid and can execute a service check
      */
     private boolean checkServiceValidity(UpgradeContext ctx, String service, Map<String, Service> clusterServices) {
-      if (!clusterServices.containsKey(service)) {
-        return false;
-      } else {
+      if (clusterServices.containsKey(service)) {
         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 != svc) {
+          // Services that only have clients such as Pig can still have service check scripts.
+          StackId stackId = m_cluster.getDesiredStackVersion();
+          try {
+            ServiceInfo si = m_metaInfo.getService(stackId.getStackName(), stackId.getStackVersion(), service);
+            CommandScriptDefinition script = si.getCommandScript();
+            if (null != script && null != script.getScript() && !script.getScript().isEmpty()) {
               ctx.setServiceDisplay(service, si.getDisplayName());
-
-              if (null == si.getCommandScript()) {
-                return false;
-              }
-            } catch (AmbariException e) {
-              return false;
+              return true;
             }
+          } catch (AmbariException e) {
+            LOG.error("Could not determine if service " + service + " can run a service check. Exception: " + e.getMessage());
           }
         }
       }
-
-      return true;
+      return false;
     }
-
   }
-
 }

ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerTest.java

@@ -191,7 +191,7 @@ public class StackManagerTest {
     Collection<ServiceInfo> services = stack.getServices();
 
     //should include all stacks in hierarchy
-    assertEquals(14, services.size());
+    assertEquals(15, services.size());
     HashSet<String> expectedServices = new HashSet<String>();
     expectedServices.add("GANGLIA");
     expectedServices.add("HBASE");
@@ -207,6 +207,7 @@ public class StackManagerTest {
     expectedServices.add("STORM");
     expectedServices.add("FLUME");
     expectedServices.add("FAKENAGIOS");
+    expectedServices.add("TEZ");
 
     ServiceInfo pigService = null;
     for (ServiceInfo service : services) {

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

@@ -25,12 +25,16 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
@@ -431,9 +435,25 @@ public class UpgradeHelperTest {
     assertNotNull(upgrade);
 
     Cluster c = makeCluster();
-    c.addService("HBASE");
-
+    // HBASE and PIG have service checks, but not TEZ.
+    Set<String> additionalServices = new HashSet<String>() {{ add("HBASE"); add("PIG"); add("TEZ"); }};
+    for(String service : additionalServices) {
+      c.addService(service);
+    }
 
+    int numServiceChecksExpected = 0;
+    Collection<Service> services = c.getServices().values();
+    for(Service service : services) {
+      ServiceInfo si = ambariMetaInfo.getService(c.getCurrentStackVersion().getStackName(),
+          c.getCurrentStackVersion().getStackVersion(), service.getName());
+      if (null != si.getCommandScript()) {
+        numServiceChecksExpected++;
+        if (service.getName().equalsIgnoreCase("TEZ")) {
+          assertTrue("Expect Tez to not have any service checks", false);
+        }
+        continue;
+      }
+    }
 
     UpgradeContext context = new UpgradeContext(m_masterHostResolver,
         UPGRADE_VERSION, Direction.UPGRADE);
@@ -444,14 +464,20 @@ public class UpgradeHelperTest {
 
     UpgradeGroupHolder holder = groups.get(3);
     assertEquals(holder.name, "SERVICE_CHECK_1");
-    assertEquals(5, holder.items.size());
-    boolean found = false;
+    assertEquals(6, holder.items.size());
+    int numServiceChecksActual = 0;
     for (StageWrapper sw : holder.items) {
-      if (sw.getText().contains("HBase")) {
-        found = true;
+      for(Service service : services) {
+        Pattern p = Pattern.compile(".*" + service.getName(), Pattern.CASE_INSENSITIVE);
+        Matcher matcher = p.matcher(sw.getText());
+        if (matcher.matches()) {
+          numServiceChecksActual++;
+          continue;
+        }
       }
     }
-    assertTrue("Expected string 'HBase' in text", found);
+
+    assertEquals(numServiceChecksActual, numServiceChecksExpected);
 
     // grab the manual task out of ZK which has placeholder text
     UpgradeGroupHolder zookeeperGroup = groups.get(1);

ambari-server/src/test/resources/stacks/HDP/2.1.1/services/PIG/metainfo.xml

@@ -20,6 +20,7 @@
   <services>
     <service>
       <name>PIG</name>
+      <displayName>Pig</displayName>
       <comment>Scripting platform for analyzing large datasets (Extended)</comment>
       <version>0.12.1.2.1.1</version>
         <components>

ambari-server/src/test/resources/stacks/HDP/2.1.1/services/TEZ/metainfo.xml

@@ -0,0 +1,34 @@
+<?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.
+-->
+<metainfo>
+  <schemaVersion>2.0</schemaVersion>
+  <services>
+    <service>
+      <name>TEZ</name>
+      <displayName>Tez</displayName>
+      <comment>Tez is the next generation Hadoop Query Processing framework written on top of YARN</comment>
+      <version>0.1.0.22-1</version>
+      <components>
+        <component>
+          <name>TEZ_CLIENT</name>
+          <category>CLIENT</category>
+        </component>
+      </components>
+    </service>
+  </services>
+</metainfo>