Allow more than one node in a cluster on same parent host in cd and non-prod

3 files changed, 52 insertions, 2 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index ade2c94a25c..e6334fcfac7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -75,6 +75,7 @@ public class NodeRepository extends AbstractComponent {
     private final CuratorDatabaseClient db;
     private final Curator curator;
     private final Clock clock;
+    private final Zone zone;
     private final NodeFlavors flavors;
     private final NameResolver nameResolver;
     private final DockerImage dockerImage;
@@ -96,6 +97,7 @@ public class NodeRepository extends AbstractComponent {
                           DockerImage dockerImage, boolean useCuratorClientCache) {
         this.db = new CuratorDatabaseClient(flavors, curator, clock, zone, useCuratorClientCache);
         this.curator = curator;
+        this.zone = zone;
         this.clock = clock;
         this.flavors = flavors;
         this.nameResolver = nameResolver;
@@ -661,6 +663,9 @@ public class NodeRepository extends AbstractComponent {
     /** Returns the time keeper of this system */
     public Clock clock() { return clock; }
 
+    /** Returns the zone of this system */
+    public Zone zone() { return zone; }
+
     /** Create a lock which provides exclusive rights to making changes to the given application */
     public Mutex lock(ApplicationId application) { return db.lock(application); }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index e459436756a..c2e2ea36c13 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.lang.MutableInteger;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -95,7 +96,7 @@ class NodeAllocation {
                 if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure)
 
                 // conditions on which we want to retire nodes that were allocated previously
-                if ( offeredNodeHasParentHostnameAlreadyAccepted(this.nodes, offered)) wantToRetireNode = true;
+                if ( violatesParentHostPolicy(this.nodes, offered)) wantToRetireNode = true;
                 if ( ! hasCompatibleFlavor(offered)) wantToRetireNode = true;
                 if ( offered.flavor().isRetired()) wantToRetireNode = true;
                 if ( offered.status().wantToRetire()) wantToRetireNode = true;
@@ -107,7 +108,7 @@ class NodeAllocation {
                 }
             }
             else if ( ! saturated() && hasCompatibleFlavor(offered)) {
-                if ( offeredNodeHasParentHostnameAlreadyAccepted(this.nodes, offered)) {
+                if ( violatesParentHostPolicy(this.nodes, offered)) {
                     ++rejectedWithClashingParentHost;
                     continue;
                 }
@@ -135,6 +136,15 @@ class NodeAllocation {
         return accepted;
     }
 
+
+    private boolean violatesParentHostPolicy(Collection<PrioritizableNode> accepted, Node offered) {
+        return checkForClashingParentHost() && offeredNodeHasParentHostnameAlreadyAccepted(accepted, offered);
+    }
+
+    private boolean checkForClashingParentHost() {
+        return nodeRepository.zone().system() == SystemName.main && nodeRepository.zone().environment().isProduction();
+    }
+
     private boolean offeredNodeHasParentHostnameAlreadyAccepted(Collection<PrioritizableNode> accepted, Node offered) {
         for (PrioritizableNode acceptedNode : accepted) {
             if (acceptedNode.node.parentHostname().isPresent() && offered.parentHostname().isPresent() &&
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java
index 9e505702270..c4af98dbbb6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java
@@ -8,6 +8,7 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.OutOfCapacityException;
 import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Before;
@@ -29,6 +30,8 @@ import static org.junit.Assert.assertNotNull;
  * @author hmusum
  * @author mpolden
  */
+// Note: Some of the tests here should be moved to DockerProvisioningTest if we stop using VMs and want
+// to remove these tests
 public class VirtualNodeProvisioningTest {
 
     private static final String flavor = "v-4-8-100";
@@ -77,6 +80,38 @@ public class VirtualNodeProvisioningTest {
     }
 
     @Test
+    public void allow_same_parent_host_for_nodes_in_a_cluster_in_cd_and_non_prod() {
+        final int containerNodeCount = 2;
+        final int contentNodeCount = 2;
+        final int groups = 1;
+
+        // Allowed to use same parent host for several nodes in same cluster in dev
+        {
+            tester = new ProvisioningTester(new Zone(Environment.dev, RegionName.from("us-east")));
+            tester.makeReadyVirtualNodes(4, "default", "parentHost1");
+
+            List<HostSpec> containerHosts = prepare(containerClusterSpec, containerNodeCount, groups);
+            List<HostSpec> contentHosts = prepare(contentClusterSpec, contentNodeCount, groups);
+            activate(containerHosts, contentHosts);
+
+            // downscaled to 1 node per cluster in dev, so 2 in total
+            assertEquals(2, getNodes(applicationId).size());
+        }
+
+        // Allowed to use same parent host for several nodes in same cluster in CD (even if prod env)
+        {
+            tester = new ProvisioningTester(new Zone(SystemName.cd, Environment.prod, RegionName.from("us-east")));
+            tester.makeReadyVirtualNodes(4, flavor, "parentHost1");
+
+            List<HostSpec> containerHosts = prepare(containerClusterSpec, containerNodeCount, groups);
+            List<HostSpec> contentHosts = prepare(contentClusterSpec, contentNodeCount, groups);
+            activate(containerHosts, contentHosts);
+
+            assertEquals(4, getNodes(applicationId).size());
+        }
+    }
+
+    @Test
     public void will_retire_clashing_active() {
         tester.makeReadyVirtualNodes(1, flavor, "parentHost1");
         tester.makeReadyVirtualNodes(1, flavor, "parentHost2");