Always use configured disk settings

5 files changed, 39 insertions, 6 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java
index a4ed22c5266..87b5133d4eb 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterResources.java
@@ -44,10 +44,12 @@ public class ClusterResources {
         return false;
     }
 
-    /** Returns true if this is within the given limits (inclusive) */
+    /** Returns true if this is within the given limits (inclusive) and is compatible with them */
     public boolean isWithin(ClusterResources min, ClusterResources max) {
         if (this.smallerThan(min)) return false;
         if (max.smallerThan(this)) return false;
+        if ( ! this.nodeResources.justNonNumbers().compatibleWith(min.nodeResources.justNonNumbers())) return false;
+        if ( ! this.nodeResources.justNonNumbers().compatibleWith(max.nodeResources.justNonNumbers())) return false;
         return true;
     }
 
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java
index 5fc05a87a7d..05b604b263f 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java
@@ -148,6 +148,11 @@ public class NodeResources {
         return with(NodeResources.DiskSpeed.any).with(StorageType.any);
     }
 
+    /** Returns this with all numbers set to 0 */
+    public NodeResources justNonNumbers() {
+        return withVcpu(0).withMemoryGb(0).withDiskGb(0).withBandwidthGbps(0);
+    }
+
     public NodeResources subtract(NodeResources other) {
         if ( ! this.isInterchangeableWith(other))
             throw new IllegalArgumentException(this + " and " + other + " are not interchangeable");
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
index bdae658f76e..93e5b160524 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
@@ -50,6 +50,11 @@ public class NodeList implements Iterable<Node> {
         return filter(node -> node.allocation().get().membership().retired());
     }
 
+    /** Returns the subset of nodes which are removable */
+    public NodeList removable() {
+        return filter(node -> node.allocation().get().isRemovable());
+    }
+
     /** Returns the subset of nodes having exactly the given resources */
     public NodeList resources(NodeResources resources) { return filter(node -> node.flavor().resources().equals(resources)); }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
index d03aa0cac91..37d0e9bbfb8 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
@@ -161,12 +161,22 @@ public class NodeRepositoryProvisioner implements Provisioner {
                                                         ClusterSpec.Id clusterId,
                                                         Capacity requested) {
         List<Node> nodes = NodeList.copyOf(nodeRepository.getNodes(applicationId, Node.State.active))
-                                   .cluster(clusterId).not().retired().asList();
-        if (nodes.size() < 1) return Optional.empty();
+                                   .cluster(clusterId)
+                                   .not().retired()
+                                   .not().removable()
+                                   .asList();
+        if (nodes.isEmpty()) return Optional.empty();
         long groups = nodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count();
-        var resources = new ClusterResources(nodes.size(), (int)groups, nodes.get(0).flavor().resources());
-        if ( ! resources.isWithin(requested.minResources(), requested.maxResources())) return Optional.empty();
-        return Optional.of(resources);
+
+        // To allow non-numeric settings to be updated without resetting to the min target, we need to use
+        // the non-numeric settings of the current min limit with the current numeric settings
+        NodeResources nodeResources = nodes.get(0).allocation().get().requestedResources()
+                                           .with(requested.minResources().nodeResources().diskSpeed())
+                                           .with(requested.maxResources().nodeResources().storageType());
+        var currentResources = new ClusterResources(nodes.size(), (int)groups, nodeResources);
+        if ( ! currentResources.isWithin(requested.minResources(), requested.maxResources())) return Optional.empty();
+
+        return Optional.of(currentResources);
     }
 
     private void logIfDownscaled(int targetNodes, int actualNodes, ClusterSpec cluster, ProvisionLogger logger) {
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 bd8be5063fd..8fc73d68785 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
@@ -354,6 +354,17 @@ public class ProvisioningTest {
             assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == NodeResources.DiskSpeed.fast));
             assertTrue(tester.nodeRepository().getNodes(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.fast));
         }
+
+        {
+            // Go back to any
+            SystemState state = prepare(application, 0, 0, 5, 3,
+                                        defaultResources.justNumbers(),
+                                        tester);
+            assertEquals(8, state.allHosts.size());
+            tester.activate(application, state.allHosts);
+            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == NodeResources.DiskSpeed.any));
+            assertTrue(tester.nodeRepository().getNodes(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.any));
+        }
     }
 
     @Test