From 6a0806c27663aeb60ac7c496f0d9322bf86b03c2 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Wed, 30 Aug 2017 14:49:48 +0200 Subject: Extract DeploymentOrder class --- .../controller/deployment/DeploymentOrder.java | 98 ++++++++++++++++++++++ .../controller/deployment/DeploymentTrigger.java | 83 +++--------------- 2 files changed, 111 insertions(+), 70 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java new file mode 100644 index 00000000000..113fe4ce1f2 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -0,0 +1,98 @@ +package com.yahoo.vespa.hosted.controller.deployment; + +import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; +import com.yahoo.vespa.hosted.controller.application.JobStatus; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.logging.Logger; + +/** + * This class determines order of deployments according to an application's deployment spec + * + * @author mpolden + */ +public class DeploymentOrder { + + private static final Logger log = Logger.getLogger(DeploymentOrder.class.getName()); + + private final Controller controller; + private final Clock clock; + + public DeploymentOrder(Controller controller) { + this.controller = controller; + this.clock = controller.clock(); + } + + /** Returns the next job(s) to trigger after the given job, or empty if none should be triggered */ + public List nextAfter(DeploymentJobs.JobType jobType, Application application) { + // Always trigger system test after component as deployment spec might not be available yet (e.g. if this is a + // new application with no previous deployments) + if (jobType == DeploymentJobs.JobType.component) { + return Collections.singletonList(DeploymentJobs.JobType.systemTest); + } + + // At this point we've at least deployed to system test, so deployment spec should be available + List zones = application.deploymentSpec().zones(); + Optional zoneForJob = zoneForJob(application, jobType); + if (!zoneForJob.isPresent()) { + return Collections.emptyList(); + } + int zoneIndex = application.deploymentSpec().zones().indexOf(zoneForJob.get()); + + // This is last zone + if (zoneIndex == zones.size() - 1) { + return Collections.emptyList(); + } + + // Skip next job if delay has not passed yet + Duration delay = delayAfter(application, zoneForJob.get()); + Optional lastSuccess = Optional.ofNullable(application.deploymentJobs().jobStatus().get(jobType)) + .flatMap(JobStatus::lastSuccess) + .map(JobStatus.JobRun::at); + if (lastSuccess.isPresent() && lastSuccess.get().plus(delay).isAfter(clock.instant())) { + log.info(String.format("Delaying next job after %s of %s by %s", jobType, application, delay)); + return Collections.emptyList(); + } + + DeploymentSpec.DeclaredZone nextZone = application.deploymentSpec().zones().get(zoneIndex + 1); + return Collections.singletonList( + DeploymentJobs.JobType.from(controller.system(), nextZone.environment(), nextZone.region().orElse(null)) + ); + } + + private Duration delayAfter(Application application, DeploymentSpec.DeclaredZone zone) { + int stepIndex = application.deploymentSpec().steps().indexOf(zone); + if (stepIndex == -1 || stepIndex == application.deploymentSpec().steps().size() - 1) { + return Duration.ZERO; + } + Duration totalDelay = Duration.ZERO; + List remainingSteps = application.deploymentSpec().steps() + .subList(stepIndex + 1, application.deploymentSpec().steps().size()); + for (DeploymentSpec.Step step : remainingSteps) { + if (!(step instanceof DeploymentSpec.Delay)) { + break; + } + totalDelay = totalDelay.plus(((DeploymentSpec.Delay) step).duration()); + } + return totalDelay; + } + + private Optional zoneForJob(Application application, DeploymentJobs.JobType jobType) { + return application.deploymentSpec() + .zones() + .stream() + .filter(zone -> zone.deploysTo( + jobType.environment(), + jobType.isProduction() ? jobType.region(controller.system()) : Optional.empty())) + .findFirst(); + } + +} 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 ce1fee4dd14..1ccbd70d4d2 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 @@ -39,6 +39,7 @@ public class DeploymentTrigger { private final Controller controller; private final Clock clock; private final BuildSystem buildSystem; + private final DeploymentOrder order; public DeploymentTrigger(Controller controller, CuratorDb curator, Clock clock) { Objects.requireNonNull(controller,"controller cannot be null"); @@ -46,6 +47,7 @@ public class DeploymentTrigger { this.controller = controller; this.clock = clock; this.buildSystem = new PolledBuildSystem(controller, curator); + this.order = new DeploymentOrder(controller); } //--- Start of methods which triggers deployment jobs ------------------------- @@ -76,11 +78,11 @@ public class DeploymentTrigger { // Trigger next if (report.success()) - application = trigger(nextAfter(report.jobType(), application), application, report.jobType() + " completed successfully", lock); + application = trigger(order.nextAfter(report.jobType(), application), application, report.jobType() + " completed successfully", lock); else if (isCapacityConstrained(report.jobType()) && shouldRetryOnOutOfCapacity(application, report.jobType())) application = trigger(report.jobType(), application, true, "Retrying due to out of capacity", lock); else if (shouldRetryNow(application)) - application = trigger(report.jobType(), application, "Retrying as job just started failing", lock); + application = trigger(report.jobType(), application, false, "Retrying as job just started failing", lock); applications().store(application, lock); } @@ -95,7 +97,7 @@ public class DeploymentTrigger { if (shouldRetryFromBeginning(application)) { // failed for a long time: Discard existing change and restart from the component job application = application.withDeploying(Optional.empty()); - application = trigger(JobType.component, application, "Retrying failing deployment from beginning: " + cause, lock); + application = trigger(JobType.component, application, false, "Retrying failing deployment from beginning: " + cause, lock); applications().store(application, lock); } else { // retry the failed job (with backoff) @@ -103,7 +105,7 @@ public class DeploymentTrigger { JobStatus jobStatus = application.deploymentJobs().jobStatus().get(jobType); if (isFailing(jobStatus)) { if (shouldRetryNow(jobStatus)) { - application = trigger(jobType, application, "Retrying failing job: " + cause, lock); + application = trigger(jobType, application, false, "Retrying failing job: " + cause, lock); applications().store(application, lock); } break; @@ -133,7 +135,7 @@ public class DeploymentTrigger { // Trigger next try (Lock lock = applications().lock(application.id())) { application = applications().require(application.id()); - application = trigger(nextAfter(lastSuccessfulJob.get().type(), application), application, + application = trigger(order.nextAfter(lastSuccessfulJob.get().type(), application), application, "Resuming delayed deployment", lock); applications().store(application, lock); } @@ -154,7 +156,7 @@ public class DeploymentTrigger { application = application.withDeploying(Optional.of(change)); if (change instanceof Change.ApplicationChange) application = application.withOutstandingChange(false); - application = trigger(JobType.systemTest, application, "Deploying change", lock); + application = trigger(JobType.systemTest, application, false, "Deploying change", lock); applications().store(application, lock); } } @@ -176,68 +178,6 @@ public class DeploymentTrigger { //--- End of methods which triggers deployment jobs ---------------------------- private ApplicationController applications() { return controller.applications(); } - - /** Returns the next job to trigger after this job, or null if none should be triggered */ - private JobType nextAfter(JobType jobType, Application application) { - // Always trigger system test after component as deployment spec might not be available yet (e.g. if this is a - // new application with no previous deployments) - if (jobType == JobType.component) { - return JobType.systemTest; - } - - // At this point we've at least deployed to system test, so deployment spec should be available - List zones = application.deploymentSpec().zones(); - Optional zoneForJob = zoneForJob(application, jobType); - if (!zoneForJob.isPresent()) { - return null; - } - int zoneIndex = application.deploymentSpec().zones().indexOf(zoneForJob.get()); - - // This is last zone - if (zoneIndex == zones.size() - 1) { - return null; - } - - // Skip next job if delay has not passed yet - Duration delay = delayAfter(application, zoneForJob.get()); - Optional lastSuccess = Optional.ofNullable(application.deploymentJobs().jobStatus().get(jobType)) - .flatMap(JobStatus::lastSuccess) - .map(JobStatus.JobRun::at); - if (lastSuccess.isPresent() && lastSuccess.get().plus(delay).isAfter(clock.instant())) { - log.info(String.format("Delaying next job after %s of %s by %s", jobType, application, delay)); - return null; - } - - DeploymentSpec.DeclaredZone nextZone = application.deploymentSpec().zones().get(zoneIndex + 1); - return JobType.from(controller.system(), nextZone.environment(), nextZone.region().orElse(null)); - } - - private Duration delayAfter(Application application, DeploymentSpec.DeclaredZone zone) { - int stepIndex = application.deploymentSpec().steps().indexOf(zone); - if (stepIndex == -1 || stepIndex == application.deploymentSpec().steps().size() - 1) { - return Duration.ZERO; - } - Duration totalDelay = Duration.ZERO; - List remainingSteps = application.deploymentSpec().steps() - .subList(stepIndex + 1, application.deploymentSpec().steps().size()); - for (DeploymentSpec.Step step : remainingSteps) { - if (!(step instanceof DeploymentSpec.Delay)) { - break; - } - totalDelay = totalDelay.plus(((DeploymentSpec.Delay) step).duration()); - } - return totalDelay; - } - - private Optional zoneForJob(Application application, JobType jobType) { - return application.deploymentSpec() - .zones() - .stream() - .filter(zone -> zone.deploysTo( - jobType.environment(), - jobType.isProduction() ? jobType.region(controller.system()) : Optional.empty())) - .findFirst(); - } private boolean isFirstJob(JobType jobType) { return jobType == JobType.component; @@ -356,8 +296,11 @@ public class DeploymentTrigger { return application.withJobTriggering(jobType, clock.instant(), controller); } - private Application trigger(JobType jobType, Application application, String cause, Lock lock) { - return trigger(jobType, application, false, cause, lock); + private Application trigger(List jobs, Application application, String cause, Lock lock) { + for (JobType job : jobs) { + application = trigger(job, application, false, cause, lock); + } + return application; } public BuildSystem buildSystem() { return buildSystem; } -- cgit v1.2.3 From fb9402d94d80e4f27c13e84f783ce825114d872e Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 31 Aug 2017 09:09:54 +0200 Subject: Trigger parallel deployments --- .../controller/application/DeploymentJobs.java | 11 -- .../controller/deployment/DeploymentOrder.java | 139 +++++++++++++++------ .../controller/deployment/DeploymentTrigger.java | 15 +-- .../vespa/hosted/controller/ControllerTest.java | 64 +++++----- .../deployment/ApplicationPackageBuilder.java | 8 ++ .../controller/deployment/DeploymentTester.java | 33 ++--- .../deployment/DeploymentTriggerTest.java | 73 ++++++++--- .../maintenance/DeploymentIssueReporterTest.java | 32 ++--- .../maintenance/FailureRedeployerTest.java | 30 ++--- .../maintenance/MetricsReporterTest.java | 2 +- .../controller/maintenance/UpgraderTest.java | 22 ++-- 11 files changed, 260 insertions(+), 169 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 61231404a94..19d60c18998 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.google.common.collect.ImmutableMap; import com.yahoo.component.Version; -import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; @@ -16,11 +15,9 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.stream.Collectors; /** * Information about which deployment jobs an application should run and their current status. @@ -252,14 +249,6 @@ public class DeploymentJobs { return from(system, new com.yahoo.config.provision.Zone(environment, region)); } - /** Returns the trigger order to use according to deployment spec */ - public static List triggerOrder(SystemName system, DeploymentSpec deploymentSpec) { - return deploymentSpec.zones().stream() - .map(declaredZone -> JobType.from(system, declaredZone.environment(), - declaredZone.region().orElse(null))) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); - } - private static Zone zone(SystemName system, String environment, String region) { return new Zone(system, Environment.from(environment), RegionName.from(region)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index 113fe4ce1f2..afa9c219048 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -3,19 +3,24 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import com.yahoo.vespa.hosted.controller.application.JobStatus; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.logging.Logger; +import java.util.stream.Collectors; + +import static java.util.stream.Collectors.collectingAndThen; /** - * This class determines order of deployments according to an application's deployment spec + * This class determines the order of deployments according to an application's deployment spec. * * @author mpolden */ @@ -31,68 +36,124 @@ public class DeploymentOrder { this.clock = controller.clock(); } - /** Returns the next job(s) to trigger after the given job, or empty if none should be triggered */ - public List nextAfter(DeploymentJobs.JobType jobType, Application application) { + /** Returns a list of jobs to trigger after the given job */ + public List nextAfter(JobType job, Application application) { // Always trigger system test after component as deployment spec might not be available yet (e.g. if this is a // new application with no previous deployments) - if (jobType == DeploymentJobs.JobType.component) { - return Collections.singletonList(DeploymentJobs.JobType.systemTest); + if (job == JobType.component) { + return Collections.singletonList(JobType.systemTest); } - // At this point we've at least deployed to system test, so deployment spec should be available - List zones = application.deploymentSpec().zones(); - Optional zoneForJob = zoneForJob(application, jobType); - if (!zoneForJob.isPresent()) { + // At this point we have deployed to system test, so deployment spec is available + List deploymentSteps = deploymentSteps(application); + Optional currentStep = fromJob(job, application); + if (!currentStep.isPresent()) { return Collections.emptyList(); } - int zoneIndex = application.deploymentSpec().zones().indexOf(zoneForJob.get()); - // This is last zone - if (zoneIndex == zones.size() - 1) { + // If this is the last deployment step there's nothing more to trigger + int currentIndex = deploymentSteps.indexOf(currentStep.get()); + if (currentIndex == deploymentSteps.size() - 1) { return Collections.emptyList(); } - // Skip next job if delay has not passed yet - Duration delay = delayAfter(application, zoneForJob.get()); - Optional lastSuccess = Optional.ofNullable(application.deploymentJobs().jobStatus().get(jobType)) - .flatMap(JobStatus::lastSuccess) - .map(JobStatus.JobRun::at); - if (lastSuccess.isPresent() && lastSuccess.get().plus(delay).isAfter(clock.instant())) { - log.info(String.format("Delaying next job after %s of %s by %s", jobType, application, delay)); + // Postpone next job if delay has not passed yet + Duration delay = delayAfter(currentStep.get(), application); + if (postponeDeployment(delay, job, application)) { + log.info(String.format("Delaying next job after %s of %s by %s", job, application, delay)); return Collections.emptyList(); } - DeploymentSpec.DeclaredZone nextZone = application.deploymentSpec().zones().get(zoneIndex + 1); - return Collections.singletonList( - DeploymentJobs.JobType.from(controller.system(), nextZone.environment(), nextZone.region().orElse(null)) - ); + DeploymentSpec.Step nextStep = deploymentSteps.get(currentIndex + 1); + if (nextStep instanceof DeploymentSpec.DeclaredZone) { + return Collections.singletonList(toJob((DeploymentSpec.DeclaredZone) nextStep)); + } else if (nextStep instanceof DeploymentSpec.ParallelZones) { + return ((DeploymentSpec.ParallelZones) nextStep).zones().stream() + .map(this::toJob) + .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + } else { + throw new IllegalStateException("Unexpected step type: " + nextStep.getClass()); + } + } + + /** Returns whether the given job is first in a deployment */ + public boolean isFirst(JobType job) { + return job == JobType.component; + } + + /** Returns whether the given job is last in a deployment */ + public boolean isLast(JobType job, Application application) { + List deploymentSteps = deploymentSteps(application); + if (deploymentSteps.isEmpty()) { // Deployment spec not yet available + return false; + } + DeploymentSpec.Step lastStep = deploymentSteps.get(deploymentSteps.size() - 1); + return fromJob(job, application).get().equals(lastStep); + } + + /** Returns jobs for given deployment spec, in the order they are declared */ + public List jobsFrom(DeploymentSpec deploymentSpec) { + if (deploymentSpec.steps().isEmpty()) { + return Arrays.asList(JobType.systemTest, JobType.stagingTest); + } + List jobs = new ArrayList<>(); + for (DeploymentSpec.Step step : deploymentSpec.steps()) { + if (step instanceof DeploymentSpec.DeclaredZone) { + jobs.add(toJob((DeploymentSpec.DeclaredZone) step)); + } else if (step instanceof DeploymentSpec.ParallelZones) { + ((DeploymentSpec.ParallelZones) step).zones().forEach(zone -> jobs.add(toJob(zone))); + } + } + return Collections.unmodifiableList(jobs); + } + + /** Resolve deployment step from job */ + private Optional fromJob(JobType job, Application application) { + for (DeploymentSpec.Step step : application.deploymentSpec().steps()) { + if (step.deploysTo(job.environment(), job.isProduction() ? job.region(controller.system()) : Optional.empty())) { + return Optional.of(step); + } + } + return Optional.empty(); + } + + /** Resolve job from deployment step */ + private JobType toJob(DeploymentSpec.DeclaredZone zone) { + return JobType.from(controller.system(), zone.environment(), zone.region().orElse(null)); + } + + /** Returns whether deployment should be postponed according to delay */ + private boolean postponeDeployment(Duration delay, JobType job, Application application) { + Optional lastSuccess = Optional.ofNullable(application.deploymentJobs().jobStatus().get(job)) + .flatMap(JobStatus::lastSuccess) + .map(JobStatus.JobRun::at); + return lastSuccess.isPresent() && lastSuccess.get().plus(delay).isAfter(clock.instant()); } - private Duration delayAfter(Application application, DeploymentSpec.DeclaredZone zone) { - int stepIndex = application.deploymentSpec().steps().indexOf(zone); + /** Find all steps that deploy to one or more zones */ + private static List deploymentSteps(Application application) { + return application.deploymentSpec().steps().stream() + .filter(step -> step instanceof DeploymentSpec.DeclaredZone || + step instanceof DeploymentSpec.ParallelZones) + .collect(Collectors.toList()); + } + + /** Determines the delay that should pass after the given step */ + private static Duration delayAfter(DeploymentSpec.Step step, Application application) { + int stepIndex = application.deploymentSpec().steps().indexOf(step); if (stepIndex == -1 || stepIndex == application.deploymentSpec().steps().size() - 1) { return Duration.ZERO; } Duration totalDelay = Duration.ZERO; List remainingSteps = application.deploymentSpec().steps() .subList(stepIndex + 1, application.deploymentSpec().steps().size()); - for (DeploymentSpec.Step step : remainingSteps) { - if (!(step instanceof DeploymentSpec.Delay)) { + for (DeploymentSpec.Step s : remainingSteps) { + if (!(s instanceof DeploymentSpec.Delay)) { break; } - totalDelay = totalDelay.plus(((DeploymentSpec.Delay) step).duration()); + totalDelay = totalDelay.plus(((DeploymentSpec.Delay) s).duration()); } return totalDelay; } - private Optional zoneForJob(Application application, DeploymentJobs.JobType jobType) { - return application.deploymentSpec() - .zones() - .stream() - .filter(zone -> zone.deploysTo( - jobType.environment(), - jobType.isProduction() ? jobType.region(controller.system()) : Optional.empty())) - .findFirst(); - } - } 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 1ccbd70d4d2..12c8c508869 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 @@ -64,7 +64,7 @@ public class DeploymentTrigger { application = application.withJobCompletion(report, clock.instant(), controller); // Handle successful first and last job - if (isFirstJob(report.jobType()) && report.success()) { // the first job tells us that a change occurred + if (order.isFirst(report.jobType()) && report.success()) { // the first job tells us that a change occurred if (application.deploying().isPresent() && ! application.deploymentJobs().hasFailures()) { // postpone until the current deployment is done applications().store(application.withOutstandingChange(true), lock); return; @@ -72,7 +72,7 @@ public class DeploymentTrigger { else { // start a new change deployment application = application.withDeploying(Optional.of(Change.ApplicationChange.unknown())); } - } else if (isLastJob(report.jobType(), application) && report.success()) { + } else if (order.isLast(report.jobType(), application) && report.success()) { application = application.withDeploying(Optional.empty()); } @@ -101,7 +101,7 @@ public class DeploymentTrigger { applications().store(application, lock); } else { // retry the failed job (with backoff) - for (JobType jobType : JobType.triggerOrder(controller.system(), application.deploymentSpec())) { // retry the *first* failing job + for (JobType jobType : order.jobsFrom(application.deploymentSpec())) { // retry the *first* failing job JobStatus jobStatus = application.deploymentJobs().jobStatus().get(jobType); if (isFailing(jobStatus)) { if (shouldRetryNow(jobStatus)) { @@ -178,15 +178,6 @@ public class DeploymentTrigger { //--- End of methods which triggers deployment jobs ---------------------------- private ApplicationController applications() { return controller.applications(); } - - private boolean isFirstJob(JobType jobType) { - return jobType == JobType.component; - } - - private boolean isLastJob(JobType jobType, Application application) { - List triggerOrder = JobType.triggerOrder(controller.system(), application.deploymentSpec()); - return triggerOrder.isEmpty() || jobType.equals(triggerOrder.get(triggerOrder.size() - 1)); - } private boolean isFailing(JobStatus jobStatusOrNull) { return jobStatusOrNull != null && !jobStatusOrNull.isSuccess(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 3dddfaf58a1..29b34747573 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -104,8 +104,8 @@ public class ControllerTest { applications.notifyJobCompletion(mockReport(app1, component, true, false)); assertFalse("Revision is currently not known", ((Change.ApplicationChange)tester.controller().applications().require(app1.id()).deploying().get()).revision().isPresent()); - tester.deployAndNotify(systemTest, app1, applicationPackage, true); - tester.deployAndNotify(stagingTest, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, systemTest); + tester.deployAndNotify(app1, applicationPackage, true, stagingTest); assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); Optional revision = ((Change.ApplicationChange)tester.controller().applications().require(app1.id()).deploying().get()).revision(); @@ -120,7 +120,7 @@ public class ControllerTest { tester.clock().advance(Duration.ofSeconds(1)); // production job (failing) - tester.deployAndNotify(productionCorpUsEast1, app1, applicationPackage, false); + tester.deployAndNotify(app1, applicationPackage, false, productionCorpUsEast1); assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); JobStatus expectedJobStatus = JobStatus.initial(productionCorpUsEast1) @@ -144,14 +144,14 @@ public class ControllerTest { // system and staging test job - succeeding applications.notifyJobCompletion(mockReport(app1, component, true, false)); - tester.deployAndNotify(systemTest, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, systemTest); assertStatus(JobStatus.initial(systemTest) .withTriggering(version1, revision, tester.clock().instant()) .withCompletion(Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); - tester.deployAndNotify(stagingTest, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, stagingTest); // production job succeeding now - tester.deployAndNotify(productionCorpUsEast1, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, productionCorpUsEast1); expectedJobStatus = expectedJobStatus .withTriggering(version1, revision, tester.clock().instant()) .withCompletion(Optional.empty(), tester.clock().instant(), tester.controller()); @@ -161,7 +161,7 @@ public class ControllerTest { assertStatus(JobStatus.initial(productionUsEast3) .withTriggering( version1, revision, tester.clock().instant()), app1.id(), tester.controller()); - tester.deployAndNotify(productionUsEast3, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3); assertEquals(5, applications.get(app1.id()).get().deploymentJobs().jobStatus().size()); @@ -189,7 +189,7 @@ public class ControllerTest { .environment(Environment.prod) .region("us-east-3") .build(); - tester.deployAndNotify(systemTest, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, systemTest); assertNull("Zone was removed", applications.require(app1.id()).deployments().get(productionCorpUsEast1.zone(SystemName.main).get())); assertNull("Deployment job was removed", applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1)); @@ -211,9 +211,9 @@ public class ControllerTest { // First deployment: An application change applications.notifyJobCompletion(mockReport(app1, component, true, false)); - tester.deployAndNotify(systemTest, app1, applicationPackage, true); - tester.deployAndNotify(stagingTest, app1, applicationPackage, true); - tester.deployAndNotify(productionUsWest1, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, systemTest); + tester.deployAndNotify(app1, applicationPackage, true, stagingTest); + tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); app1 = applications.require(app1.id()); assertEquals("First deployment gets system version", systemVersion, app1.deployedVersion().get()); @@ -234,9 +234,9 @@ public class ControllerTest { .region("us-east-3") .build(); applications.notifyJobCompletion(mockReport(app1, component, true, false)); - tester.deployAndNotify(systemTest, app1, applicationPackage, true); - tester.deployAndNotify(stagingTest, app1, applicationPackage, true); - tester.deployAndNotify(productionUsWest1, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, systemTest); + tester.deployAndNotify(app1, applicationPackage, true, stagingTest); + tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); app1 = applications.require(app1.id()); assertEquals("Application change preserves version", systemVersion, app1.deployedVersion().get()); @@ -248,7 +248,7 @@ public class ControllerTest { .region("us-west-1") .region("us-east-3") .build(); - tester.deployAndNotify(productionUsEast3, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3); app1 = applications.require(app1.id()); assertEquals("Application change preserves version", systemVersion, app1.deployedVersion().get()); assertEquals(systemVersion, tester.configServerClientMock().lastPrepareVersion.get()); @@ -256,10 +256,10 @@ public class ControllerTest { // Version upgrade changes system version Change.VersionChange change = new Change.VersionChange(newSystemVersion); applications.deploymentTrigger().triggerChange(app1.id(), change); - tester.deployAndNotify(systemTest, app1, applicationPackage, true); - tester.deployAndNotify(stagingTest, app1, applicationPackage, true); - tester.deployAndNotify(productionUsWest1, app1, applicationPackage, true); - tester.deployAndNotify(productionUsEast3, app1, applicationPackage, true); + tester.deployAndNotify(app1, applicationPackage, true, systemTest); + tester.deployAndNotify(app1, applicationPackage, true, stagingTest); + tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); + tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3); app1 = applications.require(app1.id()); assertEquals("Version upgrade changes version", newSystemVersion, app1.deployedVersion().get()); @@ -326,37 +326,37 @@ public class ControllerTest { // Initial failure Instant initialFailure = tester.clock().instant(); tester.notifyJobCompletion(component, app, true); - tester.deployAndNotify(systemTest, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at initial failure", initialFailure, firstFailing(app, tester).get().at()); // Failure again -- failingSince should remain the same tester.clock().advance(Duration.ofMillis(1000)); - tester.deployAndNotify(systemTest, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at second consecutive failure", initialFailure, firstFailing(app, tester).get().at()); // Success resets failingSince tester.clock().advance(Duration.ofMillis(1000)); - tester.deployAndNotify(systemTest, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, systemTest); assertFalse(firstFailing(app, tester).isPresent()); // Complete deployment - tester.deployAndNotify(stagingTest, app, applicationPackage, true); - tester.deployAndNotify(productionCorpUsEast1, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, stagingTest); + tester.deployAndNotify(app, applicationPackage, true, productionCorpUsEast1); // Two repeated failures again. // Initial failure tester.clock().advance(Duration.ofMillis(1000)); initialFailure = tester.clock().instant(); tester.notifyJobCompletion(component, app, true); - tester.deployAndNotify(systemTest, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at initial failure", initialFailure, firstFailing(app, tester).get().at()); // Failure again -- failingSince should remain the same tester.clock().advance(Duration.ofMillis(1000)); - tester.deployAndNotify(systemTest, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at second consecutive failure", initialFailure, firstFailing(app, tester).get().at()); } @@ -435,11 +435,11 @@ public class ControllerTest { // foo: passes system test tester.notifyJobCompletion(component, foo, true); - tester.deployAndNotify(systemTest, foo, applicationPackage, true); + tester.deployAndNotify(foo, applicationPackage, true, systemTest); // bar: passes system test tester.notifyJobCompletion(component, bar, true); - tester.deployAndNotify(systemTest, bar, applicationPackage, true); + tester.deployAndNotify(bar, applicationPackage, true, systemTest); // foo and bar: staging test jobs queued assertEquals(2, buildSystem.jobs().size()); @@ -455,14 +455,14 @@ public class ControllerTest { } // bar: Completes deployment - tester.deployAndNotify(stagingTest, bar, applicationPackage, true); - tester.deployAndNotify(productionCorpUsEast1, bar, applicationPackage, true); + tester.deployAndNotify(bar, applicationPackage, true, stagingTest); + tester.deployAndNotify(bar, applicationPackage, true, productionCorpUsEast1); // foo: 15 minutes pass, staging-test job is still failing due out of capacity, but is no longer re-queued by // out of capacity retry mechanism tester.clock().advance(Duration.ofMinutes(15)); tester.notifyJobCompletion(component, foo, true); - tester.deployAndNotify(systemTest, foo, applicationPackage, true); + tester.deployAndNotify(foo, applicationPackage, true, systemTest); tester.deploy(stagingTest, foo, applicationPackage); assertEquals(1, buildSystem.takeJobsToRun().size()); tester.notifyJobCompletion(stagingTest, foo, Optional.of(JobError.outOfCapacity)); @@ -470,7 +470,7 @@ public class ControllerTest { // bar: New change triggers another staging-test job tester.notifyJobCompletion(component, bar, true); - tester.deployAndNotify(systemTest, bar, applicationPackage, true); + tester.deployAndNotify(bar, applicationPackage, true, systemTest); assertEquals(1, buildSystem.jobs().size()); // foo: 4 hours pass in total, staging-test job is re-queued by periodic trigger mechanism and added at the 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 aa115421f6a..23451c60f08 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 @@ -13,6 +13,7 @@ import java.time.Duration; import java.time.Instant; import java.time.ZoneId; import java.time.format.DateTimeFormatter; +import java.util.Arrays; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -45,6 +46,13 @@ public class ApplicationPackageBuilder { return this; } + public ApplicationPackageBuilder parallel(String... regionName) { + environmentBody.append(" \n"); + Arrays.stream(regionName).forEach(this::region); + environmentBody.append(" \n"); + return this; + } + public ApplicationPackageBuilder delay(Duration delay) { environmentBody.append(" j.jobName().equals(DeploymentJobs.JobType.productionUsEast3.id()))); // Test environments pass - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); // Production job fails again and exhausts all immediate retries - tester.deployAndNotify(DeploymentJobs.JobType.productionUsEast3, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.productionUsEast3); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsEast3, app, false); @@ -92,7 +92,7 @@ public class FailureRedeployerTest { assertEquals("Job is retried", 1, tester.buildSystem().jobs().size()); // Production job finally succeeds - tester.deployAndNotify(DeploymentJobs.JobType.productionUsEast3, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); assertFalse("No failures", tester.application(app.id()).deploymentJobs().hasFailures()); } @@ -108,7 +108,7 @@ public class FailureRedeployerTest { Application app = tester.createApplication("app1", "tenant1", 1, 11L); tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); // staging-test starts, but does not complete assertEquals(DeploymentJobs.JobType.stagingTest.id(), tester.buildSystem().takeJobsToRun().get(0).jobName()); @@ -139,9 +139,9 @@ public class FailureRedeployerTest { Application app = tester.createApplication("app1", "tenant1", 1, 11L); tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.productionUsEast3, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); // New version is released version = Version.fromString("5.1"); @@ -151,7 +151,7 @@ public class FailureRedeployerTest { assertEquals("Application has pending upgrade to " + version, version, tester.versionChange(app.id()).get().version()); // system-test fails and exhausts all immediate retries - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.systemTest); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, app, false); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index a832a591217..3244307e91c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -84,7 +84,7 @@ public class MetricsReporterTest { // 1 app fails system-test tester.notifyJobCompletion(component, app4, true); - tester.deployAndNotify(systemTest, app4, applicationPackage, false); + tester.deployAndNotify(app4, applicationPackage, false, systemTest); metricsReporter.maintain(); assertEquals(25.0, metricsMock.getMetric(MetricsReporter.deploymentFailMetric)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index e5afcec87ad..e047a288fb9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -224,9 +224,9 @@ public class UpgraderTest { Application app = tester.createApplication("app1", "tenant1", 1, 11L); tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.productionUsEast3, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); tester.upgrader().maintain(); assertEquals("Application is on expected version: Nothing to do", 0, @@ -239,10 +239,10 @@ public class UpgraderTest { tester.upgrader().maintain(); // system-test completes successfully - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); // staging-test fails multiple times, exhausts retries and failure is recorded - tester.deployAndNotify(DeploymentJobs.JobType.stagingTest, app, applicationPackage, false); + tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.stagingTest); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, app, false); @@ -282,17 +282,17 @@ public class UpgraderTest { // Application is on 5.0 Application app = tester.createApplication("app1", "tenant1", 1, 11L); tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.productionCorpUsEast1, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionCorpUsEast1); // Canary in prod.corp-us-east-1 is upgraded to controller version tester.upgrader().maintain(); assertEquals("Upgrade started", 1, tester.buildSystem().jobs().size()); assertEquals(Vtag.currentVersion, ((Change.VersionChange) tester.application(app.id()).deploying().get()).version()); - tester.deployAndNotify(DeploymentJobs.JobType.systemTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); - tester.deployAndNotify(DeploymentJobs.JobType.productionCorpUsEast1, app, applicationPackage, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionCorpUsEast1); // System is upgraded to newer version, no upgrade triggered for canary as version is lower than controller version = Version.fromString("5.1"); -- cgit v1.2.3 From c56715fb7e7f163da95bf527d01149aaf0104e8f Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 31 Aug 2017 12:12:14 +0200 Subject: Move zones() up to simplify --- .../config/application/api/DeploymentSpec.java | 19 ++++++++------- .../controller/deployment/DeploymentOrder.java | 28 +++++++--------------- 2 files changed, 20 insertions(+), 27 deletions(-) (limited to 'controller-server') 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 fc7344aa9e8..8530679e82f 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 @@ -15,6 +15,7 @@ import java.io.Reader; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -76,13 +77,9 @@ public class DeploymentSpec { private void validateZones(List steps) { Set zones = new HashSet<>(); - steps.stream().filter(step -> step instanceof DeclaredZone) - .map(DeclaredZone.class::cast) - .forEach(zone -> ensureUnique(zone, zones)); - steps.stream().filter(step -> step instanceof ParallelZones) - .map(ParallelZones.class::cast) - .flatMap(parallelZones -> parallelZones.zones().stream()) - .forEach(zone -> ensureUnique(zone, zones)); + for (Step step : steps) + for (DeclaredZone zone : step.zones()) + ensureUnique(zone, zones); } private void ensureUnique(DeclaredZone zone, Set zones) { @@ -320,6 +317,9 @@ public class DeploymentSpec { /** Returns whether this step deploys to the given environment, and (if specified) region */ public abstract boolean deploysTo(Environment environment, Optional region); + /** Returns the zones deployed to in this step */ + public List zones() { return Collections.emptyList(); } + } /** A deployment step which is to wait for some time before progressing to the next step */ @@ -369,6 +369,9 @@ public class DeploymentSpec { /** Returns whether this zone should receive production traffic */ public boolean active() { return active; } + @Override + public List zones() { return Collections.singletonList(this); } + @Override public boolean deploysTo(Environment environment, Optional region) { if (environment != this.environment) return false; @@ -407,7 +410,7 @@ public class DeploymentSpec { this.zones = ImmutableList.copyOf(zones); } - /** The list of zones to deploy in */ + @Override public List zones() { return this.zones; } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index afa9c219048..45979b597f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -47,7 +47,7 @@ public class DeploymentOrder { // At this point we have deployed to system test, so deployment spec is available List deploymentSteps = deploymentSteps(application); Optional currentStep = fromJob(job, application); - if (!currentStep.isPresent()) { + if ( ! currentStep.isPresent()) { return Collections.emptyList(); } @@ -65,15 +65,9 @@ public class DeploymentOrder { } DeploymentSpec.Step nextStep = deploymentSteps.get(currentIndex + 1); - if (nextStep instanceof DeploymentSpec.DeclaredZone) { - return Collections.singletonList(toJob((DeploymentSpec.DeclaredZone) nextStep)); - } else if (nextStep instanceof DeploymentSpec.ParallelZones) { - return ((DeploymentSpec.ParallelZones) nextStep).zones().stream() - .map(this::toJob) - .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); - } else { - throw new IllegalStateException("Unexpected step type: " + nextStep.getClass()); - } + return nextStep.zones().stream() + .map(this::toJob) + .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } /** Returns whether the given job is first in a deployment */ @@ -96,15 +90,11 @@ public class DeploymentOrder { if (deploymentSpec.steps().isEmpty()) { return Arrays.asList(JobType.systemTest, JobType.stagingTest); } - List jobs = new ArrayList<>(); - for (DeploymentSpec.Step step : deploymentSpec.steps()) { - if (step instanceof DeploymentSpec.DeclaredZone) { - jobs.add(toJob((DeploymentSpec.DeclaredZone) step)); - } else if (step instanceof DeploymentSpec.ParallelZones) { - ((DeploymentSpec.ParallelZones) step).zones().forEach(zone -> jobs.add(toJob(zone))); - } - } - return Collections.unmodifiableList(jobs); + + return deploymentSpec.steps().stream() + .flatMap(step -> step.zones().stream()) + .map(this::toJob) + .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } /** Resolve deployment step from job */ -- cgit v1.2.3 From 5395f6ab37d3e1c43b2091895adaab11ce4aa50d Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 31 Aug 2017 13:04:39 +0200 Subject: Fix test --- .../controller/deployment/DeploymentOrder.java | 8 ++---- .../deployment/DeploymentTriggerTest.java | 31 +++++++++++++++------- 2 files changed, 23 insertions(+), 16 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index 45979b597f0..b20a459b04a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -9,10 +9,9 @@ import com.yahoo.vespa.hosted.controller.application.JobStatus; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -32,6 +31,7 @@ public class DeploymentOrder { private final Clock clock; public DeploymentOrder(Controller controller) { + Objects.requireNonNull(controller, "controller cannot be null"); this.controller = controller; this.clock = controller.clock(); } @@ -87,10 +87,6 @@ public class DeploymentOrder { /** Returns jobs for given deployment spec, in the order they are declared */ public List jobsFrom(DeploymentSpec deploymentSpec) { - if (deploymentSpec.steps().isEmpty()) { - return Arrays.asList(JobType.systemTest, JobType.stagingTest); - } - return deploymentSpec.steps().stream() .flatMap(step -> step.zones().stream()) .map(this::toJob) 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 7b658dac6f9..c26337bc922 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 @@ -6,7 +6,7 @@ import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.Change; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import org.junit.Test; @@ -24,31 +24,42 @@ public class DeploymentTriggerTest { @Test public void testTriggerFailing() { DeploymentTester tester = new DeploymentTester(); - Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); + Application app = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .upgradePolicy("default") .environment(Environment.prod) .region("us-west-1") .build(); - app1 = app1.with(app1.deploymentJobs().asSelfTriggering(false)); - tester.applications().store(app1, tester.applications().lock(app1.id())); - Version version = new Version(5, 2); - tester.deploymentTrigger().triggerChange(app1.id(), new Change.VersionChange(version)); - tester.deployAndNotify(app1, applicationPackage, false, JobType.systemTest); + Version version = new Version(5, 1); + tester.updateVersionStatus(version); + tester.upgrader().maintain(); + + // Deploy completely once + tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); + tester.deployAndNotify(app, applicationPackage, true, JobType.productionUsWest1); + + // New version is released + version = new Version(5, 2); + tester.updateVersionStatus(version); + tester.upgrader().maintain(); + + tester.deployAndNotify(app, applicationPackage, false, JobType.systemTest); assertEquals("Retried immediately", 1, tester.buildSystem().jobs().size()); tester.buildSystem().takeJobsToRun(); assertEquals("Job removed", 0, tester.buildSystem().jobs().size()); tester.clock().advance(Duration.ofHours(2)); - tester.deploymentTrigger().triggerFailing(app1.id(), "unit test"); + tester.failureRedeployer().maintain(); assertEquals("Retried job", 1, tester.buildSystem().jobs().size()); assertEquals(JobType.systemTest.id(), tester.buildSystem().jobs().get(0).jobName()); tester.buildSystem().takeJobsToRun(); assertEquals("Job removed", 0, tester.buildSystem().jobs().size()); - tester.clock().advance(Duration.ofHours(7)); - tester.deploymentTrigger().triggerFailing(app1.id(), "unit test"); + tester.clock().advance(Duration.ofHours(12).plus(Duration.ofSeconds(1))); + tester.failureRedeployer().maintain(); assertEquals("Retried from the beginning", 1, tester.buildSystem().jobs().size()); assertEquals(JobType.component.id(), tester.buildSystem().jobs().get(0).jobName()); } -- cgit v1.2.3 From 976bbfa36d5f700fb87dc2a1da93980a797b6879 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 31 Aug 2017 14:14:16 +0200 Subject: Handle jobs that complete out of order --- .../controller/application/DeploymentJobs.java | 7 +++++ .../controller/deployment/DeploymentTrigger.java | 2 +- .../deployment/DeploymentTriggerTest.java | 33 +++++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 19d60c18998..1d1d07d9513 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -126,6 +126,13 @@ public class DeploymentJobs { } return true; // other environments do not have any preconditions } + + /** Returns whether change has been deployed completely */ + public boolean isDeployed(Change change) { + return status.values().stream() + .filter(status -> status.type().isProduction()) + .allMatch(status -> isSuccessful(status.type(), change)); + } /** Returns the oldest failingSince time of the jobs of this, or null if none are failing */ public Instant failingSince() { 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 12c8c508869..5f529d21995 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 @@ -72,7 +72,7 @@ public class DeploymentTrigger { else { // start a new change deployment application = application.withDeploying(Optional.of(Change.ApplicationChange.unknown())); } - } else if (order.isLast(report.jobType(), application) && report.success()) { + } else if (order.isLast(report.jobType(), application) && report.success() && application.deploymentJobs().isDeployed(application.deploying().get())) { application = application.withDeploying(Optional.empty()); } 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 c26337bc922..298cc1982d3 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 @@ -13,6 +13,7 @@ import org.junit.Test; import java.time.Duration; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; /** @@ -50,7 +51,7 @@ public class DeploymentTriggerTest { assertEquals("Retried immediately", 1, tester.buildSystem().jobs().size()); tester.buildSystem().takeJobsToRun(); - assertEquals("Job removed", 0, tester.buildSystem().jobs().size()); + assertEquals("Job removed", 0, tester.buildSystem().jobs().size()); tester.clock().advance(Duration.ofHours(2)); tester.failureRedeployer().maintain(); assertEquals("Retried job", 1, tester.buildSystem().jobs().size()); @@ -181,6 +182,36 @@ public class DeploymentTriggerTest { assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); } + @Test + public void parallelDeploymentCompletesOutOfOrder() { + DeploymentTester tester = new DeploymentTester(); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .parallel("us-east-3", "us-west-1") + .build(); + + Application app = tester.createApplication("app1", "tenant1", 1, 11L); + tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + + // Test environments pass + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); + + // Parallel deployment + tester.deploy(DeploymentJobs.JobType.productionUsWest1, app, applicationPackage); + tester.deploy(DeploymentJobs.JobType.productionUsEast3, app, applicationPackage); + + // Last declared job completes first + tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsWest1, app, true); + assertTrue("Change is present as not all jobs are complete", + tester.applications().require(app.id()).deploying().isPresent()); + + // All jobs complete + tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsEast3, app, true); + assertFalse("Change has been deployed", + tester.applications().require(app.id()).deploying().isPresent()); + } + @Test public void testSuccessfulDeploymentApplicationPackageChanged() { DeploymentTester tester = new DeploymentTester(); -- cgit v1.2.3 From 21696e097b4e72e76f9563fd627add48557c2505 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 31 Aug 2017 15:34:53 +0200 Subject: Ensure all parallel deployments complete before next step --- .../controller/application/DeploymentJobs.java | 21 +++++++++++---------- .../controller/deployment/DeploymentOrder.java | 19 ++++++++++++++++++- .../controller/deployment/DeploymentTester.java | 1 - .../deployment/DeploymentTriggerTest.java | 17 +++++++++++++---- 4 files changed, 42 insertions(+), 16 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 1d1d07d9513..02e0d94920e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -120,9 +120,9 @@ public class DeploymentJobs { return true; } if (environment == Environment.staging) { - return isSuccessful(JobType.systemTest, change.get()); + return isSuccessful(change.get(), JobType.systemTest); } else if (environment == Environment.prod) { - return isSuccessful(JobType.stagingTest, change.get()); + return isSuccessful(change.get(), JobType.stagingTest); } return true; // other environments do not have any preconditions } @@ -131,7 +131,15 @@ public class DeploymentJobs { public boolean isDeployed(Change change) { return status.values().stream() .filter(status -> status.type().isProduction()) - .allMatch(status -> isSuccessful(status.type(), change)); + .allMatch(status -> isSuccessful(change, status.type())); + } + + /** Returns whether job has completed successfully */ + public boolean isSuccessful(Change change, JobType jobType) { + return Optional.ofNullable(jobStatus().get(jobType)) + .filter(JobStatus::isSuccess) + .filter(status -> status.lastCompletedFor(change)) + .isPresent(); } /** Returns the oldest failingSince time of the jobs of this, or null if none are failing */ @@ -154,13 +162,6 @@ public class DeploymentJobs { public Optional jiraIssueId() { return jiraIssueId; } - private boolean isSuccessful(JobType jobType, Change change) { - return Optional.ofNullable(jobStatus().get(jobType)) - .filter(JobStatus::isSuccess) - .filter(status -> status.lastCompletedFor(change)) - .isPresent(); - } - /** Job types that exist in the build system */ public enum JobType { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index b20a459b04a..448ab419853 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -57,6 +57,11 @@ public class DeploymentOrder { return Collections.emptyList(); } + // Postpone if step hasn't completed all it's jobs for this change + if (!completedSuccessfully(currentStep.get(), application)) { + return Collections.emptyList(); + } + // Postpone next job if delay has not passed yet Duration delay = delayAfter(currentStep.get(), application); if (postponeDeployment(delay, job, application)) { @@ -88,11 +93,23 @@ public class DeploymentOrder { /** Returns jobs for given deployment spec, in the order they are declared */ public List jobsFrom(DeploymentSpec deploymentSpec) { return deploymentSpec.steps().stream() - .flatMap(step -> step.zones().stream()) + .flatMap(step -> jobsFrom(step).stream()) + .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + } + + /** Returns jobs for the given step */ + private List jobsFrom(DeploymentSpec.Step step) { + return step.zones().stream() .map(this::toJob) .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } + /** Returns whether all jobs have completed successfully for given step */ + private boolean completedSuccessfully(DeploymentSpec.Step step, Application application) { + return jobsFrom(step).stream() + .allMatch(job -> application.deploymentJobs().isSuccessful(application.deploying().get(), job)); + } + /** Resolve deployment step from job */ private Optional fromJob(JobType job, Application application) { for (DeploymentSpec.Step step : application.deploymentSpec().steps()) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index e04ff8576e3..9b05101b5eb 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -179,7 +179,6 @@ public class DeploymentTester { assertEquals(job.id(), buildJob.get().jobName()); } buildSystem().removeJobs(application.id()); - } private Optional findJob(Application application, JobType jobType) { 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 298cc1982d3..7ed0ad843cc 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 @@ -158,6 +158,7 @@ public class DeploymentTriggerTest { .environment(Environment.prod) .region("us-central-1") .parallel("us-west-1", "us-east-3") + .region("eu-west-1") .build(); // Component job finishes @@ -171,14 +172,22 @@ public class DeploymentTriggerTest { assertEquals(1, tester.buildSystem().jobs().size()); tester.deployAndNotify(application, applicationPackage, true, JobType.productionUsCentral1); - // The two next regions are triggered in parallel + // Deploys in two regions in parallel assertEquals(2, tester.buildSystem().jobs().size()); assertEquals(JobType.productionUsEast3.id(), tester.buildSystem().jobs().get(0).jobName()); assertEquals(JobType.productionUsWest1.id(), tester.buildSystem().jobs().get(1).jobName()); + tester.buildSystem().takeJobsToRun(); + + tester.deploy(JobType.productionUsWest1, application, applicationPackage, false); + tester.notifyJobCompletion(JobType.productionUsWest1, application, true); + assertTrue("No more jobs triggered at this time", tester.buildSystem().jobs().isEmpty()); - // Deployment completes - tester.deployAndNotify(application, applicationPackage, true, JobType.productionUsWest1, - JobType.productionUsEast3); + tester.deploy(JobType.productionUsEast3, application, applicationPackage, false); + tester.notifyJobCompletion(JobType.productionUsEast3, application, true); + + // Last region completes + assertEquals(1, tester.buildSystem().jobs().size()); + tester.deployAndNotify(application, applicationPackage, true, JobType.productionEuWest1); assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); } -- cgit v1.2.3