Merge pull request #8560 from vespa-engine/jvenstad/deploy-version-with-tester-first-after-v4-switch

Jvenstad/deploy version with tester first after v4 switch 

3 files changed, 35 insertions, 28 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index 51d4033044f..c1194244dd8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -453,31 +453,32 @@ public class InternalStepRunner implements StepRunner {
     private void sendNotification(Run run, DualLogger logger) {
         Application application = controller.applications().require(run.id().application());
         Notifications notifications = application.deploymentSpec().notifications();
-        if (notifications != Notifications.none()) {
-            boolean newCommit = application.change().application()
-                                           .map(run.versions().targetApplication()::equals)
-                                           .orElse(false);
-            When when = newCommit ? failingCommit : failing;
-
-            List<String> recipients = new ArrayList<>(notifications.emailAddressesFor(when));
-            if (notifications.emailRolesFor(when).contains(author))
-                run.versions().targetApplication().authorEmail().ifPresent(recipients::add);
-
-            try {
-                if (run.status() == outOfCapacity && run.id().type().isProduction())
-                    controller.mailer().send(mails.outOfCapacity(run.id(), recipients));
-                if (run.status() == deploymentFailed)
-                    controller.mailer().send(mails.deploymentFailure(run.id(), recipients));
-                if (run.status() == installationFailed)
-                    controller.mailer().send(mails.installationFailure(run.id(), recipients));
-                if (run.status() == testFailure)
-                    controller.mailer().send(mails.testFailure(run.id(), recipients));
-                if (run.status() == error)
-                    controller.mailer().send(mails.systemError(run.id(), recipients));
-            }
-            catch (RuntimeException e) {
-                logger.log(INFO, "Exception trying to send mail for " + run.id(), e);
-            }
+        boolean newCommit = application.change().application()
+                                       .map(run.versions().targetApplication()::equals)
+                                       .orElse(false);
+        When when = newCommit ? failingCommit : failing;
+
+        List<String> recipients = new ArrayList<>(notifications.emailAddressesFor(when));
+        if (notifications.emailRolesFor(when).contains(author))
+            run.versions().targetApplication().authorEmail().ifPresent(recipients::add);
+
+        if (recipients.isEmpty())
+            return;
+
+        try {
+            if (run.status() == outOfCapacity && run.id().type().isProduction())
+                controller.mailer().send(mails.outOfCapacity(run.id(), recipients));
+            if (run.status() == deploymentFailed)
+                controller.mailer().send(mails.deploymentFailure(run.id(), recipients));
+            if (run.status() == installationFailed)
+                controller.mailer().send(mails.installationFailure(run.id(), recipients));
+            if (run.status() == testFailure)
+                controller.mailer().send(mails.testFailure(run.id(), recipients));
+            if (run.status() == error)
+                controller.mailer().send(mails.systemError(run.id(), recipients));
+        }
+        catch (RuntimeException e) {
+            logger.log(INFO, "Exception trying to send mail for " + run.id(), e);
         }
     }
 
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 063248071fb..6e3b6110efb 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
@@ -236,9 +236,11 @@ public class JobController {
                            .map(Deployment::applicationVersion)
                            .distinct()
                            .forEach(appVersion -> {
-                               byte[] content = controller.applications().artifacts().getApplicationPackage(application.get().id(), appVersion.id());
-                               controller.applications().applicationStore().put(application.get().id(), appVersion, content);
+                               byte[] content = controller.applications().artifacts().getApplicationPackage(id, appVersion.id());
+                               controller.applications().applicationStore().put(id, appVersion, content);
                            });
+                // Make sure any ongoing upgrade is cancelled, since future jobs will require the tester artifact.
+                application = application.withChange(application.get().change().withoutPlatform().withoutApplication());
             }
 
             long run = nextBuild(id);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index d6365eff807..686cd1c8dfa 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -73,10 +73,14 @@ public class InternalStepRunnerTest {
         tester.setEndpoints(appId, JobType.productionUsWest1.zone(tester.tester().controller().system()));
         tester.setEndpoints(appId, JobType.productionUsEast3.zone(tester.tester().controller().system()));
 
+        // Let application have an ongoing upgrade when it switches (but kill the jobs, as the tester assumes they aren't running).
+        tester.tester().upgradeSystem(new Version("7.1"));
+        tester.tester().buildService().clear();
+
         tester.deployNewSubmission();
         tester.deployNewSubmission();
 
-        tester.deployNewPlatform(new Version("7.1"));
+        tester.deployNewPlatform(new Version("7.2"));
 
         tester.jobs().unregister(appId);
         try {