diff options
2 files changed, 11 insertions, 17 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 0e32b55ae9b..bb9323ddbd8 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 @@ -50,7 +50,6 @@ import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.Job import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component; import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.systemTest; -import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.Comparator.comparing; import static java.util.Comparator.naturalOrder; @@ -286,7 +285,7 @@ public class DeploymentTrigger { application.deploymentJobs().statusOf(stagingTest) .<Instant>flatMap(job -> job.lastSuccess().map(JobRun::at))); String reason = "New change available"; - List<Job> testJobs = null; // null means "uninitialised", while empty means "don't run any jobs". + List<Job> testJobs = Collections.emptyList(); if (change.isPresent()) for (Step step : productionSteps) { @@ -300,10 +299,8 @@ public class DeploymentTrigger { && jobStateContains(application, job, idle) && stepJobs.containsAll(runningProductionJobs(application))) jobs.add(deploymentJob(application, versions, change, job, reason, completedAt.get())); - if ( ! alreadyTriggered(application, versions)) - testJobs = emptyList(); } - else if (testJobs == null) { + else if (testJobs.isEmpty()) { testJobs = testJobs(application, versions, String.format("Testing deployment for %s (%s)", job.jobName(), versions.toString()), completedAt.orElse(clock.instant())); } @@ -322,7 +319,7 @@ public class DeploymentTrigger { } } } - if (testJobs == null) + if (testJobs.isEmpty()) testJobs = testJobs(application, versions(application, application.change(), Optional .empty()), "Testing last changes outside prod", clock.instant()); jobs.addAll(testJobs); 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 1c5e686fab1..31fa78d4b5f 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 @@ -528,6 +528,7 @@ public class DeploymentTriggerTest { Supplier<Application> app = () -> tester.application(application.id()); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) + .region("us-central-1") .parallel("eu-west-1", "us-east-3") .build(); // Application version 42 and platform version 6.1. @@ -540,8 +541,12 @@ public class DeploymentTriggerTest { tester.upgradeSystem(v2); tester.deployAndNotify(application, empty(), true, systemTest); tester.deployAndNotify(application, empty(), true, stagingTest); + tester.deployAndNotify(application, empty(), true, productionUsCentral1); tester.deploymentTrigger().cancelChange(application.id(), true); tester.deploy(productionEuWest1, application, applicationPackage); + tester.deployAndNotify(application, applicationPackage, false, productionEuWest1); + tester.deployAndNotify(application, applicationPackage, false, productionUsEast3); + assertEquals(v2, app.get().deployments().get(productionUsCentral1.zone(main).get()).version()); assertEquals(v2, app.get().deployments().get(productionEuWest1.zone(main).get()).version()); assertEquals(v1, app.get().deployments().get(productionUsEast3.zone(main).get()).version()); @@ -551,26 +556,18 @@ public class DeploymentTriggerTest { tester.deployAndNotify(application, empty(), true, systemTest); assertEquals(v2, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); tester.deployAndNotify(application, empty(), true, stagingTest); - - // Tests are not re-triggered, because eu-west-1 needs their success to trigger - assertEquals(v2, app.get().deploymentJobs().jobStatus().get(systemTest).lastTriggered().get().platform()); - assertEquals(v2, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); - - // Finish old runs of the production jobs, which fail. - tester.deployAndNotify(application, applicationPackage, false, productionEuWest1); - tester.deployAndNotify(application, applicationPackage, false, productionUsEast3); - - // New upgrade is already tested for eu-west-1, and tests may now test the upgrade for us-east-3 + tester.deployAndNotify(application, empty(), true, productionUsCentral1); assertEquals(v1, app.get().deploymentJobs().jobStatus().get(systemTest).lastTriggered().get().platform()); tester.deployAndNotify(application, empty(), true, systemTest); assertEquals(v1, app.get().deploymentJobs().jobStatus().get(stagingTest).lastTriggered().get().platform()); tester.deployAndNotify(application, empty(), true, stagingTest); - tester.deployAndNotify(application, empty(), false, productionEuWest1); // The production job on version 6.2 fails and must retry -- this is OK, even though staging now has a different version. + tester.deployAndNotify(application, empty(), false, productionEuWest1); tester.deployAndNotify(application, empty(), true, productionUsEast3); tester.deployAndNotify(application, empty(), true, productionEuWest1); assertFalse(app.get().change().isPresent()); + assertEquals(43, app.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastSuccess().get().application().buildNumber().get().longValue()); assertEquals(43, app.get().deploymentJobs().jobStatus().get(productionEuWest1).lastSuccess().get().application().buildNumber().get().longValue()); assertEquals(43, app.get().deploymentJobs().jobStatus().get(productionUsEast3).lastSuccess().get().application().buildNumber().get().longValue()); } |