Assume at least 3 config server in Orchestrator

3 files changed, 115 insertions, 9 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java
index d0217710bdb..eb69f1a94a9 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java
@@ -1,12 +1,14 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.model;
 
+import com.yahoo.vespa.applicationmodel.ApplicationInstanceId;
 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.applicationmodel.ServiceStatus;
 import com.yahoo.vespa.applicationmodel.ServiceType;
+import com.yahoo.vespa.applicationmodel.TenantId;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
 import com.yahoo.vespa.orchestrator.status.HostStatus;
 
@@ -17,6 +19,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 class ClusterApiImpl implements ClusterApi {
     private final ApplicationApi applicationApi;
@@ -29,6 +32,15 @@ class ClusterApiImpl implements ClusterApi {
     private final Set<ServiceInstance> servicesNotInGroup;
     private final Set<ServiceInstance> servicesDownAndNotInGroup;
 
+    /**
+     * There are supposed to be (at least) 3 config servers in a production-like environment.
+     * However the number of config servers in the zone-config-servers application/cluster may only be 2,
+     * if only 2 have been provisioned so far, or 1 is being reprovisioned. In these cases it is
+     * important for the Orchestrator to count that third config server as down.
+     */
+    private final int missingServices;
+    private final String descriptionOfMissingServices;
+
     public ClusterApiImpl(ApplicationApi applicationApi,
                           ServiceCluster serviceCluster,
                           NodeGroup nodeGroup,
@@ -51,6 +63,15 @@ class ClusterApiImpl implements ClusterApi {
 
         servicesDownInGroup = servicesInGroup.stream().filter(this::serviceEffectivelyDown).collect(Collectors.toSet());
         servicesDownAndNotInGroup = servicesNotInGroup.stream().filter(this::serviceEffectivelyDown).collect(Collectors.toSet());
+
+        int serviceInstances = serviceCluster.serviceInstances().size();
+        if (serviceCluster.isConfigServerCluster() && serviceInstances < 3) {
+            missingServices = 3 - serviceInstances;
+            descriptionOfMissingServices = missingServices + " missing config server" + (missingServices > 1 ? "s" : "");
+        } else {
+            missingServices = 0;
+            descriptionOfMissingServices = "NA";
+        }
     }
 
     @Override
@@ -83,29 +104,31 @@ class ClusterApiImpl implements ClusterApi {
         return servicesDownInGroup.size() == servicesInGroup.size();
     }
 
+    int missingServices() { return missingServices; }
+
     @Override
     public boolean noServicesOutsideGroupIsDown() {
-        return servicesDownAndNotInGroup.size() == 0;
+        return servicesDownAndNotInGroup.size() + missingServices == 0;
     }
 
     @Override
     public int percentageOfServicesDown() {
-        int numberOfServicesDown = servicesDownAndNotInGroup.size() + servicesDownInGroup.size();
-        return numberOfServicesDown * 100 / serviceCluster.serviceInstances().size();
+        int numberOfServicesDown = servicesDownAndNotInGroup.size() + missingServices + servicesDownInGroup.size();
+        return numberOfServicesDown * 100 / (serviceCluster.serviceInstances().size() + missingServices);
     }
 
     @Override
     public int percentageOfServicesDownIfGroupIsAllowedToBeDown() {
-        int numberOfServicesDown = servicesDownAndNotInGroup.size() + servicesInGroup.size();
-        return numberOfServicesDown * 100 / serviceCluster.serviceInstances().size();
+        int numberOfServicesDown = servicesDownAndNotInGroup.size() + missingServices + servicesInGroup.size();
+        return numberOfServicesDown * 100 / (serviceCluster.serviceInstances().size() + missingServices);
     }
 
     @Override
     public String servicesDownAndNotInGroupDescription() {
         // Sort these for readability and testing stability
-        return servicesDownAndNotInGroup.stream()
-                .map(service -> service.toString())
-                .sorted()
+        return Stream
+                .concat(servicesDownAndNotInGroup.stream().map(ServiceInstance::toString).sorted(),
+                        missingServices > 0 ? Stream.of(descriptionOfMissingServices) : Stream.of())
                 .collect(Collectors.toList())
                 .toString();
     }
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java
index 0a53972b30c..c17e885df02 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java
@@ -1,10 +1,17 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.model;
 
+import com.yahoo.vespa.applicationmodel.ApplicationInstance;
+import com.yahoo.vespa.applicationmodel.ApplicationInstanceId;
+import com.yahoo.vespa.applicationmodel.ClusterId;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceCluster;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
 import com.yahoo.vespa.applicationmodel.ServiceType;
+import com.yahoo.vespa.applicationmodel.TenantId;
+import com.yahoo.vespa.orchestrator.OrchestratorUtil;
+import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException;
+import com.yahoo.vespa.orchestrator.policy.HostedVespaClusterPolicy;
 import com.yahoo.vespa.orchestrator.status.HostStatus;
 import org.junit.Test;
 
@@ -12,12 +19,18 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Optional;
+import java.util.Set;
 
+import static org.hamcrest.core.StringContains.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class ClusterApiImplTest {
     final ApplicationApi applicationApi = mock(ApplicationApi.class);
@@ -72,6 +85,69 @@ public class ClusterApiImplTest {
         assertEquals(80, clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown());
     }
 
+    /** Make a ClusterApiImpl for the cfg1 config server, with cfg3 missing from the cluster (not provisioned). */
+    private ClusterApiImpl makeCfg1ClusterApi(ServiceStatus cfg1ServiceStatus, ServiceStatus cfg2ServiceStatus) {
+        HostName cfg1Hostname = new HostName("cfg1");
+        HostName cfg2Hostname = new HostName("cfg2");
+
+        ServiceCluster serviceCluster = modelUtils.createServiceCluster(
+                ClusterId.CONFIG_SERVER.s(),
+                ServiceType.CONFIG_SERVER,
+                Arrays.asList(
+                        modelUtils.createServiceInstance("cs1", cfg1Hostname, cfg1ServiceStatus),
+                        modelUtils.createServiceInstance("cs2", cfg2Hostname, cfg2ServiceStatus))
+        );
+
+        Set<ServiceCluster> serviceClusterSet = new HashSet<>(Set.of(serviceCluster));
+
+        ApplicationInstance application = new ApplicationInstance(
+                TenantId.HOSTED_VESPA,
+                ApplicationInstanceId.CONFIG_SERVER,
+                serviceClusterSet);
+
+        serviceCluster.setApplicationInstance(application);
+
+        when(applicationApi.applicationId()).thenReturn(OrchestratorUtil.toApplicationId(application.reference()));
+
+        ClusterApiImpl clusterApi = new ClusterApiImpl(
+                applicationApi,
+                serviceCluster,
+                new NodeGroup(application, cfg1Hostname),
+                modelUtils.getHostStatusMap(),
+                modelUtils.getClusterControllerClientFactory());
+
+        assertEquals(1, clusterApi.missingServices());
+        assertFalse(clusterApi.noServicesOutsideGroupIsDown());
+
+        return clusterApi;
+    }
+
+    @Test
+    public void testCfg1SuspensionFailsWithMissingCfg3() {
+        ClusterApiImpl clusterApi = makeCfg1ClusterApi(ServiceStatus.UP, ServiceStatus.UP);
+
+        HostedVespaClusterPolicy policy = new HostedVespaClusterPolicy();
+
+        try {
+            policy.verifyGroupGoingDownIsFine(clusterApi);
+            fail();
+        } catch (HostStateChangeDeniedException e) {
+            assertThat(e.getMessage(),
+                    containsString("Changing the state of cfg1 would violate enough-services-up: Suspension percentage " +
+                            "for service type configserver would increase from 33% to 66%, over the limit of 10%. " +
+                            "These instances may be down: [1 missing config server] and these hosts are allowed to be down: []"));
+        }
+    }
+
+    @Test
+    public void testCfg1SuspendsIfDownWithMissingCfg3() throws HostStateChangeDeniedException {
+        ClusterApiImpl clusterApi = makeCfg1ClusterApi(ServiceStatus.DOWN, ServiceStatus.UP);
+
+        HostedVespaClusterPolicy policy = new HostedVespaClusterPolicy();
+
+        policy.verifyGroupGoingDownIsFine(clusterApi);
+    }
+
     @Test
     public void testNoServices() {
         HostName hostName1 = new HostName("host1");
@@ -141,10 +217,14 @@ public class ClusterApiImplTest {
                 )
         );
 
+
+        ApplicationInstance applicationInstance = modelUtils.createApplicationInstance(new ArrayList<>());
+        serviceCluster.setApplicationInstance(applicationInstance);
+
         ClusterApiImpl clusterApi = new ClusterApiImpl(
                 applicationApi,
                 serviceCluster,
-                new NodeGroup(modelUtils.createApplicationInstance(new ArrayList<>()), hostName1, hostName3),
+                new NodeGroup(applicationInstance, hostName1, hostName3),
                 new HashMap<>(),
                 modelUtils.getClusterControllerClientFactory());
 
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java
index 78f50dbfc3f..0def668e147 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java
@@ -86,6 +86,9 @@ class ModelTestUtils {
                 new ApplicationInstanceId("application-name:foo:bar:default"),
                 serviceClusterSet);
         applications.put(application.reference(), application);
+
+        serviceClusters.forEach(cluster -> cluster.setApplicationInstance(application));
+
         return application;
     }