diff options
author | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2019-01-04 11:54:34 +0100 |
---|---|---|
committer | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2019-01-04 11:54:34 +0100 |
commit | eba2eb41fa3cc8ee07f7790bbc7978ea1a2b30d1 (patch) | |
tree | 49e91a3764378a7c0b7d2f12a8b82a038bb9ccfb | |
parent | 0f4f90afc161206436679675ecf8d04b8ad896e6 (diff) |
Better API?
4 files changed, 78 insertions, 48 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java index 0375d99550a..dfd64fd951b 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Notifications.java @@ -1,11 +1,17 @@ package com.yahoo.config.application.api; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; +import java.util.Set; + +import static java.util.Collections.emptyList; +import static java.util.stream.Collectors.toMap; /** * Configuration of notifications for deployment jobs. @@ -23,41 +29,50 @@ public class Notifications { private static final Notifications none = new Notifications(Collections.emptyMap(), Collections.emptyMap()); public static Notifications none() { return none; } - private final Map<String, When> staticEmails; - private final Map<Role, When> roleEmails; + private final Map<When, List<String>> emailAddresses; + private final Map<When, List<Role>> emailRoles; - private Notifications(Map<String, When> staticEmails, Map<Role, When> roleEmails) { - this.staticEmails = ImmutableMap.copyOf(staticEmails); - this.roleEmails = ImmutableMap.copyOf(roleEmails); + private Notifications(Map<When, List<String>> emailAddresses, Map<When, List<Role>> emailRoles) { + this.emailAddresses = emailAddresses; + this.emailRoles = emailRoles; } /** * Returns a new Notifications as specified by the given String input. * - * @param when Optional string name of the default condition for sending notifications; defaults to failingCommit. - * @param staticEmails Map from email addresses to optional overrides for when to send to these. - * @param roleEmails Map from email roles to optional overrides for when to send to these. + * @param emailAddressesByWhen What email addresses to notify, indexed by when to notify them. + * @param emailRolesByWhen What roles to infer email addresses for, and notify, indexed by when to notify them. * @return The Notifications as specified. */ - public static Notifications of(Optional<String> when, Map<String, Optional<String>> staticEmails, Map<String, Optional<String>> roleEmails) { - if (staticEmails.isEmpty() && roleEmails.isEmpty()) + public static Notifications of(Map<When, List<String>> emailAddressesByWhen, Map<When, List<Role>> emailRolesByWhen) { + if (emailAddressesByWhen.isEmpty() && emailRolesByWhen.isEmpty()) return none; - When defaultWhen = when.map(When::fromValue).orElse(When.failingCommit); - return new Notifications(staticEmails.entrySet().stream() - .collect(Collectors.toMap(entry -> entry.getKey(), - entry -> entry.getValue().map(When::fromValue).orElse(defaultWhen))), - roleEmails.entrySet().stream() - .collect(Collectors.toMap(entry -> Role.fromValue(entry.getKey()), - entry -> entry.getValue().map(When::fromValue).orElse(defaultWhen)))); + ImmutableMap.Builder<When, List<String>> emailAddresses = ImmutableMap.builder(); + emailAddressesByWhen.forEach((when, addresses) -> emailAddresses.put(when, ImmutableList.copyOf(addresses))); + + ImmutableMap.Builder<When, List<Role>> emailRoles = ImmutableMap.builder(); + emailRolesByWhen.forEach((when, roles) -> emailRoles.put(when, ImmutableList.copyOf(roles))); + + return new Notifications(emailAddresses.build(), emailRoles.build()); } - public Map<String, When> staticEmails() { - return staticEmails; + /** Returns all email addresses to notify for the given condition. */ + public Set<String> emailAddressesFor(When when) { + ImmutableSet.Builder<String> addresses = ImmutableSet.builder(); + addresses.addAll(emailAddresses.getOrDefault(when, emptyList())); + for (When include : when.includes) + addresses.addAll(emailAddressesFor(include)); + return addresses.build(); } - public Map<Role, When> roleEmails() { - return roleEmails; + /** Returns all roles for which email notification is to be sent for the given condition. */ + public Set<Role> emailRolesFor(When when) { + ImmutableSet.Builder<Role> roles = ImmutableSet.builder(); + roles.addAll(emailRoles.getOrDefault(when, emptyList())); + for (When include : when.includes) + roles.addAll(emailRolesFor(include)); + return roles.build(); } @@ -89,7 +104,13 @@ public class Notifications { failing, /** Send notifications whenever a job fails while deploying a new commit. */ - failingCommit; + failingCommit(failing); + + private final List<When> includes; + + When(When... includes) { + this.includes = Arrays.asList(includes); + } public static String toValue(When when) { switch (when) { diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java index 140e4dafe32..970abd971af 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -7,6 +7,8 @@ import com.yahoo.config.application.api.DeploymentSpec.Delay; import com.yahoo.config.application.api.DeploymentSpec.ParallelZones; import com.yahoo.config.application.api.DeploymentSpec.Step; import com.yahoo.config.application.api.Notifications; +import com.yahoo.config.application.api.Notifications.Role; +import com.yahoo.config.application.api.Notifications.When; import com.yahoo.config.application.api.TimeWindow; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; @@ -121,20 +123,25 @@ public class DeploymentSpecXmlReader { if (notificationsElement == null) return Notifications.none(); - Optional<String> when = stringAttribute("when", notificationsElement); - Map<String, Optional<String>> staticEmails = new HashMap<>(); - Map<String, Optional<String>> roleEmails = new HashMap<>(); + When defaultWhen = stringAttribute("when", notificationsElement).map(When::fromValue).orElse(When.failingCommit); + Map<When, List<String>> emailAddresses = new HashMap<>(); + Map<When, List<Role>> emailRoles = new HashMap<>(); + for (When when : When.values()) { + emailAddresses.put(when, new ArrayList<>()); + emailRoles.putIfAbsent(when, new ArrayList<>()); + } for (Element emailElement : XML.getChildren(notificationsElement, "email")) { Optional<String> addressAttribute = stringAttribute("address", emailElement); - Optional<String> roleAttribute = stringAttribute("role", emailElement); + Optional<Role> roleAttribute = stringAttribute("role", emailElement).map(Role::fromValue); + When when = stringAttribute("when", emailElement).map(When::fromValue).orElse(defaultWhen); if (addressAttribute.isPresent() == roleAttribute.isPresent()) throw new IllegalArgumentException("Exactly one of 'role' and 'address' must be present in 'email' elements."); - addressAttribute.ifPresent(address -> staticEmails.put(address, stringAttribute("when", emailElement))); - roleAttribute.ifPresent(role -> roleEmails.put(role, stringAttribute("when", emailElement))); + addressAttribute.ifPresent(address -> emailAddresses.get(when).add(address)); + roleAttribute.ifPresent(role -> emailRoles.get(when).add(role)); } - return Notifications.of(when, staticEmails, roleEmails); + return Notifications.of(emailAddresses, emailRoles); } /** Imposes some constraints on tag order which are not expressible in the schema */ diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index b2ecf5368e9..b40feb3081a 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -2,6 +2,7 @@ package com.yahoo.config.application.api; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import org.junit.Test; @@ -463,11 +464,10 @@ public class DeploymentSpecTest { " <email address=\"jane@dev\"/>\n" + " </notifications>\n" + "</deployment>"); - assertEquals(ImmutableMap.of(author, failing), - spec.notifications().roleEmails()); - assertEquals(ImmutableMap.of("john@dev", failingCommit, - "jane@dev", failing), - spec.notifications().staticEmails()); + assertEquals(ImmutableSet.of(author), spec.notifications().emailRolesFor(failing)); + assertEquals(ImmutableSet.of(author), spec.notifications().emailRolesFor(failingCommit)); + assertEquals(ImmutableSet.of("john@dev", "jane@dev"), spec.notifications().emailAddressesFor(failingCommit)); + assertEquals(ImmutableSet.of("jane@dev"), spec.notifications().emailAddressesFor(failing)); } } 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 52c9b4fee7a..45165a3a2ff 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 @@ -7,6 +7,7 @@ import com.google.common.collect.ImmutableSet; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.Notifications; +import com.yahoo.config.application.api.Notifications.When; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; @@ -46,6 +47,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -56,6 +58,9 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.config.application.api.Notifications.Role.author; +import static com.yahoo.config.application.api.Notifications.When.failing; +import static com.yahoo.config.application.api.Notifications.When.failingCommit; import static com.yahoo.log.LogLevel.DEBUG; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.ACTIVATION_CONFLICT; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE; @@ -453,26 +458,23 @@ public class InternalStepRunner implements StepRunner { boolean newCommit = application.change().application() .map(run.versions().targetApplication()::equals) .orElse(false); + When when = newCommit ? failingCommit : failing; - Map<String, Notifications.When> recipients = new HashMap<>(notifications.staticEmails()); - if (notifications.roleEmails().containsKey(Notifications.Role.author)) - run.versions().targetApplication().authorEmail() - .ifPresent(address -> recipients.put(address, notifications.roleEmails().get(Notifications.Role.author))); - - if ( ! newCommit) - recipients.values().removeIf(Notifications.When.failingCommit::equals); + List<String> recipients = new ArrayList<>(notifications.emailAddressesFor(when)); + if (notifications.emailRolesFor(when).contains(author)) + run.versions().targetApplication().authorEmail().ifPresent(recipients::add); try { if (run.status() == outOfCapacity && run.id().type().isProduction()) - controller.mailer().send(mails.outOfCapacity(run.id(), recipients.keySet())); + controller.mailer().send(mails.outOfCapacity(run.id(), recipients)); if (run.status() == deploymentFailed) - controller.mailer().send(mails.deploymentFailure(run.id(), recipients.keySet())); + controller.mailer().send(mails.deploymentFailure(run.id(), recipients)); if (run.status() == installationFailed) - controller.mailer().send(mails.installationFailure(run.id(), recipients.keySet())); + controller.mailer().send(mails.installationFailure(run.id(), recipients)); if (run.status() == testFailure) - controller.mailer().send(mails.testFailure(run.id(), recipients.keySet())); + controller.mailer().send(mails.testFailure(run.id(), recipients)); if (run.status() == error) - controller.mailer().send(mails.systemError(run.id(), recipients.keySet())); + controller.mailer().send(mails.systemError(run.id(), recipients)); } catch (RuntimeException e) { logger.log(INFO, "Exception trying to send mail for " + run.id(), e); |