Store non-canonical reasons for runs, and avoid auto-aborting these from dep.orch.

7 files changed, 33 insertions, 25 deletions

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 805d727d355..6035cd274bf 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
@@ -1988,9 +1988,9 @@ public class DeploymentTriggerTest {
         app.submit();
         tester.triggerJobs();
 
-        tester.deploymentTrigger().reTrigger(app.instanceId(), productionUsEast3);
-        tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3")));
-        tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3")));
+        tester.deploymentTrigger().reTrigger(app.instanceId(), productionUsEast3, null);
+        tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3")), null);
+        tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3")), null);
 
         List<RetriggerEntry> retriggerEntries = tester.controller().curator().readRetriggerEntries();
         Assert.assertEquals(1, retriggerEntries.size());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
index d55b5cea4ee..5f443759048 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
@@ -72,7 +72,7 @@ public class DeploymentExpirerTest {
         // Dev application expires when enough time has passed since most recent attempt
         // Redeployments done by DeploymentUpgrader do not affect this
         tester.clock().advance(Duration.ofDays(12).plus(Duration.ofSeconds(1)));
-        tester.jobs().start(devApp.instanceId(), JobType.devUsEast1, lastRun.versions(), true);
+        tester.jobs().start(devApp.instanceId(), JobType.devUsEast1, lastRun.versions(), true, Optional.of("upgrade"));
         expirer.maintain();
         assertEquals(0, permanentDeployments(devApp.instance()));
         assertEquals(1, permanentDeployments(prodApp.instance()));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
index a339cde2b89..71a6fcb1d84 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
@@ -5,6 +5,7 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId;
+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.application.pkg.ApplicationPackage;
@@ -95,13 +96,13 @@ public class JobRunnerTest {
         ApplicationId id = appId.defaultInstance();
         jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
 
-        jobs.start(id, systemTest, versions);
+        start(jobs, id, systemTest);
         try {
-            jobs.start(id, systemTest, versions);
+            start(jobs, id, systemTest);
             fail("Job is already running, so this should not be allowed!");
         }
         catch (IllegalArgumentException ignored) { }
-        jobs.start(id, stagingTest, versions);
+        start(jobs, id, stagingTest);
 
         assertTrue(jobs.last(id, systemTest).get().stepStatuses().values().stream().allMatch(unfinished::equals));
         assertFalse(jobs.last(id, systemTest).get().hasEnded());
@@ -127,7 +128,7 @@ public class JobRunnerTest {
         jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
         Supplier<Run> run = () -> jobs.last(id, systemTest).get();
 
-        jobs.start(id, systemTest, versions);
+        start(jobs, id, systemTest);
         RunId first = run.get().id();
 
         Map<Step, Status> steps = run.get().stepStatuses();
@@ -188,7 +189,7 @@ public class JobRunnerTest {
         assertSame(aborted, run.get().status());
 
         // A new run is attempted.
-        jobs.start(id, systemTest, versions);
+        start(jobs, id, systemTest);
         assertEquals(first.number() + 1, run.get().id().number());
 
         // Run fails on tester deployment -- remaining run-always steps succeed, and the run finishes.
@@ -206,7 +207,7 @@ public class JobRunnerTest {
         assertEquals(2, jobs.runs(id, systemTest).size());
 
         // Start a third run, then unregister and wait for data to be deleted.
-        jobs.start(id, systemTest, versions);
+        start(jobs, id, systemTest);
         tester.applications().deleteInstance(id);
         runner.maintain();
         assertFalse(jobs.last(id, systemTest).isPresent());
@@ -234,7 +235,7 @@ public class JobRunnerTest {
         jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
 
         RunId runId = new RunId(id, systemTest, 1);
-        jobs.start(id, systemTest, versions);
+        start(jobs, id, systemTest);
         runner.maintain();
         barrier.await();
         try {
@@ -272,14 +273,14 @@ public class JobRunnerTest {
         assertFalse(jobs.lastSuccess(jobId).isPresent());
 
         for (int i = 0; i < jobs.historyLength(); i++) {
-            jobs.start(instanceId, systemTest, versions);
+            start(jobs, instanceId, systemTest);
             runner.run();
         }
 
         assertEquals(64, jobs.runs(jobId).size());
         assertTrue(jobs.details(new RunId(instanceId, systemTest, 1)).isPresent());
 
-        jobs.start(instanceId, systemTest, versions);
+        start(jobs, instanceId, systemTest);
         runner.run();
 
         assertEquals(64, jobs.runs(jobId).size());
@@ -291,7 +292,7 @@ public class JobRunnerTest {
 
         // Make all but the oldest of the 54 jobs a failure.
         for (int i = 0; i < jobs.historyLength() - 1; i++) {
-            jobs.start(instanceId, systemTest, versions);
+            start(jobs, instanceId, systemTest);
             failureRunner.run();
         }
         assertEquals(64, jobs.runs(jobId).size());
@@ -300,7 +301,7 @@ public class JobRunnerTest {
         assertEquals(66, jobs.firstFailing(jobId).get().id().number());
 
         // Oldest success is kept even though it would normally overflow.
-        jobs.start(instanceId, systemTest, versions);
+        start(jobs, instanceId, systemTest);
         failureRunner.run();
         assertEquals(65, jobs.runs(jobId).size());
         assertEquals(65, jobs.runs(jobId).keySet().iterator().next().number());
@@ -308,7 +309,7 @@ public class JobRunnerTest {
         assertEquals(66, jobs.firstFailing(jobId).get().id().number());
 
         // First failure after the last success is also kept.
-        jobs.start(instanceId, systemTest, versions);
+        start(jobs, instanceId, systemTest);
         failureRunner.run();
         assertEquals(66, jobs.runs(jobId).size());
         assertEquals(65, jobs.runs(jobId).keySet().iterator().next().number());
@@ -317,7 +318,7 @@ public class JobRunnerTest {
         assertEquals(66, jobs.firstFailing(jobId).get().id().number());
 
         // No other jobs are kept with repeated failures.
-        jobs.start(instanceId, systemTest, versions);
+        start(jobs, instanceId, systemTest);
         failureRunner.run();
         assertEquals(66, jobs.runs(jobId).size());
         assertEquals(65, jobs.runs(jobId).keySet().iterator().next().number());
@@ -327,7 +328,7 @@ public class JobRunnerTest {
         assertEquals(66, jobs.firstFailing(jobId).get().id().number());
 
         // history length returns to 256 when a new success is recorded.
-        jobs.start(instanceId, systemTest, versions);
+        start(jobs, instanceId, systemTest);
         runner.run();
         assertEquals(64, jobs.runs(jobId).size());
         assertEquals(69, jobs.runs(jobId).keySet().iterator().next().number());
@@ -365,7 +366,7 @@ public class JobRunnerTest {
         ApplicationId id = appId.defaultInstance();
         jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
 
-        jobs.start(id, systemTest, versions);
+        start(jobs, id, systemTest);
         tester.clock().advance(JobRunner.jobTimeout.plus(Duration.ofSeconds(1)));
         runner.run();
         assertSame(aborted, jobs.last(id, systemTest).get().status());
@@ -388,7 +389,7 @@ public class JobRunnerTest {
         for (RunStatus status : RunStatus.values()) {
             if (status == success || status == reset) continue; // Status not used for steps.
             outcomes.put(deployTester, status);
-            jobs.start(id, systemTest, versions);
+            start(jobs, id, systemTest);
             runner.run();
             jobs.finish(jobs.last(id, systemTest).get().id());
         }
@@ -410,6 +411,10 @@ public class JobRunnerTest {
         assertEquals(1, metric.getMetric(context::equals, JobMetrics.testFailure).get().intValue());
     }
 
+    private void start(JobController jobs, ApplicationId id, JobType type) {
+        jobs.start(id, type, versions, false, Optional.empty());
+    }
+
     public static ExecutorService inThreadExecutor() {
         return new AbstractExecutorService() {
             final AtomicBoolean shutDown = new AtomicBoolean(false);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java
index 0c1b3b39b65..ccb2b6ebb74 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java
@@ -38,10 +38,10 @@ public class RetriggerMaintainerTest {
         devApp.completeRollout();
 
         // Trigger a run (to simulate a running job)
-        tester.deploymentTrigger().reTrigger(applicationId, JobType.devUsEast1);
+        tester.deploymentTrigger().reTrigger(applicationId, JobType.devUsEast1, null);
 
         // Add a job to the queue
-        tester.deploymentTrigger().reTriggerOrAddToQueue(devApp.deploymentIdIn(ZoneId.from("dev", "us-east-1")));
+        tester.deploymentTrigger().reTriggerOrAddToQueue(devApp.deploymentIdIn(ZoneId.from("dev", "us-east-1")), null);
 
         // Should be 1 entry in the queue:
         List<RetriggerEntry> retriggerEntries = tester.controller().curator().readRetriggerEntries();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java
index f4f52b20325..237d54db20c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java
@@ -99,6 +99,7 @@ public class RunSerializerTest {
         assertEquals(applicationVersion.compileVersion(), run.versions().targetApplication().compileVersion());
         assertEquals("f00bad", run.versions().targetApplication().commit().get());
         assertEquals("https://github.com/user/repo/tree/f00bad", run.versions().targetApplication().sourceUrl().get());
+        assertEquals("because", run.reason().get());
         assertEquals(new Version(1, 2, 2), run.versions().sourcePlatform().get());
         assertEquals(ApplicationVersion.from(new SourceRevision("git@github.com:user/repo.git",
                                                                 "master",
@@ -153,8 +154,9 @@ public class RunSerializerTest {
         assertEquals(run.versions(), phoenix.versions());
         assertEquals(run.steps(), phoenix.steps());
         assertEquals(run.isDryRun(), phoenix.isDryRun());
+        assertEquals(run.reason(), phoenix.reason());
 
-        Run initial = Run.initial(id, run.versions(), run.isRedeployment(), run.start(), JobProfile.production);
+        Run initial = Run.initial(id, run.versions(), run.isRedeployment(), run.start(), JobProfile.production, Optional.empty());
         assertEquals(initial, serializer.runFromSlime(serializer.toSlime(initial)));
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json
index 54cde2bacef..85881fbfdbc 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json
@@ -53,6 +53,7 @@
         "build": 122,
         "deployedDirectly": false
       }
-    }
+    },
+    "reason": "because"
   }
 ]
\ No newline at end of file
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java
index b830a4ce512..cf6453235d3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java
@@ -49,7 +49,7 @@ public class BadgeApiTest extends ControllerContainerTest {
         for (int i = 0; i < 31; i++)
             application.failDeployment(JobType.productionUsWest1);
         application.triggerJobs();
-        tester.controller().applications().deploymentTrigger().reTrigger(application.instanceId(), JobType.testEuWest1);
+        tester.controller().applications().deploymentTrigger().reTrigger(application.instanceId(), JobType.testEuWest1, "reason");
 
         tester.assertResponse(authenticatedRequest("http://localhost:8080/badge/v1/tenant/application/default"),
                               Files.readString(Paths.get(responseFiles + "overview.svg")), 200);