Send mail only when configured

4 files changed, 38 insertions, 27 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 6641a0bc444..cb4f6f5540b 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
@@ -6,6 +6,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
+import com.yahoo.config.application.api.DeploymentSpec.Notifications;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
@@ -435,12 +436,12 @@ public class InternalStepRunner implements StepRunner {
                                                    run.hasFailed() ? Optional.of(DeploymentJobs.JobError.unknown) : Optional.empty());
                 controller.applications().deploymentTrigger().notifyOfCompletion(report);
 
-                boolean newCommit = controller.applications().require(id.application())
-                                              .change().application()
-                                              .map(run.versions().targetApplication()::equals)
-                                              .orElse(false);
+                Application application = controller.applications().require(id.application());
+                boolean newCommit = application.change().application()
+                                               .map(run.versions().targetApplication()::equals)
+                                               .orElse(false);
                 if (run.hasFailed() && newCommit)
-                    sendNotification(run, logger);
+                    sendNotification(run, application.deploymentSpec().notifications(), logger);
             });
         }
         catch (IllegalStateException e) {
@@ -450,24 +451,27 @@ public class InternalStepRunner implements StepRunner {
     }
 
     /** Sends a mail with a notification of a failed run, if one should be sent. */
-    private void sendNotification(Run run, DualLogger logger) {
-        try {
-            run.versions().targetApplication().authorEmail().ifPresent(author -> {
-                List<String> recipients = Collections.singletonList(author);
-            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);
+    private void sendNotification(Run run, Notifications notifications, DualLogger logger) {
+        if (notifications != Notifications.none()) {
+            try {
+                List<String> recipients = new ArrayList<>(notifications.staticEmails());
+                if (notifications.includeAuthor())
+                    run.versions().targetApplication().authorEmail().ifPresent(recipients::add);
+
+                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/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
index 5204a007924..09c8b4ac62c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
@@ -145,7 +145,7 @@ public class ApplicationPackageBuilder {
                 xml.append("  <author />\n");
             for (String email : notifications.staticEmails())
                 xml.append("  <email>").append(email).append("</email>\n");
-            xml.append("</notifications>");
+            xml.append("</notifications>\n");
         }
         xml.append(blockChange);
         xml.append("  <");
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index 8d245ff575e..523c43a485b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -44,6 +44,7 @@ public class InternalDeploymentTester {
             .region("us-central-1")
             .parallel("us-west-1", "us-east-3")
             .notifyAuthor(true)
+            .notifyEmails(Collections.singletonList("b@a"))
             .build();
     public static final ApplicationId appId = ApplicationId.from("tenant", "application", "default");
     public static final TesterId testerId = TesterId.of(appId);
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 dbb2ef84688..bb4b4fae5a5 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
@@ -32,6 +32,7 @@ import java.nio.file.Paths;
 import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 
 import static com.yahoo.vespa.hosted.controller.api.integration.LogEntry.Type.debug;
@@ -271,11 +272,16 @@ public class InternalStepRunnerTest {
 
     @Test
     public void notificationIsSent() {
-        RunId id = tester.startSystemTestTests();
+        tester.startSystemTestTests();
         tester.cloud().set(TesterCloud.Status.NOT_STARTED);
         tester.runner().run();
-        for (Mail mail : ((MockMailer) tester.tester().controller().mailer()).inbox("a@b"))
-            assertEquals("Vespa application tenant.application: System test failing due to system error", mail.subject());
+        MockMailer mailer = ((MockMailer) tester.tester().controller().mailer());
+        assertEquals(1, mailer.inbox("a@b").size());
+        assertEquals("Vespa application tenant.application: System test failing due to system error",
+                     mailer.inbox("a@b").get(0).subject());
+        assertEquals(1, mailer.inbox("b@a").size());
+        assertEquals("Vespa application tenant.application: System test failing due to system error",
+                     mailer.inbox("b@a").get(0).subject());
     }
 
     private void assertTestLogEntries(RunId id, Step step, LogEntry... entries) {