Merge pull request #2378 from yahoo/hakonhall/use-orchestrator-model

Makes the Orchestrator policy use the new model classes

14 files changed, 377 insertions, 302 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/BatchInternalErrorException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/BatchInternalErrorException.java
index 0762caea742..38b3f7a1e39 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/BatchInternalErrorException.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/BatchInternalErrorException.java
@@ -2,15 +2,14 @@
 package com.yahoo.vespa.orchestrator;
 
 import com.yahoo.vespa.applicationmodel.HostName;
-
-import java.util.List;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
 
 public class BatchInternalErrorException extends OrchestrationException {
 
     public BatchInternalErrorException(HostName parentHostname,
-                                       List<HostName> orderedHostNames,
+                                       NodeGroup nodeGroup,
                                        RuntimeException e) {
-        super("Failed to suspend " + orderedHostNames + " with parent host "
+        super("Failed to suspend " + nodeGroup + " with parent host "
                 + parentHostname + ": " + e.getMessage(), e);
     }
 
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java
index 222b28dae16..e3451ad9576 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.orchestrator;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.applicationmodel.HostName;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
 import com.yahoo.vespa.orchestrator.policy.BatchHostStateChangeDeniedException;
 import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException;
 import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
@@ -59,6 +60,21 @@ public interface Orchestrator {
     void suspend(HostName hostName) throws HostStateChangeDeniedException, HostNameNotFoundException;
 
     /**
+     * Suspend normal operations for a group of nodes in the same application.
+     *
+     * @param nodeGroup The group of nodes in an application.
+     * @throws HostStateChangeDeniedException if the request cannot be meet due to policy constraints.
+     * @throws HostNameNotFoundException if any hostnames in the node group is not recognized
+     */
+    void suspendGroup(NodeGroup nodeGroup) throws HostStateChangeDeniedException, HostNameNotFoundException;
+
+    /**
+     * Suspend several hosts. On failure, all hosts are resumed before exiting the method with an exception.
+     */
+    void suspendAll(HostName parentHostname, List<HostName> hostNames)
+            throws BatchInternalErrorException, BatchHostStateChangeDeniedException, BatchHostNameNotFoundException;
+
+    /**
      * Get the orchestrator status of the application instance.
      *
      * @param appId Identifier of the application to check
@@ -83,16 +99,10 @@ public interface Orchestrator {
 
 
     /**
-     * Suspend orchestration for hosts belonging to this application.
-     * I.e all suspend requests for its hosts will succeed.
+     * Suspend an application:  All hosts will allow suspension in parallel.
+     * CAUTION:  Only use this if the application is not in service.
      *
      * @param appId Identifier of the application to resume
      */
     void suspend(ApplicationId appId) throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException;
-
-    /**
-     * Suspend all hosts. On failure, all hosts are resumed before exiting the method with an exception.
-     */
-    void suspendAll(HostName parentHostname, List<HostName> hostNames)
-            throws BatchInternalErrorException, BatchHostStateChangeDeniedException, BatchHostNameNotFoundException;
 }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
index 8019ee7725a..72b644be464 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -13,9 +13,13 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerState;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse;
+import com.yahoo.vespa.orchestrator.model.ApplicationApi;
+import com.yahoo.vespa.orchestrator.model.ApplicationApiImpl;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
 import com.yahoo.vespa.orchestrator.model.VespaModelUtil;
 import com.yahoo.vespa.orchestrator.policy.BatchHostStateChangeDeniedException;
 import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException;
+import com.yahoo.vespa.orchestrator.policy.HostedVespaClusterPolicy;
 import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy;
 import com.yahoo.vespa.orchestrator.policy.Policy;
 import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
@@ -56,7 +60,7 @@ public class OrchestratorImpl implements Orchestrator {
                             OrchestratorConfig orchestratorConfig,
                             InstanceLookupService instanceLookupService)
     {
-        this(new HostedVespaPolicy(clusterControllerClientFactory),
+        this(new HostedVespaPolicy(new HostedVespaClusterPolicy(), clusterControllerClientFactory),
                 clusterControllerClientFactory,
                 statusService,
                 instanceLookupService,
@@ -116,19 +120,26 @@ public class OrchestratorImpl implements Orchestrator {
     @Override
     public void suspend(HostName hostName) throws HostStateChangeDeniedException, HostNameNotFoundException {
         ApplicationInstance<ServiceMonitorStatus> appInstance = getApplicationInstance(hostName);
+        NodeGroup nodeGroup = new NodeGroup(appInstance, hostName);
+        suspendGroup(nodeGroup);
+    }
 
+    // Public for testing purposes
+    @Override
+    public void suspendGroup(NodeGroup nodeGroup) throws HostStateChangeDeniedException, HostNameNotFoundException {
+        ApplicationInstanceReference applicationReference = nodeGroup.getApplicationReference();
 
-        try (MutableStatusRegistry hostStatusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(appInstance.reference())) {
-            final HostStatus currentHostState = hostStatusRegistry.getHostStatus(hostName);
-
-            if (HostStatus.ALLOWED_TO_BE_DOWN == currentHostState) {
+        try (MutableStatusRegistry hostStatusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(applicationReference)) {
+            ApplicationInstanceStatus appStatus = statusService.forApplicationInstance(applicationReference).getApplicationInstanceStatus();
+            if (appStatus == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) {
                 return;
             }
 
-            ApplicationInstanceStatus appStatus = statusService.forApplicationInstance(appInstance.reference()).getApplicationInstanceStatus();
-            if (appStatus == ApplicationInstanceStatus.NO_REMARKS) {
-                policy.grantSuspensionRequest(appInstance, hostName, hostStatusRegistry);
-            }
+            ApplicationApi applicationApi = new ApplicationApiImpl(
+                    nodeGroup,
+                    hostStatusRegistry,
+                    clusterControllerClientFactory);
+            policy.grantSuspensionRequest(applicationApi);
         }
     }
 
@@ -157,40 +168,45 @@ public class OrchestratorImpl implements Orchestrator {
     @Override
     public void suspendAll(HostName parentHostname, List<HostName> hostNames)
             throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException {
+        List<NodeGroup> nodeGroupsOrderedByApplication;
         try {
-            hostNames = sortHostNamesForSuspend(hostNames);
+            nodeGroupsOrderedByApplication = nodeGroupsOrderedForSuspend(hostNames);
         } catch (HostNameNotFoundException e) {
             throw new BatchHostNameNotFoundException(parentHostname, hostNames, e);
         }
 
         try {
-            for (HostName hostName : hostNames) {
+            for (NodeGroup nodeGroup : nodeGroupsOrderedByApplication) {
                 try {
-                    suspend(hostName);
+                    suspendGroup(nodeGroup);
                 } catch (HostStateChangeDeniedException e) {
-                    throw new BatchHostStateChangeDeniedException(parentHostname, hostNames, e);
+                    throw new BatchHostStateChangeDeniedException(parentHostname, nodeGroup, e);
                 } catch (HostNameNotFoundException e) {
                     // Should never get here since since we would have received HostNameNotFoundException earlier.
                     throw new BatchHostNameNotFoundException(parentHostname, hostNames, e);
                 } catch (RuntimeException e) {
-                    throw new BatchInternalErrorException(parentHostname, hostNames, e);
+                    throw new BatchInternalErrorException(parentHostname, nodeGroup, e);
                 }
             }
         } catch (Exception e) {
-            rollbackSuspendAll(hostNames, e);
+            // Rollback causes extra noise in a content clusters due to cluster version changes and calm-periods,
+            // so consider not doing a full rollback.
+            rollbackSuspendAll(nodeGroupsOrderedByApplication, e);
             throw e;
         }
     }
 
-    private void rollbackSuspendAll(List<HostName> orderedHostNames, Exception exception) {
-        List<HostName> reverseOrderedHostNames = new ArrayList<>(orderedHostNames);
+    private void rollbackSuspendAll(List<NodeGroup> orderedGroups, Exception exception) {
+        List<NodeGroup> reverseOrderedHostNames = new ArrayList<>(orderedGroups);
         Collections.reverse(reverseOrderedHostNames);
-        for (HostName hostName : reverseOrderedHostNames) {
-            try {
-                resume(hostName);
-            } catch (HostStateChangeDeniedException | HostNameNotFoundException | RuntimeException e) {
-                // We're forced to ignore these since we're already rolling back a suspension.
-                exception.addSuppressed(e);
+        for (NodeGroup nodeGroup : reverseOrderedHostNames) {
+            for (HostName hostName : nodeGroup.getHostNames()) {
+                try {
+                    resume(hostName);
+                } catch (HostStateChangeDeniedException | HostNameNotFoundException | RuntimeException e) {
+                    // We're forced to ignore these since we're already rolling back a suspension.
+                    exception.addSuppressed(e);
+                }
             }
         }
     }
@@ -221,34 +237,35 @@ public class OrchestratorImpl implements Orchestrator {
      * The solution we're using is to order the hostnames by the globally unique application instance ID,
      * e.g. hosted-vespa:routing:dev:ci-corp-us-east-1:default. In the example above, it would guarantee
      * Docker host 2 would ensure ask to suspend B2 before A2. We take care of that ordering here.
+     *
+     * NodeGroups complicate the above picture a little:  Each A1, A2, B1, and B2 is a NodeGroup that may
+     * contain several nodes (on the same Docker host).  But the argument still applies.
      */
-    List<HostName> sortHostNamesForSuspend(List<HostName> hostNames) throws HostNameNotFoundException {
-        Map<HostName, ApplicationInstanceReference> applicationReferences = new HashMap<>(hostNames.size());
+    private List<NodeGroup> nodeGroupsOrderedForSuspend(List<HostName> hostNames) throws HostNameNotFoundException {
+        Map<ApplicationInstanceReference, NodeGroup> nodeGroupMap = new HashMap<>(hostNames.size());
         for (HostName hostName : hostNames) {
-            ApplicationInstance<?> appInstance = getApplicationInstance(hostName);
-            applicationReferences.put(hostName, appInstance.reference());
+            ApplicationInstance<ServiceMonitorStatus> application = getApplicationInstance(hostName);
+
+            NodeGroup nodeGroup = nodeGroupMap.get(application.reference());
+            if (nodeGroup == null) {
+                nodeGroup = new NodeGroup(application);
+                nodeGroupMap.put(application.reference(), nodeGroup);
+            }
+
+            nodeGroup.addNode(hostName);
         }
 
-        return hostNames.stream()
-                .sorted((leftHostname, rightHostname) -> compareHostNamesForSuspend(leftHostname, rightHostname, applicationReferences))
+        return nodeGroupMap.values().stream()
+                .sorted(OrchestratorImpl::compareNodeGroupsForSuspend)
                 .collect(Collectors.toList());
     }
 
-    private int compareHostNamesForSuspend(HostName leftHostname, HostName rightHostname,
-                                           Map<HostName, ApplicationInstanceReference> applicationReferences) {
-        ApplicationInstanceReference leftApplicationReference = applicationReferences.get(leftHostname);
-        assert leftApplicationReference != null;
-
-        ApplicationInstanceReference rightApplicationReference = applicationReferences.get(rightHostname);
-        assert rightApplicationReference != null;
+    private static int compareNodeGroupsForSuspend(NodeGroup leftNodeGroup, NodeGroup rightNodeGroup) {
+        ApplicationInstanceReference leftApplicationReference = leftNodeGroup.getApplicationReference();
+        ApplicationInstanceReference rightApplicationReference = rightNodeGroup.getApplicationReference();
 
         // ApplicationInstanceReference.toString() is e.g. "hosted-vespa:routing:dev:ci-corp-us-east-1:default"
-        int diff = leftApplicationReference.toString().compareTo(rightApplicationReference.toString());
-        if (diff != 0) {
-            return diff;
-        }
-
-        return leftHostname.toString().compareTo(rightHostname.toString());
+        return leftApplicationReference.asString().compareTo(rightApplicationReference.asString());
     }
 
     private HostStatus getNodeStatus(ApplicationInstanceReference applicationRef, HostName hostName) {
@@ -294,8 +311,9 @@ public class OrchestratorImpl implements Orchestrator {
         log.log(LogLevel.INFO, String.format("Setting content clusters %s for application %s to %s",
                 contentClusterIds,application.applicationInstanceId(),state));
         for (ClusterId clusterId : contentClusterIds) {
+            List<HostName> clusterControllers = VespaModelUtil.getClusterControllerInstancesInOrder(application, clusterId);
             ClusterControllerClient client = clusterControllerClientFactory.createClient(
-                    VespaModelUtil.getClusterControllerInstancesInOrder(application, clusterId),
+                    clusterControllers,
                     clusterId.s());
             try {
                 ClusterControllerStateResponse response = client.setApplicationState(state);
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/BatchHostStateChangeDeniedException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/BatchHostStateChangeDeniedException.java
index 15bc61a3019..fb2a918fdda 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/BatchHostStateChangeDeniedException.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/BatchHostStateChangeDeniedException.java
@@ -2,16 +2,15 @@
 package com.yahoo.vespa.orchestrator.policy;
 
 import com.yahoo.vespa.applicationmodel.HostName;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
 import com.yahoo.vespa.orchestrator.OrchestrationException;
 
-import java.util.List;
-
 public class BatchHostStateChangeDeniedException extends OrchestrationException {
 
     public BatchHostStateChangeDeniedException(HostName parentHostname,
-                                               List<HostName> orderedHostNames,
+                                               NodeGroup group,
                                                HostStateChangeDeniedException e) {
-        super("Failed to suspend " + orderedHostNames + " with parent host "
+        super("Failed to suspend " + group + " with parent host "
                 + parentHostname + ": " + e.getMessage(), e);
 
     }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ClusterPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ClusterPolicy.java
new file mode 100644
index 00000000000..d0d7cd83e60
--- /dev/null
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ClusterPolicy.java
@@ -0,0 +1,14 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.orchestrator.policy;
+
+import com.yahoo.vespa.orchestrator.model.ClusterApi;
+
+public interface ClusterPolicy {
+    /**
+     * There's an implicit group of nodes known to clusterApi.  This method answers whether
+     * it would be fine, just looking at this cluster (and disregarding Cluster Controller/storage
+     * which is handled separately), whether it would be fine to allow all services on all the nodes
+     * in the group to go down.
+     */
+    void verifyGroupGoingDownIsFine(ClusterApi clusterApi) throws HostStateChangeDeniedException;
+}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ConcurrentSuspensionLimitForCluster.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ConcurrentSuspensionLimitForCluster.java
new file mode 100644
index 00000000000..1a631615ce7
--- /dev/null
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ConcurrentSuspensionLimitForCluster.java
@@ -0,0 +1,21 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.orchestrator.policy;
+
+/**
+ * How many nodes can suspend concurrently, at most.
+ */
+public enum ConcurrentSuspensionLimitForCluster {
+    ONE_NODE(0),
+    TEN_PERCENT(10),
+    ALL_NODES(100);
+
+    int percentage;
+
+    ConcurrentSuspensionLimitForCluster(int percentage) {
+        this.percentage = percentage;
+    }
+
+    public int asPercentage() {
+        return percentage;
+    }
+}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java
index ac1eca310ae..9bd584d82af 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.orchestrator.policy;
 
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceType;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
 import com.yahoo.vespa.orchestrator.OrchestrationException;
 
 /**
@@ -15,21 +16,36 @@ public class HostStateChangeDeniedException extends OrchestrationException {
 
     public HostStateChangeDeniedException(HostName hostName, String constraintName, 
                                           ServiceType serviceType, String message) {
-        super(createMessage(hostName, constraintName, serviceType, message));
-        this.constraintName = constraintName;
-        this.serviceType = serviceType;
+        this(hostName, constraintName, serviceType, message, null);
     }
 
     public HostStateChangeDeniedException(HostName hostName, String constraintName, 
                                           ServiceType serviceType, String message, Throwable cause) {
-        super(createMessage(hostName, constraintName, serviceType, message), cause);
+        this(hostName.toString(), constraintName, serviceType, message, cause);
+    }
+
+    public HostStateChangeDeniedException(NodeGroup nodeGroup,
+                                          String constraintName,
+                                          ServiceType serviceType,
+                                          String message) {
+        this(nodeGroup.toCommaSeparatedString(), constraintName, serviceType, message, null);
+    }
+
+    private HostStateChangeDeniedException(String nodes,
+                                           String constraintName,
+                                           ServiceType serviceType,
+                                           String message,
+                                           Throwable cause) {
+        super(createMessage(nodes, constraintName, serviceType, message), cause);
         this.constraintName = constraintName;
         this.serviceType = serviceType;
     }
 
-    private static String createMessage(HostName hostName, String constraintName, 
-                                        ServiceType serviceType, String message) {
-        return "Changing the state of host " + hostName + " would violate " + constraintName
+    private static String createMessage(String nodes,
+                                        String constraintName,
+                                        ServiceType serviceType,
+                                        String message) {
+        return "Changing the state of " + nodes + " would violate " + constraintName
                 + " for service type " + serviceType + ": " + message;
     }
 
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java
new file mode 100644
index 00000000000..eb2fb9dc1b9
--- /dev/null
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java
@@ -0,0 +1,52 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.orchestrator.policy;
+
+import com.yahoo.vespa.orchestrator.model.VespaModelUtil;
+import com.yahoo.vespa.orchestrator.model.ClusterApi;
+
+import static com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy.ENOUGH_SERVICES_UP_CONSTRAINT;
+
+public class HostedVespaClusterPolicy implements ClusterPolicy {
+    public void verifyGroupGoingDownIsFine(ClusterApi clusterApi)
+            throws HostStateChangeDeniedException {
+        if (clusterApi.noServicesOutsideGroupIsDown()) {
+            return;
+        }
+
+        if (clusterApi.noServicesInGroupIsUp()) {
+            return;
+        }
+
+        int percentageOfServicesAllowedToBeDown = getConcurrentSuspensionLimit(clusterApi).asPercentage();
+        if (clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown() <= percentageOfServicesAllowedToBeDown) {
+            return;
+        }
+
+        throw new HostStateChangeDeniedException(
+                clusterApi.getNodeGroup(),
+                ENOUGH_SERVICES_UP_CONSTRAINT,
+                clusterApi.serviceType(),
+                "Suspension percentage would increase from " + clusterApi.percentageOfServicesDown()
+                        + "% to " + clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown()
+                        + "%, over the limit of " + percentageOfServicesAllowedToBeDown + "%."
+                        + " These instances may be down: " + clusterApi.servicesDownAndNotInGroupDescription()
+                        + " and these hosts are allowed to be down: "
+                        + clusterApi.nodesAllowedToBeDownNotInGroupDescription());
+    }
+
+    static ConcurrentSuspensionLimitForCluster getConcurrentSuspensionLimit(ClusterApi clusterApi) {
+        if (VespaModelUtil.ADMIN_CLUSTER_ID.equals(clusterApi.clusterId())) {
+            if (VespaModelUtil.SLOBROK_SERVICE_TYPE.equals(clusterApi.serviceType())) {
+                return ConcurrentSuspensionLimitForCluster.ONE_NODE;
+            }
+
+            return ConcurrentSuspensionLimitForCluster.ALL_NODES;
+        }
+
+        if (clusterApi.isStorageCluster()) {
+            return ConcurrentSuspensionLimitForCluster.ONE_NODE;
+        }
+
+        return ConcurrentSuspensionLimitForCluster.TEN_PERCENT;
+    }
+}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
index b7574a43ce1..515a46cb1da 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
@@ -3,31 +3,19 @@ package com.yahoo.vespa.orchestrator.policy;
 
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
-import com.yahoo.vespa.applicationmodel.ClusterId;
 import com.yahoo.vespa.applicationmodel.HostName;
-import com.yahoo.vespa.applicationmodel.ServiceCluster;
-import com.yahoo.vespa.applicationmodel.ServiceInstance;
-import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerState;
-import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse;
-import com.yahoo.vespa.orchestrator.model.VespaModelUtil;
+import com.yahoo.vespa.orchestrator.model.ApplicationApi;
+import com.yahoo.vespa.orchestrator.model.ApplicationApiImpl;
+import com.yahoo.vespa.orchestrator.model.ClusterApi;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
+import com.yahoo.vespa.orchestrator.model.StorageNode;
 import com.yahoo.vespa.orchestrator.status.HostStatus;
 import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry;
 import com.yahoo.vespa.service.monitor.ServiceMonitorStatus;
 
-import java.io.IOException;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
-
-import static com.yahoo.vespa.orchestrator.OrchestratorUtil.getHostStatusMap;
-import static com.yahoo.vespa.orchestrator.OrchestratorUtil.getHostsUsedByApplicationInstance;
-import static com.yahoo.vespa.orchestrator.OrchestratorUtil.getServiceClustersUsingHost;
 
 /**
  * @author oyving
@@ -41,191 +29,69 @@ public class HostedVespaPolicy implements Policy {
 
     private static final Logger log = Logger.getLogger(HostedVespaPolicy.class.getName());
 
+    private final HostedVespaClusterPolicy clusterPolicy;
     private final ClusterControllerClientFactory clusterControllerClientFactory;
 
-    public HostedVespaPolicy(ClusterControllerClientFactory clusterControllerClientFactory) {
+    public HostedVespaPolicy(HostedVespaClusterPolicy clusterPolicy, ClusterControllerClientFactory clusterControllerClientFactory) {
+        this.clusterPolicy = clusterPolicy;
         this.clusterControllerClientFactory = clusterControllerClientFactory;
     }
 
-    private static long numContentServiceClusters(Set<? extends ServiceCluster<?>> serviceClustersOnHost) {
-        return serviceClustersOnHost.stream().filter(VespaModelUtil::isContent).count();
-    }
-
-
     @Override
-    public void grantSuspensionRequest(ApplicationInstance<ServiceMonitorStatus> applicationInstance,
-                                       HostName hostName,
-                                       MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException {
-
-        Set<ServiceCluster<ServiceMonitorStatus>> serviceClustersOnHost =
-                getServiceClustersUsingHost(applicationInstance.serviceClusters(), hostName);
-
-        Map<HostName, HostStatus> hostStatusMap = getHostStatusMap(
-                getHostsUsedByApplicationInstance(applicationInstance),
-                hostStatusService);
-
-        boolean hasUpStorageInstance = false;
-        for (ServiceCluster<ServiceMonitorStatus> serviceCluster : serviceClustersOnHost) {
-            Set<ServiceInstance<ServiceMonitorStatus>> instancesOnThisHost;
-            Set<ServiceInstance<ServiceMonitorStatus>> instancesOnOtherHosts;
-            {
-                Map<Boolean, Set<ServiceInstance<ServiceMonitorStatus>>> serviceInstancesByLocality =
-                        serviceCluster.serviceInstances().stream()
-                                .collect(
-                                        Collectors.groupingBy(
-                                                instance -> instance.hostName().equals(hostName),
-                                                Collectors.toSet()));
-                instancesOnThisHost = serviceInstancesByLocality.getOrDefault(true, Collections.emptySet());
-                instancesOnOtherHosts = serviceInstancesByLocality.getOrDefault(false, Collections.emptySet());
-            }
-
-            if (VespaModelUtil.isStorage(serviceCluster)) {
-                boolean thisHostHasSomeUpInstances = instancesOnThisHost.stream()
-                        .map(ServiceInstance::serviceStatus)
-                        .anyMatch(status -> status == ServiceMonitorStatus.UP);
-                if (thisHostHasSomeUpInstances) {
-                    hasUpStorageInstance = true;
-                }
-            }
-
-            boolean thisHostHasOnlyDownInstances = instancesOnThisHost.stream()
-                    .map(ServiceInstance::serviceStatus)
-                    .allMatch(status -> status == ServiceMonitorStatus.DOWN);
-            if (thisHostHasOnlyDownInstances) {
-                // Suspending this host will not make a difference for this cluster, so no need to investigate further.
-                continue;
-            }
-
-            Set<ServiceInstance<ServiceMonitorStatus>> possiblyDownInstancesOnOtherHosts =
-                    instancesOnOtherHosts.stream()
-                            .filter(instance -> effectivelyDown(instance, hostStatusMap))
-                            .collect(Collectors.toSet());
-            if (possiblyDownInstancesOnOtherHosts.isEmpty()) {
-                // This short-circuits the percentage calculation below and ensures that we can always upgrade
-                // any cluster by allowing one host at the time to be suspended, no matter what percentage of
-                // the cluster that host amounts to.
-                continue;
-            }
-
-            // Now calculate what the service suspension percentage will be if we suspend this host.
-            int numServiceInstancesTotal = serviceCluster.serviceInstances().size();
-            int numInstancesThatWillBeSuspended = union(possiblyDownInstancesOnOtherHosts, instancesOnThisHost).size();
-            int percentThatWillBeSuspended = numInstancesThatWillBeSuspended * 100 / numServiceInstancesTotal;
-            int suspendPercentageAllowed = ServiceClusterSuspendPolicy.getSuspendPercentageAllowed(serviceCluster);
-            if (percentThatWillBeSuspended > suspendPercentageAllowed) {
-                // It may seem like this may in some cases prevent upgrading, especially for small clusters (where the
-                // percentage of service instances affected by suspending a single host may easily exceed the allowed
-                // suspension percentage). Note that we always allow progress by allowing a single host to suspend.
-                // See previous section.
-                int currentSuspensionPercentage
-                        = possiblyDownInstancesOnOtherHosts.size() * 100 / numServiceInstancesTotal;
-                Set<HostName> otherHostsWithThisServiceCluster = instancesOnOtherHosts.stream()
-                        .map(ServiceInstance::hostName)
-                        .collect(Collectors.toSet());
-                Set<HostName> hostsAllowedToBeDown = hostStatusMap.entrySet().stream()
-                        .filter(entry -> entry.getValue() == HostStatus.ALLOWED_TO_BE_DOWN)
-                        .map(Map.Entry::getKey)
-                        .collect(Collectors.toSet());
-                Set<HostName> otherHostsAllowedToBeDown
-                        = intersection(otherHostsWithThisServiceCluster, hostsAllowedToBeDown);
-                throw new HostStateChangeDeniedException(
-                        hostName,
-                        ENOUGH_SERVICES_UP_CONSTRAINT,
-                        serviceCluster.serviceType(),
-                        "Suspension percentage would increase from " + currentSuspensionPercentage
-                                + "% to " + percentThatWillBeSuspended
-                                + "%, over the limit of " + suspendPercentageAllowed + "%."
-                                + " These instances may be down: " + possiblyDownInstancesOnOtherHosts
-                                + " and these hosts are allowed to be down: " + otherHostsAllowedToBeDown
-                );
-            }
+    public void grantSuspensionRequest(ApplicationApi application)
+            throws HostStateChangeDeniedException {
+        // Apply per-cluster policy
+        for (ClusterApi cluster : application.getClusters()) {
+            clusterPolicy.verifyGroupGoingDownIsFine(cluster);
         }
 
-        if (hasUpStorageInstance) {
-            // If there is an UP storage service on the host, we need to make sure
-            // there's sufficient redundancy before allowing the suspension. This will
-            // also avoid redistribution (which is unavoidable if the storage instance
-            // is already down).
-            setNodeStateInController(applicationInstance, hostName, ClusterControllerState.MAINTENANCE);
+        // Ask Cluster Controller to set UP storage nodes in maintenance.
+        // These storage nodes are guaranteed to be NO_REMARKS
+        for (StorageNode storageNode : application.getUpStorageNodesInGroupInClusterOrder()) {
+            storageNode.setNodeState(ClusterControllerState.MAINTENANCE);
+            log.log(LogLevel.INFO, "The storage node on " + storageNode.hostName() + " has been set to MAINTENANCE");
         }
 
-
-        // We have "go" for suspending the services on the host,store decision.
-        hostStatusService.setHostState(hostName, HostStatus.ALLOWED_TO_BE_DOWN);
-        log.log(LogLevel.INFO, hostName + " is now allowed to be down (suspended)");
+        // Ensure all nodes in the group are marked as allowed to be down
+        for (HostName hostName : application.getNodesInGroupWithStatus(HostStatus.NO_REMARKS)) {
+            application.setHostState(hostName, HostStatus.ALLOWED_TO_BE_DOWN);
+            log.log(LogLevel.INFO, hostName + " is now allowed to be down (suspended)");
+        }
     }
 
-    private static <T> Set<T> union(Set<T> setA, Set<T> setB) {
-        Set<T> union = new HashSet<>(setA);
-        union.addAll(setB);
-        return union;
+    @Override
+    public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException {
+        // Always defer to Cluster Controller whether it's OK to resume storage node
+        for (StorageNode storageNode : application.getStorageNodesAllowedToBeDownInGroupInReverseClusterOrder()) {
+            storageNode.setNodeState(ClusterControllerState.UP);
+            log.log(LogLevel.INFO, "The storage node on " + storageNode.hostName() + " has been set to UP");
+        }
+
+        for (HostName hostName : application.getNodesInGroupWithStatus(HostStatus.ALLOWED_TO_BE_DOWN)) {
+            application.setHostState(hostName, HostStatus.NO_REMARKS);
+            log.log(LogLevel.INFO, hostName + " is no longer allowed to be down (resumed)");
+        }
     }
 
-    private static <T> Set<T> intersection(Set<T> setA, Set<T> setB) {
-        Set<T> intersection = new HashSet<>(setA);
-        intersection.retainAll(setB);
-        return intersection;
+    // TODO: Remove later - currently used for backward compatibility testing
+    @Override
+    public void grantSuspensionRequest(ApplicationInstance<ServiceMonitorStatus> applicationInstance,
+                                       HostName hostName,
+                                       MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException {
+        NodeGroup nodeGroup = new NodeGroup(applicationInstance);
+        nodeGroup.addNode(hostName);
+        ApplicationApi applicationApi = new ApplicationApiImpl(nodeGroup, hostStatusService, clusterControllerClientFactory);
+        grantSuspensionRequest(applicationApi);
     }
 
+    // TODO: Remove later - currently used for backward compatibility testing
     @Override
     public void releaseSuspensionGrant(
             ApplicationInstance<ServiceMonitorStatus> applicationInstance,
             HostName hostName,
             MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException {
-        Set<ServiceCluster<ServiceMonitorStatus>> serviceClustersOnHost =
-                getServiceClustersUsingHost(applicationInstance.serviceClusters(), hostName);
-
-        // TODO: Always defer to Cluster Controller whether it's OK to resume host (if content node).
-        if (numContentServiceClusters(serviceClustersOnHost) > 0) {
-            setNodeStateInController(applicationInstance, hostName, ClusterControllerState.UP);
-        }
-        hostStatusService.setHostState(hostName, HostStatus.NO_REMARKS);
-        log.log(LogLevel.INFO, hostName + " is no longer allowed to be down (resumed)");
-    }
-
-    private static boolean effectivelyDown(ServiceInstance<ServiceMonitorStatus> serviceInstance,
-                                           Map<HostName, HostStatus> hostStatusMap) {
-        ServiceMonitorStatus instanceStatus = serviceInstance.serviceStatus();
-        HostStatus hostStatus = hostStatusMap.get(serviceInstance.hostName());
-        return hostStatus == HostStatus.ALLOWED_TO_BE_DOWN || instanceStatus == ServiceMonitorStatus.DOWN;
+        NodeGroup nodeGroup = new NodeGroup(applicationInstance, hostName);
+        ApplicationApi applicationApi = new ApplicationApiImpl(nodeGroup, hostStatusService, clusterControllerClientFactory);
+        releaseSuspensionGrant(applicationApi);
     }
-
-    private void setNodeStateInController(ApplicationInstance<?> application,
-                                          HostName hostName,
-                                          ClusterControllerState nodeState) throws HostStateChangeDeniedException {
-        ClusterId contentClusterId = VespaModelUtil.getContentClusterName(application, hostName);
-        List<HostName> clusterControllers = VespaModelUtil.getClusterControllerInstancesInOrder(application, contentClusterId);
-        ClusterControllerClient client = clusterControllerClientFactory.createClient(
-                clusterControllers,
-                contentClusterId.s());
-        int nodeIndex = VespaModelUtil.getStorageNodeIndex(application, hostName);
-
-        log.log(LogLevel.DEBUG,
-                "application " + application.applicationInstanceId() +
-                ", host " + hostName +
-                ", cluster name " + contentClusterId +
-                ", node index " + nodeIndex +
-                ", node state " + nodeState);
-
-        ClusterControllerStateResponse response;
-        try {
-            response = client.setNodeState(nodeIndex, nodeState);
-        } catch (IOException e) {
-            throw new HostStateChangeDeniedException(
-                    hostName,
-                    CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT,
-                    VespaModelUtil.CLUSTER_CONTROLLER_SERVICE_TYPE,
-                    "Failed to communicate with cluster controllers " + clusterControllers + ": " + e,
-                    e);
-        }
-
-        if ( ! response.wasModified) {
-            throw new HostStateChangeDeniedException(
-                    hostName,
-                    SET_NODE_STATE_CONSTRAINT,
-                    VespaModelUtil.CLUSTER_CONTROLLER_SERVICE_TYPE,
-                    "Failed to set state to " + nodeState + " in controller: " + response.reason);
-        }
-    }
-
 }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java
index 5b8627edcc9..fee2940efda 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java
@@ -1,9 +1,10 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.policy;
 
-import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.HostName;
+import com.yahoo.vespa.orchestrator.model.ApplicationApi;
+import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry;
 import com.yahoo.vespa.service.monitor.ServiceMonitorStatus;
 
 /**
@@ -22,6 +23,13 @@ public interface Policy {
             MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException;
 
     /**
+     * Decide whether to grant a request for temporarily suspending the services on all hosts in the group.
+     */
+    void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException;
+
+    void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException;
+
+    /**
      * Release an earlier grant for suspension.
      *
      * @throws HostStateChangeDeniedException if the release failed.
@@ -30,5 +38,4 @@ public interface Policy {
             ApplicationInstance<ServiceMonitorStatus> applicationInstance,
             HostName hostName,
             MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException;
-
 }
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
index dede55f402a..8fc6f27d3d9 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.applicationmodel.ServiceCluster;
 import com.yahoo.vespa.applicationmodel.ServiceInstance;
 import com.yahoo.vespa.applicationmodel.ServiceType;
 import com.yahoo.vespa.applicationmodel.TenantId;
+import com.yahoo.vespa.orchestrator.model.NodeGroup;
 import com.yahoo.vespa.orchestrator.model.VespaModelUtil;
 import com.yahoo.vespa.service.monitor.ServiceMonitorStatus;
 
@@ -34,7 +35,6 @@ public class DummyInstanceLookupService implements InstanceLookupService {
 
     private static final Set<ApplicationInstance<ServiceMonitorStatus>> apps = new HashSet<>();
 
-
     static {
         apps.add(new ApplicationInstance<>(
                 new TenantId("test-tenant-id"),
@@ -120,6 +120,11 @@ public class DummyInstanceLookupService implements InstanceLookupService {
         ));
     }
 
+    // A node group is tied to an application, so we need to define them after we have populated the above applications.
+    public final static NodeGroup TEST1_NODE_GROUP = new NodeGroup(new DummyInstanceLookupService().findInstanceByHost(TEST1_HOST_NAME).get(), TEST1_HOST_NAME);
+    public final static NodeGroup TEST3_NODE_GROUP = new NodeGroup(new DummyInstanceLookupService().findInstanceByHost(TEST3_HOST_NAME).get(), TEST3_HOST_NAME);
+    public final static NodeGroup TEST6_NODE_GROUP = new NodeGroup(new DummyInstanceLookupService().findInstanceByHost(TEST6_HOST_NAME).get(), TEST6_HOST_NAME);
+
 
     @Override
     public Optional<ApplicationInstance<ServiceMonitorStatus>> findInstanceById(
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
index 548d67689d2..09aa37f9610 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -19,7 +19,6 @@ import org.mockito.InOrder;
 
 import java.util.Arrays;
 import java.util.Iterator;
-import java.util.List;
 
 import static com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN;
 import static com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus.NO_REMARKS;
@@ -220,34 +219,18 @@ public class OrchestratorImplTest {
     }
 
     @Test
-    public void sortHostNamesForSuspend() throws Exception {
-        HostName parentHostName = new HostName("parentHostName");
-        List<HostName> expectedOrder = Arrays.asList(
-                DummyInstanceLookupService.TEST3_HOST_NAME,
-                DummyInstanceLookupService.TEST1_HOST_NAME);
-
-        assertEquals(expectedOrder, orchestrator.sortHostNamesForSuspend(Arrays.asList(
-                DummyInstanceLookupService.TEST1_HOST_NAME,
-                DummyInstanceLookupService.TEST3_HOST_NAME)));
-
-        assertEquals(expectedOrder, orchestrator.sortHostNamesForSuspend(Arrays.asList(
-                DummyInstanceLookupService.TEST3_HOST_NAME,
-                DummyInstanceLookupService.TEST1_HOST_NAME)));
-    }
-
-    @Test
     public void rollbackWorks() throws Exception {
         // A spy is preferential because suspendAll() relies on delegating the hard work to suspend() and resume().
         OrchestratorImpl orchestrator = spy(this.orchestrator);
 
-        doNothing().when(orchestrator).suspend(DummyInstanceLookupService.TEST3_HOST_NAME);
+        doNothing().when(orchestrator).suspendGroup(DummyInstanceLookupService.TEST3_NODE_GROUP);
 
         Throwable supensionFailure = new HostStateChangeDeniedException(
                 DummyInstanceLookupService.TEST6_HOST_NAME,
                 "some-constraint",
                 new ServiceType("foo"),
                 "error message");
-        doThrow(supensionFailure).when(orchestrator).suspend(DummyInstanceLookupService.TEST1_HOST_NAME);
+        doThrow(supensionFailure).when(orchestrator).suspendGroup(DummyInstanceLookupService.TEST6_NODE_GROUP);
 
         doThrow(new HostStateChangeDeniedException(DummyInstanceLookupService.TEST1_HOST_NAME, "foo1-constraint", new ServiceType("foo1-service"), "foo1-message"))
                 .when(orchestrator).resume(DummyInstanceLookupService.TEST1_HOST_NAME);
@@ -264,25 +247,26 @@ public class OrchestratorImplTest {
             fail();
         } catch (BatchHostStateChangeDeniedException e) {
             assertEquals(e.getSuppressed().length, 1);
-            assertEquals("Failed to suspend [test3.prod.utpoia-1.vespahosted.ut1.yahoo.com, " +
-                    "test6.prod.us-east-1.vespahosted.ne1.yahoo.com, test1.prod.utpoia-1.vespahosted.ut1.yahoo.com] " +
-                    "with parent host parentHostname: Changing the state of host " +
-                    "test6.prod.us-east-1.vespahosted.ne1.yahoo.com would violate some-constraint for " +
-                    "service type foo: error message; " +
-                    "With suppressed throwable com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException: " +
-                    "Changing the state of host test1.prod.utpoia-1.vespahosted.ut1.yahoo.com would violate " +
-                    "foo1-constraint for service type foo1-service: foo1-message", e.getMessage());
+
+            assertEquals("Failed to suspend NodeGroup{application=tenant-id-3:application-instance-3:prod:utopia-1:default, " +
+                    "hostNames=[test6.prod.us-east-1.vespahosted.ne1.yahoo.com]} with parent host parentHostname: " +
+                            "Changing the state of test6.prod.us-east-1.vespahosted.ne1.yahoo.com would violate " +
+                            "some-constraint for service type foo: error message; With suppressed throwable " +
+                            "com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException: " +
+                            "Changing the state of test1.prod.utpoia-1.vespahosted.ut1.yahoo.com would violate " +
+                            "foo1-constraint for service type foo1-service: foo1-message",
+                    e.getMessage());
         }
 
         InOrder order = inOrder(orchestrator);
-        order.verify(orchestrator).suspend(DummyInstanceLookupService.TEST3_HOST_NAME);
-        order.verify(orchestrator).suspend(DummyInstanceLookupService.TEST6_HOST_NAME);
-
-        // As of 2016-06-07:
-        //   TEST1_HOST_NAME: test-tenant-id:application:instance
-        //   TEST3_HOST_NAME: mediasearch:imagesearch:default
-        //   TEST6_HOST_NAME: tenant-id-3:application-instance-3:default
-        // Meaning the order is 3, 6, then 1. For rollback/resume the order is reversed.
+        order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST3_NODE_GROUP);
+        order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST6_NODE_GROUP);
+
+        // As of 2016-06-07 the order of the node groups are as follows:
+        //   TEST3: mediasearch:imagesearch:default
+        //   TEST6: tenant-id-3:application-instance-3:default
+        //   TEST1: test-tenant-id:application:instance
+        // Which is reversed when rolling back
         order.verify(orchestrator).resume(DummyInstanceLookupService.TEST1_HOST_NAME);
         order.verify(orchestrator).resume(DummyInstanceLookupService.TEST6_HOST_NAME);
         order.verify(orchestrator).resume(DummyInstanceLookupService.TEST3_HOST_NAME);
@@ -291,7 +275,7 @@ public class OrchestratorImplTest {
 
     private boolean isInMaintenance(ApplicationId appId, HostName hostName) throws ApplicationIdNotFoundException {
         for (ApplicationInstance<ServiceMonitorStatus> app : DummyInstanceLookupService.getApplications()) {
-            if (app.reference().equals(OrchestratorUtil.toApplicationInstanceReference(appId,new DummyInstanceLookupService()))) {
+            if (app.reference().equals(OrchestratorUtil.toApplicationInstanceReference(appId, new DummyInstanceLookupService()))) {
                 return clustercontroller.isInMaintenance(app, hostName);
             }
         }
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java
index 2af8973deef..5cba6375717 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java
@@ -16,12 +16,18 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerState;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse;
+import com.yahoo.vespa.orchestrator.model.ApplicationApi;
+import com.yahoo.vespa.orchestrator.model.ClusterApi;
+import com.yahoo.vespa.orchestrator.model.StorageNode;
 import com.yahoo.vespa.orchestrator.model.VespaModelUtil;
 import com.yahoo.vespa.orchestrator.status.HostStatus;
 import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry;
 import com.yahoo.vespa.service.monitor.ServiceMonitorStatus;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.InOrder;
 
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
@@ -38,6 +44,7 @@ import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -49,6 +56,66 @@ import static org.mockito.Mockito.when;
  * @author bakksjo
  */
 public class HostedVespaPolicyTest {
+
+    private final ClusterControllerClientFactory clientFactory = mock(ClusterControllerClientFactory.class);
+    private final ClusterControllerClient client = mock(ClusterControllerClient.class);
+
+    @Before
+    public void setUp() {
+        when(clientFactory.createClient(any(), any())).thenReturn(client);
+    }
+
+    @Test
+    public void testGrantSuspension() throws HostStateChangeDeniedException {
+        final HostedVespaClusterPolicy clusterPolicy = mock(HostedVespaClusterPolicy.class);
+        final HostedVespaPolicy policy = new HostedVespaPolicy(clusterPolicy, clientFactory);
+        final ApplicationApi applicationApi = mock(ApplicationApi.class);
+        when(applicationApi.applicationInfo()).thenReturn("tenant:app");
+
+        ClusterApi clusterApi1 = mock(ClusterApi.class);
+        ClusterApi clusterApi2 = mock(ClusterApi.class);
+        ClusterApi clusterApi3 = mock(ClusterApi.class);
+        List<ClusterApi> clusterApis = Arrays.asList(clusterApi1, clusterApi2, clusterApi3);
+        when(applicationApi.getClusters()).thenReturn(clusterApis);
+
+        StorageNode storageNode1 = mock(StorageNode.class);
+        HostName hostName1 = new HostName("storage-1");
+        when(storageNode1.hostName()).thenReturn(hostName1);
+
+        HostName hostName2 = new HostName("host-2");
+
+        StorageNode storageNode3 = mock(StorageNode.class);
+        HostName hostName3 = new HostName("storage-3");
+        when(storageNode1.hostName()).thenReturn(hostName3);
+
+        List<StorageNode> upStorageNodes = Arrays.asList(storageNode1, storageNode3);
+        when(applicationApi.getUpStorageNodesInGroupInClusterOrder()).thenReturn(upStorageNodes);
+        // setHostState
+
+        List<HostName> noRemarksHostNames = Arrays.asList(hostName1, hostName2, hostName3);
+        when(applicationApi.getNodesInGroupWithStatus(HostStatus.NO_REMARKS)).thenReturn(noRemarksHostNames);
+
+        InOrder order = inOrder(applicationApi, clusterPolicy, storageNode1, storageNode3);
+
+        policy.grantSuspensionRequest(applicationApi);
+
+        order.verify(applicationApi).getClusters();
+        order.verify(clusterPolicy).verifyGroupGoingDownIsFine(clusterApi1);
+        order.verify(clusterPolicy).verifyGroupGoingDownIsFine(clusterApi2);
+        order.verify(clusterPolicy).verifyGroupGoingDownIsFine(clusterApi3);
+
+        order.verify(applicationApi).getUpStorageNodesInGroupInClusterOrder();
+        order.verify(storageNode1).setNodeState(ClusterControllerState.MAINTENANCE);
+        order.verify(storageNode3).setNodeState(ClusterControllerState.MAINTENANCE);
+
+        order.verify(applicationApi).getNodesInGroupWithStatus(HostStatus.NO_REMARKS);
+        order.verify(applicationApi).setHostState(hostName1, HostStatus.ALLOWED_TO_BE_DOWN);
+        order.verify(applicationApi).setHostState(hostName2, HostStatus.ALLOWED_TO_BE_DOWN);
+        order.verify(applicationApi).setHostState(hostName3, HostStatus.ALLOWED_TO_BE_DOWN);
+
+        order.verifyNoMoreInteractions();
+    }
+
     private static final TenantId TENANT_ID = new TenantId("tenantId");
     private static final ApplicationInstanceId APPLICATION_INSTANCE_ID = new ApplicationInstanceId("applicationId");
     private static final HostName HOST_NAME_1 = new HostName("host-1");
@@ -59,15 +126,9 @@ public class HostedVespaPolicyTest {
     private static final ServiceType SERVICE_TYPE_1 = new ServiceType("service-1");
     private static final ServiceType SERVICE_TYPE_2 = new ServiceType("service-2");
 
-    private final ClusterControllerClientFactory clusterControllerClientFactory
-            = mock(ClusterControllerClientFactory.class);
-    private final ClusterControllerClient client = mock(ClusterControllerClient.class);
-    {
-        when(clusterControllerClientFactory.createClient(any(), any())).thenReturn(client);
-    }
-
-    private final HostedVespaPolicy policy
-            = new HostedVespaPolicy(clusterControllerClientFactory);
+    // TODO: Replace HostedVespaClusterPolicy with ClusterPolicy mock
+    private final HostedVespaPolicy policy =
+            new HostedVespaPolicy(new HostedVespaClusterPolicy(), clientFactory);
 
     private final MutableStatusRegistry mutablestatusRegistry = mock(MutableStatusRegistry.class);
     {
@@ -194,7 +255,7 @@ public class HostedVespaPolicyTest {
 
         policy.grantSuspensionRequest(applicationInstance, HOST_NAME_3, mutablestatusRegistry);
 
-        verify(mutablestatusRegistry, times(1)).setHostState(HOST_NAME_3, HostStatus.ALLOWED_TO_BE_DOWN);
+        verify(mutablestatusRegistry, times(0)).setHostState(HOST_NAME_3, HostStatus.ALLOWED_TO_BE_DOWN);
     }
 
     @Test
@@ -419,12 +480,13 @@ public class HostedVespaPolicyTest {
         // Verification phase.
 
         if (expectedNodeStateSentToClusterController.isPresent()) {
-            List<HostName> clusterControllers = CLUSTER_CONTROLLER_SERVICE_CLUSTER.serviceInstances().stream()
+            List<HostName> hostNames = CLUSTER_CONTROLLER_SERVICE_CLUSTER.serviceInstances().stream()
                     .map(service -> service.hostName())
                     .collect(Collectors.toList());
-            
-            verify(clusterControllerClientFactory, times(1))
-                    .createClient(clusterControllers, CONTENT_CLUSTER_NAME);
+            verify(clientFactory, times(1))
+                    .createClient(
+                            hostNames,
+                            CONTENT_CLUSTER_NAME);
             verify(client, times(1))
                     .setNodeState(
                             STORAGE_NODE_INDEX,
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
index 81393a28ec9..793b73ec5d8 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.applicationmodel.TenantId;
 import com.yahoo.vespa.orchestrator.InstanceLookupService;
 import com.yahoo.vespa.orchestrator.OrchestratorImpl;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock;
+import com.yahoo.vespa.orchestrator.model.ApplicationApi;
 import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException;
 import com.yahoo.vespa.orchestrator.policy.Policy;
 import com.yahoo.vespa.orchestrator.restapi.wire.BatchHostSuspendRequest;
@@ -90,8 +91,18 @@ public class HostResourceTest {
         public void grantSuspensionRequest(
                 ApplicationInstance<ServiceMonitorStatus> applicationInstance,
                 HostName hostName,
-                MutableStatusRegistry hostStatusRegistry) {
+                MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException {
+
+        }
+
+        @Override
+        public void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException {
         }
+
+        @Override
+        public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException {
+        }
+
         @Override
         public void releaseSuspensionGrant(
                 ApplicationInstance<ServiceMonitorStatus> applicationInstance,
@@ -180,6 +191,17 @@ public class HostResourceTest {
                 MutableStatusRegistry hostStatusRegistry) throws HostStateChangeDeniedException {
             doThrow();
         }
+
+        @Override
+        public void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException {
+            doThrow();
+        }
+
+        @Override
+        public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException {
+            doThrow();
+        }
+
         @Override
         public void releaseSuspensionGrant(
                 ApplicationInstance<ServiceMonitorStatus> applicationInstance,