Allow specifying min and max risk, and max idle hours, in deployment spec

4 files changed, 107 insertions, 58 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java
index 677933f3b85..9d90167a0ef 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java
@@ -1,6 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.application.api;
 
+import ai.vespa.validation.Validation;
+import com.yahoo.config.application.api.DeploymentSpec.RevisionTarget;
 import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
@@ -20,6 +22,11 @@ import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static ai.vespa.validation.Validation.require;
+import static ai.vespa.validation.Validation.requireAtLeast;
+import static ai.vespa.validation.Validation.requireInRange;
+import static com.yahoo.config.application.api.DeploymentSpec.RevisionChange.whenClear;
+import static com.yahoo.config.application.api.DeploymentSpec.RevisionTarget.next;
 import static com.yahoo.config.provision.Environment.prod;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
@@ -41,6 +48,9 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
     private final DeploymentSpec.RevisionTarget revisionTarget;
     private final DeploymentSpec.RevisionChange revisionChange;
     private final DeploymentSpec.UpgradeRollout upgradeRollout;
+    private final int minRisk;
+    private final int maxRisk;
+    private final int maxIdleHours;
     private final List<DeploymentSpec.ChangeBlocker> changeBlockers;
     private final Optional<String> globalServiceId;
     private final Optional<AthenzService> athenzService;
@@ -53,6 +63,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
                                   DeploymentSpec.RevisionTarget revisionTarget,
                                   DeploymentSpec.RevisionChange revisionChange,
                                   DeploymentSpec.UpgradeRollout upgradeRollout,
+                                  int minRisk, int maxRisk, int maxIdleHours,
                                   List<DeploymentSpec.ChangeBlocker> changeBlockers,
                                   Optional<String> globalServiceId,
                                   Optional<AthenzService> athenzService,
@@ -62,9 +73,14 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
         super(steps);
         this.name = name;
         this.upgradePolicy = upgradePolicy;
-        this.revisionTarget = revisionTarget;
-        this.revisionChange = revisionChange;
+        this.revisionTarget = require(maxRisk == 0 || revisionTarget == next, revisionTarget,
+                                      "revision-target must be 'next' when max-risk is specified");
+        this.revisionChange = require(maxRisk == 0 || revisionChange == whenClear, revisionChange,
+                                      "revision-change must be 'when-clear' when max-risk is specified");
         this.upgradeRollout = upgradeRollout;
+        this.minRisk = requireAtLeast(minRisk, "minimum risk score", 0);
+        this.maxRisk = require(maxRisk >= minRisk, maxRisk, "maximum risk cannot be less than minimum risk score");
+        this.maxIdleHours = requireInRange(maxIdleHours, "maximum idle hours", 0, 168);
         this.changeBlockers = changeBlockers;
         this.globalServiceId = globalServiceId;
         this.athenzService = athenzService;
@@ -151,7 +167,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
         instant = instant.truncatedTo(ChronoUnit.HOURS);
         Duration step = Duration.ofHours(1);
         Duration max = Duration.ofDays(maxUpgradeBlockingDays);
