Avoid re-sesnding email for repeated failures

2 files changed, 20 insertions, 3 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 e28273870d7..0d56bc286eb 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
@@ -737,6 +737,9 @@ public class InternalStepRunner implements StepRunner {
 
     /** Sends a mail with a notification of a failed run, if one should be sent. */
     private void sendEmailNotification(Run run, DualLogger logger) {
+        if ( ! isNewFailure(run))
+            return;
+
         Application application = controller.applications().requireApplication(TenantAndApplicationId.from(run.id().application()));
         Notifications notifications = application.deploymentSpec().requireInstance(run.id().application().instance()).notifications();
         boolean newCommit = application.require(run.id().application().instance()).change().application()
@@ -760,6 +763,12 @@ public class InternalStepRunner implements StepRunner {
         }
     }
 
+    private boolean isNewFailure(Run run) {
+        return controller.jobController().lastCompleted(run.id().job())
+                         .map(previous -> ! previous.hasFailed() || ! previous.versions().targetsMatch(run.versions()))
+                         .orElse(true);
+    }
+
     private void updateConsoleNotification(Run run) {
         NotificationSource source = NotificationSource.from(run.id());
         Consumer<String> updater = msg -> controller.notificationsDb().setNotification(source, Notification.Type.deployment, Notification.Level.error, msg);
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 ae92fd46f26..061cc69fc26 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
@@ -368,9 +368,7 @@ public class InternalStepRunnerTest {
 
     @Test
     public void notificationIsSent() {
-        app.startSystemTestTests();
-        tester.cloud().set(TesterCloud.Status.NOT_STARTED);
-        tester.runner().run();
+        app.submit().failDeployment(JobType.systemTest);
         MockMailer mailer = tester.controllerTester().serviceRegistry().mailer();
         assertEquals(1, mailer.inbox("a@b").size());
         assertEquals("Vespa application tenant.application: System test failing due to system error",
@@ -378,6 +376,16 @@ public class InternalStepRunnerTest {
         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());
+
+        // Re-run failing causes no additional email to be sent.
+        app.failDeployment(JobType.systemTest);
+        assertEquals(1, mailer.inbox("a@b").size());
+        assertEquals(1, mailer.inbox("b@a").size());
+
+        // Failure with new package causes new email to be sent.
+        app.submit().failDeployment(JobType.systemTest);
+        assertEquals(2, mailer.inbox("a@b").size());
+        assertEquals(2, mailer.inbox("b@a").size());
     }
 
     @Test