From 22a05632e173eebcbb43cb6fee3b7c353d9eb67d Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Thu, 17 Jun 2021 13:55:23 +0200 Subject: Remove notifications for instances no longer referenced in deployment.xml --- .../hosted/controller/ApplicationController.java | 5 +++ .../controller/notification/NotificationsDb.java | 29 +++++++++++++++++ .../notification/NotificationsDbTest.java | 37 ++++++++++++++++++++++ 3 files changed, 71 insertions(+) (limited to 'controller-server') 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 c867b97b544..ff10f3b77ca 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 @@ -445,6 +445,11 @@ public class ApplicationController { // Validate new deployment spec thoroughly before storing it. controller.jobController().deploymentStatus(application.get()); + // Clear notifications for instances that are no longer declared + for (var name : existingInstances) + if ( ! declaredInstances.contains(name)) + controller.notificationsDb().removeNotifications(NotificationSource.from(application.get().id().instance(name))); + store(application); return application; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java index 21df0c01f0f..beb771b1aa2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.notification; import com.yahoo.collections.Pair; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; @@ -16,6 +17,7 @@ import java.util.Comparator; import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -34,6 +36,13 @@ public class NotificationsDb { public NotificationsDb(Controller controller) { this(controller.clock(), controller.curator()); + + Set allDeployments = controller.applications().asList().stream() + .flatMap(application -> application.instances().values().stream()) + .flatMap(instance -> instance.deployments().keySet().stream() + .map(zone -> new DeploymentId(instance.id(), zone))) + .collect(Collectors.toSet()); + removeNotificationsForRemovedInstances(allDeployments); } NotificationsDb(Clock clock, CuratorDb curatorDb) { @@ -41,6 +50,26 @@ public class NotificationsDb { this.curatorDb = curatorDb; } + // TODO (freva): Remove after 7.423 + void removeNotificationsForRemovedInstances(Set allDeployments) { + // Prior to 7.423, notifications created for instances that were later removed by being removed from + // deployment.xml were not cleared. This should only affect notifications with type 'deployment' + allDeployments.stream() + .map(deploymentId -> deploymentId.applicationId().tenant()) + .distinct() + .flatMap(tenant -> curatorDb.readNotifications(tenant).stream() + .filter(notification -> notification.type() == Type.deployment && notification.source().zoneId().isPresent()) + .map(Notification::source)) + .filter(source -> { + ApplicationId sourceApplication = ApplicationId.from(source.tenant(), + source.application().get(), + source.instance().get()); + DeploymentId sourceDeployment = new DeploymentId(sourceApplication, source.zoneId().get()); + return ! allDeployments.contains(sourceDeployment); + }) + .forEach(source -> removeNotification(source, Type.deployment)); + } + public List listNotifications(NotificationSource source, boolean productionOnly) { return curatorDb.readNotifications(source.tenant()).stream() .filter(notification -> source.contains(notification.source()) && (!productionOnly || notification.source().isProduction())) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java index 5bd7d1db769..f6c17ffcef1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -22,7 +22,9 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -156,6 +158,41 @@ public class NotificationsDbTest { assertEquals(expected, curatorDb.readNotifications(tenant)); } + @Test + public void removes_invalid_deployment_notifications() { + curatorDb.deleteNotifications(tenant); // Remove notifications set in init() + + ZoneId z1 = ZoneId.from("prod", "us-west-1"); + ZoneId z2 = ZoneId.from("prod", "eu-south-2"); + DeploymentId d1 = new DeploymentId(ApplicationId.from("t1", "a1", "i1"), z1); + DeploymentId d2 = new DeploymentId(ApplicationId.from("t1", "a1", "i1"), z2); + DeploymentId d3 = new DeploymentId(ApplicationId.from("t1", "a1", "i2"), z1); + DeploymentId d4 = new DeploymentId(ApplicationId.from("t1", "a2", "i1"), z2); + DeploymentId d5 = new DeploymentId(ApplicationId.from("t2", "a1", "i1"), z2); + + List notifications = Stream.of(d1, d2, d3, d4, d5) + .flatMap(deployment -> Stream.of(Type.deployment, Type.feedBlock) + .map(type -> new Notification(Instant.EPOCH, type, Level.warning, NotificationSource.from(deployment), List.of("msg")))) + .collect(Collectors.toUnmodifiableList()); + notifications.stream().collect(Collectors.groupingBy(notification -> notification.source().tenant(), Collectors.toList())) + .forEach(curatorDb::writeNotifications); + + // All except d3 plus a deployment that has no notifications + Set allDeployments = Set.of(d1, d2, d4, d5, new DeploymentId(ApplicationId.from("t3", "a1", "i1"), z1)); + notificationsDb.removeNotificationsForRemovedInstances(allDeployments); + + List expectedNotifications = new ArrayList<>(notifications); + // Only the deployment notification for d3 should be cleared (the other types already correctly clear themselves) + expectedNotifications.remove(4); + + List actualNotifications = curatorDb.listNotifications().stream() + .flatMap(tenant -> curatorDb.readNotifications(tenant).stream()) + .collect(Collectors.toUnmodifiableList()); + + assertEquals(expectedNotifications.stream().map(Notification::toString).collect(Collectors.joining("\n")), + actualNotifications.stream().map(Notification::toString).collect(Collectors.joining("\n"))); + } + @Before public void init() { curatorDb.writeNotifications(tenant, notifications); -- cgit v1.2.3