Repository: incubator-slider
Updated Branches:
  refs/heads/develop f2b39adfa -> 37dad5d72


SLIDER-787 App Upgrade/Reconfig support in Slider (additional client side 
checks and tests)


Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/37dad5d7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/37dad5d7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/37dad5d7

Branch: refs/heads/develop
Commit: 37dad5d727ebbaa66977587fc1dda782b7725ac2
Parents: f2b39ad
Author: Gour Saha <gourks...@apache.org>
Authored: Fri Apr 10 20:53:14 2015 -0700
Committer: Gour Saha <gourks...@apache.org>
Committed: Fri Apr 10 20:54:31 2015 -0700

----------------------------------------------------------------------
 .../org/apache/slider/client/SliderClient.java  | 278 +++++++++++++++----
 .../slider/common/params/ActionUpgradeArgs.java |   3 +
 .../providers/agent/AgentProviderService.java   |   2 +-
 .../server/appmaster/SliderAppMaster.java       |  49 ++--
 .../actions/ActionUpgradeContainers.java        |  18 +-
 .../slider/client/TestClientBadArgs.groovy      |  85 +++++-
 .../client/TestUpgradeCommandOptions.groovy     | 224 +++++++++++++++
 7 files changed, 577 insertions(+), 82 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/main/java/org/apache/slider/client/SliderClient.java
----------------------------------------------------------------------
diff --git 
a/slider-core/src/main/java/org/apache/slider/client/SliderClient.java 
b/slider-core/src/main/java/org/apache/slider/client/SliderClient.java
index dc1a361..c166587 100644
--- a/slider-core/src/main/java/org/apache/slider/client/SliderClient.java
+++ b/slider-core/src/main/java/org/apache/slider/client/SliderClient.java
@@ -724,8 +724,25 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
     List<String> containers = upgradeArgs.containers;
     List<String> components = upgradeArgs.components;
 
-    // For upgrade spec, only --template and --resources should be specified
-    if (template != null || resources != null) {
+    // For upgrade spec, let's be little more strict with validation. If either
+    // --template or --resources is specified, then both needs to be specified.
+    // Otherwise the internal app config and resources states of the app will 
be
+    // unwantedly modified and the change will take effect to the running app
+    // immediately.
+    if (template != null && resources == null) {
+      throw new BadCommandArgumentsException(
+          "Option %s must be specified with option %s",
+          Arguments.ARG_RESOURCES, Arguments.ARG_TEMPLATE);
+    }
+    if (resources != null && template == null) {
+      throw new BadCommandArgumentsException(
+          "Option %s must be specified with option %s",
+          Arguments.ARG_TEMPLATE, Arguments.ARG_RESOURCES);
+    }
+
+    // For upgrade spec, both --template and --resources should be specified
+    // and neither of --containers or --components should be used
+    if (template != null && resources != null) {
       if (CollectionUtils.isNotEmpty(containers)) {
         throw new BadCommandArgumentsException(
             "Option %s cannot be specified with %s or %s",
@@ -738,27 +755,25 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
             Arguments.ARG_COMPONENTS, Arguments.ARG_TEMPLATE,
             Arguments.ARG_RESOURCES);
       }
-      buildInstanceDefinition(clustername, upgradeArgs, true, true);
+      
+      // not an error to try to upgrade a stopped cluster, just return success
+      // code, appropriate log messages have already been dumped
+      if (!isAppInRunningState(clustername)) {
+        return EXIT_SUCCESS;
+      }
+
+      // Now initiate the upgrade spec flow
+      buildInstanceDefinition(clustername, upgradeArgs, true, true, true);
       SliderClusterOperations clusterOperations = 
createClusterOperations(clustername);
-      clusterOperations.amSuicide("Application upgrade", 1, 1000);
-      // upgradeYarnApplicationSubmissionContext(clustername, upgradeArgs);
+      clusterOperations.amSuicide("AM restarted for application upgrade", 1, 
1000);
       return EXIT_SUCCESS;
     }
 
-    if (CollectionUtils.isNotEmpty(containers)
-        && CollectionUtils.isNotEmpty(components)) {
-      throw new BadCommandArgumentsException(
-          "Only one of option %s or %s can be specified",
-          Arguments.ARG_CONTAINERS, Arguments.ARG_COMPONENTS);
-    }
-    if (CollectionUtils.isNotEmpty(containers)) {
-      log.info("Going to stop containers (total {}) : {}", containers.size(),
-          containers);
-    }
-    if (CollectionUtils.isNotEmpty(components)) {
-      log.info("Going to stop all containers of components (total {}) : {}",
-          components.size(), components);
-    }
+    // Since neither --template or --resources were specified, it is upgrade
+    // containers flow. Here any one or both of --containers and --components
+    // can be specified. If a container is specified with --containers option
+    // and also belongs to a component type specified with --components, it 
will
+    // be upgraded only once.
     return actionUpgradeContainers(clustername, upgradeArgs);
   }
 
