Add legacy confidence, which lets upgrade move apps across majors

8 files changed, 58 insertions, 27 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 626937dc6ac..2441da19b90 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
@@ -11,6 +11,9 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList;
+import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
+import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
+import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence;
 
 import java.time.Instant;
 import java.util.Collection;
@@ -18,6 +21,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.function.Function;
 
 import static java.util.Comparator.comparing;
@@ -52,17 +56,25 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL
     }
 
     /**
-     * Returns the subset of instances whose application have a deployment on the given major, or specify it in deployment spec.
+     * Returns the subset of instances whose application have a deployment on the given major,
+     * or specify it in deployment spec,
+     * or which are on a {@link VespaVersion.Confidence#legacy} platform, and do not specify that in deployment spec.
      *
      * @param targetMajorVersion the target major version which applications returned allows upgrading to
      */
-    public InstanceList allowingMajorVersion(int targetMajorVersion) {
+    public InstanceList allowingMajorVersion(int targetMajorVersion, VersionStatus versions) {
         return matching(id -> {
             Application application = application(id);
-            return application.deploymentSpec().majorVersion().map(allowed -> targetMajorVersion <= allowed)
-                              .orElseGet(() -> application.productionDeployments().values().stream()
-                                                          .flatMap(List::stream)
-                                                          .anyMatch(deployment -> targetMajorVersion <= deployment.version().getMajor()));
+            Optional<Integer> majorVersion = application.deploymentSpec().majorVersion();
+            if (majorVersion.isPresent())
+                return majorVersion.get() >= targetMajorVersion;
+
+            for (List<Deployment> deployments : application.productionDeployments().values())
+                for (Deployment deployment : deployments) {
+                    if (deployment.version().getMajor() >= targetMajorVersion) return true;
+                    if (versions.version(deployment.version()).confidence() == Confidence.legacy) return true;
+                }
+            return false;
         });
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java
index c1e7f890d09..789b7137830 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java
@@ -54,9 +54,9 @@ public class DeploymentUpgrader extends ControllerMaintainer {
                         Versions target = new Versions(targetPlatform, last.versions().targetRevision(), Optional.of(last.versions().targetPlatform()), Optional.of(last.versions().targetRevision()));
                         if ( ! last.hasEnded()) continue;
                         if (application.revisions().get(last.versions().targetRevision()).compileVersion()
-                                .map(version -> controller().applications().versionCompatibility(instance.id()).refuse(version, target.targetPlatform()))
-                                .orElse(false)) continue;
-                        // TODO jonmv: respect major-version from deployment spec o_O
+                                       .map(version -> controller().applications().versionCompatibility(instance.id()).refuse(version, target.targetPlatform()))
+                                       .orElse(false)) continue;
+                        // TODO jonmv: respect major-version from deployment spec o_O Probably add it to ApplicationVersion.
 
                         if ( ! deployment.version().isBefore(target.targetPlatform())) continue;
                         if ( ! isLikelyNightFor(job)) continue;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
index f3c82e72ae3..037dacfcac9 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
@@ -107,7 +107,7 @@ public class Upgrader extends ControllerMaintainer {
         Map<ApplicationId, Version> targets = new LinkedHashMap<>();
         for (Version version : DeploymentStatus.targetsForPolicy(versionStatus, controller().systemVersion(versionStatus), policy)) {
             targetAndNewer.add(version);
-            InstanceList eligible = eligibleForVersion(remaining, version);
+            InstanceList eligible = eligibleForVersion(remaining, version, versionStatus);
             InstanceList outdated = cancellationCriterion.apply(eligible);
             cancelUpgradesOf(outdated.upgrading(), "Upgrading to outdated versions");
 
@@ -134,10 +134,10 @@ public class Upgrader extends ControllerMaintainer {
         }
     }
 
-    private InstanceList eligibleForVersion(InstanceList instances, Version version) {
+    private InstanceList eligibleForVersion(InstanceList instances, Version version, VersionStatus versionStatus) {
         Change change = Change.of(version);
         return instances.not().failingOn(version)
-                        .allowingMajorVersion(version.getMajor())
+                        .allowingMajorVersion(version.getMajor(), versionStatus)
                         .compatibleWithPlatform(version, controller().applications()::versionCompatibility)
                         .not().hasCompleted(change) // Avoid rescheduling change for instances without production steps.
                         .onLowerVersionThan(version)
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java
index 71b8f1cd9b7..420fed9d5d5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java
@@ -13,6 +13,7 @@ import java.util.logging.Level;
 import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.aborted;
 import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.broken;
 import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.high;
+import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.legacy;
 import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.low;
 import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.normal;
 
@@ -47,14 +48,15 @@ public class VersionStatusUpdater extends ControllerMaintainer {
     }
 
     static SystemMonitor.Confidence convert(VespaVersion.Confidence confidence) {
-        switch (confidence) {
-            case aborted: return aborted;
-            case broken:  return broken;
-            case low:     return low;
-            case normal:  return normal;
-            case high:    return high;
-            default: throw new IllegalArgumentException("Unexpected confidence '" + confidence + "'");
-        }
+        return switch (confidence) {
+            case aborted -> aborted;
+            case broken -> broken;
+            case low -> low;
+            case legacy -> legacy;
+            case normal -> normal;
+            case high -> high;
+            default -> throw new IllegalArgumentException("Unexpected confidence '" + confidence + "'");
+        };
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index 77b2182934e..db9b67636f3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -225,14 +225,14 @@ public record VersionStatus(List<VespaVersion> versions) {
         if (!confidenceIsOverridden) {
             // Always compute confidence for system and controller
             if (isSystemVersion || isControllerVersion) {
-                confidence = VespaVersion.confidenceFrom(statistics, controller);
+                confidence = VespaVersion.confidenceFrom(statistics, controller, versionStatus);
             } else {
                 // This is an older version, so we preserve the existing confidence, if any
                 confidence = versionStatus.versions().stream()
                                           .filter(v -> statistics.version().equals(v.versionNumber()))
                                           .map(VespaVersion::confidence)
                                           .findFirst()
-                                          .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller));
+                                          .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller, versionStatus));
             }
         }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
index 4cb10aa6ba9..0814e9ea6ec 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
@@ -29,11 +29,11 @@ public record VespaVersion(Version version,
                            List<NodeVersion> nodeVersions,
                            Confidence confidence) implements Comparable<VespaVersion> {
 
-    public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) {
+    public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller, VersionStatus versionStatus) {
         int thisMajorVersion = statistics.version().getMajor();
         InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList())
                                                                                                           .withProductionDeployment()))
-                                       .allowingMajorVersion(thisMajorVersion);
+                                       .allowingMajorVersion(thisMajorVersion, versionStatus);
         // 'production on this': All production deployment jobs upgrading to this version have completed without failure
         InstanceList productionOnThis = all.matching(instance -> statistics.productionSuccesses().stream().anyMatch(run -> run.id().application().equals(instance)))
                                            .not().failingUpgrade()
@@ -123,6 +123,9 @@ public record VespaVersion(Version version,
         
         /** We don't have sufficient evidence that this version is working */
         low,
+
+        /** This version works, but we want users to stop using it */
+        legacy,
         
         /** We have sufficient evidence that this version is working */
         normal,
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 044d9402629..9edcc5de312 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
@@ -2286,11 +2286,13 @@ public class DeploymentTriggerTest {
         Version version2 = new Version("6.3");
         assertEquals(version0, tester.newDeploymentContext("t", "a1", "default").submit().deploy().application().oldestDeployedPlatform().get());
 
+        // A new version, with normal confidence, is the default for a new app.
         tester.controllerTester().upgradeSystem(version1);
         tester.upgrader().overrideConfidence(version1, Confidence.normal);
         tester.controllerTester().computeVersionStatus();
         assertEquals(version1, tester.newDeploymentContext("t", "a2", "default").submit().deploy().application().oldestDeployedPlatform().get());
 
+        // A newer version has broken confidence, leaving the previous version as the default.
         tester.controllerTester().upgradeSystem(version2);
         tester.upgrader().overrideConfidence(version2, Confidence.broken);
         tester.controllerTester().computeVersionStatus();
@@ -2307,7 +2309,7 @@ public class DeploymentTriggerTest {
         }
         dev1.assertNotRunning(JobType.dev("us-east-1"));
 
-        tester.controllerTester().upgradeSystem(version2);
+        // Normal confidence lets the newest version be the default again.
         tester.upgrader().overrideConfidence(version2, Confidence.normal);
         tester.controllerTester().computeVersionStatus();
         assertEquals(version2, tester.newDeploymentContext("t", "a4", "default").submit().deploy().application().oldestDeployedPlatform().get());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
index 76f0cacb7c5..a97e2918483 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
@@ -921,8 +921,20 @@ public class UpgraderTest {
                         instance -> instance.withChange(instance.change().withoutPin()))));
         tester.upgrader().maintain();
         assertEquals(version1,
-                app1.instance().change().platform().orElseThrow(),
-                "Application upgrades to latest allowed major");
+                     app1.instance().change().platform().orElseThrow(),
+                     "Application upgrades to latest allowed major");
+
+        // Version on old major becomes legacy, so app upgrades once it does not specify the old major in deployment spec.
+        app1.runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1).runJob(productionUsEast3);
+        app1.submit(new ApplicationPackageBuilder().majorVersion(6).region("us-east-3").region("us-west-1").build()).deploy();
+        tester.upgrader().maintain();
+        assertEquals(Change.empty(), app1.instance().change());
+
+        app1.submit(new ApplicationPackageBuilder().region("us-east-3").region("us-west-1").build()).deploy();
+        tester.upgrader().overrideConfidence(version1, Confidence.legacy);
+        tester.controllerTester().computeVersionStatus();
+        tester.upgrader().maintain();
+        assertEquals(Change.of(version2), app1.instance().change());
     }
 
     @Test