Merge pull request #4002 from vespa-engine/bratseth/start-systemtests-when-ready

Start the first job in triggerReady

2 files changed, 39 insertions, 4 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 0a3e6a1beb8..e5c7b7f1c2f 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
@@ -127,17 +127,33 @@ public class DeploymentTrigger {
         for (Application application : applications.asList()) {
             try (Lock lock = applications().lock(application.id())) {
                 Optional<LockedApplication> lockedApplication = controller.applications().get(application.id(), lock);
-                if (!lockedApplication.isPresent()) continue; // application removed
+                if ( ! lockedApplication.isPresent()) continue; // application removed
                 triggerReadyJobs(lockedApplication.get());
             }
         }
     }
-    
+
+    /** Find the next step to trigger if any, and triggers it */
     private void triggerReadyJobs(LockedApplication application) {
         if ( ! application.deploying().isPresent()) return;
-        for (JobType jobType : order.jobsFrom(application.deploymentSpec())) {
+        List<JobType> jobs =  order.jobsFrom(application.deploymentSpec());
+
+        // Should the first step be triggered?
+        if ( ! jobs.isEmpty() && jobs.get(0).equals(JobType.systemTest) &&
+             application.deploying().get() instanceof Change.VersionChange) {
+            Version target = ((Change.VersionChange)application.deploying().get()).version();
+            JobStatus jobStatus = application.deploymentJobs().jobStatus().get(JobType.systemTest);
+            if (jobStatus == null || ! jobStatus.lastTriggered().isPresent() 
+                || ! jobStatus.lastTriggered().get().version().equals(target)) {
+                application = trigger(JobType.systemTest, application, false, "Upgrade to " + target);
+                controller.applications().store(application);
+            }
+        }
+
+        // Find next steps to trigger based on the state of the previous step
+        for (JobType jobType : jobs) {
             JobStatus jobStatus = application.deploymentJobs().jobStatus().get(jobType);
-            if (jobStatus == null) continue; // never run
+            if (jobStatus == null) continue; // job has never run
             if (jobStatus.isRunning(jobTimeoutLimit())) continue;
 
             // Collect the subset of next jobs which have not run with the last changes
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 f73373ac718..022fa705def 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
@@ -6,9 +6,11 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
+import com.yahoo.vespa.hosted.controller.LockedApplication;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
+import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.maintenance.BlockedChangeDeployer;
@@ -17,6 +19,7 @@ import org.junit.Test;
 
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -310,6 +313,21 @@ public class DeploymentTriggerTest {
         BuildService.BuildJob productionJob = tester.buildSystem().takeJobsToRun().get(0);
         assertEquals("production-us-west-1", productionJob.jobName());
     }
+    
+    @Test
+    public void testUpgradingButNoJobStarted() {
+        DeploymentTester tester = new DeploymentTester();
+        BlockedChangeDeployer blockedChangeDeployer = new BlockedChangeDeployer(tester.controller(),
+                                                                                Duration.ofHours(1),
+                                                                                new JobControl(tester.controllerTester().curator()));
+        LockedApplication app = (LockedApplication)tester.createAndDeploy("default0", 3, "default");
+        // Store that we are upgrading but don't start the system-tests job
+        tester.controller().applications().store(app.withDeploying(Optional.of(new Change.VersionChange(Version.fromString("6.2")))));
+        assertEquals(0, tester.buildSystem().jobs().size());
+        blockedChangeDeployer.run();
+        assertEquals(1, tester.buildSystem().jobs().size());
+        assertEquals("system-test", tester.buildSystem().jobs().get(0).jobName());
+    }
 
     @Test
     public void testHandleMultipleNotificationsFromLastJob() {
@@ -336,4 +354,5 @@ public class DeploymentTriggerTest {
                     tester.applications().require(application.id()).deploying().isPresent());
         assertTrue("All jobs consumed", buildSystem.jobs().isEmpty());
     }
+
 }