Validate that instances have increasing conservativeness

2 files changed, 88 insertions, 3 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
index 4a94dd076af..7b4e91bfe4a 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.application.api;
 
+import com.yahoo.collections.Comparables;
 import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
@@ -72,6 +73,7 @@ public class DeploymentSpec {
         this.athenzService = athenzService;
         this.xmlForm = xmlForm;
         validateTotalDelay(steps);
+        validateUpgradePoliciesOfIncreasingConservativeness(steps);
     }
 
     // TODO: Remove after October 2019
@@ -147,6 +149,23 @@ public class DeploymentSpec {
                                                " but max 24 hours is allowed");
     }
 
+    /** Throws an IllegalArgumentException if any instance has a looser upgrade policy than the previous */
+    private void validateUpgradePoliciesOfIncreasingConservativeness(List<Step> steps) {
+        UpgradePolicy previous = Collections.min(List.of(UpgradePolicy.values()));
+        for (Step step : steps) {
+            UpgradePolicy strictest = previous;
+            List<DeploymentInstanceSpec> specs = instances(List.of(step));
+            for (DeploymentInstanceSpec spec : specs) {
+                if (spec.upgradePolicy().compareTo(previous) < 0)
+                    throw new IllegalArgumentException("Instance '" + spec.name() + "' cannot have a looser upgrade " +
+                                                       "policy than the previous of '" + previous + "'");
+
+                strictest = Comparables.max(strictest, spec.upgradePolicy());
+            }
+            previous = strictest;
+        }
+    }
+
     // TODO: Remove after October 2019
     private DeploymentInstanceSpec singleInstance() {
         return singleInstance(steps);
diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
index 584e7d406de..080982cb785 100644
--- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
+++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
@@ -310,16 +310,16 @@ public class DeploymentSpecTest {
                 "<deployment version='1.0'>" +
                 "   <upgrade policy='canary'/>" +
                 "   <instance id='instance1'>" +
-                "      <upgrade policy='conservative'/>" +
                 "   </instance>" +
                 "   <instance id='instance2'>" +
+                "      <upgrade policy='conservative'/>" +
                 "   </instance>" +
                 "</deployment>"
         );
 
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
-        assertEquals("conservative", spec.requireInstance("instance1").upgradePolicy().toString());
-        assertEquals("canary", spec.requireInstance("instance2").upgradePolicy().toString());
+        assertEquals("canary", spec.requireInstance("instance1").upgradePolicy().toString());
+        assertEquals("conservative", spec.requireInstance("instance2").upgradePolicy().toString());
     }
 
     @Test
@@ -501,6 +501,72 @@ public class DeploymentSpecTest {
     }
 
     @Test(expected = IllegalArgumentException.class)
+    public void deploymentSpecWithIncreasinglyStrictUpgradePolicies() {
+        StringReader r = new StringReader(
+                "<deployment version='1.0'>" +
+                "   <instance id='instance1'>" +
+                "      <upgrade policy='conservative'/>" +
+                "   </instance>" +
+                "   <instance id='instance2' />" +
+                "</deployment>"
+        );
+        DeploymentSpec.fromXml(r);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void deploymentSpecWithIncreasinglyStrictUpgradePoliciesInParallel() {
+        StringReader r = new StringReader(
+                "<deployment version='1.0'>" +
+                "  <instance />" +
+                "  <parallel>" +
+                "     <instance id='instance1'>" +
+                "        <upgrade policy='conservative'/>" +
+                "     </instance>" +
+                "     <instance id='instance2'>" +
+                "        <upgrade policy='canary'/>" +
+                "     </instance>" +
+                "  </parallel>" +
+                "</deployment>"
+        );
+        DeploymentSpec.fromXml(r);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void deploymentSpecWithIncreasinglyStrictUpgradePoliciesAfterParallel() {
+        StringReader r = new StringReader(
+                "<deployment version='1.0'>" +
+                "  <parallel>" +
+                "     <instance id='instance1'>" +
+                "        <upgrade policy='conservative'/>" +
+                "     </instance>" +
+                "     <instance id='instance2'>" +
+                "        <upgrade policy='canary'/>" +
+                "     </instance>" +
+                "  </parallel>" +
+                "  <instance />" +
+                "</deployment>"
+        );
+        DeploymentSpec.fromXml(r);
+    }
+
+    @Test
+    public void deploymentSpecWithDifferentUpgradePoliciesInParallel() {
+        StringReader r = new StringReader(
+                "<deployment version='1.0'>" +
+                "  <parallel>" +
+                "     <instance id='instance1'>" +
+                "        <upgrade policy='conservative'/>" +
+                "     </instance>" +
+                "     <instance id='instance2' />" +
+                "  </parallel>" +
+                "</deployment>"
+        );
+        DeploymentSpec spec = DeploymentSpec.fromXml(r);
+        assertEquals(DeploymentSpec.UpgradePolicy.conservative, spec.requireInstance("instance1").upgradePolicy());
+        assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("instance2").upgradePolicy());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
     public void deploymentSpecWithIllegallyOrderedDeploymentSpec1() {
         StringReader r = new StringReader(
                 "<deployment>" +