summaryrefslogtreecommitdiffstats
path: root/orchestrator
diff options
context:
space:
mode:
authorHåkon Hallingstad <hakon@yahoo-inc.com>2017-05-03 16:05:47 +0200
committerHåkon Hallingstad <hakon@yahoo-inc.com>2017-05-03 16:05:47 +0200
commitcf41f2a58899ae6fc240da22fa154fcf106ea839 (patch)
treed25d78fc471db5abf72c9cd2f2492031b7bbfa58 /orchestrator
parent9a5239261e5b90fd9dc06d291f4a44c5fb663639 (diff)
Makes the Orchestrator policy use the new model classes
Functionally, the new code should be identical to the old one, EXCEPT if an application has 2 or more nodes on a Docker host: - In this case the new code puts these nodes into a NodeGroup, and will try to suspend them all at the same time. This PR could have been functionally identical to the old code by by putting each HostName into its own NodeGroup in OrchestratorImpl::nodeGroupsOrderedForSuspend.
Diffstat (limited to 'orchestrator')
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/BatchInternalErrorException.java7
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java26
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java104
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/BatchHostStateChangeDeniedException.java7
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ClusterPolicy.java14
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/ConcurrentSuspensionLimitForCluster.java21
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java30
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java52
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java230
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java11
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java7
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java56
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java90
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java24
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 09471f9d6ae..55de50299b3 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,