Trigger blocked jobs in maintenance

6 files changed, 80 insertions, 16 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 bf0d7c0c5c5..c00b8c6173f 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
@@ -3,11 +3,13 @@ package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ApplicationController;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.application.ApplicationList;
 import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError;
@@ -107,6 +109,58 @@ public class DeploymentTrigger {
     }
 
     /**
+     * Find jobs that can and should run but is currently not.
+     */
+    public void triggerReadyJobs() {
+        ApplicationList applications = ApplicationList.from(applications().asList());
+        applications = applications.notPullRequest();
+        for (Application application : applications.asList()) {
+            try (Lock lock = applications().lock(application.id())) {
+                triggerReadyJobs(application, lock);
+            }
+        }
+    }
+    
+    private void triggerReadyJobs(Application application, Lock lock) {
+        if ( ! application.deploying().isPresent()) return;
+        for (JobType jobType : order.jobsFrom(application.deploymentSpec())) {
+            // TODO: Do this for all jobs not just staging, and (with more work) remove triggerFailing and triggerDelayed
+            if (jobType.environment().equals(Environment.staging)) {
+                JobStatus jobStatus = application.deploymentJobs().jobStatus().get(jobType);
+                if (jobStatus.isRunning(clock.instant().minus(jobTimeout))) continue;
+
+                for (JobType nextJobType : order.nextAfter(jobType, application)) {
+                    JobStatus nextStatus = application.deploymentJobs().jobStatus().get(nextJobType);
+                    
+                    // Attempt to trigger if there are changes available - is rejected if the change is in progress,
+                    // or is currently blocked
+                    if (changesAvailable(jobStatus, nextStatus))
+                        trigger(nextJobType, application, false, "Triggering previously blocked job", lock);
+                }
+                
+            }
+        }
+    }
+
+    /**
+     * Returns true if the previous job has completed successfully with a revision and/or version which is
+     * newer (different) than the one last completed successfully in next
+     */
+    private boolean changesAvailable(JobStatus previous, JobStatus next) {
+        if ( ! previous.lastSuccess().isPresent()) return false;
+        if (next == null) return true;
+        if ( ! next.lastSuccess().isPresent()) return true;
+        
+        JobStatus.JobRun previousSuccess = previous.lastSuccess().get();
+        JobStatus.JobRun nextSuccess = next.lastSuccess().get();
+        if (previousSuccess.revision().isPresent() &&  ! previousSuccess.revision().get().equals(nextSuccess.revision().get()))
+            return true;
+        if (! previousSuccess.version().equals(nextSuccess.version()))
+            return true;
+        return false;
+    }
+    
+    /**
      * Called periodically to cause triggering of jobs in the background
      */
     public void triggerFailing(ApplicationId applicationId) {
@@ -292,9 +346,6 @@ public class DeploymentTrigger {
             return application;
         }
 
-        // TODO: Review code for retrying when no job is running but a change is in progress
-        //       or add more ...
-        
         // TODO: Remove when we can determine why this occurs
         if (jobType != JobType.component && ! application.deploying().isPresent()) {
             log.warning(String.format("Want to trigger %s for %s with reason %s, but this application is not " +
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BlockedChangeDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BlockedChangeDeployer.java
index a59e86967d1..4a68fd6cfab 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BlockedChangeDeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/BlockedChangeDeployer.java
@@ -13,6 +13,7 @@ import java.time.Duration;
  * 
  * @author bratseth
  */
+@SuppressWarnings("unused")
 public class BlockedChangeDeployer extends Maintainer {
     
     public BlockedChangeDeployer(Controller controller, Duration interval, JobControl jobControl) {
@@ -21,13 +22,7 @@ public class BlockedChangeDeployer extends Maintainer {
 
     @Override
     protected void maintain() {
-        ApplicationList applications = ApplicationList.from(controller().applications().asList()).notPullRequest();
-        applications = applications.notDeploying();
-        applications = applications.hasUndeployedSuccessfulRevisionChange();
-        for (Application application : applications.asList()) {
-//            controller().applications().deploymentTrigger().triggerChange(application.id(),
-//                                                                          Change.ApplicationChange.of());
-        }
+        controller().applications().deploymentTrigger().triggerReadyJobs();
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index 1b692ecf243..7788821f04d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -32,6 +32,7 @@ public class ControllerMaintenance extends AbstractComponent {
     private final VersionStatusUpdater versionStatusUpdater;
     private final Upgrader upgrader;
     private final DelayedDeployer delayedDeployer;
+    private final BlockedChangeDeployer blockedChangeDeployer;
 
     @SuppressWarnings("unused") // instantiated by Dependency Injection
     public ControllerMaintenance(MaintainerConfig maintainerConfig, Controller controller, CuratorDb curator,
@@ -48,6 +49,7 @@ public class ControllerMaintenance extends AbstractComponent {
         versionStatusUpdater = new VersionStatusUpdater(controller, Duration.ofMinutes(3), jobControl);
         upgrader = new Upgrader(controller, maintenanceInterval, jobControl, curator);
         delayedDeployer = new DelayedDeployer(controller, maintenanceInterval, jobControl);
+        blockedChangeDeployer = new BlockedChangeDeployer(controller, maintenanceInterval, jobControl);
     }
 
     public Upgrader upgrader() { return upgrader; }
@@ -65,6 +67,7 @@ public class ControllerMaintenance extends AbstractComponent {
         versionStatusUpdater.deconstruct();
         upgrader.deconstruct();
         delayedDeployer.deconstruct();
+        blockedChangeDeployer.deconstruct();
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobControl.java
index e05612aaf57..d7396cb2acb 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobControl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobControl.java
@@ -13,14 +13,12 @@ import java.util.logging.Logger;
  * Provides status and control over running maintenance jobs.
  * This is multithread safe.
  * 
- * Job deactivation is stored in a local file.
+ * Job deactivation is stored in zookeeper.
  * 
  * @author bratseth
  */
 public class JobControl {
     
-    private static final Logger log = Logger.getLogger(JobControl.class.getName());
-
     private final CuratorDb curator;
 
     /** This is not stored in ZooKeeper as all nodes start all jobs */
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 912aa9f2e1a..65ed5eeb95b 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
@@ -7,9 +7,12 @@ 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.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.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
+import com.yahoo.vespa.hosted.controller.maintenance.BlockedChangeDeployer;
+import com.yahoo.vespa.hosted.controller.maintenance.JobControl;
 import org.junit.Test;
 
 import java.time.Duration;
@@ -263,6 +266,9 @@ public class DeploymentTriggerTest {
     public void testBlockRevisionChange() {
         ManualClock clock = new ManualClock(Instant.parse("2017-09-26T17:30:00.00Z")); // Tuesday, 17:30
         DeploymentTester tester = new DeploymentTester(new ControllerTester(clock));
+        BlockedChangeDeployer blockedChangeDeployer = new BlockedChangeDeployer(tester.controller(),
+                                                                                Duration.ofHours(1),
+                                                                                new JobControl(tester.controllerTester().curator()));
         Version version = Version.fromString("5.0");
         tester.updateVersionStatus(version);
 
@@ -274,7 +280,10 @@ public class DeploymentTriggerTest {
 
         Application app = tester.createAndDeploy("app1", 1, applicationPackageBuilder.build());
 
-        tester.clock().advance(Duration.ofHours(1)); // Enter block window: 18:30
+        tester.clock().advance(Duration.ofHours(1)); // --------------- Enter block window: 18:30
+        
+        blockedChangeDeployer.run();
+        assertEquals(0, tester.buildSystem().jobs().size());
         
         String searchDefinition =
                 "search test {\n" +
@@ -287,9 +296,14 @@ public class DeploymentTriggerTest {
 
         tester.deployTestOnly(app, changedApplication);
 
-        tester.clock().advance(Duration.ofHours(2)); // Exit block window: 20:30
+        blockedChangeDeployer.run();
+        assertEquals(0, tester.buildSystem().jobs().size());
 
-        tester.deployCompletely(app, changedApplication);
+        tester.clock().advance(Duration.ofHours(2)); // ---------------- Exit block window: 20:30
+        tester.deploymentTrigger().triggerReadyJobs(); // Schedules the blocked production job(s)
+        assertEquals(1, tester.buildSystem().jobs().size());
+        BuildService.BuildJob productionJob = tester.buildSystem().takeJobsToRun().get(0);
+        assertEquals("production-us-west-1", productionJob.jobName());
     }
 
     @Test
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
index d8ca5e59b4f..7b6d0045133 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
@@ -4,6 +4,9 @@
       "name":"DelayedDeployer"
     },
     {
+      "name":"BlockedChangeDeployer"
+    },
+    {
       "name":"Upgrader"
     },
     {