Merge branch 'master' into jvenstad/deploy-without-application-lock

7 files changed, 84 insertions, 39 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 e5fe8a68444..66ba44ec129 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
@@ -276,17 +276,17 @@ public class ApplicationController {
                     .map(app -> new LockedApplication(app, lock))
                     .orElseGet(() -> new LockedApplication(createApplication(applicationId, Optional.empty()), lock));
 
-            boolean canDeployDirectly = options.deployDirectly || zone.environment().isManuallyDeployed();
+            boolean manuallyDeployed = options.deployDirectly || zone.environment().isManuallyDeployed();
             boolean preferOldestVersion = options.deployCurrentVersion;
 
             // Determine versions to use.
-            if (canDeployDirectly) {
-                platformVersion = options.vespaVersion.map(Version::new).orElse(application.get().deploymentSpec().majorVersion()
-                                                                                           .flatMap(this::lastCompatibleVersion)
-                                                                                           .orElse(controller.systemVersion()));
+            if (manuallyDeployed) {
                 applicationVersion = applicationVersionFromDeployer.orElse(ApplicationVersion.unknown);
                 applicationPackage = applicationPackageFromDeployer.orElseThrow(
                         () -> new IllegalArgumentException("Application package must be given when deploying to " + zone));
+                platformVersion = options.vespaVersion.map(Version::new).orElse(applicationPackage.deploymentSpec().majorVersion()
+                                                                                                  .flatMap(this::lastCompatibleVersion)
+                                                                                                  .orElse(controller.systemVersion()));
             }
             else {
                 JobType jobType = JobType.from(controller.system(), zone)
@@ -320,8 +320,10 @@ public class ApplicationController {
             });
 
             // Update application with information from application package
-            if ( ! preferOldestVersion && ! application.get().deploymentJobs().deployedInternally())
-                // TODO jvenstad: Store only on submissions (not on deployments to dev!!)
+            if (   ! preferOldestVersion
+                && ! application.get().deploymentJobs().deployedInternally()
+                && ! zone.environment().isManuallyDeployed())
+                // TODO jvenstad: Store only on submissions
                 storeWithUpdatedConfig(application, applicationPackage);
 
         }
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..1cac93a9971 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;
@@ -178,6 +174,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), 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}/deploying")) return deploying(path.get("tenant"), path.get("application"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/logs")) return logs(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request.getUri().getQuery());
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job")) return JobControllerApiHandlerHelper.jobTypeResponse(controller, appIdFromPath(path), request.getUri());
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return JobControllerApiHandlerHelper.runResponse(controller.jobController().runs(appIdFromPath(path), jobTypeFromPath(path)), request.getUri());
@@ -682,6 +680,18 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
+    private HttpResponse deploying(String tenant, String application, HttpRequest request) {
+        Application app = controller.applications().require(ApplicationId.from(tenant, application, "default"));
+        Slime slime = new Slime();
+        Cursor root = slime.setObject();
+        if (!app.change().isEmpty()) {
+            app.change().platform().ifPresent(version -> root.setString("platform", version.toString()));
+            app.change().application().ifPresent(applicationVersion -> root.setString("application", applicationVersion.id()));
+            root.setBool("pinned", app.change().isPinned());
+        }
+        return new SlimeJsonResponse(slime);
+    }
+
     private HttpResponse suspended(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
                                                      ZoneId.from(environment, region));
@@ -882,12 +892,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 +969,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/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 06417da8157..32068b006f0 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
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.ApplicationId;
@@ -426,7 +427,7 @@ public class ControllerTest {
     }
 
     @Test
