Avoid reallocation when transitioning between content and combined

3 files changed, 49 insertions, 3 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
index e1a28e3f8d7..ea4411c03ab 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
@@ -111,8 +111,10 @@ public final class ClusterSpec {
      * are ignored.
      */
     public boolean satisfies(ClusterSpec other) {
-        return other.id.equals(this.id) &&
-               other.type.equals(this.type);
+        if (!other.id.equals(this.id)) return false; // ID mismatch
+        if (other.type.isContent() || this.type.isContent()) // Allow seamless transition between content and combined
+            return other.type.isContent() == this.type.isContent();
+        return other.type.equals(this.type);
     }
 
     /** A cluster type */
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java
index dc198e5bb0d..db90b0ebff9 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java
@@ -40,7 +40,7 @@ public class ClusterSpecTest {
                 List.of(spec(ClusterSpec.Type.admin, "id1"), spec(ClusterSpec.Type.container, "id1")), false,
                 List.of(spec(ClusterSpec.Type.admin, "id1"), spec(ClusterSpec.Type.content, "id1")), false,
                 List.of(spec(ClusterSpec.Type.combined, "id1"), spec(ClusterSpec.Type.container, "id1")), false,
-                List.of(spec(ClusterSpec.Type.combined, "id1"), spec(ClusterSpec.Type.content, "id1")), false,
+                List.of(spec(ClusterSpec.Type.combined, "id1"), spec(ClusterSpec.Type.content, "id1")), true,
                 List.of(spec(ClusterSpec.Type.content, "id1"), spec(ClusterSpec.Type.content, "id1")), true
         );
         tests.forEach((specs, satisfies) -> {
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 1ab94e852ba..a0f0cd26f59 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
@@ -671,6 +671,50 @@ public class ProvisioningTest {
                 assertEquals(version2, node.allocation().get().membership().cluster().vespaVersion()));
     }
 
+    @Test
+    public void change_to_and_from_combined_cluster_does_not_change_node_allocation() {
+        var tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
+        var application = tester.makeApplicationId();
+
+        tester.makeReadyNodes(4, defaultResources);
+
+        // Application allocates two content nodes initially, with cluster type content
+        ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content,
+                                                  ClusterSpec.Id.from("music"),
+                                                  Version.fromString("1.2.3"),
+                                                  false);
+        var initalNodes = tester.activate(application, tester.prepare(application, cluster,
+                                                                       Capacity.fromCount(2, defaultResources, false, false),
+                                                                       1));
+
+        // Application is redeployed with cluster type combined
+        cluster = ClusterSpec.request(ClusterSpec.Type.combined,
+                                      ClusterSpec.Id.from("music"),
+                                      Version.fromString("1.2.3"),
+                                      false);
+        var newNodes = tester.activate(application, tester.prepare(application, cluster,
+                                                                   Capacity.fromCount(2, defaultResources, false, false),
+                                                                   1));
+
+        assertEquals("Node allocation remains the same", initalNodes, newNodes);
+        assertEquals("Cluster type is updated",
+                     Set.of(ClusterSpec.Type.combined),
+                     newNodes.stream().map(n -> n.membership().get().cluster().type()).collect(Collectors.toSet()));
+
+        // Application is redeployed with cluster type content again
+        cluster = ClusterSpec.request(ClusterSpec.Type.content,
+                                      ClusterSpec.Id.from("music"),
+                                      Version.fromString("1.2.3"),
+                                      false);
+        newNodes = tester.activate(application, tester.prepare(application, cluster,
+                                                               Capacity.fromCount(2, defaultResources, false, false),
+                                                               1));
+        assertEquals("Node allocation remains the same", initalNodes, newNodes);
+        assertEquals("Cluster type is updated",
+                     Set.of(ClusterSpec.Type.content),
+                     newNodes.stream().map(n -> n.membership().get().cluster().type()).collect(Collectors.toSet()));
+    }
+
     private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size,
                                 int content1Size, NodeResources flavor, ProvisioningTester tester) {
         return prepare(application, container0Size, container1Size, content0Size, content1Size, flavor,