Change compile version computation to account for incompatibilities

4 files changed, 75 insertions, 37 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 a87c44b02e0..48f909df6b6 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
@@ -90,6 +90,7 @@ import java.util.OptionalInt;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -97,10 +98,12 @@ import java.util.stream.Collectors;
 import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID;
 import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active;
 import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved;
+import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.low;
 import static java.util.Comparator.naturalOrder;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toMap;
+import static java.util.stream.Collectors.toSet;
 
 /**
  * A singleton owned by the Controller which contains the methods and state for controlling applications.
@@ -310,36 +313,76 @@ public class ApplicationController {
     }
 
     /**
-     * Returns a non-broken, released version at least as old as the oldest platform the given application is on.
+     * Returns the preferred Vespa version to compile against, for the given application, on an optionally restricted major.
      *
-     * If no known version is applicable, the newest version at least as old as the oldest platform is selected,
-     * among all versions released for this system. If no such versions exists, throws an IllegalStateException.
+     * A target major may be specified as an argument; or it may be set specifically for the application;
+     * or in general, for all applications; the first such specification wins.
      *
-     * If a non-empty wantedMajor is given, return a version within that major if such version is exists in this system.
+     * The returned version is not newer than the oldest deployed platform for the application, unless
+     * the target major differs from the oldest deployed platform, in which case it is not newer than
+     * the oldest available platform version on that major instead.
+     *
+     * The returned version is compatible with a platform version available in the system.
+     *
+     * A candidate is sought first among versions with non-broken confidence, then among those with forgotten confidence.
+     *
+     * The returned version is the latest in the relevant candidate set.
+     *
+     * If no such version exists, an {@link IllegalArgumentException} is thrown.
      */
     public Version compileVersion(TenantAndApplicationId id, OptionalInt wantedMajor) {
-        Version oldestPlatform = oldestInstalledPlatform(id);
+
+        // Read version status, and pick out target platforms we could run the compiled package on.
+        OptionalInt targetMajor = firstNonEmpty(wantedMajor, requireApplication(id).majorVersion(), targetMajorVersion());
         VersionStatus versionStatus = controller.readVersionStatus();
-        Set<Version> versionsInSystem = versionStatus.versions().stream()
+        Version systemVersion = controller.systemVersion(versionStatus);
+        Version oldestInstalledPlatform = oldestInstalledPlatform(id);
+
+        // Target platforms are all versions not older than the oldest installed platform, unless forcing a major version change.
+        Predicate<Version> isTargetPlatform = targetMajor.isEmpty() || targetMajor.getAsInt() == oldestInstalledPlatform.getMajor()
+                                              ? version -> ! version.isBefore(oldestInstalledPlatform)
+                                              : version -> targetMajor.getAsInt() == version.getMajor();
+        Set<Version> platformVersions = versionStatus.versions().stream()
                                                      .map(VespaVersion::versionNumber)
-                                                     .collect(Collectors.toSet());
-        Version compileVersion = versionStatus.versions().stream()
-                                              .filter(version -> version.confidence().equalOrHigherThan(VespaVersion.Confidence.low))
-                                              .filter(VespaVersion::isReleased)
-                                              .map(VespaVersion::versionNumber)
-                                              .filter(version -> !version.isAfter(oldestPlatform))
-                                              .max(Comparator.naturalOrder())
-                                              .orElseGet(() -> publishedVersionNotAfter(oldestPlatform, versionsInSystem));
-        OptionalInt targetMajor = firstNonEmpty(wantedMajor, requireApplication(id).majorVersion(), targetMajorVersion());
-        if (targetMajor.isPresent() && compileVersion.getMajor() != targetMajor.getAsInt()) {
-            // Choose the oldest version matching wanted major so that the returned version is stable in the transition
-            // to next major
-            Optional<Version> versionMatchingMajor = versionsInSystem.stream()
-                                                                     .filter(version -> version.getMajor() == targetMajor.getAsInt())
-                                                                     .min(Comparator.naturalOrder());
-            return versionMatchingMajor.orElse(compileVersion);
-        }
-        return compileVersion;
+                                                     .filter(version -> ! version.isAfter(systemVersion))
+                                                     .filter(isTargetPlatform)
+                                                     .collect(toSet());
+        if (platformVersions.isEmpty())
+            throw new IllegalArgumentException("this system has no available versions" +
+                                               (targetMajor.isPresent() ? " on specified major: " + targetMajor.getAsInt() : ""));
+
+        // The returned compile version must be compatible with at least one target platform.
+        // If it is incompatible with any of the current platforms, the system will trigger a platform change.
+        // The returned compile version should also be at least as old as both the oldest target platform version,
+        // and the oldest current platform, unless the two are incompatible, in which case only the target matters.
+        VersionCompatibility compatibility = versionCompatibility(id.defaultInstance()); // Wrong id level >_<
+        Version oldestTargetPlatform = platformVersions.stream().min(naturalOrder()).get();
+        Version newestVersion =    compatibility.accept(oldestInstalledPlatform, oldestTargetPlatform)
+                                && oldestInstalledPlatform.isBefore(oldestTargetPlatform)
+                                ? oldestInstalledPlatform
+                                : oldestTargetPlatform;
+        Predicate<Version> systemCompatible = version ->    ! version.isAfter(newestVersion)
+                                                         &&   platformVersions.stream().anyMatch(platform -> compatibility.accept(platform, version));
+
+        // Find the newest, system-compatible version with non-broken confidence.
+        Optional<Version> nonBroken = versionStatus.versions().stream()
+                                                   .filter(VespaVersion::isReleased)
+                                                   .filter(version -> version.confidence().equalOrHigherThan(low))
+                                                   .map(VespaVersion::versionNumber)
+                                                   .filter(systemCompatible)
+                                                   .max(naturalOrder());
+        if (nonBroken.isPresent()) return nonBroken.get();
+
+        // Fall back to the newest, system-compatible version with unknown confidence.
+        Set<Version> knownVersions = versionStatus.versions().stream().map(VespaVersion::versionNumber).collect(toSet());
+        Optional<Version> unknown = controller.mavenRepository().metadata().versions().stream()
+                                              .filter(version -> ! knownVersions.contains(version))
+                                              .filter(systemCompatible)
+                                              .max(naturalOrder());
+        if (unknown.isPresent()) return unknown.get();
+
+        throw new IllegalArgumentException("no suitable, released compile version exists" +
+                                           (targetMajor.isPresent() ? " for specified major: " + targetMajor.getAsInt() : ""));
     }
 
     private OptionalInt firstNonEmpty(OptionalInt... choices) {
@@ -350,16 +393,6 @@ public class ApplicationController {
         return OptionalInt.empty();
     }
 
-    /** Returns a version that is published to a Maven repository not older than oldestPlatform and known by the system */
-    private Version publishedVersionNotAfter(Version oldestPlatform, Set<Version> versionsInSystem) {
-        return controller.mavenRepository().metadata().versions().stream()
-                         .filter(version -> !version.isAfter(oldestPlatform))
-                         .filter(versionsInSystem::contains)
-                         .max(Comparator.naturalOrder())
-                         .orElseThrow(() -> new IllegalStateException("No available releases of " +
-                                                                      controller.mavenRepository().artifactId()));
-    }
-
     /**
      * Creates a new application for an existing tenant.
      *
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 8a6dac29305..5ababdd8250 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
@@ -28,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.WeightedAliasTarget;
-import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMavenRepository;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
@@ -1106,7 +1105,6 @@ public class ControllerTest {
         DeploymentContext context = tester.newDeploymentContext();
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder().region("us-west-1").build();
         TenantAndApplicationId application = TenantAndApplicationId.from(context.instanceId());
-        ((MockMavenRepository) tester.controller().mavenRepository()).addVersion("7.1", "7.2", "8.0");
 
         // No deployments result in system version
         Version version0 = Version.fromString("7.1");
@@ -1126,8 +1124,12 @@ public class ControllerTest {
 
         // A new major is released to the system
         Version version2 = Version.fromString("8.0");
-        tester.controllerTester().upgradeController(version2);
+        tester.controllerTester().upgradeSystem(version2);
         assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty()));
+        assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(8)));
+
+        // The new major is marked as incompatible with older compile versions
+        tester.controllerTester().flagSource().withListFlag(PermanentFlags.INCOMPATIBLE_VERSIONS.id(), List.of("8"), String.class);
         assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8)));
 
         // Default major version is set to 8.
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index 15a194d2ffa..75c33952754 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -274,6 +274,7 @@ public final class ControllerTester {
 
     /** Upgrade entire system to given version */
     public void upgradeSystem(Version version) {
+        ((MockMavenRepository) controller.mavenRepository()).addVersion(version);
         upgradeController(version);
         upgradeSystemApplications(version);
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 74bab37773a..8ab43f15c89 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -25,6 +25,7 @@ import com.yahoo.vespa.athenz.api.AthenzIdentity;
 import com.yahoo.vespa.athenz.api.AthenzPrincipal;
 import com.yahoo.vespa.athenz.api.AthenzUser;
 import com.yahoo.vespa.athenz.api.OAuthCredentials;
+import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.Instance;
@@ -861,6 +862,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // GET compile version for specific major
         deploymentTester.controllerTester().upgradeSystem(Version.fromString("7.0"));
+        deploymentTester.controllerTester().flagSource().withListFlag(PermanentFlags.INCOMPATIBLE_VERSIONS.id(), List.of("*"), String.class);
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/compile-version", GET)
                                       .userIdentity(USER_ID).properties(Map.of("allowMajor", "7")),
                               "{\"compileVersion\":\"7.0.0\"}");