Allow OCD to propagate revisions to instances failing upgrades

3 files changed, 30 insertions, 33 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java
index cfe84658ce5..de3f76d50cd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java
@@ -52,16 +52,6 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL
                                                                                           .orElse(defaultMajorVersion)));
     }
 
-    /** Returns the subset of instances that are allowed to start deploying its outstanding revision at the given time */
-    public InstanceList canChangeRevisionAt(Instant instant) {
-        return matching(id -> {
-            Change change = statuses.get(id).outstandingChange(id.instance());
-            return change.hasTargets() && statuses.get(id).instanceSteps().get(id.instance())
-                                                  .readyAt(change)
-                                                  .map(readyAt -> ! readyAt.isAfter(instant)).orElse(false);
-        });
-    }
-
     /** Returns the subset of instances that are allowed to upgrade to the given version at the given time */
     public InstanceList canUpgradeAt(Version version, Instant instant) {
         return matching(id -> statuses.get(id).instanceSteps().get(id.instance())
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 16fafeca6d0..51841396d6a 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
@@ -19,7 +19,6 @@ import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.InstanceList;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
-import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -32,7 +31,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalLong;
-import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
 
@@ -80,19 +78,36 @@ public class DeploymentTrigger {
         applications().lockApplicationOrThrow(id, application -> {
             application = application.withProjectId(OptionalLong.of(projectId));
             application = application.withNewSubmission(version);
+            applications().store(application);
+        });
+        triggerNewRevision(id);
+    }
+
+    /**
+     * Propagates the latest revision to ready instances.
+     * Ready instances are those whose dependencies are complete, and which aren't blocked, and, additionally,
+     * which aren't upgrading, or are already deploying an application change, or failing upgrade.
+     */
+    public void triggerNewRevision(TenantAndApplicationId id) {
+        applications().lockApplicationIfPresent(id, application -> {
             DeploymentStatus status = jobs.deploymentStatus(application.get());
-            for (ApplicationId instanceId : InstanceList.from(DeploymentStatusList.from(List.of(status)))
-                                                        .canChangeRevisionAt(clock.instant()).asList())
-                if (acceptNewApplicationVersion(status, instanceId.instance())) {
-                    application = application.with(instanceId.instance(),
-                                                   instance -> instance.withChange(status.outstandingChange(instance.name()).onTopOf(instance.change())));
-                    for (Run run : jobs.active(instanceId))
+            for (InstanceName instanceName : application.get().deploymentSpec().instanceNames()) {
+                Change outstanding = status.outstandingChange(instanceName);
+                if (   outstanding.hasTargets()
+                    && status.instanceSteps().get(instanceName)
+                             .readyAt(outstanding)
+                             .map(readyAt -> ! readyAt.isAfter(clock.instant())).orElse(false)
+                    && acceptNewApplicationVersion(status, instanceName)) {
+                    application = application.with(instanceName,
+                                                   instance -> {
+                                                       instance = instance.withChange(instance.change().with(outstanding.application().get()));
+                                                       return instance.withChange(remainingChange(instance, status));
+                                                   });
+                    for (Run run : jobs.active(application.get().id().instance(instanceName)))
                         if ( ! run.id().type().environment().isManuallyDeployed())
                             jobs.abort(run.id());
                 }
-
-            for (InstanceName instanceName : application.get().instances().keySet())
-                application = application.with(instanceName, instance -> instance.withChange(remainingChange(instance, status)));
+            }
             applications().store(application);
         });
     }
@@ -189,13 +204,6 @@ public class DeploymentTrigger {
         });
     }
 
-    public void triggerNewRevision(ApplicationId id) {
-        applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> {
-            DeploymentStatus status = jobs.deploymentStatus(application.get());
-            triggerChange(id, status.outstandingChange(id.instance()));
-        });
-    }
-
     /** Overrides the given instance's platform and application changes with any contained in the given change. */
     public void forceChange(ApplicationId instanceId, Change change) {
         applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java
index 9f3dcfb38a8..37ecef8b41e 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java
@@ -29,12 +29,11 @@ public class OutstandingChangeDeployer extends Maintainer {
 
     @Override
     protected void maintain() {
-        ApplicationList applications = ApplicationList.from(controller().applications().asList())
+        for (Application application : ApplicationList.from(controller().applications().asList())
                                                       .withProductionDeployment()
-                                                      .withDeploymentSpec();
-        for (ApplicationId instance : InstanceList.from(controller().jobController().deploymentStatuses(applications))
-                                                  .canChangeRevisionAt(controller().clock().instant()).asList())
-            controller().applications().deploymentTrigger().triggerNewRevision(instance);
+                                                      .withDeploymentSpec()
+                                                      .asList())
+            controller().applications().deploymentTrigger().triggerNewRevision(application.id());
     }
 
 }