diff options
author | Martin Polden <mpolden@mpolden.no> | 2021-12-16 12:02:54 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-16 12:02:54 +0100 |
commit | 0bb18e4d38c5dc322eae3e27818a794dffe710f6 (patch) | |
tree | 78308cdc2a835e110560e34cab0259be84527bf4 /controller-server | |
parent | df9771ee87469eb4b084aeb4e1d7a7c94ce1c005 (diff) | |
parent | 2d319a7e67b6d09e425e24f006fb3f0ecce7e438 (diff) |
Merge pull request #20543 from vespa-engine/jonmv/single-mail-per-new-failure
Avoid re-sesnding email for repeated failures
Diffstat (limited to 'controller-server')
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 |