diff options
Diffstat (limited to 'controller-server')
6 files changed, 94 insertions, 68 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index 3f12cce9aae..f36f5be7778 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -33,6 +33,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -139,10 +140,10 @@ public class DeploymentStatus { return dependents.contains(current); } - /** Whether any job is failing on anything older than version, with errors other than lack of capacity in a test zone.. */ - public boolean hasFailures(ApplicationVersion version) { + /** Whether any job is failing on versions selected by the given filter, with errors other than lack of capacity in a test zone.. */ + public boolean hasFailures(Predicate<ApplicationVersion> versionFilter) { return ! allJobs.failingHard() - .matching(job -> job.lastTriggered().get().versions().targetApplication().compareTo(version) < 0) + .matching(job -> versionFilter.test(job.lastTriggered().get().versions().targetApplication())) .isEmpty(); } @@ -210,7 +211,7 @@ public class DeploymentStatus { Versions versions = Versions.from(change, application, firstProductionJobWithDeployment.flatMap(this::deploymentFor), systemVersion); if (step.completedAt(change, firstProductionJobWithDeployment).isEmpty()) - jobs.merge(job, List.of(new Job(versions, step.readyAt(change), change)), DeploymentStatus::union); + jobs.merge(job, List.of(new Job(job.type(), versions, step.readyAt(change), change)), DeploymentStatus::union); }); return Collections.unmodifiableMap(jobs); } @@ -303,20 +304,29 @@ public class DeploymentStatus { private Map<JobId, List<Job>> productionJobs(InstanceName instance, Change change, boolean assumeUpgradesSucceed) { Map<JobId, List<Job>> jobs = new LinkedHashMap<>(); jobSteps.forEach((job, step) -> { - // When computing eager test jobs for outstanding changes, assume current upgrade completes successfully. - Optional<Deployment> deployment = deploymentFor(job) - .map(existing -> assumeUpgradesSucceed ? withChange(existing, change.withoutApplication()) : existing); + // When computing eager test jobs for outstanding changes, assume current change completes successfully. + Optional<Deployment> deployment = deploymentFor(job); + Optional<Version> existingPlatform = deployment.map(Deployment::version); + Optional<ApplicationVersion> existingApplication = deployment.map(Deployment::applicationVersion); + if (assumeUpgradesSucceed) { + Change currentChange = application.require(instance).change(); + Versions target = Versions.from(currentChange, application, deployment, systemVersion); + existingPlatform = Optional.of(target.targetPlatform()); + existingApplication = Optional.of(target.targetApplication()); + } if (job.application().instance().equals(instance) && job.type().isProduction()) { - List<Job> toRun = new ArrayList<>(); List<Change> changes = changes(job, step, change); if (changes.isEmpty()) return; for (Change partial : changes) { - toRun.add(new Job(Versions.from(partial, application, deployment, systemVersion), - step.readyAt(partial, Optional.of(job)), - partial)); + Job jobToRun = new Job(job.type(), + Versions.from(partial, application, existingPlatform, existingApplication, systemVersion), + step.readyAt(partial, Optional.of(job)), + partial); + toRun.add(jobToRun); // Assume first partial change is applied before the second. - deployment = deployment.map(existing -> withChange(existing, partial)); + existingPlatform = Optional.of(jobToRun.versions.targetPlatform()); + existingApplication = Optional.of(jobToRun.versions.targetApplication()); } jobs.put(job, toRun); } @@ -324,17 +334,6 @@ public class DeploymentStatus { return jobs; } - private static Deployment withChange(Deployment deployment, Change change) { - return new Deployment(deployment.zone(), - change.application().orElse(deployment.applicationVersion()), - change.platform().orElse(deployment.version()), - deployment.at(), - deployment.metrics(), - deployment.activity(), - deployment.quota(), - deployment.cost()); - } - /** Changes to deploy with the given job, possibly split in two steps. */ private List<Change> changes(JobId job, StepStatus step, Change change) { // Signal strict completion criterion by depending on job itself. @@ -432,7 +431,8 @@ public class DeploymentStatus { declaredTest(job.application(), testType).ifPresent(testJob -> { for (Job productionJob : versionsList) if (allJobs.successOn(productionJob.versions()).get(testJob).isEmpty()) - testJobs.merge(testJob, List.of(new Job(productionJob.versions(), + testJobs.merge(testJob, List.of(new Job(testJob.type(), + productionJob.versions(), jobSteps().get(testJob).readyAt(productionJob.change), productionJob.change)), DeploymentStatus::union); @@ -448,7 +448,8 @@ public class DeploymentStatus { && testJobs.get(test).stream().anyMatch(testJob -> testJob.versions().equals(productionJob.versions())))) { JobId testJob = firstDeclaredOrElseImplicitTest(testType); testJobs.merge(testJob, - List.of(new Job(productionJob.versions(), + List.of(new Job(testJob.type(), + productionJob.versions(), jobSteps.get(testJob).readyAt(productionJob.change), productionJob.change)), DeploymentStatus::union); @@ -872,8 +873,8 @@ public class DeploymentStatus { private final Optional<Instant> readyAt; private final Change change; - public Job(Versions versions, Optional<Instant> readyAt, Change change) { - this.versions = versions; + public Job(JobType type, Versions versions, Optional<Instant> readyAt, Change change) { + this.versions = type == systemTest ? versions.withoutSources() : versions; this.readyAt = readyAt; this.change = 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 bb8180e9f14..20fa539c820 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.config.application.api.DeploymentInstanceSpec; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.InstanceName; import com.yahoo.text.Text; @@ -30,6 +31,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.OptionalLong; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -197,7 +199,7 @@ public class DeploymentTrigger { DeploymentStatus status = jobs.deploymentStatus(application); Versions versions = Versions.from(instance.change(), application, status.deploymentFor(job), controller.readSystemVersion()); - DeploymentStatus.Job toTrigger = new DeploymentStatus.Job(versions, Optional.of(controller.clock().instant()), instance.change()); + DeploymentStatus.Job toTrigger = new DeploymentStatus.Job(job.type(), versions, Optional.of(controller.clock().instant()), instance.change()); Map<JobId, List<DeploymentStatus.Job>> jobs = status.testJobs(Map.of(job, List.of(toTrigger))); if (jobs.isEmpty() || ! requireTests) jobs = Map.of(job, List.of(toTrigger)); @@ -388,9 +390,13 @@ public class DeploymentTrigger { private boolean acceptNewApplicationVersion(DeploymentStatus status, InstanceName instance, ApplicationVersion version) { if (status.application().deploymentSpec().instance(instance).isEmpty()) return false; // Unknown instance. boolean isChangingRevision = status.application().require(instance).change().application().isPresent(); - switch (status.application().deploymentSpec().requireInstance(instance).revisionChange()) { + DeploymentInstanceSpec spec = status.application().deploymentSpec().requireInstance(instance); + Predicate<ApplicationVersion> versionFilter = spec.revisionTarget() == DeploymentSpec.RevisionTarget.next + ? failing -> status.application().require(instance).change().application().get().compareTo(failing) == 0 + : failing -> version.compareTo(failing) > 0; + switch (spec.revisionChange()) { case whenClear: return ! isChangingRevision; - case whenFailing: return ! isChangingRevision || status.hasFailures(version); + case whenFailing: return ! isChangingRevision || status.hasFailures(versionFilter); case always: return true; default: throw new IllegalStateException("Unknown revision upgrade policy"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index 1e183d44377..0ea18d9cfa2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -37,6 +37,11 @@ public class Versions { this.sourceApplication = requireNonNull(sourceApplication); } + /** A copy of this, without source versions. */ + public Versions withoutSources() { + return new Versions(targetPlatform, targetApplication, Optional.empty(), Optional.empty()); + } + /** Target platform version for this */ public Version targetPlatform() { return targetPlatform; @@ -98,36 +103,36 @@ public class Versions { } /** Create versions using given change and application */ + public static Versions from(Change change, Application application, Optional<Version> existingPlatform, + Optional<ApplicationVersion> existingApplication, Version defaultPlatformVersion) { + return new Versions(targetPlatform(application, change, existingPlatform, defaultPlatformVersion), + targetApplication(application, change, existingApplication), + existingPlatform, + existingApplication); + } + + /** Create versions using given change and application */ public static Versions from(Change change, Application application, Optional<Deployment> deployment, Version defaultPlatformVersion) { - return new Versions(targetPlatform(application, change, deployment, defaultPlatformVersion), - targetApplication(application, change, deployment), + return new Versions(targetPlatform(application, change, deployment.map(Deployment::version), defaultPlatformVersion), + targetApplication(application, change, deployment.map(Deployment::applicationVersion)), deployment.map(Deployment::version), deployment.map(Deployment::applicationVersion)); } - public static Versions from(Change change, Deployment deployment) { - return new Versions(change.platform().filter(version -> change.isPinned() || deployment.version().isBefore(version)) - .orElse(deployment.version()), - change.application().filter(version -> deployment.applicationVersion().compareTo(version) < 0) - .orElse(deployment.applicationVersion()), - Optional.of(deployment.version()), - Optional.of(deployment.applicationVersion())); - } - - private static Version targetPlatform(Application application, Change change, Optional<Deployment> deployment, + private static Version targetPlatform(Application application, Change change, Optional<Version> existing, Version defaultVersion) { if (change.isPinned() && change.platform().isPresent()) return change.platform().get(); - return max(change.platform(), deployment.map(Deployment::version)) + return max(change.platform(), existing) .orElseGet(() -> application.oldestDeployedPlatform().orElse(defaultVersion)); } private static ApplicationVersion targetApplication(Application application, Change change, - Optional<Deployment> deployment) { + Optional<ApplicationVersion> existing) { return change.application() - .or(() -> deployment.map(Deployment::applicationVersion)) + .or(() -> existing) .orElseGet(() -> defaultApplicationVersion(application)); } 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 bfedd325ae7..805d727d355 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 @@ -251,9 +251,6 @@ public class DeploymentTriggerTest { .build(); DeploymentContext app = tester.newDeploymentContext() .submit(appPackage); - Optional<ApplicationVersion> revision0 = app.lastSubmission(); - - app.submit(appPackage); Optional<ApplicationVersion> revision1 = app.lastSubmission(); app.submit(appPackage); @@ -262,17 +259,47 @@ public class DeploymentTriggerTest { app.submit(appPackage); Optional<ApplicationVersion> revision3 = app.lastSubmission(); - assertEquals(revision0, app.instance().change().application()); - assertEquals(revision1, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + app.submit(appPackage); + Optional<ApplicationVersion> revision4 = app.lastSubmission(); + + app.submit(appPackage); + Optional<ApplicationVersion> revision5 = app.lastSubmission(); - tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(revision1.get())); + // 5 revisions submitted; the first is rolling out, and the others are queued. + tester.outstandingChangeDeployer().run(); assertEquals(revision1, app.instance().change().application()); assertEquals(revision2, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); - app.deploy(); + // The second revision is set as the target by user interaction. + tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(revision2.get())); tester.outstandingChangeDeployer().run(); assertEquals(revision2, app.instance().change().application()); assertEquals(revision3, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + + // The second revision deploys completely, and the third starts rolling out. + app.runJob(systemTest).runJob(stagingTest) + .runJob(productionUsEast3); + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + assertEquals(revision3, app.instance().change().application()); + assertEquals(revision4, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + + // The third revision fails, and the fourth is chosen to replace it. + app.triggerJobs().timeOutConvergence(systemTest); + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + assertEquals(revision4, app.instance().change().application()); + assertEquals(revision5, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + + // Tests for outstanding change are relevant when current revision completes. + app.runJob(systemTest).runJob(systemTest) + .jobAborted(stagingTest).runJob(stagingTest).runJob(stagingTest) + .runJob(productionUsEast3); + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + assertEquals(revision5, app.instance().change().application()); + assertEquals(Change.empty(), app.deploymentStatus().outstandingChange(InstanceName.defaultName())); + app.runJob(productionUsEast3); } @Test @@ -1810,7 +1837,8 @@ public class DeploymentTriggerTest { // System and staging tests both require unknown versions, and are broken. tester.controller().applications().deploymentTrigger().forceTrigger(app.instanceId(), productionCdUsEast1, "user", false); app.runJob(productionCdUsEast1) - .abortJob(systemTest) + .triggerJobs() + .jobAborted(systemTest) .jobAborted(stagingTest) .runJob(systemTest) // Run test for aws zone again. .runJob(stagingTest) // Run test for aws zone again. diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index b05be36a24a..ed82039d923 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -144,13 +144,6 @@ "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" - }, - "sourcePlatform": "6.1.0", - "sourceApplication": { - "build": 2, - "compileVersion": "6.1.0", - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" } }, "steps": [ @@ -209,13 +202,6 @@ "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" - }, - "sourcePlatform": "6.1.0", - "sourceApplication": { - "build": 1, - "compileVersion": "6.1.0", - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" } }, "steps": [ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json index c5286d4a04b..211aa57d8ed 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json @@ -194,7 +194,7 @@ { "name": "system-test", "coolingDownUntil": "(ignore)", - "pending": "platform" + "pending": "application" }, { "name": "staging-test", |