Merge pull request #21700 from vespa-engine/mpolden/compile-version-major

Add allowMajor parameter to compile-version endpoint

4 files changed, 94 insertions, 30 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 61fce68b5d5..c6785a098be 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
@@ -86,10 +86,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.function.Consumer;
-import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -299,6 +299,48 @@ public class ApplicationController {
     }
 
     /**
+     * Returns a non-broken, released version at least as old as the oldest platform the given application is on.
+     *
+     * 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.
+     *
+     * If a non-empty wantedMajor is given, return a version within that major if such version is exists in this system.
+     */
+    public Version compileVersion(TenantAndApplicationId id, OptionalInt wantedMajor) {
+        Version oldestPlatform = oldestInstalledPlatform(id);
+        VersionStatus versionStatus = controller.readVersionStatus();
+        Set<Version> versionsInSystem = 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));
+        if (wantedMajor.isPresent() && compileVersion.getMajor() != wantedMajor.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() == wantedMajor.getAsInt())
+                                                                     .min(Comparator.naturalOrder());
+            return versionMatchingMajor.orElse(compileVersion);
+        }
+        return compileVersion;
+    }
+
+    /** 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.
      *
      * @throws IllegalArgumentException if the application already exists
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 6490ade655a..c2146dd5329 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -146,6 +146,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Scanner;
 import java.util.SortedSet;
 import java.util.StringJoiner;
@@ -248,7 +249,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}/validate")) return validateSecretStore(path.get("tenant"), path.get("name"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), Optional.empty(), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/compile-version")) return compileVersion(path.get("tenant"), path.get("application"));
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/compile-version")) return compileVersion(path.get("tenant"), path.get("application"), request.getProperty("allowMajor"));
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deployment")) return JobControllerApiHandlerHelper.overviewResponse(controller, TenantAndApplicationId.from(path.get("tenant"), path.get("application")), request.getUri());
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/package")) return applicationPackage(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/diff/{number}")) return applicationPackageDiff(path.get("tenant"), path.get("application"), path.get("number"));
@@ -852,10 +853,18 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
-    private HttpResponse compileVersion(String tenantName, String applicationName) {
+    private HttpResponse compileVersion(String tenantName, String applicationName, String allowMajorParam) {
         Slime slime = new Slime();
-        slime.setObject().setString("compileVersion",
-                                    compileVersion(TenantAndApplicationId.from(tenantName, applicationName)).toFullString());
+        OptionalInt allowMajor = OptionalInt.empty();
+        if (allowMajorParam != null) {
+            try {
+                allowMajor = OptionalInt.of(Integer.parseInt(allowMajorParam));
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("Invalid major version '" + allowMajorParam + "'", e);
+            }
+        }
+        Version compileVersion = controller.applications().compileVersion(TenantAndApplicationId.from(tenantName, applicationName), allowMajor);
+        slime.setObject().setString("compileVersion", compileVersion.toFullString());
         return new SlimeJsonResponse(slime);
     }
 
@@ -1698,31 +1707,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         return controller.zoneRegistry().getMonitoringSystemUri(deploymentId);
     }
 
-    /**
-     * Returns a non-broken, released version at least as old as the oldest platform the given application is on.
-     *
-     * 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.
-     */
-    private Version compileVersion(TenantAndApplicationId id) {
-        Version oldestPlatform = controller.applications().oldestInstalledPlatform(id);
-        VersionStatus versionStatus = controller.readVersionStatus();
-        return 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(() -> controller.mavenRepository().metadata().versions().stream()
-                                                  .filter(version -> ! version.isAfter(oldestPlatform))
-                                                  .filter(version -> ! versionStatus.versions().stream()
-                                                                                    .map(VespaVersion::versionNumber)
-                                                                                    .collect(Collectors.toSet()).contains(version))
-                                                  .max(Comparator.naturalOrder())
-                                                  .orElseThrow(() -> new IllegalStateException("No available releases of " +
-                                                                                               controller.mavenRepository().artifactId())));
-    }
-
     private HttpResponse setGlobalRotationOverride(String tenantName, String applicationName, String instanceName, String environment, String region, boolean inService, HttpRequest request) {
         Instance instance = controller.applications().requireInstance(ApplicationId.from(tenantName, applicationName, instanceName));
         ZoneId zone = requireZone(environment, region);
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 ca4e56fca3a..f3fdd71e3f4 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,9 +28,11 @@ 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;
+import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext;
@@ -1099,4 +1101,34 @@ public class ControllerTest {
         }
     }
 
+    @Test
+    public void testCompileVersion() {
+        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");
+        tester.controllerTester().upgradeSystem(version0);
+        assertEquals(version0, tester.applications().compileVersion(application, OptionalInt.empty()));
+        context.submit(applicationPackage).deploy();
+
+        // System is upgraded
+        Version version1 = Version.fromString("7.2");
+        tester.controllerTester().upgradeSystem(version1);
+        assertEquals(version0, tester.applications().compileVersion(application, OptionalInt.empty()));
+
+        // Application is upgraded and compile version is bumped
+        tester.upgrader().maintain();
+        context.deployPlatform(version1);
+        assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty()));
+
+        // A new major is released to the system
+        Version version2 = Version.fromString("8.0");
+        tester.controllerTester().upgradeController(version2);
+        assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty()));
+        assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8)));
+    }
+
 }
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 d9192598452..74bab37773a 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
@@ -859,6 +859,12 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .userIdentity(USER_ID),
                               "{\"message\":\"Aborting run 2 of staging-test for tenant1.application1.instance1\"}");
 
+        // GET compile version for specific major
+        deploymentTester.controllerTester().upgradeSystem(Version.fromString("7.0"));
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/compile-version", GET)
+                                      .userIdentity(USER_ID).properties(Map.of("allowMajor", "7")),
+                              "{\"compileVersion\":\"7.0.0\"}");
+
         // OPTIONS return 200 OK
         tester.assertResponse(request("/application/v4/", Request.Method.OPTIONS)
                                       .userIdentity(USER_ID),