Verify the actual change we are deploying

This code verified the change we believe we are currently deploying
according to application.deploying().
Since we now allow new changes to enter while an application change is
already in progress, application.deploying() is not reliable.

Moving the check to after we update application.deploying()
to what we received in this deploy request should solve this
problem.

2 files changed, 9 insertions, 10 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 79994b6ad9c..672f50f83d7 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
@@ -274,7 +274,6 @@ public class ApplicationController {
             // Determine what we are doing
             Application application = get(applicationId).orElse(new Application(applicationId));
 
-            // Decide version to deploy, if applicable.
             Version version;
             if (options.deployCurrentVersion)
                 version = application.currentVersion(controller, zone);
@@ -285,13 +284,6 @@ public class ApplicationController {
             else
                 version = application.currentDeployVersion(controller, zone);
 
-            // Ensure that the deploying change is tested
-            if (! canDeployDirectlyTo(zone, options) &&
-                ! application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying()))
-                throw new IllegalArgumentException("Rejecting deployment of " + application + " to " + zone +
-                                                   " as pending " + application.deploying().get() +
-                                                   " is untested");
-
             DeploymentJobs.JobType jobType = DeploymentJobs.JobType.from(controller.zoneRegistry().system(), zone);
             ApplicationRevision revision = toApplicationPackageRevision(applicationPackage, options.screwdriverBuildJob);
 
@@ -323,6 +315,12 @@ public class ApplicationController {
                 store(application, lock); // store missing information even if we fail deployment below
             }
 
+            // Ensure that the deploying change is tested
+            if (! canDeployDirectlyTo(zone, options) &&
+                ! application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying()))
+                throw new IllegalArgumentException("Rejecting deployment of " + application + " to " + zone +
+                                                   " as " + application.deploying().get() + " is not tested");
+
             // Carry out deployment
             DeploymentId deploymentId = new DeploymentId(applicationId, zone);
             ApplicationRotation rotationInDns = registerRotationInDns(deploymentId, getOrAssignRotation(deploymentId, 
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 db913d8c8d5..6fc787d940e 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
@@ -518,7 +518,8 @@ public class ControllerTest {
     @Test
     public void testDeployUntestedChangeFails() {
         ControllerTester tester = new ControllerTester();
-        ApplicationController applications = tester.controller().applications();TenantId tenant = tester.createTenant("tenant1", "domain1", 11L);
+        ApplicationController applications = tester.controller().applications();
+        TenantId tenant = tester.createTenant("tenant1", "domain1", 11L);
         Application app = tester.createApplication(tenant, "app1", "default", 1);
 
         app = app.withDeploying(Optional.of(new Change.VersionChange(Version.fromString("6.3"))));
@@ -527,7 +528,7 @@ public class ControllerTest {
             tester.deploy(app, new Zone(Environment.prod, RegionName.from("us-east-3")));
             fail("Expected exception");
         } catch (IllegalArgumentException e) {
-            assertEquals("Rejecting deployment of application 'tenant1.app1' to zone prod.us-east-3 as pending version change to 6.3 is untested", e.getMessage());
+            assertEquals("Rejecting deployment of application 'tenant1.app1' to zone prod.us-east-3 as version change to 6.3 is not tested", e.getMessage());
         }
     }