Add and use completeRollout() instead of hard coded jobs

7 files changed, 73 insertions, 56 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java
index 9881002659d..510171512f9 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java
@@ -20,7 +20,7 @@ import static com.yahoo.config.provision.SystemName.main;
 /** Job types that exist in the build system */
 public enum JobType {
 //     | enum name ------------| job name ------------------| Zone in main system ---------------------------------------| Zone in CD system -------------------------------------------
-    component              ("component",
+    component              ("component", // TODO jonmv: remove when no longer present in serialized data
                             Map.of()),
 
     systemTest             ("system-test",
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 f4ec1d26d8e..a39880b9125 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
@@ -322,13 +322,6 @@ public class DeploymentTrigger {
 
     /** Returns the set of all jobs which have changes to propagate from the upstream steps. */
     private List<Job> computeReadyJobs() {
-        ApplicationList applications = ApplicationList.from(applications().asList());
-        applications = applications.withProjectId();
-        applications = applications.withChanges();
-        var jobs = applications.idList().stream()
-                .map(this::computeReadyJobs)
-                .flatMap(Collection::stream)
-                .collect(Collectors.toList());
         return ApplicationList.from(applications().asList())
                            .withProjectId()
                            .withChanges()
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 2d1188e2f29..fb1d6e7ad38 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -197,6 +197,14 @@ public class JobController {
         locked(id, run -> run.with(testerCertificate));
     }
 
+    /** Returns a list of all applications which have registered. */
+    public List<TenantAndApplicationId> applications() {
+        return copyOf(controller.applications().asList().stream()
+                                .filter(Application::internal)
+                                .map(Application::id)
+                                .iterator());
+    }
+
     /** Returns a list of all instances of applications which have registered. */
     public List<ApplicationId> instances() {
         return copyOf(controller.applications().asList().stream()
@@ -241,12 +249,19 @@ public class JobController {
 
     /** Returns a list of all active runs. */
     public List<Run> active() {
-        return copyOf(instances().stream()
-                                 .flatMap(id -> Stream.of(JobType.values())
-                                                         .map(type -> last(id, type))
-                                                         .flatMap(Optional::stream)
-                                                         .filter(run -> ! run.hasEnded()))
-                                 .iterator());
+        return copyOf(applications().stream()
+                                    .flatMap(id -> active(id).stream())
+                                    .iterator());
+    }
+
+    /** Returns a list of all active runs for the given instance. */
+    public List<Run> active(TenantAndApplicationId id) {
+        return copyOf(controller.applications().requireApplication(id).instances().keySet().stream()
+                                .flatMap(name -> Stream.of(JobType.values())
+                                                       .map(type -> last(id.instance(name), type))
+                                                       .flatMap(Optional::stream)
+                                                       .filter(run -> ! run.hasEnded()))
+                                .iterator());
     }
 
     /** Changes the status of the given step, for the given run, provided it is still active. */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index 6f7b94acfad..5b5eaa60ce9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -5,7 +5,7 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
-import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.log.LogLevel;
 import com.yahoo.security.KeyAlgorithm;
 import com.yahoo.security.KeyUtils;
@@ -13,22 +13,24 @@ import com.yahoo.security.SignatureAlgorithm;
 import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.controller.Application;
-import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.ApplicationController;
+import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
+import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint;
+import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGeneratorMock;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud;
-import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
-import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock;
+import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock;
-import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGeneratorMock;
 import com.yahoo.vespa.hosted.controller.maintenance.JobControl;
 import com.yahoo.vespa.hosted.controller.maintenance.JobRunner;
 import com.yahoo.vespa.hosted.controller.maintenance.JobRunnerTest;
@@ -40,14 +42,18 @@ import java.security.cert.X509Certificate;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.logging.Logger;
 
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 public class InternalDeploymentTester {
@@ -144,24 +150,31 @@ public class InternalDeploymentTester {
                                                                                          zone.environment().value()))));
     }
 
-    /**
-     * Completely deploys a new submission and returns the new version.
-     */
-    public ApplicationVersion deployNewSubmission() {
-        ApplicationVersion applicationVersion = newSubmission();
+    /** Runs and returns all remaining jobs for the application, at most once, and asserts the current change is rolled out. */
+    public List<JobType> completeRollout() {
+        tester.readyJobTrigger().run();
+        Set<JobType> jobs = new HashSet<>();
+        List<Run> activeRuns;
+        while ( ! (activeRuns = jobs().active(appId)).isEmpty())
+            for (Run run : activeRuns)
+                if (jobs.add(run.id().type())) {
+                    runJob(run.id().type());
+                    tester.readyJobTrigger().run();
+                }
+                else
+                    throw new AssertionError("Job '" + run.id().type() + "' was run twice for '" + instanceId + "'");
+
+        assertFalse("Change should have no targets, but was " + application().change(), application().change().hasTargets());
+        return List.copyOf(jobs);
+    }
 
+    /** Completely deploys the given application version, assuming it is the last to be submitted. */
+    public void deployNewSubmission(ApplicationVersion version) {
         assertFalse(instance().deployments().values().stream()
-                              .anyMatch(deployment -> deployment.applicationVersion().equals(applicationVersion)));
-        assertEquals(applicationVersion, application().change().application().get());
+                              .anyMatch(deployment -> deployment.applicationVersion().equals(version)));
+        assertEquals(version, application().change().application().get());
         assertFalse(application().change().platform().isPresent());
-
-        runJob(JobType.systemTest);
-        runJob(JobType.stagingTest);
-        runJob(JobType.productionUsCentral1);
-        runJob(JobType.productionUsWest1);
-        runJob(JobType.productionUsEast3);
-
-        return applicationVersion;
+        completeRollout();
     }
 
     /**
@@ -174,22 +187,16 @@ public class InternalDeploymentTester {
         assertEquals(version, application().change().platform().get());
         assertFalse(application().change().application().isPresent());
 
-        runJob(JobType.systemTest);
-        runJob(JobType.stagingTest);
-        runJob(JobType.productionUsCentral1);
-        runJob(JobType.productionUsWest1);
-        runJob(JobType.productionUsEast3);
+        completeRollout();
+
         assertTrue(instance().productionDeployments().values().stream()
                              .allMatch(deployment -> deployment.version().equals(version)));
-        assertTrue(tester.configServer().nodeRepository()
-                         .list(JobType.productionAwsUsEast1a.zone(tester.controller().system()), instanceId).stream()
-                         .allMatch(node -> node.currentVersion().equals(version)));
-        assertTrue(tester.configServer().nodeRepository()
-                         .list(JobType.productionUsEast3.zone(tester.controller().system()), instanceId).stream()
-                         .allMatch(node -> node.currentVersion().equals(version)));
-        assertTrue(tester.configServer().nodeRepository()
-                         .list(JobType.productionUsEast3.zone(tester.controller().system()), instanceId).stream()
-                         .allMatch(node -> node.currentVersion().equals(version)));
+
+        for (JobType type : new DeploymentSteps(application().deploymentSpec(), tester.controller()::system).productionJobs())
+            assertTrue(tester.configServer().nodeRepository()
+                             .list(type.zone(tester.controller().system()), instanceId).stream()
+                             .allMatch(node -> node.currentVersion().equals(version)));
+
         assertFalse(application().change().hasTargets());
     }
 
@@ -203,7 +210,7 @@ public class InternalDeploymentTester {
                       .findAny()
                       .orElseThrow(() -> new AssertionError(type + " is not among the active: " + jobs.active()));
         assertFalse(run.hasFailed());
-        assertNotSame(aborted, run.status());
+        assertNotEquals(aborted, run.status());
 
         ZoneId zone = type.zone(tester.controller().system());
         DeploymentId deployment = new DeploymentId(instanceId, zone);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index 4a3554b8408..eed685cc20a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -83,7 +83,7 @@ public class InternalStepRunnerTest {
 
     @Test
     public void canRegisterAndRunDirectly() {
-        tester.deployNewSubmission();
+        tester.deployNewSubmission(tester.newSubmission());
 
         tester.deployNewPlatform(new Version("7.1"));
     }
@@ -100,8 +100,8 @@ public class InternalStepRunnerTest {
         tester.tester().upgradeSystem(new Version("7.1"));
         tester.tester().buildService().clear();
 
-        tester.deployNewSubmission();
-        tester.deployNewSubmission();
+        tester.deployNewSubmission(tester.newSubmission());
+        tester.deployNewSubmission(tester.newSubmission());
 
         tester.deployNewPlatform(new Version("7.2"));
 
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 44785407874..0ea4abc2203 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
@@ -186,7 +186,8 @@ public class MetricsReporterTest {
     @Test
     public void test_build_time_reporting() {
         InternalDeploymentTester tester = new InternalDeploymentTester();
-        ApplicationVersion version = tester.deployNewSubmission();
+        ApplicationVersion version = tester.newSubmission();
+        tester.deployNewSubmission(version);
         assertEquals(1000, version.buildTime().get().toEpochMilli());
 
         MetricsReporter reporter = createReporter(tester.tester().controller());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java
index 6cb808133e3..b4b4045819e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java
@@ -54,7 +54,8 @@ public class JobControllerApiHandlerHelperTest {
         tester.clock().setInstant(Instant.EPOCH);
 
         // Revision 1 gets deployed everywhere.
-        ApplicationVersion revision1 = tester.deployNewSubmission();
+        ApplicationVersion revision1 = tester.newSubmission();
+        tester.deployNewSubmission(revision1);
         assertEquals(2, tester.application().projectId().getAsLong());
 
         tester.clock().advance(Duration.ofMillis(1000));