Simplify

3 files changed, 19 insertions, 25 deletions

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 19ff6df3ccb..bde493a4237 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
@@ -49,7 +49,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFact
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Logs;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
@@ -70,7 +69,6 @@ import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.athenz.impl.ZmsClientFacade;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel;
-import com.yahoo.vespa.hosted.controller.maintenance.InfrastructureUpgrader;
 import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse;
 import com.yahoo.vespa.hosted.controller.restapi.MessageResponse;
 import com.yahoo.vespa.hosted.controller.restapi.ResourceResponse;
@@ -95,13 +93,11 @@ import java.time.DayOfWeek;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Scanner;
-import java.util.function.Function;
 import java.util.logging.Level;
 
 import static java.util.stream.Collectors.joining;
@@ -882,12 +878,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
             }
             // To avoid second guessing the orchestrated upgrades of system applications
             // we don't allow to deploy these during an system upgrade (i.e when new vespa is being rolled out)
-            Version version = wantedSystemVersion(zone, SystemApplication.zone);
-            if (!controller.systemVersion().equals(version)) {
-                throw new RuntimeException("Deployment of system applications during a system upgrade is not allowed");
+            if (controller.versionStatus().isUpgrading()) {
+                throw new IllegalArgumentException("Deployment of system applications during a system upgrade is not allowed");
+            }
+            Optional<VespaVersion> systemVersion = controller.versionStatus().systemVersion();
+            if (systemVersion.isEmpty()) {
+                throw new IllegalArgumentException("Deployment of system applications is not permitted until system version is determined");
             }
             ActivateResult result = controller.applications()
-                    .deploySystemApplicationPackage(SystemApplication.zone, zone, version);
+                    .deploySystemApplicationPackage(SystemApplication.zone, zone, systemVersion.get().versionNumber());
             return new SlimeJsonResponse(toSlime(result));
         }
 
@@ -956,23 +955,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(toSlime(result));
     }
 
-    /** Find the minimum value of a version field in a zone */
-    private Version wantedSystemVersion(ZoneId zone, SystemApplication application) {
-        try {
-            return controller.configServer()
-                    .nodeRepository()
-                    .list(zone, application.id())
-                    .stream()
-                    .filter(node -> node.state().equals(Node.State.active))
-                    .map(Node::wantedVersion)
-                    .min(Comparator.naturalOrder()).orElseThrow(
-                            () -> new RuntimeException("System version not found in node repo"));
-        } catch (Exception e) {
-            throw new RuntimeException(String.format("Failed to get version for %s in %s: %s",
-                    application.id(), zone, Exceptions.toMessageString(e)));
-        }
-    }
-
     private HttpResponse deleteTenant(String tenantName, HttpRequest request) {
         Optional<Tenant> tenant = controller.tenants().tenant(tenantName);
         if ( ! tenant.isPresent()) return ErrorResponse.notFoundError("Could not delete tenant '" + tenantName + "': Tenant not found"); // NOTE: The Jersey implementation would silently ignore this
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 e367de35d46..26410280566 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
@@ -72,6 +72,12 @@ public class VersionStatus {
         return versions().stream().filter(VespaVersion::isSystemVersion).findFirst();
     }
 
+    /** Returns whether the system is currently upgrading */
+    public boolean isUpgrading() {
+        return systemVersion().map(VespaVersion::versionNumber).orElse(Version.emptyVersion)
+                              .isBefore(controllerVersion().map(VespaVersion::versionNumber).orElse(Version.emptyVersion));
+    }
+
     /** 
      * Lists all currently active Vespa versions, with deployment statistics, 
      * sorted from lowest to highest version number.
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 7697cf00b86..29c59b61604 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
@@ -672,6 +672,12 @@ public class ApplicationApiTest extends ControllerContainerTest {
         // POST (deploy) a system application with an application package
         HttpEntity noAppEntity = createApplicationDeployData(Optional.empty(), true);
         tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/routing/environment/prod/region/us-central-1/instance/default/deploy", POST)
+                                      .data(noAppEntity)
+                                      .userIdentity(USER_ID),
+                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Deployment of system applications during a system upgrade is not allowed\"}",
+                              400);
+        tester.upgradeSystem(tester.controller().versionStatus().controllerVersion().get().versionNumber());
+        tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/routing/environment/prod/region/us-central-1/instance/default/deploy", POST)
                         .data(noAppEntity)
                         .userIdentity(USER_ID),
                 new File("deploy-result.json"));