Ensure prod node count requirement is respected

4 files changed, 52 insertions, 22 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
index 3f392674b20..19489cf230f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
@@ -3,10 +3,10 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 
 import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.config.provision.Zone;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeFlavors;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
 
 import java.util.Optional;
 
@@ -26,14 +26,14 @@ public class CapacityPolicies {
     }
 
     public int decideSize(Capacity requestedCapacity) {
-        int requestedNodes = requestedCapacity.nodeCount();
+        int requestedNodes = ensureRedundancy(requestedCapacity.nodeCount());
         if (requestedCapacity.isRequired()) return requestedNodes;
 
         switch(zone.environment()) {
             case dev : case test : return 1;
             case perf : return Math.min(requestedCapacity.nodeCount(), 3);
             case staging: return requestedNodes <= 1 ? requestedNodes : Math.max(2, requestedNodes / 10);
-            case prod : return ensureRedundancy(requestedCapacity.nodeCount());
+            case prod : return requestedNodes;
             default : throw new IllegalArgumentException("Unsupported environment " + zone.environment());
         }
     }
@@ -55,13 +55,13 @@ public class CapacityPolicies {
     }
 
     /**
-     * Throw if the node count is 1
+     * Throw if the node count is 1 and we're in a production zone
      *
      * @return the argument node count
      * @throws IllegalArgumentException if only one node is requested
      */
     private int ensureRedundancy(int nodeCount) {
-        if (nodeCount == 1 && zone.system() != SystemName.cd) {
+        if (nodeCount == 1 && zone.environment().isProduction() && zone.system() != SystemName.cd) {
             throw new IllegalArgumentException("Deployments to prod require at least 2 nodes per cluster for redundancy");
         }
         return nodeCount;
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java
index 9587fedb7b2..cb2a752215f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/MultigroupProvisioningTest.java
@@ -94,7 +94,7 @@ public class MultigroupProvisioningTest {
 
     @Test
     public void test_one_node_and_group_to_two() {
-        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")));
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.perf, RegionName.from("us-east")));
 
         ApplicationId application1 = tester.makeApplicationId();
 
@@ -106,7 +106,7 @@ public class MultigroupProvisioningTest {
 
     @Test
     public void test_one_node_and_group_to_two_with_flavor_migration() {
-        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")));
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.perf, RegionName.from("us-east")));
 
         ApplicationId application1 = tester.makeApplicationId();
 
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 26e0e714201..801bee314ab 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
@@ -41,6 +41,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -728,6 +729,16 @@ public class ProvisioningTest {
         assertEquals(4, tester.getNodes(application, Node.State.active).size());
     }
 
+    @Test
+    public void required_capacity_respects_prod_redundancy_requirement() {
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")));
+        ApplicationId application = tester.makeApplicationId();
+        try {
+            prepare(application, 1, 0, 1, 0, true, "default", Version.fromString("6.42"), tester);
+            fail("Expected exception");
+        } catch (IllegalArgumentException ignored) {}
+    }
+
     private void assertCorrectFlavorPreferences(boolean largeIsStock) {
         FlavorConfigBuilder b = new FlavorConfigBuilder();
         b.addFlavor("large", 4., 8., 100, Flavor.Type.BARE_METAL).cost(10).stock(largeIsStock);
@@ -762,21 +773,31 @@ public class ProvisioningTest {
         tester.assertNumberOfNodesWithFlavor(contentNodes, "large-variant-variant", 3);
     }
 
-    private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size, int content1Size, String flavor, ProvisioningTester tester) {
-        return prepare(application, container0Size, container1Size, content0Size, content1Size, flavor, Version.fromString("6.42"), tester);
+    private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size,
+                                int content1Size, String flavor, ProvisioningTester tester) {
+        return prepare(application, container0Size, container1Size, content0Size, content1Size, flavor,
+                       Version.fromString("6.42"), tester);
+    }
+
+    private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size,
+                                int content1Size, String flavor, Version wantedVersion, ProvisioningTester tester) {
+        return prepare(application, container0Size, container1Size, content0Size, content1Size, false, flavor,
+                       wantedVersion, tester);
     }
 
-    private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size, int content1Size, String flavor, Version wantedVersion, ProvisioningTester tester) {
+    private SystemState prepare(ApplicationId application, int container0Size, int container1Size, int content0Size,
+                                int content1Size, boolean required, String flavor, Version wantedVersion,
+                                ProvisioningTester tester) {
         // "deploy prepare" with a two container clusters and a storage cluster having of two groups
         ClusterSpec containerCluster0 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container0"), wantedVersion);
         ClusterSpec containerCluster1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container1"), wantedVersion);
         ClusterSpec contentCluster0 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content0"), wantedVersion);
         ClusterSpec contentCluster1 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("content1"), wantedVersion);
 
-        Set<HostSpec> container0 = prepare(application, containerCluster0, container0Size, 1, flavor, tester);
-        Set<HostSpec> container1 = prepare(application, containerCluster1, container1Size, 1, flavor, tester);
-        Set<HostSpec> content0 = prepare(application, contentCluster0, content0Size, 1, flavor, tester);
-        Set<HostSpec> content1 = prepare(application, contentCluster1, content1Size, 1, flavor, tester);
+        Set<HostSpec> container0 = prepare(application, containerCluster0, container0Size, 1, required, flavor, tester);
+        Set<HostSpec> container1 = prepare(application, containerCluster1, container1Size, 1, required, flavor, tester);
+        Set<HostSpec> content0 = prepare(application, contentCluster0, content0Size, 1, required, flavor, tester);
+        Set<HostSpec> content1 = prepare(application, contentCluster1, content1Size, 1, required, flavor, tester);
 
         Set<HostSpec> allHosts = new HashSet<>();
         allHosts.addAll(container0);
@@ -784,10 +805,11 @@ public class ProvisioningTest {
         allHosts.addAll(content0);
         allHosts.addAll(content1);
 
-        int expectedContainer0Size = tester.capacityPolicies().decideSize(Capacity.fromNodeCount(container0Size));
-        int expectedContainer1Size = tester.capacityPolicies().decideSize(Capacity.fromNodeCount(container1Size));
-        int expectedContent0Size = tester.capacityPolicies().decideSize(Capacity.fromNodeCount(content0Size));
-        int expectedContent1Size = tester.capacityPolicies().decideSize(Capacity.fromNodeCount(content1Size));
+        Function<Integer, Capacity> capacity = required ? Capacity::fromRequiredNodeCount : Capacity::fromNodeCount;
+        int expectedContainer0Size = tester.capacityPolicies().decideSize(capacity.apply(container0Size));
+        int expectedContainer1Size = tester.capacityPolicies().decideSize(capacity.apply(container1Size));
+        int expectedContent0Size = tester.capacityPolicies().decideSize(capacity.apply(content0Size));
+        int expectedContent1Size = tester.capacityPolicies().decideSize(capacity.apply(content1Size));
 
         assertEquals("Hosts in each group cluster is disjunct and the total number of unretired nodes is correct",
                      expectedContainer0Size + expectedContainer1Size + expectedContent0Size + expectedContent1Size,
@@ -806,9 +828,10 @@ public class ProvisioningTest {
         return new SystemState(allHosts, container0, container1, content0, content1);
     }
 
-    private Set<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, String flavor, ProvisioningTester tester) {
+    private Set<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups,
+                                  boolean required, String flavor, ProvisioningTester tester) {
         if (nodeCount == 0) return Collections.emptySet(); // this is a shady practice
-        return new HashSet<>(tester.prepare(application, cluster, nodeCount, groups, flavor));
+        return new HashSet<>(tester.prepare(application, cluster, nodeCount, groups, required, flavor));
     }
 
     private static class SystemState {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
index edc44c7a135..0f9d06a69cc 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
@@ -128,7 +128,14 @@ public class ProvisioningTester {
     public void patchNode(Node node) { nodeRepository.write(node); }
 
     public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, String flavor) {
-        return prepare(application, cluster, Capacity.fromNodeCount(nodeCount, Optional.ofNullable(flavor)), groups);
+        return prepare(application, cluster, nodeCount, groups, false, flavor);
+    }
+
+    public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, int nodeCount, int groups, boolean required, String flavor) {
+        Capacity capacity = required
+                ? Capacity.fromRequiredNodeCount(nodeCount)
+                : Capacity.fromNodeCount(nodeCount, Optional.ofNullable(flavor));
+        return prepare(application, cluster, capacity, groups);
     }
 
     public List<HostSpec> prepare(ApplicationId application, ClusterSpec cluster, Capacity capacity, int groups) {