@@ -771,54 +786,117 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
     log.debug("actionUpgradeContainers({}, reason={}, wait={})", clustername,
         text, waittime);
 
+    // not an error to try to upgrade a stopped cluster, just return success
+    // code, appropriate log messages have already been dumped
+    if (!isAppInRunningState(clustername)) {
+      return EXIT_SUCCESS;
+    }
+
+    // Create sets of containers and components to get rid of duplicates and
+    // for quick lookup during checks below
+    Set<String> containers = new HashSet<>();
+    if (upgradeArgs.containers != null) {
+      containers.addAll(new ArrayList<>(upgradeArgs.containers));
+    }
+    Set<String> components = new HashSet<>();
+    if (upgradeArgs.components != null) {
+      components.addAll(new ArrayList<>(upgradeArgs.components));
+    }
+
+    // check validity of component names and running containers here
+    List<ContainerInformation> liveContainers = getContainers(clustername);
+    Set<String> validContainers = new HashSet<>();
+    Set<String> validComponents = new HashSet<>();
+    for (ContainerInformation liveContainer : liveContainers) {
+      boolean allContainersAndComponentsAccountedFor = true;
+      if (CollectionUtils.isNotEmpty(containers)) {
+        if (containers.contains(liveContainer.containerId)) {
+          containers.remove(liveContainer.containerId);
+          validContainers.add(liveContainer.containerId);
+        }
+        allContainersAndComponentsAccountedFor = false;
+      }
+      if (CollectionUtils.isNotEmpty(components)) {
+        if (components.contains(liveContainer.component)) {
+          components.remove(liveContainer.component);
+          validComponents.add(liveContainer.component);
+        }
+        allContainersAndComponentsAccountedFor = false;
+      }
+      if (allContainersAndComponentsAccountedFor) {
+        break;
+      }
+    }
+
+    // If any item remains in containers or components then they are invalid.
+    // Log warning for them and proceed.
+    if (CollectionUtils.isNotEmpty(containers)) {
+      log.warn("Invalid set of containers provided {}", containers);
+    }
+    if (CollectionUtils.isNotEmpty(components)) {
+      log.warn("Invalid set of components provided {}", components);
+    }
+
+    // If not a single valid container or component is specified do not proceed
+    if (CollectionUtils.isEmpty(validContainers)
+        && CollectionUtils.isEmpty(validComponents)) {
+      log.error("Not a single valid container or component specified. Nothing 
to do.");
+      return EXIT_FALSE;
+    }
+
+    SliderClusterProtocol appMaster = connect(findInstance(clustername));
+    Messages.UpgradeContainersRequestProto r =
+      Messages.UpgradeContainersRequestProto
+              .newBuilder()
+              .setMessage(text)
+              .addAllContainer(validContainers)
+              .addAllComponent(validComponents)
+              .build();
+    appMaster.upgradeContainers(r);
+    log.info("Cluster upgrade issued for -");
+    if (CollectionUtils.isNotEmpty(validContainers)) {
+      log.info(" Containers (total {}): {}", validContainers.size(),
+          validContainers);
+    }
+    if (CollectionUtils.isNotEmpty(validComponents)) {
+      log.info(" Components (total {}): {}", validComponents.size(),
+          validComponents);
+    }
+
+    return EXIT_SUCCESS;
+  }
+
+  // returns true if and only if app is in RUNNING state
+  private boolean isAppInRunningState(String clustername) throws YarnException,
+      IOException {
     // is this actually a known cluster?
     sliderFileSystem.locateInstanceDefinition(clustername);
     ApplicationReport app = findInstance(clustername);
     if (app == null) {
       // exit early
       log.info("Cluster {} not running", clustername);
-      // not an error to try to upgrade a stopped cluster
-      return EXIT_SUCCESS;
+      return false;
     }
     log.debug("App to upgrade was found: {}:\n{}", clustername,
         new SliderUtils.OnDemandReportStringifier(app));
     if (app.getYarnApplicationState().ordinal() >= 
YarnApplicationState.FINISHED
         .ordinal()) {
-      log.info("Cluster {} is in a terminated state {}", clustername,
-          app.getYarnApplicationState());
-      return EXIT_SUCCESS;
+      log.info(
+          "Cluster {} is in a terminated state {}. Use command '{}' instead.",
+          clustername, app.getYarnApplicationState(),
+          SliderActions.ACTION_UPDATE);
+      return false;
     }
 
     // IPC request to upgrade containers is possible if the app is running.
     if (app.getYarnApplicationState().ordinal() < YarnApplicationState.RUNNING
         .ordinal()) {
-      log.info("Cluster {} is in a pre-running state {}. It needs to be "
-          + "RUNNING to upgrade.", clustername, app.getYarnApplicationState());
-      return EXIT_SUCCESS;
-    }
-
-    try {
-      SliderClusterProtocol appMaster = connect(app);
-      Messages.UpgradeContainersRequestProto r =
-        Messages.UpgradeContainersRequestProto
-                .newBuilder()
-                .setMessage(text)
-                .addAllContainer(upgradeArgs.containers)
-                .addAllComponent(upgradeArgs.components)
-                .build();
-      appMaster.upgradeContainers(r);
-      log.debug("Cluster upgrade containers issued");
-    } catch (YarnException e) {
-      log.warn("Exception while trying to upgrade containers {}", clustername,
-          e);
-      return EXIT_FALSE;
-    } catch (IOException e) {
-      log.warn("Exception while trying to upgrade containers {}", clustername,
-          e);
-      return EXIT_FALSE;
+      log.info("Cluster {} is in a pre-running state {}. To upgrade it needs "
+          + "to be RUNNING.", clustername, app.getYarnApplicationState());
+      return false;
     }
 
-    return EXIT_SUCCESS;
+    return true;
   }
 
   private static void checkForCredentials(Configuration conf,
@@ -1431,10 +1509,18 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
    * @throws YarnException
    * @throws IOException
    */
-  
+
   public void buildInstanceDefinition(String clustername,
-      AbstractClusterBuildingActionArgs buildInfo, boolean overwrite, boolean 
liveClusterAllowed)
-        throws YarnException, IOException {
+      AbstractClusterBuildingActionArgs buildInfo, boolean overwrite,
+      boolean liveClusterAllowed) throws YarnException, IOException {
+    buildInstanceDefinition(clustername, buildInfo, overwrite,
+        liveClusterAllowed, false);
+  }
+
+  public void buildInstanceDefinition(String clustername,
+      AbstractClusterBuildingActionArgs buildInfo, boolean overwrite,
+      boolean liveClusterAllowed, boolean isUpgradeFlow) throws YarnException,
+      IOException {
     // verify that a live cluster isn't there
     SliderUtils.validateClusterName(clustername);
     verifyBindingsDefined();
@@ -1493,6 +1579,13 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
       }
     }
 
