Ensure comp.platform for all changes of change, and do not override present ones

3 files changed, 38 insertions, 13 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 50492077e20..8dbe85bedb5 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
@@ -178,18 +178,18 @@ public class DeploymentStatus {
                                                  .map(application.revisions()::get)
                                                  .flatMap(ApplicationVersion::compileVersion);
 
-        // If the revision requires a certain platform for compatibility, add that here.
+        // If the revision requires a certain platform for compatibility, add that here, unless we're already deploying a compatible platform.
         VersionCompatibility compatibility = versionCompatibility.apply(instance);
         Predicate<Version> compatibleWithCompileVersion = version -> compileVersion.map(compiled -> compatibility.accept(version, compiled)).orElse(true);
+        if (change.platform().map(compatibleWithCompileVersion::test).orElse(false))
+            return change;
+
         if (   application.productionDeployments().isEmpty() // TODO: replace with adding this for test jobs when needed
             || application.productionDeployments().getOrDefault(instance, List.of()).stream()
-                     .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) {
-            return targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy())
-                    .stream() // Pick the latest platform with appropriate confidence, which is compatible with the compile version.
-                    .filter(compatibleWithCompileVersion)
-                    .findFirst()
-                    .map(platform -> change.withoutPin().with(platform))
-                    .orElse(change);
+                          .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) {
+            for (Version platform : targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy()))
+                if (compatibleWithCompileVersion.test(platform))
+                    return change.withoutPin().with(platform);
         }
         return change;
     }
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 7c0a4a83e4f..c1032bb5920 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
@@ -91,10 +91,8 @@ public class DeploymentTrigger {
                                             && acceptNewRevision(status, instanceName, outstanding.revision().get());
                 application = application.with(instanceName,
                                                instance -> withRemainingChange(instance,
-                                                                               status.withCompatibilityPlatform((deployOutstanding ? outstanding
-                                                                                                                                   : Change.empty())
-                                                                                                                        .onTopOf(instance.change()),
-                                                                                                         instanceName),
+                                                                               deployOutstanding ? outstanding.onTopOf(instance.change())
+                                                                                                 : instance.change(),
                                                                                status));
             }
             applications().store(application);
@@ -430,7 +428,8 @@ public class DeploymentTrigger {
             remaining = remaining.withoutPlatform();
         if (status.hasCompleted(instance.name(), change.withoutPlatform()))
             remaining = remaining.withoutApplication();
-        return instance.withChange(remaining);
+
+        return instance.withChange(status.withCompatibilityPlatform(remaining, instance.name()));
     }
 
     // ---------- Version and job helpers ----------
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 3b8d0374a45..6f7c8403a2b 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
@@ -2222,6 +2222,32 @@ public class DeploymentTriggerTest {
         newApp.deploy();
         assertEquals(version2, tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetPlatform());
         assertEquals(version2, newApp.application().revisions().get(tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetRevision()).compileVersion().get());
+
+        // New app compiles against old major, and downgrades when a pin is also applied.
+        newApp.submit(new ApplicationPackageBuilder().compileVersion(version1)
+                                                     .systemTest()
+                                                     .region("us-east-3")
+                                                     .build());
+        newRevision = newApp.lastSubmission().get();
+
+        assertEquals(Change.of(newRevision).with(version1), newApp.instance().change());
+        tester.triggerJobs();
+        newApp.assertNotRunning(systemTest); // Without a pin, the platform won't downgrade, and 8 is incompatible with compiled 7.
+
+        tester.outstandingChangeDeployer().run();
+        assertEquals(Change.of(newRevision).with(version1), newApp.instance().change());
+        tester.upgrader().run();
+        assertEquals(Change.of(newRevision).with(version1), newApp.instance().change());
+
+        tester.deploymentTrigger().forceChange(newApp.instanceId(), newApp.instance().change().withPin());
+        tester.outstandingChangeDeployer().run();
+        assertEquals(Change.of(newRevision).with(version1).withPin(), newApp.instance().change());
+        tester.upgrader().run();
+        assertEquals(Change.of(newRevision).with(version1).withPin(), newApp.instance().change());
+
+        newApp.deploy();
+        assertEquals(version1, tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetPlatform());
+        assertEquals(version1, newApp.application().revisions().get(tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetRevision()).compileVersion().get());
     }
 
     @Test