Ensure that applications can upgrade to latest allowed major

3 files changed, 72 insertions, 29 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
index 936b0f3fce2..dec1847e751 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.ApplicationController;
 
 import java.time.Instant;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
@@ -64,13 +65,18 @@ public class ApplicationList {
     }
 
     /** Returns the subset of applications which are not pinned to a certain Vespa version. */
-    public ApplicationList notPinned() {
+    public ApplicationList unpinned() {
         return listOf(list.stream().filter(application -> ! application.change().isPinned()));
     }
 
     /** Returns the subset of applications which are currently not upgrading to the given version */
     public ApplicationList notUpgradingTo(Version version) {
-        return listOf(list.stream().filter(application -> ! isUpgradingTo(version, application)));
+        return notUpgradingTo(Collections.singletonList(version));
+    }
+
+    /** Returns the subset of applications which are currently not upgrading to any of the given versions */
+    public ApplicationList notUpgradingTo(Collection<Version> versions) {
+        return listOf(list.stream().filter(application -> versions.stream().noneMatch(version -> isUpgradingTo(version, application))));
     }
 
     /**
@@ -78,7 +84,7 @@ public class ApplicationList {
      * or returns all if no version is specified
      */
     public ApplicationList notUpgradingTo(Optional<Version> version) {
-        if ( ! version.isPresent()) return this;
+        if (version.isEmpty()) return this;
         return notUpgradingTo(version.get());
     }
 
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 4f1f7d4b277..3180cc33b8c 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
@@ -8,21 +8,21 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
 import com.yahoo.vespa.hosted.controller.application.Change;
-import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence;
-import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
-import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Comparator;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.logging.Level;
+import java.util.function.BinaryOperator;
+import java.util.function.Function;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM;
 
@@ -50,8 +50,8 @@ public class Upgrader extends Maintainer {
     public void maintain() {
         // Determine target versions for each upgrade policy
         Optional<Version> canaryTarget = controller().versionStatus().systemVersion().map(VespaVersion::versionNumber);
-        Optional<Version> defaultTarget = newestVersionWithConfidence(Confidence.normal);
-        Optional<Version> conservativeTarget = newestVersionWithConfidence(Confidence.high);
+        Collection<Version> defaultTargets = targetVersions(Confidence.normal);
+        Collection<Version> conservativeTargets = targetVersions(Confidence.high);
 
         // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation)
         for (VespaVersion version : controller().versionStatus().versions()) {
@@ -66,34 +66,31 @@ public class Upgrader extends Maintainer {
 
         // Cancel *failed* upgrades to earlier versions, as the new version may fix it
         String reason = "Failing on outdated version";
-        cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().notUpgradingTo(defaultTarget), reason);
-        cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().notUpgradingTo(conservativeTarget), reason);
+        cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().notUpgradingTo(defaultTargets), reason);
+        cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().notUpgradingTo(conservativeTargets), reason);
 
         // Schedule the right upgrades
         canaryTarget.ifPresent(target -> upgrade(applications().with(UpgradePolicy.canary), target));
-        defaultTarget.ifPresent(target -> upgrade(applications().with(UpgradePolicy.defaultPolicy), target));
-        conservativeTarget.ifPresent(target -> upgrade(applications().with(UpgradePolicy.conservative), target));
+        defaultTargets.forEach(target -> upgrade(applications().with(UpgradePolicy.defaultPolicy), target));
+        conservativeTargets.forEach(target -> upgrade(applications().with(UpgradePolicy.conservative), target));
     }
 
-    private Optional<Version> newestVersionWithConfidence(Confidence confidence) {
-        return reversed(controller().versionStatus().versions()).stream()
-                                                                // Ensure we never pick a version newer than the system
-                                                                .filter(v -> !v.versionNumber().isAfter(controller().systemVersion()))
-                                                                .filter(v -> v.confidence().equalOrHigherThan(confidence))
-                                                                .findFirst()
-                                                                .map(VespaVersion::versionNumber);
-    }
-
-    private List<VespaVersion> reversed(List<VespaVersion> versions) {
-        List<VespaVersion> reversed = new ArrayList<>(versions.size());
-        for (int i = 0; i < versions.size(); i++)
-            reversed.add(versions.get(versions.size() - 1 - i));
-        return reversed;
+    /** Returns the target versions for given confidence, one per major version in the system */
+    private Collection<Version> targetVersions(Confidence confidence) {
+        return controller().versionStatus().versions().stream()
+                           // Ensure we never pick a version newer than the system
+                           .filter(v -> !v.versionNumber().isAfter(controller().systemVersion()))
+                           .filter(v -> v.confidence().equalOrHigherThan(confidence))
+                           .map(VespaVersion::versionNumber)
+                           .collect(Collectors.toMap(Version::getMajor, // Key on major version
+                                                     Function.identity(),  // Use version as value
+                                                     BinaryOperator.<Version>maxBy(Comparator.naturalOrder()))) // Pick highest version when merging versions within this major
+                           .values();
     }
 
     /** Returns a list of all applications, except those which are pinned — these should not be manipulated by the Upgrader */
     private ApplicationList applications() {
-        return ApplicationList.from(controller().applications().asList()).notPinned();
+        return ApplicationList.from(controller().applications().asList()).unpinned();
     }
 
     private void upgrade(ApplicationList applications, Version version) {
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 25631a48dc4..7953b462ab2 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
@@ -1233,4 +1233,44 @@ public class UpgraderTest {
         assertFalse(tester.application(application.id()).change().hasTargets());
     }
 
+    @Test
+    public void upgradesToLatestAllowedMajor() {
+        DeploymentTester tester = new DeploymentTester();
+        Version version0 = Version.fromString("6.1");
+        tester.upgradeSystem(version0);
+
+        // Apps target 6 by default
+        tester.upgrader().setTargetMajorVersion(Optional.of(6));
+
+        // All applications deploy on current version
+        Application app1 = tester.createAndDeploy("app1", 1, "default");
+        Application app2 = tester.createAndDeploy("app2", 1, "default");
+
+        // Keep app 1 on current version
+        tester.controller().applications().lockIfPresent(app1.id(), app -> tester.controller().applications().store(app.withChange(app.get().change().withPin())));
+
+        // New version is released
+        Version version1 = Version.fromString("6.2");
+        tester.upgradeSystem(version1);
+        tester.upgrader().maintain();
+
+        // App 2 upgrades
+        tester.completeUpgrade(app2, version1, "default");
+
+        // New major version is released
+        Version version2 = Version.fromString("7.1");
+        tester.upgradeSystem(version2);
+
+        // App 2 is allowed on new major and upgrades
+        tester.controller().applications().lockIfPresent(app2.id(), app -> tester.applications().store(app.withMajorVersion(7)));
+        tester.upgrader().maintain();
+        assertEquals(version2, tester.controller().applications().require(app2.id()).change().platform().get());
+
+        // App 1 is unpinned and upgrades to latest 6
+        tester.controller().applications().lockIfPresent(app1.id(), app -> tester.controller().applications().store(app.withChange(app.get().change().withoutPin())));
+        tester.upgrader().maintain();
+        assertEquals("Application upgrades to latest allowed major", version1,
+                     tester.controller().applications().require(app1.id()).change().platform().get());
+    }
+
 }