Consider major version compatibility when computing confidence

8 files changed, 69 insertions, 32 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 045e43f532c..b9432fdc375 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -237,7 +237,7 @@ public class ApplicationController {
     }
 
     /** Sets the default target major version. Set to empty to determine target version normally (by confidence) */
-    public void setTargetMajorVersion(Optional<Integer> targetMajorVersion) {
+    public void setTargetMajorVersion(OptionalInt targetMajorVersion) {
         curator.writeTargetMajorVersion(targetMajorVersion);
     }
 
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 a2fb0df626f..1932dc65657 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
@@ -23,7 +23,6 @@ import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.Random;
 import java.util.Set;
@@ -182,7 +181,7 @@ public class Upgrader extends ControllerMaintainer {
     }
 
     /** Sets the default target major version. Set to empty to determine target version normally (by confidence) */
-    public void setTargetMajorVersion(Optional<Integer> targetMajorVersion) {
+    public void setTargetMajorVersion(OptionalInt targetMajorVersion) {
         controller().applications().setTargetMajorVersion(targetMajorVersion);
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index f02f49e7114..54e98877ba3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -1,9 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.persistence;
 
-import com.yahoo.component.annotation.Inject;
 import com.yahoo.collections.Pair;
 import com.yahoo.component.Version;
+import com.yahoo.component.annotation.Inject;
 import com.yahoo.concurrent.UncheckedTimeoutException;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
@@ -41,6 +41,7 @@ import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
+
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
@@ -53,6 +54,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeoutException;
@@ -271,9 +273,9 @@ public class CuratorDb {
         return read(targetMajorVersionPath(), ByteBuffer::wrap).map(ByteBuffer::getInt);
     }
 
-    public void writeTargetMajorVersion(Optional<Integer> targetMajorVersion) {
+    public void writeTargetMajorVersion(OptionalInt targetMajorVersion) {
         if (targetMajorVersion.isPresent())
-            curator.set(targetMajorVersionPath(), ByteBuffer.allocate(Integer.BYTES).putInt(targetMajorVersion.get()).array());
+            curator.set(targetMajorVersionPath(), ByteBuffer.allocate(Integer.BYTES).putInt(targetMajorVersion.getAsInt()).array());
         else
             curator.delete(targetMajorVersionPath());
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
index 25ac90ac0ea..776fcbfd03b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
@@ -35,6 +35,7 @@ import java.security.Principal;
 import java.security.cert.X509Certificate;
 import java.time.Instant;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Scanner;
 import java.util.function.Function;
 import java.util.logging.Level;
@@ -60,13 +61,13 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler {
     @Override
     public HttpResponse auditAndHandle(HttpRequest request) {
         try {
-            switch (request.getMethod()) {
-                case GET: return get(request);
-                case POST: return post(request);
-                case DELETE: return delete(request);
-                case PATCH: return patch(request);
-                default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported");
-            }
+            return switch (request.getMethod()) {
+                case GET -> get(request);
+                case POST -> post(request);
+                case DELETE -> delete(request);
+                case PATCH -> patch(request);
+                default -> ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported");
+            };
         }
         catch (IllegalArgumentException e) {
             return ErrorResponse.badRequest(Exceptions.toMessageString(e));
@@ -165,8 +166,8 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler {
         if (inspect.field(upgradesPerMinuteField).valid()) {
             upgrader.setUpgradesPerMinute(inspect.field(upgradesPerMinuteField).asDouble());
         } else if (inspect.field(targetMajorVersionField).valid()) {
-            int target = (int)inspect.field(targetMajorVersionField).asLong();
-            upgrader.setTargetMajorVersion(Optional.ofNullable(target == 0 ? null : target)); // 0 is the default value
+            int target = (int) inspect.field(targetMajorVersionField).asLong();
+            upgrader.setTargetMajorVersion(target == 0 ? OptionalInt.empty() : OptionalInt.of(target)); // 0 is the default value
         } else {
             return ErrorResponse.badRequest("No such modifiable field(s)");
         }
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 7f33f612cd0..e078df0267f 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
@@ -15,7 +15,8 @@ import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy;
 
 /**
  * Information about a particular Vespa version.
- * VespaVersions are identified by their version number and ordered by increasing version numbers.
+ *
+ * Vespa versions are identified by their version number and ordered by increasing version numbers.
  * 
  * @author bratseth
  */
@@ -29,8 +30,11 @@ public record VespaVersion(Version version,
                            Confidence confidence) implements Comparable<VespaVersion> {
 
     public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) {
+        int thisMajorVersion = statistics.version().getMajor();
+        int defaultMajorVersion = controller.applications().targetMajorVersion().orElse(thisMajorVersion);
         InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList())
-                                                                                                          .withProductionDeployment()));
+                                                                                                          .withProductionDeployment()))
+                                       .allowingMajorVersion(thisMajorVersion, defaultMajorVersion);
         // '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()
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 157f6b27487..a8c18957773 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -1153,7 +1153,7 @@ public class ControllerTest {
         assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8)));
 
         // Default major version is set to 8.
-        tester.applications().setTargetMajorVersion(Optional.of(8));
+        tester.applications().setTargetMajorVersion(OptionalInt.of(8));
         assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7)));
         assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty()));
 
