diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2022-01-21 15:49:34 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-01-21 15:49:34 +0100 |
commit | 2c3ac8ec9f5ac8285c7e443b9fee1be8cbef2e3e (patch) | |
tree | 617735f0e27692796da28493ba2878777ed60604 | |
parent | a5583941af71c18f109a87338d71c88429446af8 (diff) | |
parent | 15b0ed57773bf6f54c98f10f52a6661d6bdabd6d (diff) |
Merge pull request #20895 from vespa-engine/jonmv/deployment-orchestration-for-long-pipelines
Jonmv/deployment orchestration for long pipelines
10 files changed, 132 insertions, 15 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index cac9d21ee1f..d946dd972f4 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -194,9 +194,10 @@ "public" ], "methods": [ - "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, java.time.Instant)", + "public void <init>(com.yahoo.config.provision.InstanceName, java.util.List, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision, 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, java.time.Instant)", "public com.yahoo.config.provision.InstanceName name()", "public com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy upgradePolicy()", + "public com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision upgradeRevision()", "public com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout upgradeRollout()", "public java.util.List changeBlocker()", "public java.util.Optional globalServiceId()", @@ -365,6 +366,23 @@ "public static final enum com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy conservative" ] }, + "com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision": { + "superClass": "java.lang.Enum", + "interfaces": [], + "attributes": [ + "public", + "final", + "enum" + ], + "methods": [ + "public static com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision[] values()", + "public static com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision valueOf(java.lang.String)" + ], + "fields": [ + "public static final enum com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision separate", + "public static final enum com.yahoo.config.application.api.DeploymentSpec$UpgradeRevision latest" + ] + }, "com.yahoo.config.application.api.DeploymentSpec$UpgradeRollout": { "superClass": "java.lang.Enum", "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 67ddb9ef83c..ea38860c29b 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 @@ -31,6 +31,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { private final InstanceName name; private final DeploymentSpec.UpgradePolicy upgradePolicy; + private final DeploymentSpec.UpgradeRevision upgradeRevision; private final DeploymentSpec.UpgradeRollout upgradeRollout; private final List<DeploymentSpec.ChangeBlocker> changeBlockers; private final Optional<String> globalServiceId; @@ -41,6 +42,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { public DeploymentInstanceSpec(InstanceName name, List<DeploymentSpec.Step> steps, DeploymentSpec.UpgradePolicy upgradePolicy, + DeploymentSpec.UpgradeRevision upgradeRevision, DeploymentSpec.UpgradeRollout upgradeRollout, List<DeploymentSpec.ChangeBlocker> changeBlockers, Optional<String> globalServiceId, @@ -51,6 +53,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { super(steps); this.name = name; this.upgradePolicy = upgradePolicy; + this.upgradeRevision = upgradeRevision; this.upgradeRollout = upgradeRollout; this.changeBlockers = changeBlockers; this.globalServiceId = globalServiceId; @@ -150,6 +153,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 revision strategy of this, which is separate if none is specified */ + public DeploymentSpec.UpgradeRevision upgradeRevision() { return upgradeRevision; } + /** Returns the upgrade rollout strategy of this, which is separate if none is specified */ public DeploymentSpec.UpgradeRollout upgradeRollout() { return upgradeRollout; } @@ -198,6 +204,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { DeploymentInstanceSpec other = (DeploymentInstanceSpec) o; return globalServiceId.equals(other.globalServiceId) && upgradePolicy == other.upgradePolicy && + upgradeRevision == other.upgradeRevision && upgradeRollout == other.upgradeRollout && changeBlockers.equals(other.changeBlockers) && steps().equals(other.steps()) && @@ -208,7 +215,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { @Override public int hashCode() { - return Objects.hash(globalServiceId, upgradePolicy, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints); + return Objects.hash(globalServiceId, upgradePolicy, upgradeRevision, 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 88363db6e49..4b019bd9f7a 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 @@ -550,6 +550,15 @@ public class DeploymentSpec { } + /** Determines when application changes deploy, when an older revision is already rolling out. */ + public enum UpgradeRevision { + /** Separate: Application changes wait for previous application changes to complete, unless they fail. */ + separate, + /** Latest: Application changes immediately supersede previous application changes, unless currently blocked. */ + latest + } + + /** 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. */ 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 8f866654d56..b031af9faf2 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 @@ -165,6 +165,7 @@ public class DeploymentSpecXmlReader { // Values where the parent may provide a default DeploymentSpec.UpgradePolicy upgradePolicy = readUpgradePolicy(instanceTag, parentTag); + DeploymentSpec.UpgradeRevision upgradeRevision = readUpgradeRevision(instanceTag, parentTag); DeploymentSpec.UpgradeRollout upgradeRollout = readUpgradeRollout(instanceTag, parentTag); List<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceTag, parentTag); Optional<AthenzService> athenzService = mostSpecificAttribute(instanceTag, athenzServiceAttribute).map(AthenzService::from); @@ -183,6 +184,7 @@ public class DeploymentSpecXmlReader { .map(name -> new DeploymentInstanceSpec(InstanceName.from(name), steps, upgradePolicy, + upgradeRevision, upgradeRollout, changeBlockers, globalServiceId.asOptional(), @@ -472,6 +474,25 @@ public class DeploymentSpecXmlReader { } } + private DeploymentSpec.UpgradeRevision readUpgradeRevision(Element parent, Element fallbackParent) { + Element upgradeElement = XML.getChild(parent, upgradeTag); + if (upgradeElement == null) + upgradeElement = XML.getChild(fallbackParent, upgradeTag); + if (upgradeElement == null) + return DeploymentSpec.UpgradeRevision.separate; + + String revision = upgradeElement.getAttribute("revision"); + if (revision.isEmpty()) + return DeploymentSpec.UpgradeRevision.separate; + + switch (revision) { + case "separate": return DeploymentSpec.UpgradeRevision.separate; + case "latest": return DeploymentSpec.UpgradeRevision.latest; + default: throw new IllegalArgumentException("Illegal upgrade revision '" + revision + "': " + + "Must be one of " + Arrays.toString(DeploymentSpec.UpgradeRevision.values())); + } + } + private DeploymentSpec.UpgradeRollout readUpgradeRollout(Element parent, Element fallbackParent) { Element upgradeElement = XML.getChild(parent, upgradeTag); if (upgradeElement == null) @@ -487,8 +508,8 @@ public class DeploymentSpecXmlReader { 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())); + default: throw new IllegalArgumentException("Illegal upgrade rollout '" + rollout + "': " + + "Must be one of " + Arrays.toString(DeploymentSpec.UpgradeRollout.values())); } } 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 2fa2ba83291..a97faf5995d 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 @@ -121,6 +121,7 @@ public class DeploymentSpecTest { assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); + assertEquals(DeploymentSpec.UpgradeRevision.separate, spec.requireInstance("default").upgradeRevision()); assertEquals(DeploymentSpec.UpgradeRollout.separate, spec.requireInstance("default").upgradeRollout()); } @@ -364,6 +365,21 @@ public class DeploymentSpecTest { } @Test + public void productionSpecWithUpgradeRevision() { + StringReader r = new StringReader( + "<deployment>" + + " <instance id='default'>" + + " <upgrade revision='latest' />" + + " </instance>" + + " <instance id='custom'/>" + + "</deployment>" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals("latest", spec.requireInstance("default").upgradeRevision().toString()); + assertEquals("separate", spec.requireInstance("custom").upgradeRevision().toString()); + } + + @Test public void productionSpecWithUpgradeRollout() { StringReader r = new StringReader( "<deployment>" + @@ -397,10 +413,10 @@ public class DeploymentSpecTest { public void upgradePolicyDefault() { StringReader r = new StringReader( "<deployment version='1.0'>" + - " <upgrade policy='canary' rollout='leading'/>" + + " <upgrade policy='canary' rollout='leading' revision='latest' />" + " <instance id='instance1'/>" + " <instance id='instance2'>" + - " <upgrade policy='conservative' rollout='separate'/>" + + " <upgrade policy='conservative' rollout='separate' revision='separate'/>" + " </instance>" + "</deployment>" ); @@ -408,6 +424,8 @@ public class DeploymentSpecTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("canary", spec.requireInstance("instance1").upgradePolicy().toString()); assertEquals("conservative", spec.requireInstance("instance2").upgradePolicy().toString()); + assertEquals("latest", spec.requireInstance("instance1").upgradeRevision().toString()); + assertEquals("separate", spec.requireInstance("instance2").upgradeRevision().toString()); assertEquals("leading", spec.requireInstance("instance1").upgradeRollout().toString()); assertEquals("separate", spec.requireInstance("instance2").upgradeRollout().toString()); } diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 819e6b79fbb..3e751a379d4 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -52,6 +52,7 @@ ParallelInstances = element parallel { Upgrade = element upgrade { attribute policy { xsd:string }? & + attribute revision { xsd:string }? & attribute rollout { xsd:string }? } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java index 3fe5240ce34..cc68d47666d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java @@ -188,7 +188,7 @@ public class Instance { return change; } - /** Returns the application version that last completedd roll-out to this instance. */ + /** Returns the application version that last rolled out to this instance. */ public Optional<ApplicationVersion> latestDeployed() { return latestDeployed; } 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 d3f2d4e4501..9790ece421e 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 @@ -364,11 +364,13 @@ public class DeploymentTrigger { // ---------- Change management o_O ---------- 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(); + if (status.application().deploymentSpec().instance(instance).isEmpty()) return false; // Unknown instance. + if (status.hasFailures()) return true; // Allow changes to fix upgrade or previous revision problems. + DeploymentInstanceSpec spec = status.application().deploymentSpec().requireInstance(instance); + Change change = status.application().require(instance).change(); + if (change.platform().isPresent() && spec.upgradeRollout() == DeploymentSpec.UpgradeRollout.separate) return false; + if (change.application().isPresent() && spec.upgradeRevision() == DeploymentSpec.UpgradeRevision.separate) return false; + return true; } private Instance withRemainingChange(Instance instance, Change change, DeploymentStatus status) { 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 a338efd856c..bc5a70b1fa0 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 @@ -57,6 +57,7 @@ public class ApplicationPackageBuilder { private OptionalInt majorVersion = OptionalInt.empty(); private String instances = "default"; private String upgradePolicy = null; + private String upgradeRevision = "latest"; private String upgradeRollout = null; private String globalServiceId = null; private String athenzIdentityAttributes = "athenz-domain='domain' athenz-service='service'"; @@ -80,6 +81,11 @@ public class ApplicationPackageBuilder { return this; } + public ApplicationPackageBuilder upgradeRevision(String upgradeRevision) { + this.upgradeRevision = upgradeRevision; + return this; + } + public ApplicationPackageBuilder upgradeRollout(String upgradeRollout) { this.upgradeRollout = upgradeRollout; return this; @@ -253,9 +259,10 @@ public class ApplicationPackageBuilder { } xml.append(">\n"); xml.append(" <instance id='").append(instances).append("'>\n"); - if (upgradePolicy != null || upgradeRollout != null) { + if (upgradePolicy != null || upgradeRevision != null || upgradeRollout != null) { xml.append(" <upgrade "); if (upgradePolicy != null) xml.append("policy='").append(upgradePolicy).append("' "); + if (upgradeRevision != null) xml.append("revision='").append(upgradeRevision).append("' "); if (upgradeRollout != null) xml.append("rollout='").append(upgradeRollout).append("' "); xml.append("/>\n"); } 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 52521775ddd..cd1e12312a8 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,40 @@ public class DeploymentTriggerTest { } @Test + public void separateRevisionMakesApplicationChangeWaitForPreviousToComplete() { + DeploymentContext app = tester.newDeploymentContext(); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .upgradeRevision(null) // separate by default, but we override this in test builder + .region("us-east-3") + .test("us-east-3") + .build(); + + app.submit(applicationPackage).runJob(systemTest).runJob(stagingTest).runJob(productionUsEast3); + Optional<ApplicationVersion> v0 = app.lastSubmission(); + + app.submit(applicationPackage); + Optional<ApplicationVersion> v1 = app.lastSubmission(); + assertEquals(v0, app.instance().change().application()); + + // Eager tests still run before new revision rolls out. + app.runJob(systemTest).runJob(stagingTest); + + // v0 rolls out completely. + app.runJob(testUsEast3); + assertEquals(Optional.empty(), app.instance().change().application()); + + // v1 starts rolling when v0 is done. + tester.outstandingChangeDeployer().run(); + assertEquals(v1, app.instance().change().application()); + + // v1 fails, so v2 starts immediately. + app.runJob(productionUsEast3).failDeployment(testUsEast3); + app.submit(applicationPackage); + Optional<ApplicationVersion> v2 = app.lastSubmission(); + assertEquals(v2, app.instance().change().application()); + } + + @Test public void leadingUpgradeAllowsApplicationChangeWhileUpgrading() { var applicationPackage = new ApplicationPackageBuilder().region("us-east-3") .upgradeRollout("leading") @@ -826,6 +860,7 @@ public class DeploymentTriggerTest { DeploymentContext i4 = tester.newDeploymentContext("t", "a", "i4"); ApplicationPackage applicationPackage = ApplicationPackageBuilder .fromDeploymentXml("<deployment version='1'>\n" + + " <upgrade revision='separate' />\n" + " <parallel>\n" + " <instance id='i1'>\n" + " <prod>\n" + @@ -910,8 +945,7 @@ public class DeploymentTriggerTest { tester.clock().advance(Duration.ofHours(3)); // v1 is all done in i1 and i2, but does not yet roll out in i3; v2 is not completely rolled out there yet. - // TODO jonmv: thie belowh new revision policy, but must be faked for now, as v1 would not wait for v0 to complete. - //tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); assertEquals(v0, i3.instance().change().application()); // i3 completes v0, which rolls out to i4; v1 is ready for i3, but v2 is not. |