Merge pull request #11770 from vespa-engine/mpolden/restore-content-combined-substition

Revert "Remove unnecessary content/combined substitution"

3 files changed, 34 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 16369d82f9f..f1a8b579866 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
@@ -90,8 +90,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
+        // TODO(mpolden): Remove this after January 2019, once all nodes in combined clusters have type combined.
+        if (other.type.isContent() || this.type.isContent()) 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 a54afd1c229..b4e8b2266fb 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
@@ -625,6 +625,35 @@ public class ProvisioningTest {
         tester.activate(application, state.allHosts);
     }
 
+    @Test
+    public void change_to_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. This is the old behaviour where combined clusters has type
+        // content
+        ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content,
+                                                  ClusterSpec.Id.from("combined"),
+                                                  Version.fromString("1.2.3"),
+                                                  false);
+        var initialNodes = 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("combined"),
+                                      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", initialNodes, newNodes);
+    }
+
     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,