@@ -1169,13 +1169,13 @@ public class ControllerTest {
 
         // Application upgrades to major 8; only major version from deployment spec should cause a downgrade.
         context.submit(new ApplicationPackageBuilder().region("us-west-1").compileVersion(version2).build()).deploy();
-        tester.applications().setTargetMajorVersion(Optional.empty());
+        tester.applications().setTargetMajorVersion(OptionalInt.empty());
         tester.applications().lockApplicationOrThrow(application, locked -> tester.applications().store(locked.withMajorVersion(null)));
         assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7)));
         assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty()));
 
         // Default major version across all apps should not cause a downgrade.
-        tester.applications().setTargetMajorVersion(Optional.of(7));
+        tester.applications().setTargetMajorVersion(OptionalInt.of(7));
         assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7)));
         assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty()));
 
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 5968b490c09..f053ab7a4a6 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
@@ -36,7 +36,6 @@ import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.pro
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.stagingTest;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.systemTest;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.ALL;
-import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.APPLICATION;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PIN;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM;
 import static org.junit.Assert.assertEquals;
@@ -746,7 +745,7 @@ public class UpgraderTest {
         var default1 = tester.newDeploymentContext("tenant1", "default1", "default").submit(DeploymentContext.applicationPackage()).deploy();
 
         // New major version is released, but we don't want to upgrade to it yet
-        tester.upgrader().setTargetMajorVersion(Optional.of(6));
+        tester.upgrader().setTargetMajorVersion(OptionalInt.of(6));
         version = Version.fromString("7.0");
         tester.controllerTester().upgradeSystem(version);
         assertEquals(version, tester.controller().readVersionStatus().systemVersion().get().versionNumber());
@@ -771,7 +770,7 @@ public class UpgraderTest {
         assertEquals(0, tester.jobs().active().size());
 
         // Now we want to upgrade the latest application
-        tester.upgrader().setTargetMajorVersion(Optional.empty());
+        tester.upgrader().setTargetMajorVersion(OptionalInt.empty());
         tester.upgrader().maintain();
         tester.triggerJobs();
         assertEquals(2, tester.jobs().active().size());
@@ -959,7 +958,7 @@ public class UpgraderTest {
         tester.controllerTester().upgradeSystem(version0);
 
         // Apps target 6 by default
-        tester.upgrader().setTargetMajorVersion(Optional.of(6));
+        tester.upgrader().setTargetMajorVersion(OptionalInt.of(6));
 
         // All applications deploy on current version
         var app1 = createAndDeploy("app1", "default");
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
index 7a137d4e410..7b045e508a9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
@@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
+import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
@@ -27,6 +28,7 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.List;
 import java.util.Map;
+import java.util.OptionalInt;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -218,7 +220,7 @@ public class VersionStatusTest {
         tester.upgrader().maintain();
 
         // Setup applications - all running on version0
-        ApplicationPackage canaryPolicy = applicationPackage("canary");
+        ApplicationPackage canaryPolicy = applicationPackage("canary", 7);
         var canary0 = tester.newDeploymentContext("tenant1", "canary0", "default")
                             .submit(canaryPolicy)
                             .deploy();
@@ -231,7 +233,7 @@ public class VersionStatusTest {
 
         ApplicationPackage defaultPolicy = applicationPackage("default");
         var default0 = tester.newDeploymentContext("tenant1", "default0", "default")
-                             .submit(defaultPolicy)
+                             .submit(applicationPackage("default", 7))
                              .deploy();
         var default1 = tester.newDeploymentContext("tenant1", "default1", "default")
                              .submit(defaultPolicy)
@@ -379,6 +381,28 @@ public class VersionStatusTest {
         assertTrue(versions.get(0).isReleased());
         assertFalse(versions.get(1).isReleased()); // tesst quirk: maven repo lost during controller recreation; useful to test status though
         assertTrue(versions.get(2).isReleased());
+
+        // A new major version is released and all canaries upgrade
+        Version version4 = new Version("7.1");
+        tester.controller().applications().setTargetMajorVersion(OptionalInt.of(version3.getMajor())); // Previous remains the default
+        tester.controllerTester().upgradeSystem(version4);
+        tester.upgrader().maintain();
+        tester.triggerJobs();
+        canary0.deployPlatform(version4);
+        canary1.deployPlatform(version4);
+        canary2.deployPlatform(version4);
+        tester.controllerTester().computeVersionStatus();
+        assertEquals(Confidence.normal, confidence(tester.controller(), version4));
+
+        // The single application allowing this major upgrades and confidence becomes 'high'
+        tester.upgrader().maintain();
+        tester.triggerJobs();
+        assertEquals(Change.of(version4), default0.instance().change());
+        default0.jobAborted(systemTest)
+                .jobAborted(stagingTest)
+                .deployPlatform(version4);
+        tester.controllerTester().computeVersionStatus();
+        assertEquals(Confidence.high, confidence(tester.controller(), version4));
     }
 
     @Test
@@ -687,6 +711,14 @@ public class VersionStatusTest {
                          .orElseThrow(() -> new IllegalArgumentException("Expected to find version: " + version));
     }
 
+    private static ApplicationPackage applicationPackage(String upgradePolicy, int majorVersion) {
+        return new ApplicationPackageBuilder().upgradePolicy(upgradePolicy)
+                                              .region("us-west-1")
+                                              .region("us-east-3")
+                                              .majorVersion(majorVersion)
+                                              .build();
+    }
+
     private static final ApplicationPackage canaryApplicationPackage =
             new ApplicationPackageBuilder().upgradePolicy("canary")
                                            .region("us-west-1")
@@ -707,12 +739,12 @@ public class VersionStatusTest {
 
     /** Returns empty prebuilt applications for efficiency */
     private ApplicationPackage applicationPackage(String upgradePolicy) {
-        switch (upgradePolicy) {
-            case "canary" : return canaryApplicationPackage;
-            case "default" : return defaultApplicationPackage;
-            case "conservative" : return conservativeApplicationPackage;
-            default : throw new IllegalArgumentException("No upgrade policy '" + upgradePolicy + "'");
-        }
+        return switch (upgradePolicy) {
+            case "canary" -> canaryApplicationPackage;
+            case "default" -> defaultApplicationPackage;
+            case "conservative" -> conservativeApplicationPackage;
+            default -> throw new IllegalArgumentException("No upgrade policy '" + upgradePolicy + "'");
+        };
     }
 
 }