Test that force triggering triggers prerequisites

4 files changed, 86 insertions, 56 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 80a325e3dc8..0006845b888 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
@@ -57,6 +57,7 @@ import static java.util.Optional.empty;
 import static java.util.stream.Collectors.groupingBy;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.partitioningBy;
+import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
 /**
@@ -226,17 +227,21 @@ public class DeploymentTrigger {
                &&   buildService.isRunning(BuildJob.of(application.id(), application.deploymentJobs().projectId().getAsLong(), jobType.jobName()));
     }
 
-    public void forceTrigger(ApplicationId applicationId, JobType jobType) {
+    public List<JobType> forceTrigger(ApplicationId applicationId, JobType jobType) {
         Application application = applications().require(applicationId);
         if (jobType == component) {
             buildService.trigger(BuildJob.of(applicationId, application.deploymentJobs().projectId().getAsLong(), jobType.jobName()));
-            return;
+            return singletonList(component);
         }
         State target = targetFor(application, application.change(), jobType);
-        if (isVerified(application, target, jobType))
-            trigger(deploymentJob(application, target, application.change(), jobType, ">:o:< Triggered by force! (-o-) |-o-| (=oo=) ", clock.instant(), Collections.emptySet()));
-        else
-            ; // TODO jvenstad: Test it!
+        String reason = ">:o:< Triggered by force! (-o-) |-o-| (=oo=)";
+        if (isVerified(application, target, jobType)) {
+            trigger(deploymentJob(application, target, application.change(), jobType, reason, clock.instant(), Collections.emptySet()));
+            return singletonList(jobType);
+        }
+        List<Job> testJobs = testJobsFor(application, target, reason, clock.instant());
+        testJobs.forEach(this::trigger);
+        return testJobs.stream().map(Job::jobType).collect(toList());
     }
 
     private Job deploymentJob(Application application, State target, Change change, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith) {
@@ -267,26 +272,38 @@ public class DeploymentTrigger {
      * Finds the next step to trigger for the given application, if any, and returns these as a list.
      */
     private List<Job> computeReadyJobs(ApplicationId id) {
+        /*
+            find testJobs
+            find productionSteps
+
+            completedAt = EPOCH
+
+
+
+
+
+
+
+
+
+
+         */
+
+
+
+
+
         List<Job> jobs = new ArrayList<>();
         applications().get(id).ifPresent(application -> {
             List<Step> steps = application.deploymentSpec().steps().isEmpty()
                     ? singletonList(new DeploymentSpec.DeclaredZone(test))
                     : application.deploymentSpec().steps();
-            List<Step> productionSteps = new ArrayList<>(steps);
-            productionSteps.removeIf(step -> ! step.deploysTo(prod) && ! step.zones().isEmpty());
-            List<Step> testSteps = new ArrayList<>(steps);
-            testSteps.removeAll(productionSteps);
+            List<Step> productionSteps = steps.stream().filter(step -> step.deploysTo(prod) || step.zones().isEmpty()).collect(toList());
 
-            boolean complete = ! productionSteps.isEmpty();
-            Optional<Instant> completedAt = application.deploymentJobs().statusOf(stagingTest).flatMap(JobStatus::lastSuccess).map(JobRun::at);
+            Optional<Instant> completedAt = Optional.of(Instant.EPOCH); // Delays before first production job are ignored.
             String reason = "New change available";
-            State testTarget = null;
-            Instant testAvailableSince = null;
-            JobType testFor = null;
+            List<Job> testJobs = null;
 
-            // Loop through all production steps and find
-            //   a) any incomplete and ready production jobs which are already verified, and / or
-            //   b) any tests which are required to verify the first incomplete production job
             for (Step step : productionSteps) {
                 Set<JobType> stepJobs = step.zones().stream().map(order::toJob).collect(toSet());
                 Map<Optional<Instant>, List<JobType>> jobsByCompletion = stepJobs.stream().collect(groupingBy(job -> completedAt(application.change(), application, job)));
@@ -297,16 +314,9 @@ public class DeploymentTrigger {
                             if (completedAt.isPresent())
                                 jobs.add(deploymentJob(application, target, application.change(), job, reason, completedAt.get(), stepJobs));
                         }
-                        else if (testTarget == null) {
-                            testTarget = target;
-                            testAvailableSince = completedAt.orElse(clock.instant());
-                            testFor = job;
-                        }
+                        else if (testJobs == null)
+                            testJobs = testJobsFor(application, target, "Testing deployment for " + job.jobName(), completedAt.orElse(clock.instant()));
                     }
-
-                    complete = false;
-                    completedAt = empty();
-                    break;
                 }
                 else { // All jobs are complete -- find the time of completion of this step.
                     if (stepJobs.isEmpty()) { // No jobs means this is delay step.
@@ -321,29 +331,34 @@ public class DeploymentTrigger {
                 }
             }
 
-            if (productionSteps.isEmpty()) {
-                testTarget = targetFor(application, application.change(), systemTest);
-                testFor = systemTest;
-                testAvailableSince = clock.instant();
-            }
-            if (testTarget != null)
-                for (Step step : testSteps) {
-                    for (JobType jobType : step.zones().stream().map(order::toJob).collect(Collectors.toList())) {
-                        Optional<JobRun> completion = successOn(application, jobType, testTarget.targetPlatform, testTarget.targetApplication);
-                        if (completion.isPresent())
-                            testAvailableSince = completion.get().at();
-                        else if (isVerified(application, testTarget, jobType))
-                            jobs.add(deploymentJob(application, testTarget, application.change(), jobType, "Testing deployment for " + testFor.jobName(), testAvailableSince, emptySet()));
-                    }
-                }
+            if (testJobs == null)
+                testJobs = testJobsFor(application, targetFor(application, application.change(), systemTest), "Testing last changes outside prod", clock.instant());
+            jobs.addAll(testJobs);
 
             // TODO jvenstad: Replace with completion of individual parts of Change.
-            if (complete)
+            if (steps.stream().flatMap(step -> step.zones().stream()).map(order::toJob)
+                     .allMatch(job -> completedAt(application.change(), application, job).isPresent()))
                 applications().lockIfPresent(id, lockedApplication -> applications().store(lockedApplication.withChange(Change.empty())));
         });
         return jobs;
     }
 
