Add configuration for minimum number of slobroks per container cluster

* By default, we use between 1 and 3/(number of container clusters) nodes per
  container cluster for slobroks. If a customer has 3 or more container clusters
  we end up with just one slobrok per cluster. These might end up on the same
  Docker host when running on Docker.
* Make minimum number of slobroks per container cluster configurable (default 1 as before)
  so that we can migrate apps that have more than 3 container clusters in a controlled manner

4 files changed, 133 insertions, 45 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
index 08843d306b7..fe757ba58b3 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.model.builder.xml.dom;
 import com.yahoo.component.Version;
 import com.yahoo.config.model.api.ConfigServerSpec;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.vespa.model.HostResource;
 import com.yahoo.vespa.model.HostSystem;
 import com.yahoo.vespa.model.admin.*;
@@ -50,16 +49,17 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         Optional<NodesSpecification> requestedLogservers = 
                 NodesSpecification.optionalDedicatedFromParent(adminElement.getChild("logservers"), version);
 
-        assignSlobroks(requestedSlobroks.orElse(NodesSpecification.nonDedicated(3, version)), admin);
+        int minSlobroksPerCluster = getMinSlobroksPerContainerCluster(adminElement);
+        assignSlobroks(requestedSlobroks.orElse(NodesSpecification.nonDedicated(3, version)), admin, minSlobroksPerCluster);
         assignLogserver(requestedLogservers.orElse(NodesSpecification.nonDedicated(1, version)), admin);
     }
 
-    private void assignSlobroks(NodesSpecification nodesSpecification, Admin admin) {
+    private void assignSlobroks(NodesSpecification nodesSpecification, Admin admin, int minSlobroksPerContainerCluster) {
         if (nodesSpecification.isDedicated()) {
             createSlobroks(admin, allocateHosts(admin.getHostSystem(), "slobroks", nodesSpecification));
         }
         else {
-            createSlobroks(admin, pickContainerHosts(nodesSpecification.count()));
+            createSlobroks(admin, pickContainerHosts(nodesSpecification.count(), minSlobroksPerContainerCluster));
         }
     }
 
@@ -88,14 +88,16 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
      *
      * @param count the desired number of nodes. More nodes may be returned to ensure a smooth transition
      *        on topology changes, and less nodes may be returned if fewer are available
+     * @param minHostsPerContainerCluster the desired number of hosts per cluster
      */
-    private List<HostResource> pickContainerHosts(int count) {
+    private List<HostResource> pickContainerHosts(int count, int minHostsPerContainerCluster) {
         // Pick from all container clusters to make sure we don't lose all nodes at once if some clusters are removed.
         // This will overshoot the desired size (due to ceil and picking at least one node per cluster).
         List<HostResource> picked = new ArrayList<>();
         for (ContainerModel containerModel : containerModels)
             picked.addAll(pickContainerHostsFrom(containerModel,
-                                                 (int)Math.max(1, Math.ceil((double)count/containerModels.size()))));
+                                                 (int) Math.max(minHostsPerContainerCluster,
+                                                                Math.ceil((double) count / containerModels.size()))));
         return picked;
     }
 
@@ -105,7 +107,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
 
         // if we can return multiple hosts, include retired nodes which would have been picked before
         // (probably - assuming all previous nodes were retired, which is always true for a single cluster
-        // at the moment (Sept 2015)) // to ensure a smoother transition between the old and new topology
+        // at the moment (Sept 2015)) to ensure a smoother transition between the old and new topology
         // by including both new and old nodes during the retirement period
         picked.addAll(sortedContainerHostsFrom(model, count, retired));
 
@@ -143,4 +145,11 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         admin.addSlobroks(slobroks);
     }
 
+    private int getMinSlobroksPerContainerCluster(ModelElement adminElement) {
+        ModelElement minNodes = adminElement.getChild("minSlobroksPerCluster");
+        if (minNodes == null) return 1; //default
+
+        return (int) minNodes.asLong();
+    }
+
 }
diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc
index d8782bebc8a..b16129e473a 100644
--- a/config-model/src/main/resources/schema/admin.rnc
+++ b/config-model/src/main/resources/schema/admin.rnc
@@ -5,6 +5,7 @@ AdminV2 =
  element admin {
    attribute version { "2.0" } &
    element adminserver { service.attlist } &
+   element minSlobroksPerCluster { xsd:positiveInteger }? &
    GenericConfig* &
    LogServer? &
    (ConfigServer | ConfigServers)? &
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
index 7e61451fd7c..715e0614d8e 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
@@ -30,8 +30,21 @@ import static org.junit.Assert.assertNotNull;
  */
 public class DedicatedAdminV4Test {
 
-    private static final String services =
-                "<services>" +
+    private static final String hosts = "<hosts>"
+            + "  <host name=\"myhost0\">"
+            + "    <alias>node0</alias>"
+            + "  </host>"
+            + "  <host name=\"myhost1\">"
+            + "    <alias>node1</alias>"
+            + "  </host>"
+            + "  <host name=\"myhost2\">"
+            + "    <alias>node2</alias>"
+            + "  </host>"
+            + "</hosts>";
+
+    @Test
+    public void testModelBuilding() throws IOException, SAXException {
+        String services = "<services>" +
                 "  <admin version='4.0'>" +
                 "    <slobroks><nodes count='2' dedicated='true'/></slobroks>" +
                 "    <logservers><nodes count='1' dedicated='true'/></logservers>" +
@@ -46,45 +59,20 @@ public class DedicatedAdminV4Test {
                 "  </admin>" +
                 "</services>";
 
-    @Test
-    public void testModelBuilding() throws IOException, SAXException {
-        String hosts = "<hosts>"
-                + " <host name=\"myhost0\">"
-                + "  <alias>node0</alias>"
-                + " </host>"
-                + " <host name=\"myhost1\">"
-                + "  <alias>node1</alias>"
-                + " </host>"
-                + " <host name=\"myhost2\">"
-                + "  <alias>node2</alias>"
-                + " </host>"
-                + "</hosts>";
-        ApplicationPackage app = new MockApplicationPackage.Builder().withHosts(hosts).withServices(services).build();
-        VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder().applicationPackage(app).modelHostProvisioner(new InMemoryProvisioner(Hosts.readFrom(app.getHosts()), true)).build());
+        VespaModel model = createModel(hosts, services);
         assertEquals(3, model.getHosts().size());
 
-        Set<String> serviceNames0 = serviceNames(model.getConfig(SentinelConfig.class, "hosts/myhost0"));
-        assertEquals(3, serviceNames0.size());
-        assertTrue(serviceNames0.contains("slobrok"));
-        assertTrue(serviceNames0.contains("logd"));
-        assertTrue(serviceNames0.contains("filedistributorservice"));
-
-        Set<String> serviceNames1 = serviceNames(model.getConfig(SentinelConfig.class, "hosts/myhost1"));
-        assertEquals(3, serviceNames1.size());
-        assertTrue(serviceNames1.contains("slobrok"));
-        assertTrue(serviceNames1.contains("logd"));
-        assertTrue(serviceNames1.contains("filedistributorservice"));
-
-        Set<String> serviceNames2 = serviceNames(model.getConfig(SentinelConfig.class, "hosts/myhost2"));
-        assertEquals(3, serviceNames2.size());
-        assertTrue(serviceNames2.contains("logserver"));
-        assertTrue(serviceNames2.contains("logd"));
-        assertTrue(serviceNames2.contains("filedistributorservice"));
-        
+        assertHostContainsServices(model, "hosts/myhost0",
+                                   "slobrok", "logd", "filedistributorservice");
+        assertHostContainsServices(model, "hosts/myhost1",
+                                   "slobrok", "logd", "filedistributorservice");
+        assertHostContainsServices(model, "hosts/myhost2",
+                                   "logserver", "logd", "filedistributorservice");
+
         Yamas yamas = model.getAdmin().getYamas();
         assertEquals("vespa.routing", yamas.getClustername());
-        assertEquals(60L, (long)yamas.getIntervalSeconds());
-        
+        assertEquals(60L, (long) yamas.getIntervalSeconds());
+
         MetricsConsumer consumer = model.getAdmin().getLegacyUserMetricsConsumers().get(VESPA_CONSUMER_ID);
         assertNotNull(consumer);
         assertEquals(3, consumer.getMetrics().size());
@@ -93,8 +81,97 @@ public class DedicatedAdminV4Test {
         assertEquals("nginx.upstreams.down", metric.outputName);
     }
 
-    private Set<String> serviceNames(SentinelConfig config) {
+    @Test
+    public void testModelBuildingWithConfiguredMinSlobrokCountPerCluster() throws IOException, SAXException {
+        String hosts = "<hosts>"
+                + "  <host name=\"myhost0\">"
+                + "    <alias>node0</alias>"
+                + "  </host>"
+                + "  <host name=\"myhost1\">"
+                + "    <alias>node1</alias>"
+                + "  </host>"
+                + "  <host name=\"myhost2\">"
+                + "    <alias>node2</alias>"
+                + "  </host>"
+                + "  <host name=\"myhost3\">"
+                + "    <alias>node3</alias>"
+                + "  </host>"
+                + "</hosts>";
+
+        {
+            VespaModel model = createModel(hosts, servicesWithMinSlobroksPerCluster(1));
+            assertEquals(4, model.getHosts().size());
+
+            // 3 slobroks, 1 per cluster
+            assertHostContainsServices(model, "hosts/myhost0",
+                                       "slobrok", "logd", "filedistributorservice", "logserver", "qrserver");
+            assertHostContainsServices(model, "hosts/myhost1",
+                                       "logd", "filedistributorservice", "qrserver");
+            assertHostContainsServices(model, "hosts/myhost2",
+                                       "slobrok", "logd", "filedistributorservice", "qrserver");
+            assertHostContainsServices(model, "hosts/myhost3",
+                                       "slobrok", "logd", "filedistributorservice", "qrserver");
+        }
+
+        {
+            VespaModel model = createModel(hosts, servicesWithMinSlobroksPerCluster(2));
+            assertEquals(4, model.getHosts().size());
+
+            // 4 slobroks, 2 per cluster where possible
+            assertHostContainsServices(model, "hosts/myhost0",
+                                       "slobrok", "logd", "filedistributorservice", "logserver", "qrserver");
+            assertHostContainsServices(model, "hosts/myhost1",
+                                       "slobrok", "logd", "filedistributorservice", "qrserver");
+            assertHostContainsServices(model, "hosts/myhost2",
+                                       "slobrok", "logd", "filedistributorservice", "qrserver");
+            assertHostContainsServices(model, "hosts/myhost3",
+                                       "slobrok", "logd", "filedistributorservice", "qrserver");
+        }
+    }
+
+    private String servicesWithMinSlobroksPerCluster(int count) {
+        return "<services>" +
+                "  <admin version='4.0'>" +
+                "    <minSlobroksPerCluster>" + count + "</minSlobroksPerCluster>" +
+                "    <nodes count='1' dedicated='true' />" +
+                "  </admin>" +
+                "  <jdisc id='a' version='1.0'>" +
+                "    <search />" +
+                "    <nodes count='2' dedicated='true' />" +
+                "  </jdisc>" +
+                "  <jdisc id='b' version='1.0'>" +
+                "    <search />" +
+                "    <nodes count='1' dedicated='true' />" +
+                "  </jdisc>" +
+                "  <jdisc id='c' version='1.0'>" +
+                "    <search />" +
+                "    <nodes count='1' dedicated='true' />" +
+                "  </jdisc>" +
+                "</services>";
+    }
+
+    private Set<String> serviceNames(VespaModel model, String hostname) {
+        SentinelConfig config = model.getConfig(SentinelConfig.class, hostname);
         return config.service().stream().map(SentinelConfig.Service::name).collect(Collectors.toSet());
     }
 
+    private void assertHostContainsServices(VespaModel model, String hostname, String... expectedServices) {
+        Set<String> serviceNames = serviceNames(model, hostname);
+        assertEquals(expectedServices.length, serviceNames.size());
+        for (String serviceName : expectedServices) {
+            assertTrue(serviceNames.contains(serviceName));
+        }
+    }
+
+    private VespaModel createModel(String hosts, String services) throws IOException, SAXException {
+        ApplicationPackage app = new MockApplicationPackage.Builder()
+                .withHosts(hosts)
+                .withServices(services)
+                .build();
+        return new VespaModel(new NullConfigModelRegistry(),
+                              new DeployState.Builder().applicationPackage(app).modelHostProvisioner(
+                                      new InMemoryProvisioner(Hosts.readFrom(app.getHosts()), true))
+                                                       .build());
+    }
+
 }
diff --git a/config-model/src/test/schema-test-files/services.xml b/config-model/src/test/schema-test-files/services.xml
index c8f8ba7ddb1..7a03b019703 100644
--- a/config-model/src/test/schema-test-files/services.xml
+++ b/config-model/src/test/schema-test-files/services.xml
@@ -7,6 +7,7 @@
   </config>
 
   <admin version="2.0">
+    <minSlobroksPerCluster>1</minSlobroksPerCluster>
     <adminserver hostalias="adminserver" />
     <logserver hostalias="logserver" />
     <slobroks>