From d8086d8aeba03320f88e62cce61ed9b0e6f2a8bf Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 21 Apr 2021 20:57:31 +0200 Subject: Expose notifications in /applications/v4/ --- .../hosted/controller/api/role/PathGroup.java | 1 + .../yahoo/vespa/hosted/controller/Controller.java | 7 ++ .../controller/notification/NotificationsDb.java | 76 +++++++++++++++ .../restapi/application/ApplicationApiHandler.java | 43 ++++++++- .../notification/NotificationsDbTest.java | 106 +++++++++++++++++++++ .../restapi/application/ApplicationApiTest.java | 21 ++++ .../responses/notifications-tenant1-app2.json | 15 +++ .../responses/notifications-tenant1.json | 23 +++++ 8 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1-app2.json create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1.json diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index a0da8cbddba..d052a000860 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -54,6 +54,7 @@ enum PathGroup { tenantInfo(Matcher.tenant, "/application/v4/tenant/{tenant}/application/", "/application/v4/tenant/{tenant}/info/", + "/application/v4/tenant/{tenant}/notifications", "/routing/v1/status/tenant/{tenant}/{*}"), tenantKeys(Matcher.tenant, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 5b2c2d74d20..2de8fa6457a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -23,6 +23,7 @@ import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder; import com.yahoo.vespa.hosted.controller.metric.ConfigServerMetrics; +import com.yahoo.vespa.hosted.controller.notification.NotificationsDb; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.JobControlFlags; import com.yahoo.vespa.hosted.controller.security.AccessControl; @@ -82,6 +83,7 @@ public class Controller extends AbstractComponent { private final ControllerConfig controllerConfig; private final SecretStore secretStore; private final CuratorArchiveBucketDb archiveBucketDb; + private final NotificationsDb notificationsDb; /** * Creates a controller @@ -118,6 +120,7 @@ public class Controller extends AbstractComponent { auditLogger = new AuditLogger(curator, clock); jobControl = new JobControl(new JobControlFlags(curator, flagSource)); archiveBucketDb = new CuratorArchiveBucketDb(this); + notificationsDb = new NotificationsDb(this); this.controllerConfig = controllerConfig; this.secretStore = secretStore; @@ -306,4 +309,8 @@ public class Controller extends AbstractComponent { public CuratorArchiveBucketDb archiveBucketDb() { return archiveBucketDb; } + + public NotificationsDb notificationsDb() { + return notificationsDb; + } } 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 new file mode 100644 index 00000000000..c91382ae4bb --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java @@ -0,0 +1,76 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.notification; + +import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; + +import java.time.Clock; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Adds, updates and removes tenant notifications in ZK + * + * @author freva + */ +public class NotificationsDb { + + private final Clock clock; + private final CuratorDb curatorDb; + + public NotificationsDb(Controller controller) { + this(controller.clock(), controller.curator()); + } + + NotificationsDb(Clock clock, CuratorDb curatorDb) { + this.clock = clock; + this.curatorDb = curatorDb; + } + + public List listNotifications(NotificationSource source) { + return curatorDb.readNotifications(source.tenant()).stream() + .filter(notification -> source.contains(notification.source())) + .collect(Collectors.toUnmodifiableList()); + } + + public void addNotification(NotificationSource source, Notification.Type type, String message) { + addNotification(source, type, List.of(message)); + } + + public void addNotification(NotificationSource source, Notification.Type type, List messages) { + try (Lock lock = curatorDb.lockNotifications(source.tenant())) { + List notifications = curatorDb.readNotifications(source.tenant()).stream() + .filter(notification -> !source.equals(notification.source()) || type != notification.type()) + .collect(Collectors.toCollection(ArrayList::new)); + notifications.add(new Notification(clock.instant(), type, source, messages)); + curatorDb.writeNotifications(source.tenant(), notifications); + } + } + + public void removeNotification(NotificationSource source, Notification.Type type) { + try (Lock lock = curatorDb.lockNotifications(source.tenant())) { + List initial = curatorDb.readNotifications(source.tenant()); + List filtered = initial.stream() + .filter(notification -> !source.equals(notification.source()) || type != notification.type()) + .collect(Collectors.toUnmodifiableList()); + if (initial.size() > filtered.size()) + curatorDb.writeNotifications(source.tenant(), filtered); + } + } + + public void removeNotifications(NotificationSource source) { + if (source.application().isEmpty()) // Source is tenant + curatorDb.deleteNotifications(source.tenant()); + + try (Lock lock = curatorDb.lockNotifications(source.tenant())) { + List initial = curatorDb.readNotifications(source.tenant()); + List filtered = initial.stream() + .filter(notification -> !source.contains(notification.source())) + .collect(Collectors.toUnmodifiableList()); + if (initial.size() > filtered.size()) + curatorDb.writeNotifications(source.tenant(), filtered); + } + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 78dab2cceb6..7b8530c07ef 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -87,6 +87,8 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToC import com.yahoo.vespa.hosted.controller.deployment.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.TestConfigSerializer; +import com.yahoo.vespa.hosted.controller.notification.Notification; +import com.yahoo.vespa.hosted.controller.notification.NotificationSource; import com.yahoo.vespa.hosted.controller.rotation.RotationId; import com.yahoo.vespa.hosted.controller.rotation.RotationState; import com.yahoo.vespa.hosted.controller.rotation.RotationStatus; @@ -136,8 +138,6 @@ import java.util.stream.Stream; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.CONFLICT; -import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; -import static com.yahoo.jdisc.Response.Status.NOT_FOUND; import static java.util.Map.Entry.comparingByKey; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; @@ -223,6 +223,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant")) return tenants(request); if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/info")) return tenantInfo(path.get("tenant"), request); + if (path.matches("/application/v4/tenant/{tenant}/notifications")) return notifications(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}/validate")) return validateSecretStore(path.get("tenant"), path.get("name"), request); if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), Optional.empty(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request); @@ -480,6 +481,44 @@ public class ApplicationApiHandler extends LoggingRequestHandler { .withAddress(updateTenantInfoAddress(insp.field("address"), oldContact.address())); } + private HttpResponse notifications(String tenantName, HttpRequest request) { + NotificationSource notificationSource = new NotificationSource(TenantName.from(tenantName), + Optional.ofNullable(request.getProperty("application")).map(ApplicationName::from), + Optional.ofNullable(request.getProperty("instance")).map(InstanceName::from), + Optional.empty(), Optional.empty(), Optional.empty(), OptionalLong.empty()); + + Slime slime = new Slime(); + Cursor notificationsArray = slime.setObject().setArray("notifications"); + controller.notificationsDb().listNotifications(notificationSource) + .forEach(notification -> toSlime(notificationsArray.addObject(), notification)); + return new SlimeJsonResponse(slime); + } + + private static void toSlime(Cursor cursor, Notification notification) { + cursor.setLong("at", notification.at().toEpochMilli()); + cursor.setString("type", notificationTypeAsString(notification.type())); + Cursor messagesArray = cursor.setArray("messages"); + notification.messages().forEach(messagesArray::addString); + + notification.source().application().ifPresent(application -> cursor.setString("application", application.value())); + notification.source().instance().ifPresent(instance -> cursor.setString("instance", instance.value())); + notification.source().zoneId().ifPresent(zoneId -> { + cursor.setString("environment", zoneId.environment().value()); + cursor.setString("region", zoneId.region().value()); + }); + notification.source().clusterId().ifPresent(clusterId -> cursor.setString("clusterId", clusterId.value())); + notification.source().jobType().ifPresent(jobType -> cursor.setString("jobType", jobType.jobName())); + notification.source().runNumber().ifPresent(runNumber -> cursor.setLong("runNumber", runNumber)); + } + + private static String notificationTypeAsString(Notification.Type type) { + switch (type) { + case APPLICATION_PACKAGE_WARNING: return "APPLICATION_PACKAGE_WARNING"; + case DEPLOYMENT_FAILURE: return "DEPLOYMENT_FAILURE"; + default: throw new IllegalArgumentException("No serialization defined for notification type " + type); + } + } + private HttpResponse applications(String tenantName, Optional applicationName, HttpRequest request) { TenantName tenant = TenantName.from(tenantName); if (controller.tenants().get(tenantName).isEmpty()) 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 new file mode 100644 index 00000000000..ea86a3103a6 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -0,0 +1,106 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.notification; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.path.Path; +import com.yahoo.test.ManualClock; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; +import org.junit.Before; +import org.junit.Test; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author freva + */ +public class NotificationsDbTest { + + private static final TenantName tenant = TenantName.from("tenant1"); + private static final List notifications = List.of( + notification(1001, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(tenant), "tenant msg"), + notification(1101, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(TenantAndApplicationId.from(tenant.value(), "app1")), "app msg"), + notification(1201, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg"), + notification(1301, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(new DeploymentId(ApplicationId.from(tenant.value(), "app2", "instance2"), ZoneId.from("prod", "us-north-2"))), "deployment msg"), + notification(1401, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(new DeploymentId(ApplicationId.from(tenant.value(), "app1", "instance1"), ZoneId.from("dev", "us-south-1")), ClusterSpec.Id.from("cluster1")), "clusterMsg msg"), + notification(1501, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(new RunId(ApplicationId.from(tenant.value(), "app1", "instance1"), JobType.devUsEast1, 4)), "run id msg")); + + private final ManualClock clock = new ManualClock(Instant.ofEpochSecond(12345)); + private final MockCuratorDb curatorDb = new MockCuratorDb(); + private final NotificationsDb notificationsDb = new NotificationsDb(clock, curatorDb); + + @Test + public void list_test() { + assertEquals(notifications, notificationsDb.listNotifications(NotificationSource.from(tenant))); + assertEquals(notificationIndices(2, 3), notificationsDb.listNotifications(NotificationSource.from(TenantAndApplicationId.from(tenant.value(), "app2")))); + assertEquals(notificationIndices(4, 5), notificationsDb.listNotifications(NotificationSource.from(ApplicationId.from(tenant.value(), "app1", "instance1")))); + assertEquals(notificationIndices(5), notificationsDb.listNotifications(NotificationSource.from(new RunId(ApplicationId.from(tenant.value(), "app1", "instance1"), JobType.devUsEast1, 5)))); + assertEquals(List.of(), notificationsDb.listNotifications(NotificationSource.from(new RunId(ApplicationId.from(tenant.value(), "app1", "instance1"), JobType.productionUsEast3, 4)))); + } + + @Test + public void add_test() { + Notification notification1 = notification(12345, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2"); + Notification notification2 = notification(12345, Notification.Type.DEPLOYMENT_FAILURE, NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), "instance msg #3"); + + // Replace the 3rd notification + notificationsDb.addNotification(notification1.source(), notification1.type(), notification1.messages()); + + // Notification for a new app, add without replacement + notificationsDb.addNotification(notification2.source(), notification2.type(), notification2.messages()); + + List expected = notificationIndices(0, 1, 3, 4, 5); + expected.addAll(List.of(notification1, notification2)); + assertEquals(expected, curatorDb.readNotifications(tenant)); + } + + @Test + public void remove_single_test() { + // Remove the 3rd notification + notificationsDb.removeNotification(NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), Notification.Type.DEPLOYMENT_FAILURE); + + // Removing something that doesn't exist is OK + notificationsDb.removeNotification(NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), Notification.Type.DEPLOYMENT_FAILURE); + + assertEquals(notificationIndices(0, 1, 3, 4, 5), curatorDb.readNotifications(tenant)); + } + + @Test + public void remove_multiple_test() { + // Remove the 3rd notification + notificationsDb.removeNotifications(NotificationSource.from(ApplicationId.from(tenant.value(), "app1", "instance1"))); + assertEquals(notificationIndices(0, 1, 2, 3), curatorDb.readNotifications(tenant)); + assertTrue(curatorDb.curator().exists(Path.fromString("/controller/v1/notifications/" + tenant.value()))); + + notificationsDb.removeNotifications(NotificationSource.from(tenant)); + assertEquals(List.of(), curatorDb.readNotifications(tenant)); + assertFalse(curatorDb.curator().exists(Path.fromString("/controller/v1/notifications/" + tenant.value()))); + } + + @Before + public void init() { + curatorDb.writeNotifications(tenant, notifications); + } + + private static List notificationIndices(int... indices) { + return Arrays.stream(indices).mapToObj(notifications::get).collect(Collectors.toCollection(ArrayList::new)); + } + + private static Notification notification(long secondsSinceEpoch, Notification.Type type, NotificationSource source, String... messages) { + return new Notification(Instant.ofEpochSecond(secondsSinceEpoch), type, source, List.of(messages)); + } +} \ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index b1b1c7ffe7a..4419921cdfd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -39,6 +39,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -58,6 +59,8 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import com.yahoo.vespa.hosted.controller.metric.ApplicationMetrics; +import com.yahoo.vespa.hosted.controller.notification.Notification; +import com.yahoo.vespa.hosted.controller.notification.NotificationSource; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import com.yahoo.vespa.hosted.controller.routing.GlobalRouting; @@ -801,6 +804,13 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID), ""); + addNotifications(TenantName.from("tenant1")); + tester.assertResponse(request("/application/v4/tenant/tenant1/notifications", GET).userIdentity(USER_ID), + new File("notifications-tenant1.json")); + tester.assertResponse(request("/application/v4/tenant/tenant1/notifications", GET) + .properties(Map.of("application", "app2")).userIdentity(USER_ID), + new File("notifications-tenant1-app2.json")); + // DELETE the application which no longer has any deployments tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", DELETE) .userIdentity(USER_ID) @@ -1628,6 +1638,17 @@ public class ApplicationApiTest extends ControllerContainerTest { )); } + private void addNotifications(TenantName tenantName) { + tester.controller().notificationsDb().addNotification( + NotificationSource.from(TenantAndApplicationId.from(tenantName.value(), "app1")), + Notification.Type.APPLICATION_PACKAGE_WARNING, + "Something something deprecated..."); + tester.controller().notificationsDb().addNotification( + NotificationSource.from(new RunId(ApplicationId.from(tenantName.value(), "app2", "instance1"), JobType.systemTest, 12)), + Notification.Type.DEPLOYMENT_FAILURE, + "Failed to deploy: Out of capacity"); + } + private void assertGlobalRouting(DeploymentId deployment, GlobalRouting.Status status, GlobalRouting.Agent agent) { var changedAt = tester.controller().clock().instant(); var westPolicies = tester.controller().routing().policies().get(deployment); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1-app2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1-app2.json new file mode 100644 index 00000000000..7f583a7d803 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1-app2.json @@ -0,0 +1,15 @@ +{ + "notifications": [ + { + "at": "(ignore)", + "type": "DEPLOYMENT_FAILURE", + "messages": [ + "Failed to deploy: Out of capacity" + ], + "application": "app2", + "instance": "instance1", + "jobType": "system-test", + "runNumber": 12 + } + ] +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1.json new file mode 100644 index 00000000000..0ed8e9201a0 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/notifications-tenant1.json @@ -0,0 +1,23 @@ +{ + "notifications": [ + { + "at": "(ignore)", + "type": "APPLICATION_PACKAGE_WARNING", + "messages": [ + "Something something deprecated..." + ], + "application": "app1" + }, + { + "at": "(ignore)", + "type": "DEPLOYMENT_FAILURE", + "messages": [ + "Failed to deploy: Out of capacity" + ], + "application": "app2", + "instance": "instance1", + "jobType": "system-test", + "runNumber": 12 + } + ] +} -- cgit v1.2.3