Handle deployment spec, and mock it

4 files changed, 37 insertions, 27 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 66725d40b29..6e1940931e7 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
@@ -306,18 +306,8 @@ public class ApplicationController {
             validate(applicationPackage.deploymentSpec());
 
             // Update application with information from application package
-            if ( ! preferOldestVersion) {
-                // Store information about application package
-                application = application.with(applicationPackage.deploymentSpec());
-                application = application.with(applicationPackage.validationOverrides());
-
-                // Delete zones not listed in DeploymentSpec, if allowed
-                // We do this at deployment time to be able to return a validation failure message when necessary
-                application = deleteRemovedDeployments(application);
-
-                // Clean up deployment jobs that are no longer referenced by deployment spec
-                application = deleteUnreferencedDeploymentJobs(application);
-
+            if ( ! preferOldestVersion && ! application.get().deploymentJobs().builtInternally()) {
+                application = withUpdatedConfig(application, applicationPackage);
                 store(application); // store missing information even if we fail deployment below
             }
 
@@ -341,6 +331,24 @@ public class ApplicationController {
         }
     }
 
+    /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */
+    public LockedApplication withUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) {
+        // Store information about application package
+        application = application.with(applicationPackage.deploymentSpec());
+        application = application.with(applicationPackage.validationOverrides());
+
+        // Delete zones not listed in DeploymentSpec, if allowed
+        // We do this at deployment time for externally built applications, and at submission time
+        // for internally built ones, to be able to return a validation failure message when necessary
+        application = withoutDeletedDeployments(application);
+
+        // Clean up deployment jobs that are no longer referenced by deployment spec
+        application = withoutUnreferencedDeploymentJobs(application);
+
+        store(application);
+        return(application);
+    }
+
     /** Deploy a system application to given zone */
     public void deploy(SystemApplication application, ZoneId zone, Version version) {
         if (application.hasApplicationPackage()) {
@@ -403,7 +411,7 @@ public class ApplicationController {
         return new ActivateResult(new RevisionId("0"), prepareResponse, 0);
     }
 
-    private LockedApplication deleteRemovedDeployments(LockedApplication application) {
+    private LockedApplication withoutDeletedDeployments(LockedApplication application) {
         List<Deployment> deploymentsToRemove = application.get().productionDeployments().values().stream()
                 .filter(deployment -> ! application.get().deploymentSpec().includes(deployment.zone().environment(),
                                                                                     Optional.of(deployment.zone().region())))
@@ -428,7 +436,7 @@ public class ApplicationController {
         return applicationWithRemoval;
     }
 
-    private LockedApplication deleteUnreferencedDeploymentJobs(LockedApplication application) {
+    private LockedApplication withoutUnreferencedDeploymentJobs(LockedApplication application) {
         for (JobType job : JobList.from(application.get()).production().mapToList(JobStatus::type)) {
             ZoneId zone = job.zone(controller.system());
             if (application.get().deploymentSpec().includes(zone.environment(), Optional.of(zone.region())))
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index a8c6cb7d5cf..4a14d116c1c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -1,7 +1,6 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.google.common.collect.ImmutableMap;
-import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -11,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.LogStore;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
+import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
@@ -172,8 +172,6 @@ public class JobController {
                                      byte[] applicationPackage, byte[] applicationTestPackage) {
         AtomicReference<ApplicationVersion> version = new AtomicReference<>();
         controller.applications().lockOrThrow(id, application -> {
-            controller.applications().store(application.withBuiltInternally(true));
-
             long run = nextBuild(id);
             version.set(ApplicationVersion.from(revision, run));
 
@@ -184,6 +182,10 @@ public class JobController {
                                                                    version.toString(),
                                                                    applicationTestPackage);
 
+            application = application.withBuiltInternally(true);
+            controller.applications().store(controller.applications().withUpdatedConfig(application,
+                                                                                        new ApplicationPackage(applicationPackage)));
+
             notifyOfNewSubmission(id, revision, run);
         });
         return version.get();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java
index faf8977beb7..955980e89d6 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java
@@ -48,17 +48,17 @@ public class ArtifactRepositoryMock extends AbstractComponent implements Artifac
 
     @Override
     public void putApplicationPackage(ApplicationId application, String applicationVersion, byte[] applicationPackage) {
-        ;
+        repository.put(artifactHash(application, applicationVersion), new Artifact(applicationPackage));
     }
 
     @Override
     public byte[] getTesterPackage(ApplicationId tester, String applicationVersion) {
-        return new byte[0];
+        return getApplicationPackage(tester, applicationVersion);
     }
 
     @Override
     public void putTesterPackage(ApplicationId tester, String applicationVersion, byte[] testerPackage) {
-        ;
+        putApplicationPackage(tester, applicationVersion, testerPackage);
     }
 
     @Override
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
index 4dfd419e5fc..ee1567dae11 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
@@ -59,12 +59,12 @@ import static org.junit.Assert.fail;
 public class JobRunnerTest {
 
     private static final Versions versions = new Versions(Version.fromString("1.2.3"),
-                                                  ApplicationVersion.from(new SourceRevision("repo",
-                                                                                             "branch",
-                                                                                             "bada55"),
-                                                                          321),
-                                                  Optional.empty(),
-                                                  Optional.empty());
+                                                          ApplicationVersion.from(new SourceRevision("repo",
+                                                                                                     "branch",
+                                                                                                     "bada55"),
+                                                                                  321),
+                                                          Optional.empty(),
+                                                          Optional.empty());
 
     @Test
     public void multiThreadedExecutionFinishes() throws InterruptedException {
@@ -226,7 +226,7 @@ public class JobRunnerTest {
         assertEquals(Optional.empty(), jobs.last(id, systemTest));
     }
 
-    private static ExecutorService inThreadExecutor() {
+    public static ExecutorService inThreadExecutor() {
         return new AbstractExecutorService() {
             AtomicBoolean shutDown = new AtomicBoolean(false);
             @Override public void shutdown() { shutDown.set(true); }