+    if (isUpgradeFlow) {
+      ActionUpgradeArgs upgradeInfo = (ActionUpgradeArgs) buildInfo;
+      if (!upgradeInfo.force) {
+        validateClientAndClusterResource(clustername, resources);
+      }
+    }
+    
     AppDefinitionPersister appDefinitionPersister = new 
AppDefinitionPersister(sliderFileSystem);
     appDefinitionPersister.processSuppliedDefinitions(clustername, buildInfo, 
appConf);
 
@@ -1507,7 +1600,7 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
     for (Map.Entry<String, String> roleEntry : argsRoleMap.entrySet()) {
       String count = roleEntry.getValue();
       String key = roleEntry.getKey();
-      log.debug("{} => {}", key, count);
+      log.info("{} => {}", key, count);
       resources.getOrAddComponent(key)
                  .put(ResourceKeys.COMPONENT_INSTANCES, count);
     }
@@ -1605,6 +1698,85 @@ public class SliderClient extends 
AbstractSliderLaunchedService implements RunSe
     validateInstanceDefinition(provider, instanceDescription, 
sliderFileSystem);
   }
 
+  private void validateClientAndClusterResource(String clustername,
+      ConfTreeOperations clientResources) throws BadClusterStateException,
+      SliderException, IOException {
+    log.info("Validating client resource with cluster resource definition 
(components and instance count)");
+    Map<String, Integer> clientComponentNameInstanceMap = new HashMap<>();
+    for (String componentName : clientResources.getComponentNames()) {
+      if (!SliderKeys.COMPONENT_AM.equals(componentName)) {
+        clientComponentNameInstanceMap.put(componentName, clientResources
+            .getComponentOptInt(componentName,
+                ResourceKeys.COMPONENT_INSTANCES, -1));
+      }
+    }
+
+    AggregateConf clusterConf = null;
+    try {
+      clusterConf = loadPersistedClusterDescription(clustername);
+    } catch (LockAcquireFailedException e) {
+      log.warn("Failed to get a Lock on cluster resource : {}", e);
+      throw new BadClusterStateException(
+          "Failed to validate client resource definition " + clustername + ": "
+              + e);
+    }
+    Map<String, Integer> clusterComponentNameInstanceMap = new HashMap<>();
+    for (Map.Entry<String, Map<String, String>> component : clusterConf
+        .getResources().components.entrySet()) {
+      if (!SliderKeys.COMPONENT_AM.equals(component.getKey())) {
+        clusterComponentNameInstanceMap.put(
+            component.getKey(),
+            Integer.decode(component.getValue().get(
+                ResourceKeys.COMPONENT_INSTANCES)));
+      }
+    }
+
+    // client and cluster should be an exact match
+    Iterator<Map.Entry<String, Integer>> clientComponentNameInstanceIterator = 
clientComponentNameInstanceMap
+        .entrySet().iterator();
+    while (clientComponentNameInstanceIterator.hasNext()) {
+      Map.Entry<String, Integer> clientComponentNameInstanceEntry = 
clientComponentNameInstanceIterator
+          .next();
+      if (clusterComponentNameInstanceMap
+          .containsKey(clientComponentNameInstanceEntry.getKey())) {
+        // compare instance count now and remove from both maps if they match
+        if (clusterComponentNameInstanceMap
+            .get(clientComponentNameInstanceEntry.getKey()) == 
clientComponentNameInstanceEntry
+            .getValue()) {
+          clusterComponentNameInstanceMap
+              .remove(clientComponentNameInstanceEntry.getKey());
+          clientComponentNameInstanceIterator.remove();
+        }
+      }
+    }
+
+    if (!clientComponentNameInstanceMap.isEmpty()
+        || !clusterComponentNameInstanceMap.isEmpty()) {
+      log.error("Mismatch found in client and cluster resource specification");
+      if (!clientComponentNameInstanceMap.isEmpty()) {
+        log.info("  Following are the client resources that do not match");
+        for (Map.Entry<String, Integer> clientComponentNameInstanceEntry : 
clientComponentNameInstanceMap
+            .entrySet()) {
+          log.info("    Component Name: {}, Instance count: {}",
+              clientComponentNameInstanceEntry.getKey(),
+              clientComponentNameInstanceEntry.getValue());
+        }
+      }
+      if (!clusterComponentNameInstanceMap.isEmpty()) {
+        log.info("  Following are the cluster resources that do not match");
+        for (Map.Entry<String, Integer> clusterComponentNameInstanceEntry : 
clusterComponentNameInstanceMap
+            .entrySet()) {
+          log.info("    Component Name: {}, Instance count: {}",
+              clusterComponentNameInstanceEntry.getKey(),
+              clusterComponentNameInstanceEntry.getValue());
+        }
+      }
+      throw new BadConfigException("\n\nResource definition provided for "
+          + "upgrade does not match with that of the currently running "
+          + "cluster.\nIf you are aware of what you are doing, rerun the "
+          + "command with " + Arguments.ARG_FORCE + " option.\n");
+    }
+  }
 
   protected void persistInstanceDefinition(boolean overwrite,
                                          Path appconfdir,

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/main/java/org/apache/slider/common/params/ActionUpgradeArgs.java
----------------------------------------------------------------------
diff --git 
a/slider-core/src/main/java/org/apache/slider/common/params/ActionUpgradeArgs.java
 
b/slider-core/src/main/java/org/apache/slider/common/params/ActionUpgradeArgs.java
index ec46f50..832e1cc 100644
--- 
a/slider-core/src/main/java/org/apache/slider/common/params/ActionUpgradeArgs.java
+++ 
b/slider-core/src/main/java/org/apache/slider/common/params/ActionUpgradeArgs.java
@@ -67,4 +67,7 @@ public class ActionUpgradeArgs extends 
AbstractClusterBuildingActionArgs
       description = "stop all containers of specific components")
   public List<String> components = new ArrayList<String>(0);
 
+  @Parameter(names = {ARG_FORCE},
+      description = "force spec upgrade operation")
+  public boolean force;
 }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java
