Merge pull request #20282 from vespa-engine/jonmv/avoid-setting-no-op-change

Recompute change to avoid setting a change which triggers no jobs

5 files changed, 22 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 3892ceeddf9..4e8f17b6098 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
@@ -277,8 +277,13 @@ public class DeploymentTrigger {
     /** 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 -> {
-            applications().store(application.with(instanceId.instance(),
-                                                  instance -> instance.withChange(change.onTopOf(application.get().require(instanceId.instance()).change()))));
+            Change newChange = change.onTopOf(application.get().require(instanceId.instance()).change());
+            application = application.with(instanceId.instance(),
+                                           instance -> instance.withChange(newChange));
+            DeploymentStatus newStatus = jobs.deploymentStatus(application.get());
+            application = application.with(instanceId.instance(),
+                                           instance -> instance.withChange(remainingChange(instance, newStatus)));
+            applications().store(application);
         });
     }
 
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 1f5fa243838..102dfde16ec 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
@@ -465,6 +465,21 @@ public class DeploymentTriggerTest {
     }
 
     @Test
+    public void settingANoOpChangeIsANoOp() {
+        var app = tester.newDeploymentContext().submit().deploy();
+        ApplicationVersion appVersion0 = app.lastSubmission().get();
+        app.submit().deploy();
+        ApplicationVersion appVersion1 = app.lastSubmission().get();
+
+        // Triggering a roll-out of an already deployed application is a no-op.
+        assertEquals(Change.empty(), app.instance().change());
+        tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(appVersion0));
+        assertEquals(Change.empty(), app.instance().change());
+        tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(appVersion1));
+        assertEquals(Change.empty(), app.instance().change());
+    }
+
+    @Test
     public void stepIsCompletePreciselyWhenItShouldBe() {
         var app1 = tester.newDeploymentContext("tenant1", "app1", "default");
         var app2 = tester.newDeploymentContext("tenant1", "app2", "default");
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 8b8a7fc0c62..6e8445102c3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -358,9 +358,6 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                        ATHENZ_TENANT_DOMAIN_2,
                                        id2.application());
 
-        // Trigger upgrade and then application change
-        deploymentTester.applications().deploymentTrigger().triggerChange(id2, Change.of(Version.fromString("7.0")));
-
         // POST an application package and a test jar, submitting a new application for production deployment.
         tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/submit", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID)
@@ -389,11 +386,6 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // GET application having both change and outstanding change
         tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", GET)
-                                      .userIdentity(USER_ID),
-                              new File("application2.json"));
-
-        // GET application having both change and outstanding change
-        tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", GET)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               new File("application2.json"));
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json
index 28732acb1df..df3f9699677 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json
@@ -23,9 +23,6 @@
     {
       "instance": "instance1",
       "deploying": {
-        "version": "7"
-      },
-      "outstandingChange": {
         "revision": {
           "buildNumber": 1,
           "hash": "1.0.1-commit1",
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json
index d009af005e4..9ef46247629 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json
@@ -22,9 +22,6 @@
     {
       "instance": "instance1",
       "deploying": {
-        "version": "7"
-      },
-      "outstandingChange": {
         "revision": {
           "buildNumber": 1,
           "hash": "1.0.1-commit1",