Prepare to iterate over groups 2

2 files changed, 15 insertions, 14 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
index a556dcf9ced..d1a5a91e985 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
@@ -43,18 +43,21 @@ public class AllocationOptimizer {
         // This is the group size, since we (for now) assume the group size is decided by someone wiser than us
         // and we decide the number of groups.
         // The exception is when we only have one group, where we can add and remove single nodes in it.
-        boolean singleGroupMode = current.groups() == 1 && ( limits.isEmpty() || limits.min().groups() == 1 );
-        int nodeIncrement = singleGroupMode ? 1 : current.groupSize();
 
         Optional<AllocatableClusterResources> bestAllocation = Optional.empty();
-
-        for (int nodes = minNodes(limits, singleGroupMode, current); nodes <= maxNodes(limits, singleGroupMode, current); nodes += nodeIncrement) {
+        for (int nodes = minNodes(limits); nodes <= maxNodes(limits); nodes++) {
+            boolean singleGroupMode = current.groups() == 1 && ( limits.isEmpty() || limits.min().groups() == 1 );
             int groups = singleGroupMode ? 1 : nodes / current.groupSize();
             if ( ! limits.isEmpty() && ( groups < limits.min().groups() || groups > limits.max().groups())) continue;
 
+            if (nodes % groups != 0) continue;
+            int groupSize = nodes / groups;
+            if (groups != 1 && groupSize != current.groupSize()) continue; // TODO: Remove this line
+
+
             // Adjust for redundancy: Node in group if groups = 1, an extra group if multiple groups
             // TODO: Make the best choice based on size and redundancy setting instead
-            int nodesAdjustedForRedundancy = target.adjustForRedundancy() ? nodes - nodeIncrement : nodes;
+            int nodesAdjustedForRedundancy = target.adjustForRedundancy() ? ( groups == 1 ? nodes - 1 : nodes - groupSize ) : nodes;
             if (nodesAdjustedForRedundancy < 1) continue;
             int groupsAdjustedForRedundancy = target.adjustForRedundancy() ? ( groups == 1 ? 1 : groups - 1 ) : groups;
 
@@ -71,16 +74,14 @@ public class AllocationOptimizer {
         return bestAllocation;
     }
 
-    private int minNodes(Limits limits, boolean singleGroupMode, AllocatableClusterResources current) {
+    private int minNodes(Limits limits) {
         if (limits.isEmpty()) return minimumNodes;
-        if (singleGroupMode) return limits.min().nodes();
-        return Math.max(limits.min().nodes(), limits.min().groups() * current.groupSize() );
+        return limits.min().nodes();
     }
 
-    private int maxNodes(Limits limits, boolean singleGroupMode, AllocatableClusterResources current) {
+    private int maxNodes(Limits limits) {
         if (limits.isEmpty()) return maximumNodes;
-        if (singleGroupMode) return limits.max().nodes();
-        return Math.min(limits.max().nodes(), limits.max().groups() * current.groupSize() );
+        return limits.max().nodes();
     }
 
     /**
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index f72998f07b9..694c1f2d9fc 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -450,21 +450,21 @@ public class ProvisioningTest {
         tester.activate(app1, cluster1, Capacity.from(resources(2, 1, 10, 30,  10),
                                                       resources(4, 2, 14, 40, 13)));
         tester.assertNodes("A mix of min and max",
-                           2, 1, 10, 30, 13.0,
+                           3, 1, 10, 30, 13.0,
                            app1, cluster1);
 
         // Changing group size
         tester.activate(app1, cluster1, Capacity.from(resources(6, 3, 8, 25,  10),
                                                       resources(9, 3, 12, 35, 15)));
         tester.assertNodes("Groups changed",
-                           6, 3, 8, 30, 13,
+                           9, 3, 8, 30, 13,
                            app1, cluster1);
 
         // Stop specifying node resources
         tester.activate(app1, cluster1, Capacity.from(new ClusterResources(6, 3, NodeResources.unspecified),
                                                       new ClusterResources(9, 3, NodeResources.unspecified)));
         tester.assertNodes("No change",
-                           6, 3, 8, 30, 13,
+                           9, 3, 8, 30, 13,
                            app1, cluster1);
     }