diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-08-19 11:49:10 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2021-08-19 11:49:10 +0200 |
commit | d360cfdd5dc6dc5c6475c17dc6b961786e6b7c34 (patch) | |
tree | cfb60d5f7608c78a2bebc6ea42a8075637a5cac8 | |
parent | 5b61adcd248e9bd9f191c21c6d0a6dc39cf78d60 (diff) |
Allow configuring upgrade rollout in deployment.xml
10 files changed, 131 insertions, 25 deletions
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 <init>(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 <init>(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<DeploymentSpec.ChangeBlocker> changeBlockers; private final Optional<String> globalServiceId; private final Optional<AthenzService> athenzService; @@ -36,6 +37,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { public DeploymentInstanceSpec(InstanceName name, List<DeploymentSpec.Step> steps, DeploymentSpec.UpgradePolicy upgradePolicy, + DeploymentSpec.UpgradeRollout upgradeRollout, List<DeploymentSpec.ChangeBlocker> changeBlockers, Optional<String> globalServiceId, Optional<AthenzService> 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<DeploymentSpec.ChangeBlocker> 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<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceTag, parentTag); Optional<AthenzService> 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 @@ -360,33 +361,43 @@ public class DeploymentSpecTest { } @Test + public void productionSpecWithUpgradeRollout() { + StringReader r = new StringReader( + "<deployment>" + + " <instance id='default'>" + + " <upgrade rollout='leading' />" + + " </instance>" + + " <instance id='custom'/>" + + "</deployment>" + ); + 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( "<deployment>" + " <instance id='default'>" + " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + " </instance>" + + " <instance id='custom'/>" + "</deployment>" ); - 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( "<deployment version='1.0'>" + - " <upgrade policy='canary'/>" + - " <instance id='instance1'>" + - " </instance>" + + " <upgrade policy='canary' rollout='leading'/>" + + " <instance id='instance1'/>" + " <instance id='instance2'>" + - " <upgrade policy='conservative'/>" + + " <upgrade policy='conservative' rollout='separate'/>" + " </instance>" + "</deployment>" ); @@ -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 @@ -277,18 +278,23 @@ public class DeploymentSpecWithoutInstanceTest { } @Test + public void productionSpecWithUpgradeRollout() { + StringReader r = new StringReader( + "<deployment>" + + " <upgrade rollout='leading'/>" + + "</deployment>" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals("leading", spec.requireInstance("default").upgradeRollout().toString()); + } + + @Test public void productionSpecWithUpgradePolicy() { StringReader r = new StringReader( "<deployment>" + " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + "</deployment>" ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("canary", spec.requireInstance("default").upgradePolicy().toString()); } diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 171112f6bd7..77e3ce8f573 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -51,7 +51,8 @@ ParallelInstances = element parallel { } Upgrade = element upgrade { - attribute policy { xsd:string } + attribute policy { xsd:string }? & + attribute rollout { xsd:string }? } BlockChange = element block-change { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 4e6df1921b6..aab4e943b94 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -411,6 +411,8 @@ public class DeploymentTrigger { private boolean acceptNewApplicationVersion(DeploymentStatus status, InstanceName instance) { if (status.application().require(instance).change().application().isPresent()) return true; // Replacing a previous application change is ok. if (status.hasFailures()) return true; // Allow changes to fix upgrade problems. + if (status.application().deploymentSpec().instance(instance) // Leading upgrade allows app change to join in. + .map(spec -> spec.upgradeRollout() == DeploymentSpec.UpgradeRollout.leading).orElse(false)) return true; return status.application().require(instance).change().platform().isEmpty(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index 808409cf793..b234ab4960b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -53,6 +53,7 @@ public class ApplicationPackageBuilder { private OptionalInt majorVersion = OptionalInt.empty(); private String instances = "default"; private String upgradePolicy = null; + private String upgradeRollout = null; private String globalServiceId = null; private String athenzIdentityAttributes = null; private String searchDefinition = "search test { }"; @@ -75,6 +76,11 @@ public class ApplicationPackageBuilder { return this; } + public ApplicationPackageBuilder upgradeRollout(String upgradeRollout) { + this.upgradeRollout = upgradeRollout; + return this; + } + public ApplicationPackageBuilder globalServiceId(String globalServiceId) { this.globalServiceId = globalServiceId; return this; @@ -221,10 +227,11 @@ public class ApplicationPackageBuilder { } xml.append(">\n"); xml.append(" <instance id='").append(instances).append("'>\n"); - if (upgradePolicy != null) { - xml.append(" <upgrade policy='"); - xml.append(upgradePolicy); - xml.append("'/>\n"); + if (upgradePolicy != null || upgradeRollout != null) { + xml.append(" <upgrade "); + if (upgradePolicy != null) xml.append("policy='").append(upgradePolicy).append("' "); + if (upgradeRollout != null) xml.append("rollout='").append(upgradeRollout).append("' "); + xml.append("/>\n"); } xml.append(notifications); if (explicitSystemTest) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 7077e14a648..70a967ecef9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -107,6 +107,27 @@ public class DeploymentTriggerTest { } @Test + public void leadingUpgradeAllowsApplicationChangeWhileUpgrading() { + var applicationPackage = new ApplicationPackageBuilder().region("us-east-3") + .upgradeRollout("leading") + .build(); + var app = tester.newDeploymentContext(); + + app.submit(applicationPackage).deploy(); + + Change upgrade = Change.of(new Version("7.8.9")); + tester.controllerTester().upgradeSystem(upgrade.platform().get()); + tester.upgrader().maintain(); + app.runJob(systemTest).runJob(stagingTest); + tester.triggerJobs(); + app.assertRunning(productionUsEast3); + assertEquals(upgrade, app.instance().change()); + + app.submit(applicationPackage); + assertEquals(upgrade.with(app.lastSubmission().get()), app.instance().change()); + } + + @Test public void abortsJobsOnNewApplicationChange() { var app = tester.newDeploymentContext(); app.submit() |