+    private List<Job> testJobsFor(Application application, State target, String reason, Instant availableSince) {
+        List<Step> steps = application.deploymentSpec().steps();
+        if (steps.isEmpty()) steps = singletonList(new DeploymentSpec.DeclaredZone(test));
+        List<Job> jobs = new ArrayList<>();
+        for (Step step : steps.stream().filter(step -> step.deploysTo(test) || step.deploysTo(staging)).collect(toList())) {
+            for (JobType jobType : step.zones().stream().map(order::toJob).collect(toList())) {
+                Optional<JobRun> completion = successOn(application, jobType, target.targetPlatform, target.targetApplication);
+                if (completion.isPresent())
+                    availableSince = completion.get().at();
+                else if (isVerified(application, target, jobType))
+                    jobs.add(deploymentJob(application, target, application.change(), jobType, reason, availableSince, emptySet()));
+            }
+        }
+        return jobs;
+    }
+
     private boolean isVerified(Application application, State state, JobType jobType) {
         if (jobType.environment() == staging)
             return successOn(application, systemTest, state.targetPlatform, state.targetApplication).isPresent();
@@ -403,7 +418,7 @@ public class DeploymentTrigger {
         return application.deploymentJobs().jobStatus().keySet().parallelStream()
                           .filter(job -> job.isProduction())
                           .filter(job -> isRunning(application, job))
-                          .collect(Collectors.toList());
+                          .collect(toList());
     }
 
     private ApplicationController applications() {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
index 6a5201f4fdb..3139a7efb29 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.restapi.screwdriver;
 
+import com.google.common.base.Joiner;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
@@ -28,6 +29,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.joining;
 
 /**
  * This API lists deployment jobs that are queued for execution on Screwdriver.
@@ -88,11 +90,13 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler {
                                   .map(JobType::fromJobName)
                                   .orElse(JobType.component);
 
-        controller.applications().deploymentTrigger().forceTrigger(ApplicationId.from(tenantName, applicationName, "default"), jobType);
+        String triggered = controller.applications().deploymentTrigger()
+                                     .forceTrigger(ApplicationId.from(tenantName, applicationName, "default"), jobType)
+                                     .stream().map(JobType::jobName).collect(joining(", "));
 
         Slime slime = new Slime();
         Cursor cursor = slime.setObject();
-        cursor.setString("message", "Triggered " + jobType.jobName() + " for " + tenantName + "." + applicationName);
+        cursor.setString("message", "Triggered " + triggered + " for " + tenantName + "." + applicationName);
         return new SlimeJsonResponse(slime);
     }
 
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 59ba9f33c56..fc4b8cb9621 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
@@ -120,7 +120,6 @@ public class DeploymentTriggerTest {
 
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)
-                .delay(Duration.ofSeconds(30))
                 .region("us-west-1")
                 .delay(Duration.ofMinutes(2))
                 .delay(Duration.ofMinutes(2)) // Multiple delays are summed up
@@ -133,14 +132,8 @@ public class DeploymentTriggerTest {
 
         // Test jobs pass
         tester.deployAndNotify(application, applicationPackage, true, JobType.systemTest);
-        tester.clock().advance(Duration.ofSeconds(1)); // Make staging test sort as the last successful job
         tester.deployAndNotify(application, applicationPackage, true, JobType.stagingTest);
-        assertTrue("No more jobs triggered at this time", mockBuildService.jobs().isEmpty());
-
-        // 30 seconds pass, us-west-1 is triggered
-        tester.clock().advance(Duration.ofSeconds(30));
         tester.deploymentTrigger().triggerReadyJobs();
-
         assertEquals(1, mockBuildService.jobs().size());
         tester.assertRunning(application.id(), productionUsWest1);
 
@@ -383,11 +376,15 @@ public class DeploymentTriggerTest {
         tester.jobCompletion(productionUsCentral1).application(application).unsuccessful().submit();
         tester.deployAndNotify(application, empty(), true, systemTest);
         tester.deployAndNotify(application, empty(), true, stagingTest);
-        tester.deployAndNotify(application, empty(), true, productionUsCentral1);
+        tester.deployAndNotify(application, empty(), false, productionUsCentral1);
 
         // The last job has a different target, and the tests need to run again.
+        // These may now start, since the first job has been triggered once, and thus is verified already.
         tester.deployAndNotify(application, empty(), true, systemTest);
         tester.deployAndNotify(application, empty(), true, stagingTest);
+
+        // Finally, the two production jobs complete, in order.
+        tester.deployAndNotify(application, empty(), true, productionUsCentral1);
         tester.deployAndNotify(application, empty(), true, productionEuWest1);
         assertEquals(appVersion1, app.get().deployments().get(ZoneId.from("prod.us-central-1")).applicationVersion());
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java
index 91d5d687531..a0cbf39e5ab 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java
@@ -13,6 +13,7 @@ import org.junit.Test;
 
 import java.io.File;
 import java.nio.charset.StandardCharsets;
+import java.util.Optional;
 import java.util.OptionalLong;
 
 /**
@@ -67,12 +68,25 @@ public class ScrewdriverApiTest extends ControllerContainerTest {
                                                                                                 Optional.of(new SourceRevision("repo", "branch", "commit")),
                                                                                                 Optional.empty()));
 
-        // Triggers specific job when given -- fails here because the job has never run before, and so application version can't be resolved.
+        // Triggers specific job when given -- triggers the prerequisites here, since they are not yet fulfilled.
+        assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/" +
+                                   app.id().tenant().value() + "/application/" + app.id().application().value(),
+                                   "staging-test".getBytes(StandardCharsets.UTF_8), Request.Method.POST),
+                       200, "{\"message\":\"Triggered system-test for tenant1.application1\"}");
+
+        tester.controller().applications().deploymentTrigger().notifyOfCompletion(new JobReport(app.id(),
+                                                                                                DeploymentJobs.JobType.systemTest,
+                                                                                                1,
+                                                                                                42,
+                                                                                                Optional.empty(),
+                                                                                                Optional.empty()));
+
+        // Triggers specific job when given, and when it is verified.
         assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/" +
                                    app.id().tenant().value() + "/application/" + app.id().application().value(),
                                    "staging-test".getBytes(StandardCharsets.UTF_8), Request.Method.POST),
                        200, "{\"message\":\"Triggered staging-test for tenant1.application1\"}");
-        // TODO jvenstad: This should trigger system-test instead, unless they are allowed to run in parallel.
+
     }
 
 }