Trigger next on completion through triggerReadyJobs

10 files changed, 44 insertions, 43 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 6c954191075..767ffbaa7ea 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
@@ -109,9 +109,8 @@ public class DeploymentTrigger {
             // TODO jvenstad: Don't trigger.
             // Trigger next
             if (report.success()) {
-                List<JobType> jobs = order.nextAfter(report.jobType(), application);
-                for (JobType job : jobs)
-                     application = trigger(new Triggering(application, job, false, report.jobType().jobName() + " completed"), jobs, false);
+                triggerReadyJobs(application);
+                return; // Don't overwrite below.
             }
             else if (retryBecauseOutOfCapacity(application, report.jobType())) {
                 triggerReadyJobs(application);
@@ -138,8 +137,6 @@ public class DeploymentTrigger {
 
     /** Find the next step to trigger if any, and triggers it */
     public void triggerReadyJobs(LockedApplication application) {
-        if ( ! application.change().isPresent()) return;
-
         List<JobType> jobs =  order.jobsFrom(application.deploymentSpec());
 
         // Should the first step be triggered?
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 50bf20c4405..895ba195b08 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
@@ -146,11 +146,13 @@ public class ControllerTest {
         tester.jobCompletion(productionCorpUsEast1).application(app1).unsuccessful().submit();
 
         // system and staging test job - succeeding
-        tester.jobCompletion(component).application(app1).submit();
+        tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
+        applicationVersion = tester.application("app1").change().application().get();
         tester.deployAndNotify(app1, applicationPackage, true, false, systemTest);
         assertStatus(JobStatus.initial(systemTest)
                               .withTriggering(version1, applicationVersion, "", tester.clock().instant().minus(Duration.ofMillis(1)))
-                              .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller());
+                              .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()),
+                     app1.id(), tester.controller());
         tester.deployAndNotify(app1, applicationPackage, true, stagingTest);
 
         // production job succeeding now
@@ -186,7 +188,7 @@ public class ControllerTest {
         JobStatus jobStatus = applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1);
         assertNotNull("Deployment job was not removed", jobStatus);
         assertEquals(42, jobStatus.lastCompleted().get().id());
-        assertEquals("staging-test completed", jobStatus.lastCompleted().get().reason());
+        assertEquals("Available change in staging-test", jobStatus.lastCompleted().get().reason());
 
         // prod zone removal is allowed with override
         applicationPackage = new ApplicationPackageBuilder()
@@ -661,7 +663,7 @@ public class ControllerTest {
                     .environment(Environment.prod)
                     .allow(ValidationId.deploymentRemoval)
                     .build();
-            tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit();
+            tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
             tester.deployAndNotify(app1, applicationPackage, true, systemTest);
             tester.applications().deactivate(app1, ZoneId.from(Environment.test, RegionName.from("us-east-1")));
             tester.applications().deactivate(app1, ZoneId.from(Environment.staging, RegionName.from("us-east-3")));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java
index bb8840a7b1f..f1e6108710d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java
@@ -104,6 +104,7 @@ public class BuildJob {
     /** Upload given application package to artifact repository as part of this job */
     public BuildJob uploadArtifact(ApplicationPackage applicationPackage) {
         Objects.requireNonNull(job, "job cannot be null");
+        Objects.requireNonNull(applicationId, "applicationId cannot be null");
         if (job != DeploymentJobs.JobType.component) {
             throw new IllegalStateException(job + " cannot upload artifact");
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java
index 155c9f14f77..548f7c33fa1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java
@@ -134,7 +134,7 @@ public class DeploymentIssueReporterTest {
 
 
         // app3 now has a new failure past max failure age; see that a new issue is filed.
-        tester.jobCompletion(component).application(app3).submit();
+        tester.jobCompletion(component).application(app3).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app3, applicationPackage, false, systemTest);
         tester.clock().advance(maxInactivity.plus(maxFailureAge));
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
index 5ed4c3a186e..f6216856317 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
@@ -74,7 +74,7 @@ public class DnsMaintainerTest {
                 .environment(Environment.prod)
                 .allow(ValidationId.deploymentRemoval)
                 .build();
-        tester.jobCompletion(component).application(application).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(application).nextBuildNumber().uploadArtifact(applicationPackage).submit();
 
         tester.deployAndNotify(application, applicationPackage, true, systemTest);
         tester.applications().deactivate(application, ZoneId.from(Environment.test, RegionName.from("us-east-1")));
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 088895408fc..021fe0954c2 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
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
+import com.yahoo.vespa.hosted.controller.deployment.BuildJob;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import org.junit.Test;
@@ -152,7 +153,7 @@ public class UpgraderTest {
 
         // --- Failing application is repaired by changing the application, causing confidence to move above 'high' threshold
         // Deploy application change
-        tester.deployCompletely("default0");
+        tester.deployCompletely(tester.application("default0"), DeploymentTester.applicationPackage("default"), BuildJob.defaultBuildNumber + 1);
 
         tester.updateVersionStatus(version);
         assertEquals(VespaVersion.Confidence.high, tester.controller().versionStatus().systemVersion().get().confidence());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json
index 63832531c7d..31949cce282 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json
@@ -1,6 +1,6 @@
 {
   "id": "tenant1:app1:default",
-  "deploymentSpecField": "<deployment version='1.0'>\n  <test />\n  <staging />\n  <prod>\n     <region active=\"true\">cd-us-central-1</region>\n     <region active=\"true\">cd-us-central-2</region>\n  </prod>\n</deployment>\n",
+  "deploymentSpecField": "<deployment version='1.0'>\n  <test />\n  <staging />\n  <prod>\n     <region active=\"true\">us-central-1</region>\n     <region active=\"true\">us-west-1</region>\n  </prod>\n</deployment>\n",
   "validationOverrides": "<deployment version='1.0'/>",
   "deployments": [],
   "deploymentJobs": {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
index 9cd9329e36f..06d562d40f2 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
@@ -48,7 +48,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -62,7 +62,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -76,7 +76,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       }
     },
@@ -94,7 +94,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -108,7 +108,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -122,7 +122,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       }
     },
@@ -140,7 +140,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -154,7 +154,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -168,7 +168,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       }
     },
@@ -186,7 +186,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "production-us-west-1 completed",
+        "reason": "Available change in production-us-west-1",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -200,7 +200,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "production-us-west-1 completed",
+        "reason": "Available change in production-us-west-1",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -214,7 +214,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "production-us-west-1 completed",
+        "reason": "Available change in production-us-west-1",
         "at": "(ignore)"
       }
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
index f75f24aa74b..a067d19fb6a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
@@ -58,7 +58,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -72,7 +72,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -86,7 +86,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       }
     },
@@ -104,7 +104,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -118,7 +118,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -132,7 +132,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       }
     },
@@ -150,7 +150,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -164,7 +164,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       },
       "firstFailing": {
@@ -178,7 +178,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       }
     },
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
index 3109cce5731..3c4cc32111c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
@@ -58,7 +58,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -72,7 +72,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -86,7 +86,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "component completed",
+        "reason": "Available change in component",
         "at": "(ignore)"
       }
     },
@@ -104,7 +104,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -118,7 +118,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -132,7 +132,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "system-test completed",
+        "reason": "Available change in system-test",
         "at": "(ignore)"
       }
     },
@@ -150,7 +150,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -164,7 +164,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       },
       "firstFailing": {
@@ -178,7 +178,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "staging-test completed",
+        "reason": "Available change in staging-test",
         "at": "(ignore)"
       }
     },