diff options
3 files changed, 49 insertions, 6 deletions
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..cded075a7f3 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") |