-    public void testDeployDirectly() {
+    public void testIntegrationTestDeployment() {
         DeploymentTester tester = new DeploymentTester();
         Version six = Version.fromString("6.1");
         tester.upgradeSystem(six);
@@ -461,6 +462,28 @@ public class ControllerTest {
     }
 
     @Test
+    public void testDevDeployment() {
+        DeploymentTester tester = new DeploymentTester();
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.dev)
+                .majorVersion(6)
+                .region("us-east-1")
+                .build();
+
+        // Create application
+        Application app = tester.createApplication("app1", "tenant1", 1, 2L);
+        ZoneId zone = ZoneId.from("dev", "us-east-1");
+
+        // Deploy
+        tester.controller().applications().deploy(app.id(), zone, Optional.of(applicationPackage), DeployOptions.none());
+        assertTrue("Application deployed and activated",
+                   tester.controllerTester().configServer().application(app.id()).get().activated());
+        assertTrue("No job status added",
+                   tester.applications().require(app.id()).deploymentJobs().jobStatus().isEmpty());
+        assertEquals("DeploymentSpec is not persisted", DeploymentSpec.empty, tester.applications().require(app.id()).deploymentSpec());
+    }
+
+    @Test
     public void testSuspension() {
         DeploymentTester tester = new DeploymentTester();
         Application app = tester.createApplication("app1", "tenant1", 1, 11L);
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..552bcd34fe4 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
@@ -392,27 +392,39 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .userIdentity(USER_ID)
                                       .data("6.1.0"),
                               "{\"message\":\"Triggered pin to 6.1 for tenant1.application1\"}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying", GET)
+                                      .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying/pin", GET)
+                                      .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}");
 
         // DELETE only the pin to a given version
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying/pin", DELETE)
                                       .userIdentity(USER_ID),
                               "{\"message\":\"Changed deployment from 'pin to 6.1' to 'upgrade to 6.1' for application 'tenant1.application1'\"}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying", GET)
+                                      .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":false}");
 
         // POST pinning again
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying/pin", POST)
                                       .userIdentity(USER_ID)
                                       .data("6.1"),
                               "{\"message\":\"Triggered pin to 6.1 for tenant1.application1\"}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying", GET)
+                                      .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}");
 
         // DELETE only the version, but leave the pin
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying/platform", DELETE)
                                       .userIdentity(USER_ID),
                               "{\"message\":\"Changed deployment from 'pin to 6.1' to 'pin to current platform' for application 'tenant1.application1'\"}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying", GET)
+                                      .userIdentity(USER_ID), "{\"pinned\":true}");
 
         // DELETE also the pin to a given version
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying/pin", DELETE)
                                       .userIdentity(USER_ID),
                               "{\"message\":\"Changed deployment from 'pin to current platform' to 'no change' for application 'tenant1.application1'\"}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/deploying", GET)
+                                      .userIdentity(USER_ID), "{}");
 
         // POST a pause to a production job
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default/job/production-us-west-1/pause", POST)
@@ -672,6 +684,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"));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
index 443a2c23b6a..3744e44152a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
@@ -66,7 +66,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Testing deployment for production-us-central-1 (platform 6.1, application 1.0.42-commit1)",
+        "reason": "Testing last changes outside prod",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -80,7 +80,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Testing deployment for production-us-central-1 (platform 6.1, application 1.0.42-commit1)",
+        "reason": "Testing last changes outside prod",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -94,7 +94,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Testing deployment for production-us-central-1 (platform 6.1, application 1.0.42-commit1)",
+        "reason": "Testing last changes outside prod",
         "at": "(ignore)"
       }
     },
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
index 64c92f9cde4..822bc447d8d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
@@ -66,7 +66,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Testing deployment for production-us-central-1 (platform 6.1, application 1.0.42-commit1)",
+        "reason": "Testing last changes outside prod",
         "at": "(ignore)"
       },
       "lastCompleted": {
@@ -80,7 +80,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Testing deployment for production-us-central-1 (platform 6.1, application 1.0.42-commit1)",
+        "reason": "Testing last changes outside prod",
         "at": "(ignore)"
       },
       "lastSuccess": {
@@ -94,7 +94,7 @@
             "gitCommit": "commit1"
           }
         },
-        "reason": "Testing deployment for production-us-central-1 (platform 6.1, application 1.0.42-commit1)",
+        "reason": "Testing last changes outside prod",
         "at": "(ignore)"
       }
     },