diff options
author | hakonhall <hakon@yahoo-inc.com> | 2017-05-08 15:40:55 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-05-08 15:40:55 +0200 |
commit | 964f92513a1bdecf7dcb664c13a29909e33967b2 (patch) | |
tree | 13893430deed34a53fd77c65b4a39900815709f4 /orchestrator | |
parent | efb0a993ac3e696703f0d5003738f7d80fa20ae1 (diff) | |
parent | cf41f2a58899ae6fc240da22fa154fcf106ea839 (diff) |
Merge pull request #2378 from yahoo/hakonhall/use-orchestrator-model
Makes the Orchestrator policy use the new model classes
Diffstat (limited to 'orchestrator')
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, |