-        for (Instant current = instant; !canUpgradeAt(current); current = current.plus(step)) {
+        for (Instant current = instant; ! canUpgradeAt(current); current = current.plus(step)) {
             Duration blocked = Duration.between(instant, current);
             if (blocked.compareTo(max) > 0) {
                 return false;
@@ -172,6 +188,15 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
     /** Returns the upgrade rollout strategy of this, which is {@link DeploymentSpec.UpgradeRollout#separate} by default */
     public DeploymentSpec.UpgradeRollout upgradeRollout() { return upgradeRollout; }
 
+    /** Minimum cumulative, enqueued risk required for a new revision to roll out to this instance. 0 by default. */
+    public int minRisk() { return minRisk; }
+
+    /** Maximum cumulative risk that will automatically roll out to this instance, as long as this is possible. 0 by default. */
+    public int maxRisk() { return maxRisk; }
+
+    /* Maximum number of hours to wait for enqueued risk to reach the minimum, before rolling out whatever revisions are enqueued. 8 by default. */
+    public int maxIdleHours() { return maxIdleHours; }
+
     /** Returns time windows where upgrades are disallowed for these instances */
     public List<DeploymentSpec.ChangeBlocker> changeBlocker() { return changeBlockers; }
 
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 9135e9f49ff..e5ea65b6d4e 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
@@ -566,9 +566,9 @@ public class DeploymentSpec {
 
     /** Determines what application changes to deploy to the instance. */
     public enum RevisionTarget {
-        /** Next: Application changes are rolled through this instance in the same manner as they become ready. */
+        /** Next: Application changes are rolled through this instance in the same manner as they become ready, optionally adjusted further by min and max risk settings. */
         next,
-        /** Latest: Application changes are merged, so the latest available is always chosen for roll-out. */
+        /** Latest: Application changes are always merged, so the latest available is always chosen for roll-out. */
         latest
     }
 
@@ -594,7 +594,6 @@ public class DeploymentSpec {
         simultaneous
     }
 
-
     /** A blocking of changes in a given time window */
     public static class ChangeBlocker {
         
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
index 1915aa66d4c..ddb2a53b767 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.application.api.xml;
 
+import ai.vespa.validation.Validation;
 import com.yahoo.config.application.api.DeploymentInstanceSpec;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.DeploymentSpec.DeclaredTest;
@@ -8,8 +9,12 @@ import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone;
 import com.yahoo.config.application.api.DeploymentSpec.Delay;
 import com.yahoo.config.application.api.DeploymentSpec.DeprecatedElement;
 import com.yahoo.config.application.api.DeploymentSpec.ParallelSteps;
+import com.yahoo.config.application.api.DeploymentSpec.RevisionChange;
+import com.yahoo.config.application.api.DeploymentSpec.RevisionTarget;
 import com.yahoo.config.application.api.DeploymentSpec.Step;
 import com.yahoo.config.application.api.DeploymentSpec.Steps;
+import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy;
+import com.yahoo.config.application.api.DeploymentSpec.UpgradeRollout;
 import com.yahoo.config.application.api.Endpoint;
 import com.yahoo.config.application.api.Notifications;
 import com.yahoo.config.application.api.Notifications.Role;
@@ -39,6 +44,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -164,10 +170,13 @@ public class DeploymentSpecXmlReader {
             validateTagOrder(instanceTag);
 
         // Values where the parent may provide a default
-        DeploymentSpec.UpgradePolicy upgradePolicy = readUpgradePolicy(instanceTag, parentTag);
-        DeploymentSpec.RevisionTarget revisionTarget = readRevisionTarget(instanceTag, parentTag);
-        DeploymentSpec.RevisionChange revisionChange = readRevisionChange(instanceTag, parentTag);
-        DeploymentSpec.UpgradeRollout upgradeRollout = readUpgradeRollout(instanceTag, parentTag);
+        DeploymentSpec.UpgradePolicy upgradePolicy = getWithFallback(instanceTag, parentTag, upgradeTag, "policy", this::readUpgradePolicy, UpgradePolicy.defaultPolicy);
+        DeploymentSpec.RevisionTarget revisionTarget = getWithFallback(instanceTag, parentTag, upgradeTag, "revision-target", this::readRevisionTarget, RevisionTarget.latest);
+        DeploymentSpec.RevisionChange revisionChange = getWithFallback(instanceTag, parentTag, upgradeTag, "revision-change", this::readRevisionChange, RevisionChange.whenFailing);
+        DeploymentSpec.UpgradeRollout upgradeRollout = getWithFallback(instanceTag, parentTag, upgradeTag, "rollout", this::readUpgradeRollout, UpgradeRollout.separate);
+        int minRisk = getWithFallback(instanceTag, parentTag, upgradeTag, "min-risk", Integer::parseInt, 0);
+        int maxRisk = getWithFallback(instanceTag, parentTag, upgradeTag, "max-risk", Integer::parseInt, 0);
+        int maxIdleHours = getWithFallback(instanceTag, parentTag, upgradeTag, "max-idle-hours", Integer::parseInt, 8);
         List<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceTag, parentTag);
         Optional<AthenzService> athenzService = mostSpecificAttribute(instanceTag, athenzServiceAttribute).map(AthenzService::from);
         Notifications notifications = readNotifications(instanceTag, parentTag);
@@ -188,6 +197,7 @@ public class DeploymentSpecXmlReader {
                                                              revisionTarget,
                                                              revisionChange,
                                                              upgradeRollout,
+                                                             minRisk, maxRisk, maxIdleHours,
                                                              changeBlockers,
                                                              globalServiceId.asOptional(),
                                                              athenzService,
@@ -456,82 +466,51 @@ public class DeploymentSpecXmlReader {
         return value == null || value.isEmpty() || value.equals("true");
     }
 
-    private DeploymentSpec.UpgradePolicy readUpgradePolicy(Element parent, Element fallbackParent) {
-        Element upgradeElement = XML.getChild(parent, upgradeTag);
-        if (upgradeElement == null)
-            upgradeElement = XML.getChild(fallbackParent, upgradeTag);
-        if (upgradeElement == null)
-            return DeploymentSpec.UpgradePolicy.defaultPolicy;
-
-        String policy = upgradeElement.getAttribute("policy");
-        if (policy.isEmpty())
-            return DeploymentSpec.UpgradePolicy.defaultPolicy;
+    private <T> T getWithFallback(Element parent, Element fallbackParent, String tagName, String attributeName,
+                                  Function<String, T> mapper, T fallbackValue) {
+        Element element = XML.getChild(parent, tagName);
+        if (element == null) element = XML.getChild(fallbackParent, tagName);
+        if (element == null) return fallbackValue;
+        String attribute = element.getAttribute(attributeName);
+        return attribute.isBlank() ? fallbackValue : mapper.apply(attribute);
+    }
 
+    private DeploymentSpec.UpgradePolicy readUpgradePolicy(String policy) {
         switch (policy) {
             case "canary": return DeploymentSpec.UpgradePolicy.canary;
             case "default": return DeploymentSpec.UpgradePolicy.defaultPolicy;
             case "conservative": return DeploymentSpec.UpgradePolicy.conservative;
             default: throw new IllegalArgumentException("Illegal upgrade policy '" + policy + "': " +
-                                                        "Must be one of " + Arrays.toString(DeploymentSpec.UpgradePolicy.values()));
+                                                        "Must be one of 'canary', 'default', 'conservative'");
         }
     }
 
-    private DeploymentSpec.RevisionChange readRevisionChange(Element parent, Element fallbackParent) {
-        Element upgradeElement = XML.getChild(parent, upgradeTag);
-        if (upgradeElement == null)
-            upgradeElement = XML.getChild(fallbackParent, upgradeTag);
-        if (upgradeElement == null)
-            return DeploymentSpec.RevisionChange.whenFailing;
-
-        String revision = upgradeElement.getAttribute("revision-change");
-        if (revision.isEmpty())
-            return DeploymentSpec.RevisionChange.whenFailing;
-
+    private DeploymentSpec.RevisionChange readRevisionChange(String revision) {
         switch (revision) {
             case "when-clear": return DeploymentSpec.RevisionChange.whenClear;
             case "when-failing": return DeploymentSpec.RevisionChange.whenFailing;
             case "always": return DeploymentSpec.RevisionChange.always;
             default: throw new IllegalArgumentException("Illegal upgrade revision change policy '" + revision + "': " +
-                                                        "Must be one of " + Arrays.toString(DeploymentSpec.RevisionTarget.values()));
+                                                        "Must be one of 'always', 'when-failing', 'when-clear'");
         }
     }
 
-    private DeploymentSpec.RevisionTarget readRevisionTarget(Element parent, Element fallbackParent) {
-        Element upgradeElement = XML.getChild(parent, upgradeTag);
-        if (upgradeElement == null)
-            upgradeElement = XML.getChild(fallbackParent, upgradeTag);
-        if (upgradeElement == null)
-            return DeploymentSpec.RevisionTarget.latest;
-
-        String revision = upgradeElement.getAttribute("revision-target");
-        if (revision.isEmpty())
-            return DeploymentSpec.RevisionTarget.latest;
-
+    private DeploymentSpec.RevisionTarget readRevisionTarget(String revision) {
         switch (revision) {
             case "next": return DeploymentSpec.RevisionTarget.next;
             case "latest": return DeploymentSpec.RevisionTarget.latest;
             default: throw new IllegalArgumentException("Illegal upgrade revision target '" + revision + "': " +
-                                                        "Must be one of " + Arrays.toString(DeploymentSpec.RevisionTarget.values()));
+                                                        "Must be one of 'next', 'latest'");
         }
     }
 
-    private DeploymentSpec.UpgradeRollout readUpgradeRollout(Element parent, Element fallbackParent) {
-        Element upgradeElement = XML.getChild(parent, upgradeTag);
-        if (upgradeElement == null)
-            upgradeElement = XML.getChild(fallbackParent, upgradeTag);
-        if (upgradeElement == null)
-            return DeploymentSpec.UpgradeRollout.separate;
-
-        String rollout = upgradeElement.getAttribute("rollout");
-        if (rollout.isEmpty())
-            return DeploymentSpec.UpgradeRollout.separate;
-
+    private DeploymentSpec.UpgradeRollout readUpgradeRollout(String rollout) {
         switch (rollout) {
             case "separate": return DeploymentSpec.UpgradeRollout.separate;
             case "leading": return DeploymentSpec.UpgradeRollout.leading;
             case "simultaneous": return DeploymentSpec.UpgradeRollout.simultaneous;
             default: throw new IllegalArgumentException("Illegal upgrade rollout '" + rollout + "': " +
-                                                        "Must be one of " + Arrays.toString(DeploymentSpec.UpgradeRollout.values()));
+                                                        "Must be one of 'separate', 'leading', 'simultaneous'");
         }
     }
 
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 5efa49b3656..5073c6b9fb2 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
@@ -26,6 +26,7 @@ import static com.yahoo.config.application.api.Notifications.When.failingCommit;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -125,6 +126,9 @@ public class DeploymentSpecTest {
         assertEquals(DeploymentSpec.RevisionTarget.latest, spec.requireInstance("default").revisionTarget());
         assertEquals(DeploymentSpec.RevisionChange.whenFailing, spec.requireInstance("default").revisionChange());
         assertEquals(DeploymentSpec.UpgradeRollout.separate, spec.requireInstance("default").upgradeRollout());
+        assertEquals(0, spec.requireInstance("default").minRisk());
+        assertEquals(0, spec.requireInstance("default").maxRisk());
+        assertEquals(8, spec.requireInstance("default").maxIdleHours());
     }
 
     @Test
@@ -371,7 +375,7 @@ public class DeploymentSpecTest {
         StringReader r = new StringReader(
                 "<deployment>" +
                 "   <instance id='default'>" +
-                "      <upgrade revision-change='when-clear' revision-target='next' />" +
+                "      <upgrade revision-change='when-clear' revision-target='next' min-risk='3' max-risk='12' max-idle-hours='32' />" +
                 "   </instance>" +
                 "   <instance id='custom'>" +
                 "      <upgrade revision-change='always' />" +
@@ -383,6 +387,48 @@ public class DeploymentSpecTest {
         assertEquals("latest", spec.requireInstance("custom").revisionTarget().toString());
         assertEquals("whenClear", spec.requireInstance("default").revisionChange().toString());
         assertEquals("always", spec.requireInstance("custom").revisionChange().toString());
+        assertEquals(3, spec.requireInstance("default").minRisk());
+        assertEquals(12, spec.requireInstance("default").maxRisk());
+        assertEquals(32, spec.requireInstance("default").maxIdleHours());
+    }
+
+    @Test
+    public void productionSpecsWithIllegalRevisionSettings() {
+        assertEquals("revision-change must be 'when-clear' when max-risk is specified, but got: 'always'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("<deployment>" +
+                                                               "   <instance id='default'>" +
+                                                               "      <upgrade revision-change='always' revision-target='next' min-risk='3' max-risk='12' max-idle-hours='32' />" +
+                                                               "   </instance>" +
+                                                               "</deployment>"))
+                             .getMessage());
+
+        assertEquals("revision-target must be 'next' when max-risk is specified, but got: 'latest'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("<deployment>" +
+                                                               "   <instance id='default'>" +
+                                                               "      <upgrade revision-change='when-clear' min-risk='3' max-risk='12' max-idle-hours='32' />" +
+                                                               "   </instance>" +
+                                                               "</deployment>"))
+                             .getMessage());
+
+        assertEquals("maximum risk cannot be less than minimum risk score, but got: '12'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("<deployment>" +
+                                                               "   <instance id='default'>" +
+                                                               "      <upgrade revision-change='when-clear' revision-target='next' min-risk='13' max-risk='12' max-idle-hours='32' />" +
+                                                               "   </instance>" +
+                                                               "</deployment>"))
+                             .getMessage());
+
+        assertEquals("maximum risk cannot be less than minimum risk score, but got: '0'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("<deployment>" +
+                                                               "   <instance id='default'>" +
+                                                               "      <upgrade min-risk='3' />" +
+                                                               "   </instance>" +
+                                                               "</deployment>"))
+                             .getMessage());
     }
 
     @Test