From d360cfdd5dc6dc5c6475c17dc6b961786e6b7c34 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 19 Aug 2021 11:49:10 +0200 Subject: Allow configuring upgrade rollout in deployment.xml --- config-model-api/abi-spec.json | 20 ++++++++++++- .../application/api/DeploymentInstanceSpec.java | 9 +++++- .../config/application/api/DeploymentSpec.java | 12 ++++++++ .../api/xml/DeploymentSpecXmlReader.java | 23 +++++++++++++-- .../config/application/api/DeploymentSpecTest.java | 33 +++++++++++++++------- .../api/DeploymentSpecWithoutInstanceTest.java | 18 ++++++++---- 6 files changed, 95 insertions(+), 20 deletions(-) (limited to 'config-model-api') diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 5561c400745..49290618692 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -193,9 +193,10 @@ "public" ], "methods": [ - "public void (com.yahoo.config.provision.InstanceName, java.util.List, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.List, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)", + "public void (com.yahoo.config.provision.InstanceName, java.util.List, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout, java.util.List, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)", "public com.yahoo.config.provision.InstanceName name()", "public com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy upgradePolicy()", + "public com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout upgradeRollout()", "public java.util.List changeBlocker()", "public java.util.Optional globalServiceId()", "public boolean canUpgradeAt(java.time.Instant)", @@ -350,6 +351,23 @@ "public static final enum com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy conservative" ] }, + "com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout": { + "superClass": "java.lang.Enum", + "interfaces": [], + "attributes": [ + "public", + "final", + "enum" + ], + "methods": [ + "public static com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout[] values()", + "public static com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout valueOf(java.lang.String)" + ], + "fields": [ + "public static final enum com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout separate", + "public static final enum com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout leading" + ] + }, "com.yahoo.config.application.api.DeploymentSpec": { "superClass": "java.lang.Object", "interfaces": [], 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 8813eaf9c8c..cd051ecda7b 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 @@ -27,6 +27,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { private final InstanceName name; private final DeploymentSpec.UpgradePolicy upgradePolicy; + private final DeploymentSpec.UpgradeRollout upgradeRollout; private final List changeBlockers; private final Optional globalServiceId; private final Optional athenzService; @@ -36,6 +37,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { public DeploymentInstanceSpec(InstanceName name, List steps, DeploymentSpec.UpgradePolicy upgradePolicy, + DeploymentSpec.UpgradeRollout upgradeRollout, List changeBlockers, Optional globalServiceId, Optional athenzService, @@ -44,6 +46,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { super(steps); this.name = name; this.upgradePolicy = upgradePolicy; + this.upgradeRollout = upgradeRollout; this.changeBlockers = changeBlockers; this.globalServiceId = globalServiceId; this.athenzService = athenzService; @@ -136,6 +139,9 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { /** Returns the upgrade policy of this, which is defaultPolicy if none is specified */ public DeploymentSpec.UpgradePolicy upgradePolicy() { return upgradePolicy; } + /** Returns the upgrade rollout strategy of this, which is separate if none is specified */ + public DeploymentSpec.UpgradeRollout upgradeRollout() { return upgradeRollout; } + /** Returns time windows where upgrades are disallowed for these instances */ public List changeBlocker() { return changeBlockers; } @@ -181,6 +187,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { DeploymentInstanceSpec other = (DeploymentInstanceSpec) o; return globalServiceId.equals(other.globalServiceId) && upgradePolicy == other.upgradePolicy && + upgradeRollout == other.upgradeRollout && changeBlockers.equals(other.changeBlockers) && steps().equals(other.steps()) && athenzService.equals(other.athenzService) && @@ -190,7 +197,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { @Override public int hashCode() { - return Objects.hash(globalServiceId, upgradePolicy, changeBlockers, steps(), athenzService, notifications, endpoints); + return Objects.hash(globalServiceId, upgradePolicy, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints); } @Override 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 24d83e2b7b1..7305f794965 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 @@ -510,6 +510,18 @@ public class DeploymentSpec { conservative } + + /** Determines when application changes deploy, when there is already an ongoing platform upgrade. */ + public enum UpgradeRollout { + /** Separate: Application changes wait for upgrade to complete, unless upgrade fails. */ + separate, + /** Leading: Application changes are allowed to start and catch up to the platform upgrade. */ + leading + // /** Simultaneous: Application changes deploy independently of platform upgrades. */ + // 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 4bd819b3b6a..88c366e5d96 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 @@ -149,6 +149,7 @@ public class DeploymentSpecXmlReader { // Values where the parent may provide a default DeploymentSpec.UpgradePolicy upgradePolicy = readUpgradePolicy(instanceTag, parentTag); + DeploymentSpec.UpgradeRollout upgradeRollout = readUpgradeRollout(instanceTag, parentTag); List changeBlockers = readChangeBlockers(instanceTag, parentTag); Optional athenzService = mostSpecificAttribute(instanceTag, athenzServiceAttribute).map(AthenzService::from); Notifications notifications = readNotifications(instanceTag, parentTag); @@ -165,6 +166,7 @@ public class DeploymentSpecXmlReader { .map(name -> new DeploymentInstanceSpec(InstanceName.from(name), steps, upgradePolicy, + upgradeRollout, changeBlockers, globalServiceId.asOptional(), athenzService, @@ -397,13 +399,13 @@ public class DeploymentSpecXmlReader { } private DeploymentSpec.UpgradePolicy readUpgradePolicy(Element parent, Element fallbackParent) { + String policy; Element upgradeElement = XML.getChild(parent, upgradeTag); if (upgradeElement == null) upgradeElement = XML.getChild(fallbackParent, upgradeTag); - if (upgradeElement == null) + if (upgradeElement == null || (policy = upgradeElement.getAttribute("policy")).isEmpty()) return DeploymentSpec.UpgradePolicy.defaultPolicy; - String policy = upgradeElement.getAttribute("policy"); switch (policy) { case "canary": return DeploymentSpec.UpgradePolicy.canary; case "default": return DeploymentSpec.UpgradePolicy.defaultPolicy; @@ -413,6 +415,23 @@ public class DeploymentSpecXmlReader { } } + private DeploymentSpec.UpgradeRollout readUpgradeRollout(Element parent, Element fallbackParent) { + String rollout; + Element upgradeElement = XML.getChild(parent, upgradeTag); + if (upgradeElement == null) + upgradeElement = XML.getChild(fallbackParent, upgradeTag); + if (upgradeElement == null || (rollout = upgradeElement.getAttribute("rollout")).isEmpty()) + return DeploymentSpec.UpgradeRollout.separate; + + switch (rollout) { + case "separate": return DeploymentSpec.UpgradeRollout.separate; + case "leading": return DeploymentSpec.UpgradeRollout.leading; + // case "simultaneous": return DeploymentSpec.UpgradePolicy.conservative; + default: throw new IllegalArgumentException("Illegal upgrade policy '" + rollout + "': " + + "Must be one of " + Arrays.toString(DeploymentSpec.UpgradePolicy.values())); + } + } + private boolean readActive(Element regionTag) { String activeValue = regionTag.getAttribute("active"); if ("true".equals(activeValue)) return true; 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 5561ebdef63..c5f07444ead 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 @@ -118,6 +118,7 @@ public class DeploymentSpecTest { assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); + assertEquals(DeploymentSpec.UpgradeRollout.separate, spec.requireInstance("default").upgradeRollout()); } @Test @@ -359,34 +360,44 @@ public class DeploymentSpecTest { assertEquals("qrs", spec.requireInstance("default").globalServiceId().get()); } + @Test + public void productionSpecWithUpgradeRollout() { + StringReader r = new StringReader( + "" + + " " + + " " + + " " + + " " + + "" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals("leading", spec.requireInstance("default").upgradeRollout().toString()); + assertEquals("separate", spec.requireInstance("custom").upgradeRollout().toString()); + } + @Test public void productionSpecWithUpgradePolicy() { StringReader r = new StringReader( "" + " " + " " + - " " + - " us-west-1" + - " us-central-1" + - " us-east-3" + - " " + " " + + " " + "" ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("canary", spec.requireInstance("default").upgradePolicy().toString()); + assertEquals("defaultPolicy", spec.requireInstance("custom").upgradePolicy().toString()); } @Test public void upgradePolicyDefault() { StringReader r = new StringReader( "" + - " " + - " " + - " " + + " " + + " " + " " + - " " + + " " + " " + "" ); @@ -394,6 +405,8 @@ public class DeploymentSpecTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("canary", spec.requireInstance("instance1").upgradePolicy().toString()); assertEquals("conservative", spec.requireInstance("instance2").upgradePolicy().toString()); + assertEquals("leading", spec.requireInstance("instance1").upgradeRollout().toString()); + assertEquals("separate", spec.requireInstance("instance2").upgradeRollout().toString()); } @Test diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java index 77ce5c2175d..c71ae92b47a 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java @@ -111,6 +111,7 @@ public class DeploymentSpecWithoutInstanceTest { assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); + assertEquals(DeploymentSpec.UpgradeRollout.separate, spec.requireInstance("default").upgradeRollout()); } @Test @@ -276,19 +277,24 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals("qrs", spec.requireInstance("default").globalServiceId().get()); } + @Test + public void productionSpecWithUpgradeRollout() { + StringReader r = new StringReader( + "" + + " " + + "" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals("leading", spec.requireInstance("default").upgradeRollout().toString()); + } + @Test public void productionSpecWithUpgradePolicy() { StringReader r = new StringReader( "" + " " + - " " + - " us-west-1" + - " us-central-1" + - " us-east-3" + - " " + "" ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("canary", spec.requireInstance("default").upgradePolicy().toString()); } -- cgit v1.2.3 From 290dc43262a9f55589d982e066d1f5b701584925 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 19 Aug 2021 16:36:14 +0200 Subject: Split assignment and if-condition --- .../application/api/xml/DeploymentSpecXmlReader.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'config-model-api') 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 88c366e5d96..14b72f79525 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 @@ -399,11 +399,14 @@ public class DeploymentSpecXmlReader { } private DeploymentSpec.UpgradePolicy readUpgradePolicy(Element parent, Element fallbackParent) { - String policy; Element upgradeElement = XML.getChild(parent, upgradeTag); if (upgradeElement == null) upgradeElement = XML.getChild(fallbackParent, upgradeTag); - if (upgradeElement == null || (policy = upgradeElement.getAttribute("policy")).isEmpty()) + if (upgradeElement == null) + return DeploymentSpec.UpgradePolicy.defaultPolicy; + + String policy = upgradeElement.getAttribute("policy"); + if (policy.isEmpty()) return DeploymentSpec.UpgradePolicy.defaultPolicy; switch (policy) { @@ -416,11 +419,14 @@ public class DeploymentSpecXmlReader { } private DeploymentSpec.UpgradeRollout readUpgradeRollout(Element parent, Element fallbackParent) { - String rollout; Element upgradeElement = XML.getChild(parent, upgradeTag); if (upgradeElement == null) upgradeElement = XML.getChild(fallbackParent, upgradeTag); - if (upgradeElement == null || (rollout = upgradeElement.getAttribute("rollout")).isEmpty()) + if (upgradeElement == null) + return DeploymentSpec.UpgradeRollout.separate; + + String rollout = upgradeElement.getAttribute("rollout"); + if (rollout.isEmpty()) return DeploymentSpec.UpgradeRollout.separate; switch (rollout) { -- cgit v1.2.3