Bladeren bron

AMBARI-9109 - Manual Upgrade Tasks Should Be More Explicit And Parameterized (jonathanhurley)

Jonathan Hurley 10 jaren geleden
bovenliggende
commit
a4c94d7057

+ 9 - 2
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java

@@ -70,6 +70,12 @@ public class UpgradeGroupResourceProvider extends AbstractControllerResourceProv
   @Inject
   private static UpgradeDAO m_dao = null;
 
+  /**
+   * Used to generated the correct tasks and stages during an upgrade.
+   */
+  @Inject
+  private static UpgradeHelper s_upgradeHelper;
+
   static {
     // properties
     PROPERTY_IDS.add(UPGRADE_REQUEST_ID);
@@ -122,7 +128,7 @@ public class UpgradeGroupResourceProvider extends AbstractControllerResourceProv
 
       List<UpgradeGroupEntity> groups = upgrade.getUpgradeGroups();
       if (null != groups) {
-        UpgradeHelper helper = new UpgradeHelper();
+
 
         for (UpgradeGroupEntity group : upgrade.getUpgradeGroups()) {
           Resource r = toResource(upgrade, group, requestPropertyIds);
@@ -132,7 +138,8 @@ public class UpgradeGroupResourceProvider extends AbstractControllerResourceProv
             stageIds.add(itemEntity.getStageId());
           }
 
-          Set<Resource> stages = helper.getStageResources(clusterName, upgrade.getRequestId(), stageIds);
+          Set<Resource> stages = s_upgradeHelper.getStageResources(clusterName,
+              upgrade.getRequestId(), stageIds);
 
           aggregate(r, stages, requestPropertyIds);
 

+ 8 - 4
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java

@@ -68,6 +68,12 @@ public class UpgradeItemResourceProvider extends ReadOnlyResourceProvider {
   @Inject
   private static UpgradeDAO m_dao = null;
 
+  /**
+   * Used to generated the correct tasks and stages during an upgrade.
+   */
+  @Inject
+  private static UpgradeHelper s_upgradeHelper;
+
 
   static {
     // properties
@@ -188,10 +194,8 @@ public class UpgradeItemResourceProvider extends ReadOnlyResourceProvider {
 
       if (!resultMap.isEmpty()) {
         if (null != clusterName) {
-          UpgradeHelper helper = new UpgradeHelper();
-
-          Set<Resource> stages = helper.getStageResources(clusterName, requestId,
-              new ArrayList<Long>(resultMap.keySet()));
+          Set<Resource> stages = s_upgradeHelper.getStageResources(clusterName,
+              requestId, new ArrayList<Long>(resultMap.keySet()));
 
           for (Resource stage : stages) {
             Long l = (Long) stage.getPropertyValue(StageResourceProvider.STAGE_STAGE_ID);

+ 30 - 22
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java

@@ -107,19 +107,31 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider
 
   @Inject
   private static UpgradeDAO m_upgradeDAO = null;
+
   @Inject
   private static Provider<AmbariMetaInfo> m_metaProvider = null;
+
   @Inject
   private static RepositoryVersionDAO m_repoVersionDAO = null;
+
   @Inject
   private static Provider<RequestFactory> requestFactory;
+
   @Inject
   private static Provider<StageFactory> stageFactory;
+
   @Inject
   private static Provider<AmbariActionExecutionHelper> actionExecutionHelper;
+
   @Inject
   private static Provider<AmbariCustomCommandExecutionHelper> commandExecutionHelper;
 
+  /**
+   * Used to generated the correct tasks and stages during an upgrade.
+   */
+  @Inject
+  private static UpgradeHelper s_upgradeHelper;
+
   private static Map<String, String> REQUEST_PROPERTY_MAP = new HashMap<String, String>();
 
   static {
@@ -222,14 +234,14 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider
         upgrades = m_upgradeDAO.findUpgrades(cluster.getClusterId());
       }
 
-      UpgradeHelper helper = new UpgradeHelper();
       for (UpgradeEntity entity : upgrades) {
         Resource r = toResource(entity, clusterName, requestPropertyIds);
         results.add(r);
 
         // !!! not terribly efficient, but that's ok in this case.  The handful-per-year
         // an upgrade is done won't kill performance.
-        Resource r1 = helper.getRequestResource(clusterName, entity.getRequestId());
+        Resource r1 = s_upgradeHelper.getRequestResource(clusterName,
+            entity.getRequestId());
         for (Entry<String, String> entry : REQUEST_PROPERTY_MAP.entrySet()) {
           Object o = r1.getPropertyValue(entry.getKey());
 
@@ -332,27 +344,24 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider
    * @param cluster Cluster
    * @param upgradeItem the item whose tasks will be injected.
    */
-  private void injectVariables(ConfigHelper configHelper, Cluster cluster, UpgradeItemEntity upgradeItem) {
+  private void injectVariables(ConfigHelper configHelper, Cluster cluster,
+      UpgradeItemEntity upgradeItem) {
     final String regexp = "(\\{\\{.*?\\}\\})";
 
     String task = upgradeItem.getTasks();
     if (task != null && !task.isEmpty()) {
       Matcher m = Pattern.compile(regexp).matcher(task);
-      while(m.find()) {
+      while (m.find()) {
         String origVar = m.group(1);
-        String formattedVar = origVar.substring(2, origVar.length() - 2).trim();
-
-        int posConfigFile = formattedVar.indexOf("/");
-        if (posConfigFile > 0) {
-          String configType = formattedVar.substring(0, posConfigFile);
-          String propertyName = formattedVar.substring(posConfigFile + 1, formattedVar.length());
-          try {
-            String configValue = configHelper.getPropertyValueFromStackDefinitions(cluster, configType, propertyName);
-            task = task.replace(origVar, configValue);
-          } catch (Exception err) {
-            LOG.error(String.format("Exception trying to retrieve property %s/%s. Error: %s", configType, propertyName, err.getMessage()));
-          }
+        String configValue = configHelper.getPlaceholderValueFromDesiredConfigurations(
+            cluster, origVar);
+
+        if (null != configValue) {
+          task = task.replace(origVar, configValue);
+        } else {
+          LOG.error("Unable to retrieve value for {}", origVar);
         }
+
       }
       upgradeItem.setTasks(task);
     }
@@ -371,16 +380,17 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider
     ConfigHelper configHelper = getManagementController().getConfigHelper();
 
     MasterHostResolver mhr = new MasterHostResolver(cluster);
-    UpgradeHelper helper = new UpgradeHelper();
-
     String forceDowngrade = (String) requestMap.get(UPGRADE_FORCE_DOWNGRADE);
 
     List<UpgradeGroupHolder> groups = null;
 
+    // the version being upgraded or downgraded to (ie hdp-2.2.1.0-1234)
+    final String version = (String) requestMap.get(UPGRADE_VERSION);
+
     if (null != forceDowngrade && Boolean.parseBoolean(forceDowngrade)) {
-      groups = helper.createDowngrade(cluster, mhr, pack);
+      groups = s_upgradeHelper.createDowngrade(mhr, pack, version);
     } else {
-      groups = helper.createUpgrade(cluster, mhr, pack);
+      groups = s_upgradeHelper.createUpgrade(mhr, pack, version);
     }
 
     if (groups.isEmpty()) {
@@ -388,8 +398,6 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider
     }
 
     List<UpgradeGroupEntity> groupEntities = new ArrayList<UpgradeGroupEntity>();
-
-    final String version = (String) requestMap.get(UPGRADE_VERSION);
     RequestStageContainer req = createRequest(version);
 
     for (UpgradeGroupHolder group : groups) {

+ 34 - 24
ambari-server/src/main/java/org/apache/ambari/server/stack/MasterHostResolver.java

@@ -58,6 +58,16 @@ public class MasterHostResolver {
     this.cluster = cluster;
   }
 
+  /**
+   * Gets the cluster that this instace of the {@link MasterHostResolver} is
+   * initialized with.
+   *
+   * @return the cluster (not {@code null}).
+   */
+  public Cluster getCluster() {
+    return cluster;
+  }
+
   /**
    * Get the master hostname of the given service and component.
    * @param serviceName Service
@@ -75,7 +85,7 @@ public class MasterHostResolver {
     if (0 == componentHosts.size()) {
       return null;
     }
-    
+
     hostsType.hosts = componentHosts;
 
     Service s = null;
@@ -119,12 +129,12 @@ public class MasterHostResolver {
    */
   private Map<Status, String> getNameNodePair(Set<String> hosts) {
     Map<Status, String> stateToHost = new HashMap<Status, String>();
-    
+
     if (hosts != null && hosts.size() == 2) {
       for (String hostname : hosts) {
         String state = queryJmxBeanValue(hostname, 50070,
             "Hadoop:service=NameNode,name=NameNodeStatus", "State", true);
-        
+
         if (null != state &&
             (state.equalsIgnoreCase(Status.ACTIVE.toString()) ||
                 state.equalsIgnoreCase(Status.STANDBY.toString()))) {
@@ -132,7 +142,7 @@ public class MasterHostResolver {
             stateToHost.put(status, hostname);
           }
         }
-        
+
       if (stateToHost.containsKey(Status.ACTIVE) && stateToHost.containsKey(Status.STANDBY) && !stateToHost.get(Status.ACTIVE).equalsIgnoreCase(stateToHost.get(Status.STANDBY))) {
         return stateToHost;
       }
@@ -140,37 +150,37 @@ public class MasterHostResolver {
 
     return null;
   }
-  
+
   private void resolveResourceManagers(HostsType hostType) {
     // !!! for RM, only the master returns jmx
     Set<String> orderedHosts = new LinkedHashSet<String>(hostType.hosts);
-    
+
     for (String hostname : hostType.hosts) {
-      
+
       String value = queryJmxBeanValue(hostname, 8088,
           "Hadoop:service=ResourceManager,name=RMNMInfo", "modelerType", true);
-      
+
       if (null != value) {
         if (null == hostType.master) {
           hostType.master = hostname;
         }
-        
+
         // !!! quick and dirty to make sure the master is last in the list
         orderedHosts.remove(hostname);
         orderedHosts.add(hostname);
       }
-      
+
     }
     hostType.hosts = orderedHosts;
   }
-  
+
   private void resolveHBaseMasters(HostsType hostsType) {
-    
+
     for (String hostname : hostsType.hosts) {
-      
+
       String value = queryJmxBeanValue(hostname, 60010,
           "Hadoop:service=HBase,name=Master,sub=Server", "tag.isActiveMaster", false);
-      
+
       if (null != value) {
         Boolean bool = Boolean.valueOf(value);
         if (bool.booleanValue()) {
@@ -179,36 +189,36 @@ public class MasterHostResolver {
           hostsType.secondary = hostname;
         }
       }
-      
+
     }
   }
-  
+
   private String queryJmxBeanValue(String hostname, int port, String beanName, String attributeName,
       boolean asQuery) {
-    
+
     String endPoint = asQuery ?
         String.format("http://%s:%s/jmx?qry=%s", hostname, port, beanName) :
           String.format("http://%s:%s/jmx?get=%s::%s", hostname, port, beanName, attributeName);
-    
+
     String response = HTTPUtils.requestURL(endPoint);
-    
+
     if (null == response || response.isEmpty()) {
       return null;
     }
-    
+
     Type type = new TypeToken<Map<String, ArrayList<HashMap<String, String>>>>() {}.getType();
 
     try {
       Map<String, ArrayList<HashMap<String, String>>> jmxBeans =
           StageUtils.getGson().fromJson(response, type);
-      
+
       return jmxBeans.get("beans").get(0).get(attributeName);
     } catch (Exception e) {
       LOG.info("Could not load JMX from {}/{} from {}", beanName, attributeName, hostname, e);
     }
-    
+
     return null;
-    
+
   }
-  
+
 }

+ 49 - 0
ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java

@@ -490,6 +490,55 @@ public class ConfigHelper {
     return null;
   }
 
+  /**
+   * Gets the configuration value referenced by the specified placeholder from
+   * the cluster configuration. This will take a configuration placeholder such
+   * as {{hdfs-site/foo}} and return the value of {@code foo} defined in
+   * {@code hdfs-site}.
+   *
+   * @param cluster
+   *          the cluster to use when rendering the placeholder value (not
+   *          {@code null}).
+   * @param placeholder
+   *          the placeholder value, such as {{hdfs-site/foobar}} (not
+   *          {@code null} )
+   * @return the configuration value, or {@code null} if none.
+   * @throws AmbariException
+   *           if there was a problem parsing the placeholder or retrieving the
+   *           referenced value.
+   */
+  public String getPlaceholderValueFromDesiredConfigurations(Cluster cluster,
+      String placeholder) {
+    // remove the {{ and }} from the placholder
+    if (placeholder.startsWith("{{") && placeholder.endsWith("}}")) {
+      placeholder = placeholder.substring(2, placeholder.length() - 2).trim();
+    }
+
+    // break up hdfs-site/foobar into hdfs-site and foobar
+    int delimiterPosition = placeholder.indexOf("/");
+    if (delimiterPosition < 0) {
+      return placeholder;
+    }
+
+    String configType = placeholder.substring(0, delimiterPosition);
+    String propertyName = placeholder.substring(delimiterPosition + 1,
+        placeholder.length());
+
+    // return the value if it exists, otherwise return the placeholder
+    Map<String, DesiredConfig> desiredConfigs = cluster.getDesiredConfigs();
+    DesiredConfig desiredConfig = desiredConfigs.get(configType);
+    Config config = cluster.getConfig(configType, desiredConfig.getTag());
+    Map<String, String> configurationProperties = config.getProperties();
+    if (null != configurationProperties) {
+      String value = configurationProperties.get(propertyName);
+      if( null != value ) {
+        return value;
+      }
+    }
+
+    return placeholder;
+  }
+
   public ServiceInfo getPropertyOwnerService(Cluster cluster, String configType, String propertyName) throws AmbariException {
     StackId stackId = cluster.getCurrentStackVersion();
     StackInfo stack = ambariMetaInfo.getStack(stackId.getStackName(), stackId.getStackVersion());

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

@@ -24,6 +24,8 @@ import java.util.LinkedHashSet;
 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.controller.internal.RequestResourceProvider;
@@ -47,60 +49,130 @@ import org.apache.ambari.server.state.stack.UpgradePack;
 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.StageWrapper;
 import org.apache.ambari.server.state.stack.upgrade.StageWrapperBuilder;
+import org.apache.ambari.server.state.stack.upgrade.Task;
+import org.apache.ambari.server.state.stack.upgrade.Task.Type;
 import org.apache.ambari.server.state.stack.upgrade.TaskWrapper;
 import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
 /**
  * Class to assist with upgrading a cluster.
  */
+@Singleton
 public class UpgradeHelper {
 
-  private static Logger LOG = LoggerFactory.getLogger(UpgradeHelper.class);
+  private static final Logger LOG = LoggerFactory.getLogger(UpgradeHelper.class);
+
+  /**
+   * Matches on placeholder values such as
+   *
+   * <pre>
+   * {{host}}
+   * </pre>
+   * and
+   * <pre>
+   * {{hdfs-site/foo}}
+   * </pre>
+   */
+  private static final Pattern PLACEHOLDER_REGEX = Pattern.compile("(\\{\\{.*?\\}\\})");
+
+  /**
+   * A placeholder token that represents all of the hosts that a component is
+   * deployed on. This can be used for cases where text needs to be rendered
+   * with all of the hosts mentioned by their FQDN.
+   */
+  private static final String PLACEHOLDER_HOST_ALL = "{{hosts.all}}";
+
+  /**
+   * A placeholder token that represents a single, active master that a
+   * component is deployed on. This can be used for cases where text needs to eb
+   * rendered with a single master host FQDN inserted.
+   */
+  private static final String PLACEHOLDER_HOST_MASTER = "{{hosts.master}}";
+
+  /**
+   * The version that the stack is being upgraded or downgraded to, such as
+   * {@code 2.2.1.0-1234}.
+   */
+  private static final String PLACEHOLDER_VERSION = "{{version}}";
 
   /**
-   * Generates a list of UpgradeGroupHolder items that are used to execute a downgrade
-   * @param cluster     the cluster
-   * @param mhr         Master Host Resolver needed to get master and secondary
-   *                    hosts of several components like NAMENODE
-   * @param upgradePack the upgrade pack
+   * Used to render parameter placeholders in {@link ManualTask}s after the
+   * {@link StageWrapperBuilder} has finished building out all of the stages.
+   */
+  @Inject
+  private Provider<ConfigHelper> m_configHelper;
+
+  /**
+   * Generates a list of UpgradeGroupHolder items that are used to execute a
+   * downgrade
+   *
+   * @param mhr
+   *          Master Host Resolver needed to get master and secondary hosts of
+   *          several components like NAMENODE
+   * @param upgradePack
+   *          the upgrade pack
+   * @param version
+   *          the version of the stack that the downgrade to, such as
+   *          {@code 2.2.0.0-2041}.
    * @return the list of holders
    */
-  public List<UpgradeGroupHolder> createDowngrade(Cluster cluster, MasterHostResolver mhr, UpgradePack upgradePack) throws AmbariException {
-    return createSequence(cluster, mhr, upgradePack, false);
+  public List<UpgradeGroupHolder> createDowngrade(MasterHostResolver mhr,
+      UpgradePack upgradePack, String version) throws AmbariException {
+    return createSequence(mhr, upgradePack, version, false);
   }
 
   /**
-   * Generates a list of UpgradeGroupHolder items that are used to execute an upgrade
-   * @param cluster     the cluster
-   * @param mhr         Master Host Resolver needed to get master and secondary
-   *                    hosts of several components like NAMENODE
-   * @param upgradePack the upgrade pack
+   * Generates a list of UpgradeGroupHolder items that are used to execute an
+   * upgrade
+   *
+   * @param mhr
+   *          Master Host Resolver needed to get master and secondary hosts of
+   *          several components like NAMENODE
+   * @param upgradePack
+   *          the upgrade pack
+   * @param version
+   *          the version of the stack that the upgrade is to, such as
+   *          {@code 2.2.0.0-2041}.
    * @return the list of holders
    */
-  public List<UpgradeGroupHolder> createUpgrade(Cluster cluster, MasterHostResolver mhr, UpgradePack upgradePack) throws AmbariException {
-    return createSequence(cluster, mhr, upgradePack, true);
+  public List<UpgradeGroupHolder> createUpgrade(MasterHostResolver mhr,
+      UpgradePack upgradePack, String version) throws AmbariException {
+    return createSequence(mhr, upgradePack, version, true);
   }
 
 
   /**
-   * Generates a list of UpgradeGroupHolder items that are used to execute an upgrade
-   * @param cluster     the cluster
-   * @param mhr         Master Host Resolver needed to get master and secondary
-   *                    hosts of several components like NAMENODE
-   * @param upgradePack the upgrade pack
-   * @param forUpgrade  {@code true} if the sequence is for an upgrade, {@code false} if for a downgrade
+   * Generates a list of UpgradeGroupHolder items that are used to execute an
+   * upgrade
+   *
+   * @param mhr
+   *          Master Host Resolver needed to get master and secondary hosts of
+   *          several components like NAMENODE
+   * @param upgradePack
+   *          the upgrade pack
+   * @param version
+   *          the version of the stack that the upgrade or downgrade is to, such
+   *          as {@code 2.2.0.0-2041}.
+   * @param forUpgrade
+   *          {@code true} if the sequence is for an upgrade, {@code false} if
+   *          for a downgrade
    * @return the list of holders
    *
    */
-  private List<UpgradeGroupHolder> createSequence(Cluster cluster,
-      MasterHostResolver mhr, UpgradePack upgradePack, boolean forUpgrade)
+  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;
@@ -179,6 +251,9 @@ public class UpgradeHelper {
 
       List<StageWrapper> proxies = builder.build();
 
+      // post process all of the tasks
+      postProcessTasks(proxies, mhr, version);
+
       if (!proxies.isEmpty()) {
         groupHolder.items = proxies;
         if (forUpgrade) {
@@ -208,6 +283,79 @@ public class UpgradeHelper {
     return groups;
   }
 
+  /**
+   * Walks through the generated upgrade hierarchy and applies special
+   * processing to any tasks that require it. An example of this are manual
+   * tasks that have some parameter placeholders rendered.
+   *
+   * @param stageWrappers
+   *          the stage wrappers (not {@code null}).
+   * @param mhr
+   *          a helper to resolve masters, slaves, and hosts (not {@code null}).
+   * @param version
+   *          the version of the stack being upgraded or dowgraded to, such as
+   *          {@code 2.2.0.0-1234} (not {@code null}).
+   */
+  private void postProcessTasks(List<StageWrapper> stageWrappers,
+      MasterHostResolver mhr, String version) throws AmbariException {
+    Cluster cluster = mhr.getCluster();
+
+    for (StageWrapper stageWrapper : stageWrappers) {
+      List<TaskWrapper> taskWrappers = stageWrapper.getTasks();
+      for (TaskWrapper taskWrapper : taskWrappers) {
+        List<Task> tasks = taskWrapper.getTasks();
+
+        // if the task is a manual task, then render any placeholders such
+        // as {{hdfs-site/foo}}
+        for (Task task : tasks) {
+          if (task.getType() == Type.MANUAL) {
+            List<String> tokens = new ArrayList<String>(5);
+            ManualTask manualTask = (ManualTask) task;
+
+            // if there is no message for some reason, skip this one
+            if (null == manualTask.message) {
+              continue;
+            }
+
+            Matcher matcher = PLACEHOLDER_REGEX.matcher(manualTask.message);
+            while (matcher.find()) {
+              tokens.add(matcher.group(1));
+            }
+
+            // iterate through all of the matched tokens
+            for (String token : tokens) {
+              String value = token;
+              if (token.equals(PLACEHOLDER_HOST_ALL)) {
+                HostsType hostsType = mhr.getMasterAndHosts(
+                    taskWrapper.getService(), taskWrapper.getComponent());
+
+                if (null != hostsType) {
+                  value = StringUtils.join(hostsType.hosts, ", ");
+                }
+              } else if (token.equals(PLACEHOLDER_HOST_MASTER)) {
+                HostsType hostsType = mhr.getMasterAndHosts(
+                    taskWrapper.getService(), taskWrapper.getComponent());
+
+                if (null != hostsType) {
+                  value = hostsType.master;
+                }
+              } else if (token.equals(PLACEHOLDER_VERSION)) {
+                value = version;
+              } else {
+                value = m_configHelper.get().getPlaceholderValueFromDesiredConfigurations(
+                    cluster, token);
+              }
+
+              // replace the token in the message with the value
+              if (null != value) {
+                manualTask.message = manualTask.message.replace(token, value);
+              }
+            }
+          }
+        }
+      }
+    }
+  }
 
   /**
    * Short-lived objects that hold information about upgrade groups

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

@@ -213,7 +213,7 @@
           <task xsi:type="restart" />
         </upgrade>
       </component>
-      
+
       <component name="HDFS_CLIENT">
         <upgrade>
           <task xsi:type="restart" />
@@ -233,7 +233,7 @@
           <task xsi:type="restart" />
         </upgrade>
       </component>
-      
+
       <component name="MAPREDUCE2_CLIENT">
         <upgrade>
           <task xsi:type="restart" />
@@ -273,7 +273,7 @@
         </upgrade>
       </component>
     </service>
-    
+
     <service name="HBASE">
       <component name="HBASE_MASTER">
         <pre-upgrade>
@@ -292,7 +292,7 @@
           <task xsi:type="restart" />
         </upgrade>
       </component>
-      
+
       <component name="HBASE_CLIENT">
         <upgrade>
           <task xsi:type="restart" />
@@ -307,7 +307,7 @@
         </upgrade>
       </component>
     </service>
-    
+
     <service name="PIG">
       <component name="PIG">
         <upgrade>
@@ -320,10 +320,10 @@
       <component name="HIVE_METASTORE">
         <pre-upgrade>
           <task xsi:type="manual">
-            <message>Backup the Hive Metastore database.</message>
+            <message>Backup the Hive Metastore database located on the following host(s): {{hosts.all}}</message>
           </task>
           <task xsi:type="manual">
-            <message>Run the SQL file at /usr/hdp/$version/hive/scripts/metastore/upgrade to update the Hive Metastore schema.</message>
+            <message>Consult the README documentation at /usr/hdp/{{version}}/hive/scripts/metastore/upgrade to correctly upgrade the Hive Metastore database. This database upgrade should be performed on the following host(s): {{hosts.all}}</message>
           </task>
         </pre-upgrade>
         <upgrade>
@@ -334,7 +334,7 @@
       <component name="HIVE_SERVER">
         <pre-upgrade>
           <task xsi:type="manual">
-            <message>The HiveServer port will now change to 10010. You can use "netstat -anp | grep 10010" to determine if the port is available on each HiveServer host. If the port is not available, the process using it must be terminated.</message>
+            <message>The HiveServer port will now change to 10010. You can use "netstat -anp | grep 10010" to determine if the port is available on each of following HiveServer host(s): {{hosts.all}}. If the port is not available, the process using it must be terminated.</message>
           </task>
 
           <task xsi:type="configure">
@@ -343,10 +343,10 @@
             <value>10010</value>
           </task>
         </pre-upgrade>
-        
+
         <pre-downgrade>
           <task xsi:type="manual">
-            <message>The HiveServer port will now change to 10000. You can use "netstat -anp | grep 10000" to determine if the port is available on each HiveServer host. If the port is not available, the process using it must be terminated.</message>
+            <message>The HiveServer port will now change to 10000. You can use "netstat -anp | grep 10000" to determine if the port is available on each of following HiveServer host(s): {{hosts.all}}. If the port is not available, the process using it must be terminated.</message>
           </task>
 
           <task xsi:type="configure">
@@ -378,7 +378,7 @@
       <component name="OOZIE_SERVER">
         <pre-upgrade>
           <task xsi:type="manual">
-            <message>Backup the Oozie Server database.</message>
+            <message>Backup the Oozie Server database on {{oozie-env/oozie_hostname}}</message>
           </task>
         </pre-upgrade>
         <upgrade>

+ 35 - 3
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java

@@ -17,6 +17,7 @@
  */
 package org.apache.ambari.server.controller.internal;
 
+import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -52,19 +53,24 @@ import org.apache.ambari.server.orm.entities.UpgradeGroupEntity;
 import org.apache.ambari.server.orm.entities.UpgradeItemEntity;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
+import org.apache.ambari.server.state.ConfigHelper;
 import org.apache.ambari.server.state.Host;
 import org.apache.ambari.server.state.RepositoryVersionState;
 import org.apache.ambari.server.state.Service;
 import org.apache.ambari.server.state.ServiceComponent;
 import org.apache.ambari.server.state.StackId;
 import org.apache.ambari.server.view.ViewRegistry;
+import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.google.inject.Binder;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
+import com.google.inject.Module;
 import com.google.inject.persist.PersistService;
+import com.google.inject.util.Modules;
 
 /**
  * UpgradeResourceDefinition tests.
@@ -77,11 +83,23 @@ public class UpgradeResourceProviderTest {
   private Clusters clusters;
   private OrmTestHelper helper;
   AmbariManagementController amc;
+  private ConfigHelper configHelper;
 
   @Before
   public void before() throws Exception {
+    // setup the config helper for placeholder resolution
+    configHelper = EasyMock.createNiceMock(ConfigHelper.class);
+    expect(
+        configHelper.getPlaceholderValueFromDesiredConfigurations(
+            EasyMock.anyObject(Cluster.class), EasyMock.eq("{{foo/bar}}"))).andReturn(
+        "placeholder-rendered-properly").anyTimes();
+
+    EasyMock.replay(configHelper);
+
+    // create an injector which will inject the mocks
+    injector = Guice.createInjector(Modules.override(
+        new InMemoryDefaultTestModule()).with(new MockModule()));
 
-    injector = Guice.createInjector(new InMemoryDefaultTestModule());
     injector.getInstance(GuiceJpaInitializer.class);
 
     helper = injector.getInstance(OrmTestHelper.class);
@@ -145,6 +163,7 @@ public class UpgradeResourceProviderTest {
 
   public org.apache.ambari.server.controller.spi.RequestStatus testCreateResources() throws Exception {
 
+
     Cluster cluster = clusters.getCluster("c1");
 
     List<UpgradeEntity> upgrades = upgradeDao.findUpgrades(cluster.getClusterId());
@@ -172,7 +191,9 @@ public class UpgradeResourceProviderTest {
     UpgradeGroupEntity group = upgradeGroups.get(1);
     assertEquals(4, group.getItems().size());
 
-    assertTrue(group.getItems().get(0).getText().contains("Preparing"));
+    assertTrue(group.getItems().get(0).getText().contains(
+        "placeholder of placeholder-rendered-properly"));
+
     assertTrue(group.getItems().get(1).getText().contains("Restarting"));
     assertTrue(group.getItems().get(2).getText().contains("Updating"));
     assertTrue(group.getItems().get(3).getText().contains("Service Check"));
@@ -314,5 +335,16 @@ public class UpgradeResourceProviderTest {
     return new UpgradeResourceProvider(amc);
   }
 
-
+  /**
+   *
+   */
+  private class MockModule implements Module {
+    /**
+   *
+   */
+    @Override
+    public void configure(Binder binder) {
+      binder.bind(ConfigHelper.class).toInstance(configHelper);
+    }
+  }
 }

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

@@ -38,29 +38,50 @@ import org.apache.ambari.server.stack.HostsType;
 import org.apache.ambari.server.stack.MasterHostResolver;
 import org.apache.ambari.server.state.UpgradeHelper.UpgradeGroupHolder;
 import org.apache.ambari.server.state.stack.UpgradePack;
+import org.apache.ambari.server.state.stack.upgrade.ManualTask;
 import org.apache.ambari.server.state.stack.upgrade.StageWrapper;
 import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.google.inject.Binder;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
+import com.google.inject.Module;
 import com.google.inject.persist.PersistService;
+import com.google.inject.util.Modules;
 
 /**
  * Tests the {@link UpgradeHelper} class
  */
 public class UpgradeHelperTest {
 
+  private static final String UPGRADE_VERSION = "2.2.1.0-1234";
+  private static final String DOWNGRADE_VERSION = "2.2.0.0-1234";
+
   private Injector injector;
   private AmbariMetaInfo ambariMetaInfo;
   private OrmTestHelper helper;
   private MasterHostResolver m_masterHostResolver;
+  private UpgradeHelper m_upgradeHelper;
+  private ConfigHelper m_configHelper;
 
   @Before
   public void before() throws Exception {
-    injector = Guice.createInjector(new InMemoryDefaultTestModule());
+    // configure the mock to return data given a specific placeholder
+    m_configHelper = EasyMock.createNiceMock(ConfigHelper.class);
+
+    expect(
+        m_configHelper.getPlaceholderValueFromDesiredConfigurations(
+            EasyMock.anyObject(Cluster.class), EasyMock.eq("{{foo/bar}}"))).andReturn(
+        "placeholder-rendered-properly").anyTimes();
+
+    replay(m_configHelper);
+
+    // create an injector which will inject the mocks
+    injector = Guice.createInjector(Modules.override(
+        new InMemoryDefaultTestModule()).with(new MockModule()));
 
     injector.getInstance(GuiceJpaInitializer.class);
 
@@ -68,6 +89,7 @@ public class UpgradeHelperTest {
     ambariMetaInfo = injector.getInstance(AmbariMetaInfo.class);
     ambariMetaInfo.init();
 
+    m_upgradeHelper = injector.getInstance(UpgradeHelper.class);
     m_masterHostResolver = EasyMock.createMock(MasterHostResolver.class);
   }
 
@@ -88,8 +110,8 @@ public class UpgradeHelperTest {
 
     Cluster cluster = makeCluster();
 
-    UpgradeHelper helper = new UpgradeHelper();
-    List<UpgradeGroupHolder> groups = helper.createUpgrade(cluster, m_masterHostResolver, upgrade);
+    List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
+        m_masterHostResolver, upgrade, UPGRADE_VERSION);
 
     assertEquals(5, groups.size());
 
@@ -124,12 +146,14 @@ public class UpgradeHelperTest {
 
     Cluster cluster = makeCluster();
 
-    UpgradeHelper helper = new UpgradeHelper();
-    List<UpgradeGroupHolder> groups = helper.createDowngrade(cluster, m_masterHostResolver, upgrade);
+    List<UpgradeGroupHolder> groups = m_upgradeHelper.createDowngrade(
+        m_masterHostResolver, upgrade, DOWNGRADE_VERSION);
 
     assertEquals(5, groups.size());
 
-    assertEquals("PRE_CLUSTER", groups.get(0).name);
+    UpgradeGroupHolder preGroup = groups.get(0);
+    assertEquals("PRE_CLUSTER", preGroup.name);
+
     assertEquals("CORE_SLAVES", groups.get(1).name);
     assertEquals("CORE_MASTER", groups.get(2).name);
     assertEquals("ZOOKEEPER", groups.get(3).name);
@@ -160,13 +184,41 @@ public class UpgradeHelperTest {
 
     Cluster cluster = makeCluster();
 
-    UpgradeHelper helper = new UpgradeHelper();
-    List<UpgradeGroupHolder> groups = helper.createUpgrade(cluster, m_masterHostResolver, upgrade);
+    List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
+        m_masterHostResolver, upgrade, UPGRADE_VERSION);
 
     assertEquals(1, groups.size());
     UpgradeGroupHolder group = groups.iterator().next();
 
-    assertEquals(7, group.items.size());
+    assertEquals(6, group.items.size());
+  }
+
+  @Test
+  public void testManualTaskPostProcessing() throws Exception {
+    Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("foo", "bar");
+    assertTrue(upgrades.isEmpty());
+
+    upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1");
+    assertTrue(upgrades.containsKey("upgrade_test"));
+    UpgradePack upgrade = upgrades.get("upgrade_test");
+    assertNotNull(upgrade);
+
+    Cluster cluster = makeCluster();
+
+    List<UpgradeGroupHolder> groups = m_upgradeHelper.createUpgrade(
+        m_masterHostResolver, upgrade, UPGRADE_VERSION);
+
+    assertEquals(5, 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);
   }
 
   /**
@@ -250,11 +302,23 @@ public class UpgradeHelperTest {
     type.hosts = new HashSet<String>(Arrays.asList("h1", "h3"));
     expect(m_masterHostResolver.getMasterAndHosts("YARN", "NODEMANAGER")).andReturn(type).anyTimes();
 
+    expect(m_masterHostResolver.getCluster()).andReturn(c).anyTimes();
 
     replay(m_masterHostResolver);
 
     return c;
   }
 
-
+  /**
+   *
+   */
+  private class MockModule implements Module {
+    /**
+    *
+    */
+    @Override
+    public void configure(Binder binder) {
+      binder.bind(ConfigHelper.class).toInstance(m_configHelper);
+    }
+  }
 }

+ 1 - 1
ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_bucket_test.xml

@@ -61,7 +61,7 @@
             <command>ls</command>
           </task>
 
-          <task xsi:type="manual">
+          <task xsi:type="execute">
             <command>ls</command>
           </task>
           

+ 1 - 1
ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml

@@ -98,7 +98,7 @@
         <pre-upgrade>
           <task xsi:type="manual">
             <summary>SUMMARY OF PREPARE</summary>
-            <message>Preparing with a manual</message>
+            <message>This is a manual task with a placeholder of {{foo/bar}}</message>
           </task>
         </pre-upgrade>
         <upgrade>