Don't constrain groups to 1 if group-size is set

3 files changed, 104 insertions, 8 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
index f44849e185f..105a9669d1f 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.model.provision;
 
+import com.yahoo.collections.IntRange;
 import com.yahoo.collections.ListMap;
 import com.yahoo.collections.Pair;
 import com.yahoo.config.model.api.HostProvisioner;
@@ -153,13 +154,18 @@ public class InMemoryProvisioner implements HostProvisioner {
             requested = requested.withLimits(requested.minResources().withNodes(1),
                                              requested.maxResources().withNodes(1));
         }
-        if (useMaxResources)
-            return prepare(cluster, requested.maxResources(), requested.isRequired(), requested.canFail());
-        else
-            return prepare(cluster, requested.minResources(), requested.isRequired(), requested.canFail());
+        IntRange groupRange = IntRange.of(requested.minResources().groups(), requested.maxResources().groups());
+        if (useMaxResources) {
+            int groups = groupRange.fit(requested.maxResources().nodes() / requested.groupSize().to().orElse(1));
+            return prepare(cluster, requested.maxResources(),groups, requested.isRequired(), requested.canFail());
+        }
+        else {
+            int groups = groupRange.fit(requested.minResources().nodes() / requested.groupSize().from().orElse(1));
+            return prepare(cluster, requested.minResources(), groups, requested.isRequired(), requested.canFail());
+        }
     }
 
-    public List<HostSpec> prepare(ClusterSpec cluster, ClusterResources requested, boolean required, boolean canFail) {
+    public List<HostSpec> prepare(ClusterSpec cluster, ClusterResources requested, int groups, boolean required, boolean canFail) {
         if (cluster.group().isPresent() && requested.groups() > 1)
             throw new IllegalArgumentException("Cannot both be specifying a group and ask for groups to be created");
 
@@ -169,7 +175,7 @@ public class InMemoryProvisioner implements HostProvisioner {
         if (alwaysReturnOneNode)
             nodes = 1;
 
-        int groups = Math.min(requested.groups(), nodes);
+        groups = Math.min(groups, nodes);
 
         List<HostSpec> allocation = new ArrayList<>();
         if (groups == 1) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
index b4f49f10285..7a9ee51c29b 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
@@ -124,8 +124,9 @@ public class NodesSpecification {
         var nodes =  IntRange.from(nodesElement.stringAttribute("count", ""));
         var groups = IntRange.from(nodesElement.stringAttribute("groups", ""));
         var groupSize = IntRange.from(nodesElement.stringAttribute("group-size", ""));
+        int defaultMaxGroups = groupSize.isEmpty() ? 1 : nodes.to().orElse(1); // Don't constrain the number of groups if group size is set
         var min = new ClusterResources(nodes.from().orElse(1),  groups.from().orElse(1),  nodeResources(nodesElement).getFirst());
-        var max = new ClusterResources(nodes.to().orElse(1), groups.to().orElse(1), nodeResources(nodesElement).getSecond());
+        var max = new ClusterResources(nodes.to().orElse(1), groups.to().orElse(defaultMaxGroups), nodeResources(nodesElement).getSecond());
         return new ResourceConstraints(min, max, groupSize);
     }
 
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index a6ddce4cec5..452a9fcdf87 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -595,6 +595,95 @@ public class ModelProvisioningTest {
     }
 
     @Test
+    public void testUsingGroups() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>\n" +
+                "<services>" +
+                "  <admin version='4.0'/>" +
+                "  <container version='1.0' id='foo'>" +
+                "     <nodes count='10'/>" +
+                "  </container>" +
+                "  <content version='1.0' id='bar'>" +
+                "     <redundancy>2</redundancy>" +
+                "     <documents>" +
+                "       <document type='type1' mode='index'/>" +
+                "     </documents>" +
+                "     <nodes count='30' groups='2'/>" +
+                "  </content>" +
+                "  <content version='1.0' id='baz'>" +
+                "     <redundancy>1</redundancy>" +
+                "     <documents>" +
+                "       <document type='type1' mode='index'/>" +
+                "     </documents>" +
+                "     <nodes count='30' groups='30'/>" +
+                "   </content>" +
+                "</services>";
+
+        int numberOfHosts = 73;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(numberOfHosts);
+        VespaModel model = tester.createModel(services, true);
+        assertEquals(numberOfHosts, model.getRoot().hostSystem().getHosts().size());
+
+        ContentCluster cluster = model.getContentClusters().get("bar");
+        List<StorageGroup> subGroups = cluster.getRootGroup().getSubgroups();
+        assertEquals( 0, cluster.getRootGroup().getNodes().size());
+        assertEquals( 2, subGroups.size());
+        assertEquals(15, subGroups.get(0).getNodes().size());
+
+        cluster = model.getContentClusters().get("baz");
+        subGroups = cluster.getRootGroup().getSubgroups();
+        assertEquals( 0, cluster.getRootGroup().getNodes().size());
+        assertEquals(30, subGroups.size());
+        assertEquals( 1, subGroups.get(0).getNodes().size());
+    }
+
+    // Same as the test above but setting groupSize only
+    @Test
+    public void testUsingGroupSizeNotGroups() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>\n" +
+                "<services>" +
+                "  <admin version='4.0'/>" +
+                "  <container version='1.0' id='foo'>" +
+                "     <nodes count='10'/>" +
+                "  </container>" +
+                "  <content version='1.0' id='bar'>" +
+                "     <redundancy>2</redundancy>" +
+                "     <documents>" +
+                "       <document type='type1' mode='index'/>" +
+                "     </documents>" +
+                "     <nodes count='30' group-size='[15, 30]'/>" +
+                "  </content>" +
+                "  <content version='1.0' id='baz'>" +
+                "     <redundancy>1</redundancy>" +
+                "     <documents>" +
+                "       <document type='type1' mode='index'/>" +
+                "     </documents>" +
+                "     <nodes count='30' group-size='1'/>" +
+                "   </content>" +
+                "</services>";
+
+        int numberOfHosts = 73;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(numberOfHosts);
+        VespaModel model = tester.createModel(services, true);
+        assertEquals(numberOfHosts, model.getRoot().hostSystem().getHosts().size());
+
+        ContentCluster cluster = model.getContentClusters().get("bar");
+        List<StorageGroup> subGroups = cluster.getRootGroup().getSubgroups();
+        assertEquals( 0, cluster.getRootGroup().getNodes().size());
+        assertEquals( 2, subGroups.size());
+        assertEquals(15, subGroups.get(0).getNodes().size());
+
+        cluster = model.getContentClusters().get("baz");
+        subGroups = cluster.getRootGroup().getSubgroups();
+        assertEquals( 0, cluster.getRootGroup().getNodes().size());
+        assertEquals(30, subGroups.size());
+        assertEquals( 1, subGroups.get(0).getNodes().size());
+    }
+
+    @Test
     public void testSlobroksOnContainersIfNoContentClusters() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +
@@ -1449,7 +1538,7 @@ public class ModelProvisioningTest {
         tester.addHosts(new NodeResources(85,   200, 1000_000_000, 0.3), 20);
         tester.addHosts(new NodeResources( 0.5,   2,           10, 0.3), 3);
         VespaModel model = tester.createModel(services, true);
-        assertEquals(totalHosts + 3, model.getRoot().hostSystem().getHosts().size());
+        assertEquals(4 + 6 + 1, model.getRoot().hostSystem().getHosts().size());
     }
 
     @Test