Some more testing of dual change with multiple next-when-clear instances

4 files changed, 76 insertions, 5 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
index f94b9f32088..5eee0ccf6ff 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
@@ -335,7 +335,6 @@ public class DeploymentStatus {
             }
             List<Job> toRun = new ArrayList<>();
             List<Change> changes = deployingCompatibilityChange ? List.of(change) : changes(job, step, change);
-            if (changes.isEmpty()) return;
             for (Change partial : changes) {
                 Job jobToRun = new Job(job.type(),
                                        Versions.from(partial, application, existingPlatform, existingApplication, systemVersion),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 39c5dd2eae0..4219c52be20 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -146,7 +146,7 @@ public class DeploymentTester {
         int triggered;
         int triggeredTotal = 0;
         do {
-            triggered = (int)deploymentTrigger().triggerReadyJobs();
+            triggered = (int) deploymentTrigger().triggerReadyJobs();
             triggeredTotal += triggered;
         } while (triggered > 0);
         return triggeredTotal;
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 abdd4233339..8c166174a1b 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
@@ -1077,6 +1077,66 @@ public class DeploymentTriggerTest {
     }
 
     @Test
+    public void testMultipleInstancesWithRevisionCatchingUpToUpgrade() {
+        String spec = "<deployment>\n" +
+                      "    <instance id='alpha'>\n" +
+                      "        <upgrade rollout=\"simultaneous\" revision-target=\"next\" />\n" +
+                      "        <test />\n" +
+                      "        <staging />\n" +
+                      "    </instance>\n" +
+                      "    <instance id='beta'>\n" +
+                      "        <upgrade rollout=\"simultaneous\" revision-change=\"when-clear\" revision-target=\"next\" />\n" +
+                      "        <prod>\n" +
+                      "            <region>us-east-3</region>\n" +
+                      "            <test>us-east-3</test>\n" +
+                      "        </prod>\n" +
+                      "    </instance>\n" +
+                      "</deployment>\n";
+        ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(spec);
+        DeploymentContext alpha = tester.newDeploymentContext("t", "a", "alpha");
+        DeploymentContext beta = tester.newDeploymentContext("t", "a", "beta");
+        alpha.submit(applicationPackage).deploy();
+        Optional<ApplicationVersion> revision1 = alpha.lastSubmission();
+
+        Version version1 = new Version("7.1");
+        tester.controllerTester().upgradeSystem(version1);
+        tester.upgrader().run();
+        alpha.runJob(systemTest).runJob(stagingTest);
+        assertEquals(Change.empty(), alpha.instance().change());
+        assertEquals(Change.empty(), beta.instance().change());
+
+        tester.upgrader().run();
+        assertEquals(Change.empty(), alpha.instance().change());
+        assertEquals(Change.of(version1), beta.instance().change());
+
+        beta.triggerJobs();
+        tester.runner().run();
+        beta.triggerJobs();
+        tester.outstandingChangeDeployer().run();
+        beta.assertRunning(productionUsEast3);
+        beta.assertNotRunning(testUsEast3);
+
+        alpha.submit(applicationPackage);
+        Optional<ApplicationVersion> revision2 = alpha.lastSubmission();
+        assertEquals(Change.of(revision2.get()), alpha.instance().change());
+        assertEquals(Change.of(version1), beta.instance().change());
+
+        alpha.runJob(systemTest).runJob(stagingTest);
+        assertEquals(Change.empty(), alpha.instance().change());
+        assertEquals(Change.of(version1), beta.instance().change());
+
+        tester.outstandingChangeDeployer().run();
+        assertEquals(Change.of(version1).with(revision2.get()), beta.instance().change());
+
+        beta.triggerJobs();
+        tester.runner().run();
+        beta.triggerJobs();
+
+        beta.assertRunning(productionUsEast3);
+        beta.assertNotRunning(testUsEast3);
+    }
+
+    @Test
     public void testMultipleInstances() {
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
                 .instances("instance1,instance2")
@@ -1874,7 +1934,7 @@ public class DeploymentTriggerTest {
         // Deploy manually again, then submit new package.
         app.runJob(productionCdUsEast1, cdPackage);
         app.submit(cdPackage);
-        app.triggerJobs().jobAborted(systemTest).runJob(systemTest);
+        app.triggerJobs().runJob(systemTest);
         // Staging test requires unknown initial version, and is broken.
         tester.controller().applications().deploymentTrigger().forceTrigger(app.instanceId(), productionCdUsEast1, "user", false, true, true);
         app.runJob(productionCdUsEast1)
@@ -1894,7 +1954,7 @@ public class DeploymentTriggerTest {
                 "    </instance>\n" +
                 "    <instance id='default'>\n" +
                 "        <prod>\n" +
-                "            <region active='true'>eu-west-1</region>\n" +
+                "            <region>eu-west-1</region>\n" +
                 "            <test>eu-west-1</test>\n" +
                 "        </prod>\n" +
                 "    </instance>\n" +
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml
index a3642acb21a..cc9feb90d90 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/playground/deployment.xml
@@ -31,7 +31,7 @@
         </prod>
     </instance>
     <instance id='prod25'>
-        <upgrade rollout="simultaneous" revision-change="when-failing" revision-target="next" />
+        <upgrade rollout="simultaneous" revision-change="when-clear" revision-target="next" />
         <prod>
             <parallel>
                 <steps>
@@ -52,6 +52,17 @@
     <instance id='prod100'>
         <upgrade rollout="simultaneous" revision-change="when-clear" revision-target="next" />
         <prod>
+            <steps>
+                <parallel>
+                    <steps>
+                        <region>eu-west-1</region>
+                        <test>eu-west-1</test>
+                    </steps>
+                    <steps>
+                        <region>ap-northeast-1</region>
+                        <test>ap-northeast-1</test>
+                    </steps>
+                </parallel>
             <parallel>
                 <steps>
                     <region>us-east-3</region>
@@ -66,6 +77,7 @@
                     <test>us-west-1</test>
                 </steps>
             </parallel>
+            </steps>
         </prod>
     </instance>
 </deployment>