----------------------------------------------------------------------
diff --git 
a/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java
 
b/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java
index 3ee71f2..76353ff 100644
--- 
a/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java
+++ 
b/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java
@@ -1111,7 +1111,7 @@ public class AgentProviderService extends 
AbstractProviderService implements
     this.isInUpgradeMode = inUpgradeMode;
   }
 
-  public void addUpgradeContainers(List<String> upgradeContainers) {
+  public void addUpgradeContainers(Set<String> upgradeContainers) {
     this.upgradeContainers.addAll(upgradeContainers);
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/main/java/org/apache/slider/server/appmaster/SliderAppMaster.java
----------------------------------------------------------------------
diff --git 
a/slider-core/src/main/java/org/apache/slider/server/appmaster/SliderAppMaster.java
 
b/slider-core/src/main/java/org/apache/slider/server/appmaster/SliderAppMaster.java
index 6ef5f8e..3ab4501 100644
--- 
a/slider-core/src/main/java/org/apache/slider/server/appmaster/SliderAppMaster.java
+++ 
b/slider-core/src/main/java/org/apache/slider/server/appmaster/SliderAppMaster.java
@@ -176,9 +176,11 @@ import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -1622,7 +1624,9 @@ public class SliderAppMaster extends 
AbstractSliderLaunchedService
   }
 
   /**
-   * Signal that containers are being upgraded
+   * Signal that containers are being upgraded. Containers specified with
+   * --containers option and all containers of all roles specified with
+   * --components option are merged and upgraded.
    * 
    * @param upgradeContainersRequest
    *          request containing upgrade details
@@ -1630,24 +1634,32 @@ public class SliderAppMaster extends 
AbstractSliderLaunchedService
   public synchronized void onUpgradeContainers(
       ActionUpgradeContainers upgradeContainersRequest) throws IOException,
       SliderException {
-    LOG_YARN.info("onUpgradeContainers([{}]",
+    LOG_YARN.info("onUpgradeContainers({})",
         upgradeContainersRequest.getMessage());
-    List<String> containers = upgradeContainersRequest.getContainers();
-    if (CollectionUtils.isEmpty(containers)) {
-      // components will not be null here, since it is pre-checked
-      List<String> components = upgradeContainersRequest.getComponents();
+    Set<String> containers = upgradeContainersRequest.getContainers() == null 
? new HashSet<String>()
+        : upgradeContainersRequest.getContainers();
+    LOG_YARN.info("  Container list provided (total {}) : {}",
+        containers.size(), containers);
+    Set<String> components = upgradeContainersRequest.getComponents() == null 
? new HashSet<String>()
+        : upgradeContainersRequest.getComponents();
+    LOG_YARN.info("  Component list provided (total {}) : {}",
+        components.size(), components);
+    // If components are specified as well, then grab all the containers of
+    // each of the components (roles)
+    if (CollectionUtils.isNotEmpty(components)) {
       Map<ContainerId, RoleInstance> liveContainers = appState.getLiveNodes();
-      containers = new ArrayList<String>();
-      Map<String, List<String>> roleContainerMap = 
prepareRoleContainerMap(liveContainers);
-      for (String component : components) {
-        List<String> roleContainers = roleContainerMap.get(component);
-        if (roleContainers != null) {
-          containers.addAll(roleContainers);
+      if (CollectionUtils.isNotEmpty(liveContainers.keySet())) {
+        Map<String, Set<String>> roleContainerMap = 
prepareRoleContainerMap(liveContainers);
+        for (String component : components) {
+          Set<String> roleContainers = roleContainerMap.get(component);
+          if (roleContainers != null) {
+            containers.addAll(roleContainers);
+          }
         }
       }
     }
-    LOG_YARN.info("Containers to be upgraded (total {}) : {}", 
containers.size(),
-        containers);
+    LOG_YARN.info("Final list of containers to be upgraded (total {}) : {}",
+        containers.size(), containers);
     if (providerService instanceof AgentProviderService) {
       AgentProviderService agentProviderService = (AgentProviderService) 
providerService;
       agentProviderService.setInUpgradeMode(true);
@@ -1655,17 +1667,18 @@ public class SliderAppMaster extends 
AbstractSliderLaunchedService
     }
   }
 
-  // create a reverse map of roles -> list of all live containers
-  private Map<String, List<String>> prepareRoleContainerMap(
+  // create a reverse map of roles -> set of all live containers
+  private Map<String, Set<String>> prepareRoleContainerMap(
       Map<ContainerId, RoleInstance> liveContainers) {
-    Map<String, List<String>> roleContainerMap = new HashMap<String, 
List<String>>();
+    // liveContainers is ensured to be not empty
+    Map<String, Set<String>> roleContainerMap = new HashMap<>();
     for (Map.Entry<ContainerId, RoleInstance> liveContainer : liveContainers
         .entrySet()) {
       RoleInstance role = liveContainer.getValue();
       if (roleContainerMap.containsKey(role.role)) {
         roleContainerMap.get(role.role).add(liveContainer.getKey().toString());
       } else {
-        List<String> containers = new ArrayList<String>();
+        Set<String> containers = new HashSet<String>();
         containers.add(liveContainer.getKey().toString());
         roleContainerMap.put(role.role, containers);
       }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/main/java/org/apache/slider/server/appmaster/actions/ActionUpgradeContainers.java
----------------------------------------------------------------------
diff --git 
a/slider-core/src/main/java/org/apache/slider/server/appmaster/actions/ActionUpgradeContainers.java
 
b/slider-core/src/main/java/org/apache/slider/server/appmaster/actions/ActionUpgradeContainers.java
index ad3bb92..05fcbcc 100644
--- 
a/slider-core/src/main/java/org/apache/slider/server/appmaster/actions/ActionUpgradeContainers.java
+++ 
b/slider-core/src/main/java/org/apache/slider/server/appmaster/actions/ActionUpgradeContainers.java
@@ -18,7 +18,9 @@
 
 package org.apache.slider.server.appmaster.actions;
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.collections.CollectionUtils;
@@ -30,8 +32,8 @@ public class ActionUpgradeContainers extends AsyncAction {
   private int exitCode;
   private FinalApplicationStatus finalApplicationStatus;
   private String message;
-  private List<String> containers;
-  private List<String> components;
+  private Set<String> containers = new HashSet<>();
+  private Set<String> components = new HashSet<>();
 
   public ActionUpgradeContainers(String name,
       long delay,
@@ -44,8 +46,8 @@ public class ActionUpgradeContainers extends AsyncAction {
     super(name, delay, timeUnit);
     this.exitCode = exitCode;
     this.finalApplicationStatus = finalApplicationStatus;
-    this.containers = containers;
-    this.components = components;
+    this.containers.addAll(containers);
+    this.components.addAll(components);
     this.message = message;
   }
 
@@ -85,19 +87,19 @@ public class ActionUpgradeContainers extends AsyncAction {
     this.message = message;
   }
 
-  public List<String> getContainers() {
+  public Set<String> getContainers() {
     return containers;
   }
 
-  public void setContainers(List<String> containers) {
+  public void setContainers(Set<String> containers) {
     this.containers = containers;
   }
 
-  public List<String> getComponents() {
+  public Set<String> getComponents() {
     return components;
   }
 
-  public void setComponents(List<String> components) {
+  public void setComponents(Set<String> components) {
     this.components = components;
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/test/groovy/org/apache/slider/client/TestClientBadArgs.groovy
----------------------------------------------------------------------
diff --git 
a/slider-core/src/test/groovy/org/apache/slider/client/TestClientBadArgs.groovy 
b/slider-core/src/test/groovy/org/apache/slider/client/TestClientBadArgs.groovy
index 8aacf6a..f81738f 100644
--- 
a/slider-core/src/test/groovy/org/apache/slider/client/TestClientBadArgs.groovy
+++ 
b/slider-core/src/test/groovy/org/apache/slider/client/TestClientBadArgs.groovy
@@ -78,7 +78,7 @@ class TestClientBadArgs extends ServiceLauncherBaseTest {
                              [SliderActions.ACTION_HELP,
                              "hello, world"])
   }
-  
+
   @Test
   public void testBadImageArg() throws Throwable {
     launchExpectingException(SliderClient,
@@ -87,7 +87,7 @@ class TestClientBadArgs extends ServiceLauncherBaseTest {
                             [SliderActions.ACTION_HELP,
                              Arguments.ARG_IMAGE])
   }
-  
+
   @Test
   public void testRegistryUsage() throws Throwable {
     def exception = launchExpectingException(SliderClient,
@@ -139,4 +139,85 @@ class TestClientBadArgs extends ServiceLauncherBaseTest {
     assert exception instanceof UsageException
     log.info(exception.toString())
   }
+
+  @Test
+  public void testUpgradeUsage() throws Throwable {
+    def exception = launchExpectingException(SliderClient,
+        new Configuration(),
+        "org.apache.slider.core.exceptions.BadCommandArgumentsException: Not 
enough arguments for action: upgrade Expected minimum 1 but got 0",
+        [SliderActions.ACTION_UPGRADE])
+    assert exception instanceof BadCommandArgumentsException
+    log.info(exception.toString())
+  }
+
+  @Test
+  public void testUpgradeWithTemplateOptionOnly() throws Throwable {
+    String appName = "test_hbase"
+    def exception = launchExpectingException(SliderClient,
+        new Configuration(),
+        "BadCommandArgumentsException: Option --resources must be specified 
with option --template",
+        [SliderActions.ACTION_UPGRADE,
+            appName,
+            Arguments.ARG_TEMPLATE,
+            "/tmp/appConfig.json",
+        ])
+    assert exception instanceof BadCommandArgumentsException
+    log.info(exception.toString())
+  }
+
+  @Test
+  public void testUpgradeWithResourcesOptionOnly() throws Throwable {
+    String appName = "test_hbase"
+    def exception = launchExpectingException(SliderClient,
+        new Configuration(),
+        "BadCommandArgumentsException: Option --template must be specified 
with option --resources",
+        [SliderActions.ACTION_UPGRADE,
+            appName,
+            Arguments.ARG_RESOURCES,
+            "/tmp/resources.json",
+        ])
+    assert exception instanceof BadCommandArgumentsException
+    log.info(exception.toString())
+  }
+
+  @Test
+  public void testUpgradeWithTemplateResourcesAndContainersOption() throws 
Throwable {
+    String appName = "test_hbase"
+    def exception = launchExpectingException(SliderClient,
+        new Configuration(),
+        "BadCommandArgumentsException: Option --containers cannot be "
+        + "specified with --template or --resources",
+        [SliderActions.ACTION_UPGRADE,
+            appName,
+            Arguments.ARG_TEMPLATE,
+            "/tmp/appConfig.json",
+            Arguments.ARG_RESOURCES,
+            "/tmp/resources.json",
+            Arguments.ARG_CONTAINERS,
+            "container_1"
+        ])
+    assert exception instanceof BadCommandArgumentsException
+    log.info(exception.toString())
+  }
+
+  @Test
+  public void testUpgradeWithTemplateResourcesAndComponentsOption() throws 
Throwable {
+    String appName = "test_hbase"
+    def exception = launchExpectingException(SliderClient,
+        new Configuration(),
+        "BadCommandArgumentsException: Option --components cannot be "
+        + "specified with --template or --resources",
+        [SliderActions.ACTION_UPGRADE,
+            appName,
+            Arguments.ARG_TEMPLATE,
+            "/tmp/appConfig.json",
+            Arguments.ARG_RESOURCES,
+            "/tmp/resources.json",
+            Arguments.ARG_COMPONENTS,
+            "HBASE_MASTER"
+        ])
+    assert exception instanceof BadCommandArgumentsException
+    log.info(exception.toString())
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/37dad5d7/slider-core/src/test/groovy/org/apache/slider/client/TestUpgradeCommandOptions.groovy
----------------------------------------------------------------------
diff --git 
a/slider-core/src/test/groovy/org/apache/slider/client/TestUpgradeCommandOptions.groovy
 
b/slider-core/src/test/groovy/org/apache/slider/client/TestUpgradeCommandOptions.groovy
new file mode 100644
index 0000000..f760c84
--- /dev/null
+++ 
b/slider-core/src/test/groovy/org/apache/slider/client/TestUpgradeCommandOptions.groovy
@@ -0,0 +1,224 @@
+/*
+ * 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.slider.client
+
+import java.io.File
+import java.io.IOException
+import java.io.FileNotFoundException
+
+import org.apache.commons.io.FileUtils
+import org.apache.commons.logging.Log
+import org.apache.commons.logging.LogFactory
+import org.apache.hadoop.fs.FileSystem
+import org.apache.hadoop.fs.FileUtil
+import org.apache.hadoop.fs.Path
+import org.apache.hadoop.fs.RawLocalFileSystem
+import org.apache.hadoop.yarn.api.records.ApplicationReport
+import org.apache.hadoop.yarn.conf.YarnConfiguration
+
+import org.apache.slider.agent.AgentMiniClusterTestBase
+import org.apache.slider.common.SliderKeys
+import org.apache.slider.common.params.Arguments
+import org.apache.slider.common.params.ClientArgs
+import org.apache.slider.common.params.SliderActions
+import org.apache.slider.common.tools.SliderFileSystem
+import org.apache.slider.common.tools.SliderUtils
+import org.apache.slider.core.exceptions.BadCommandArgumentsException
+import org.apache.slider.core.exceptions.BadConfigException
+import org.apache.slider.core.exceptions.SliderException
+import org.apache.slider.core.exceptions.UnknownApplicationInstanceException
+import org.apache.slider.core.main.ServiceLauncher
+import org.apache.slider.providers.agent.AgentKeys
+
+import org.junit.Before
+import org.junit.Test
+
+/**
+ * Test the package commands options
+ */
+class TestUpgradeCommandOptions extends AgentMiniClusterTestBase {
+  final shouldFail = new GroovyTestCase().&shouldFail
+  private Log log = LogFactory.getLog(this.class)
+  private static SliderFileSystem testFileSystem
+  private static String APP_NAME = "HBASE"
+  private static String APP_VERSION = "1.0.0"
+  private YarnConfiguration yarnConfig = new YarnConfiguration(configuration)
+  private ServiceLauncher<SliderClient> launcher = null
+  
+  @Before
+  public void setupFilesystem() {
+    FileSystem fileSystem = new RawLocalFileSystem()
+    YarnConfiguration configuration = SliderUtils.createConfiguration()
+    fileSystem.setConf(configuration)
+    testFileSystem = new SliderFileSystem(fileSystem, configuration)
+    File testFolderDir = new File(testFileSystem.buildPackageDirPath(APP_NAME,
+      null).toUri().path)
+    testFolderDir.deleteDir()
+    File testFolderDirWithVersion = new 
File(testFileSystem.buildPackageDirPath(
+      APP_NAME, APP_VERSION).toUri().path)
+    testFolderDirWithVersion.deleteDir()
+  }
+
+  @Test
+  public void testUpgradeAppNotRunning() throws Throwable {
+    describe("Calling upgrade")
+    YarnConfiguration conf = SliderUtils.createConfiguration()
+    try {
+      ServiceLauncher launcher = launch(TestSliderClient,
+          conf,
+          [
+            ClientArgs.ACTION_UPGRADE,
+            APP_NAME,
+            ClientArgs.ARG_TEMPLATE,
+            "/tmp/appConfig.json",
+            ClientArgs.ARG_RESOURCES,
+            "/tmp/resources.json"
+          ])
+      fail("Upgrade command should have failed")
+    } catch (SliderException e) {
+      assert e instanceof UnknownApplicationInstanceException
+      assert e.getMessage().contains("Unknown application instance")
+      log.info(e.toString())
+    }
+  }
+
+  @Test
+  public void testAll() {
+    // Create a single test to reduce the amount of test execution time
+    describe("Create mini cluster")
+    String clustername = createMiniCluster("", yarnConfig, 1, true)
+    describe("Created cluster - " + clustername)
+
+    // start the app and AM
+    describe("Starting the app")
+    launcher = createStandaloneAM(clustername, true, false)
+    SliderClient sliderClient = launcher.service
+    ApplicationReport report = waitForClusterLive(sliderClient)
+    addToTeardown(sliderClient)
+
+    // now call all the tests
+    testUpgradeInvalidResourcesFile(clustername)
+    testUpgradeInvalidConfigFile(clustername)
+    testUpgradeSuccess(clustername)
+  }
+
+  public void testUpgradeInvalidResourcesFile(String clustername) 
+    throws Throwable {
+    String appConfigFile = Path.getPathWithoutSchemeAndAuthority(testFileSystem
+      .buildClusterDirPath(clustername)).toString() + "/app_config.json"
+
+    describe("Calling upgrade - testUpgradeInvalidResourcesFile")
+    try {
+      launcher = launchClientAgainstMiniMR(
+          //config includes RM binding info
+          yarnConfig,
+          //varargs list of command line params
+          [
+              ClientArgs.ACTION_UPGRADE,
+              clustername,
+              ClientArgs.ARG_TEMPLATE,
+              appConfigFile,
+              ClientArgs.ARG_RESOURCES,
+              "/tmp/resources.json"
+          ]
+      )
+      fail("Upgrade command should have failed")
+    } catch (SliderException e) {
+      assert e instanceof BadConfigException
+      assert e.getMessage().contains("incorrect argument to --resources: " +
+        "\"/tmp/resources.json\" : java.io.FileNotFoundException: " +
+        "/tmp/resources.json (No such file or directory)")
+      log.info(e.toString())
+    }
+  }
+
+  public void testUpgradeInvalidConfigFile(String clustername)
+    throws Throwable {
+    String resourceFile = Path.getPathWithoutSchemeAndAuthority(testFileSystem
+      .buildClusterDirPath(clustername)).toString() + "/resources.json"
+
+    describe("Calling upgrade - testUpgradeInvalidConfigFile")
+    try {
+      launcher = launchClientAgainstMiniMR(
+          //config includes RM binding info
+          yarnConfig,
+          //varargs list of command line params
+          [
+              ClientArgs.ACTION_UPGRADE,
+              clustername,
+              ClientArgs.ARG_TEMPLATE,
+              "/tmp/appConfig.json",
+              ClientArgs.ARG_RESOURCES,
+              resourceFile
+          ]
+      )
+      fail("Upgrade command should have failed")
+    } catch (SliderException e) {
+      assert e instanceof BadConfigException
+      assert e.getMessage().contains("incorrect argument to --template: " +
+        "\"/tmp/appConfig.json\" : java.io.FileNotFoundException: " +
+        "/tmp/appConfig.json (No such file or directory)")
+      log.info(e.toString())
+    }
+  }
+
+  public void testUpgradeSuccess(String clustername)
+    throws Throwable {
+    String resourceFile = Path.getPathWithoutSchemeAndAuthority(testFileSystem
+      .buildClusterDirPath(clustername)).toString() + "/resources.json"
+    String appConfigFile = Path.getPathWithoutSchemeAndAuthority(testFileSystem
+      .buildClusterDirPath(clustername)).toString() + "/app_config.json"
+
+    describe("Calling upgrade - testUpgradeSuccess")
+    try {
+      launcher = launchClientAgainstMiniMR(
+          //config includes RM binding info
+          yarnConfig,
+          //varargs list of command line params
+          [
+              ClientArgs.ACTION_UPGRADE,
+              clustername,
+              ClientArgs.ARG_TEMPLATE,
+              appConfigFile,
+              ClientArgs.ARG_RESOURCES,
+              resourceFile
+          ]
+      )
+    } catch (SliderException e) {
+      fail("Upgrade command should have failed")
+      log.info(e.toString())
+    }
+    assert launcher.serviceExitCode == 0
+  }
+
+  private File getTempLocation () {
+    return new File(System.getProperty("user.dir") + "/target/_")
+  }
+
+  static class TestSliderClient extends SliderClient {
+    public TestSliderClient() {
+      super()
+    }
+
+    @Override
+    protected void initHadoopBinding() throws IOException, SliderException {
+      sliderFileSystem = testFileSystem
+    }
+  }
+}

Reply via email to