diff options
author | Jon Bratseth <bratseth@vespa.ai> | 2023-05-09 12:49:50 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@vespa.ai> | 2023-05-09 12:49:50 +0200 |
commit | 232f4384b13af3c3b94a6bd25fc9d11114c87ff2 (patch) | |
tree | 25fdf78282a3d8b6a276841d5fa58b6b6c587729 /config-model-api/src | |
parent | 3cf58185936ca332f86214999c86a7b3c1213933 (diff) |
Support a default BCP deadline
Diffstat (limited to 'config-model-api/src')
5 files changed, 85 insertions, 55 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java index 48464904f44..7464373df9e 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java @@ -6,6 +6,7 @@ import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -39,23 +40,30 @@ import java.util.stream.Collectors; */ public class Bcp { - private static final Bcp empty = new Bcp(List.of()); + private static final Bcp empty = new Bcp(List.of(), Optional.empty()); + private final Optional<Duration> defaultDeadline; private final List<Group> groups; - public Bcp(List<Group> groups) { + public Bcp(List<Group> groups, Optional<Duration> defaultDeadline) { totalMembershipSumsToOne(groups); + this.defaultDeadline = defaultDeadline; this.groups = List.copyOf(groups); } + public Optional<Duration> defaultDeadline() { return defaultDeadline; } public List<Group> groups() { return groups; } + public Bcp withGroups(List<Group> groups) { + return new Bcp(groups, defaultDeadline); + } + /** Returns the set of regions declared in the groups of this. */ public Set<RegionName> regions() { return groups.stream().flatMap(group -> group.members().stream()).map(member -> member.region()).collect(Collectors.toSet()); } - public boolean isEmpty() { return groups.isEmpty(); } + public boolean isEmpty() { return groups.isEmpty() && defaultDeadline.isEmpty(); } /** Returns this bcp spec, or if it is empty, the given bcp spec. */ public Bcp orElse(Bcp other) { @@ -81,7 +89,9 @@ public class Bcp { @Override public String toString() { if (isEmpty()) return "empty BCP"; - return "BCP of " + groups; + return "BCP of " + + ( groups.isEmpty() ? "no groups" : groups ) + + (defaultDeadline.isEmpty() ? "" : ", deadline: " + defaultDeadline.get()); } public static class Group { diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java index 4b30734365d..7c0c8b27e62 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java @@ -107,6 +107,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { validateZones(new HashSet<>(), new HashSet<>(), this); validateEndpoints(globalServiceId, this.endpoints); validateChangeBlockers(changeBlockers, now); + validateBcp(bcp); } public InstanceName name() { return name; } @@ -181,7 +182,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { .collect(Collectors.toSet()); } - private void validateChangeBlockers(List<DeploymentSpec.ChangeBlocker> changeBlockers, Instant now) { // Find all possible dates an upgrade block window can start Stream<Instant> blockingFrom = changeBlockers.stream() diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 699010417bf..1f44e599e11 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -46,7 +46,6 @@ public class DeploymentSpec { Optional.empty(), Optional.empty(), List.of(), - Bcp.empty(), "<deployment version='1.0'/>", List.of()); @@ -58,7 +57,6 @@ public class DeploymentSpec { private final Optional<AthenzService> athenzService; private final Optional<CloudAccount> cloudAccount; private final List<Endpoint> endpoints; - private final Bcp bcp; private final List<DeprecatedElement> deprecatedElements; private final String xmlForm; @@ -69,7 +67,6 @@ public class DeploymentSpec { Optional<AthenzService> athenzService, Optional<CloudAccount> cloudAccount, List<Endpoint> endpoints, - Bcp bcp, String xmlForm, List<DeprecatedElement> deprecatedElements) { this.steps = List.copyOf(Objects.requireNonNull(steps)); @@ -79,13 +76,11 @@ public class DeploymentSpec { this.cloudAccount = Objects.requireNonNull(cloudAccount); this.xmlForm = Objects.requireNonNull(xmlForm); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); - this.bcp = Objects.requireNonNull(bcp); this.deprecatedElements = List.copyOf(Objects.requireNonNull(deprecatedElements)); validateTotalDelay(steps); validateUpgradePoliciesOfIncreasingConservativeness(steps); validateAthenz(); validateApplicationEndpoints(); - validateBcp(); } public boolean isEmpty() { return this == empty; } @@ -164,11 +159,6 @@ public class DeploymentSpec { } } - private void validateBcp() { - for (var instance : instances()) - instance.validateBcp(instance.bcp().orElse(bcp())); - } - /** Returns the major version this application is pinned to, or empty (default) to allow all major versions */ public Optional<Integer> majorVersion() { return majorVersion; } @@ -212,8 +202,9 @@ public class DeploymentSpec { .orElse(ZoneEndpoint.defaultEndpoint); } - /** Returns the default BCP spec for instances, or Bcp.empty() if none are defined. */ - public Bcp bcp() { return bcp; } + /** @deprecated returns Bcp.empty(). */ + @Deprecated // Remove after June 2023 + public Bcp bcp() { return Bcp.empty(); } /** Returns the XML form of this spec, or null if it was not created by fromXml, nor is empty */ public String xmlForm() { return xmlForm; } 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 d04bb7ecfe0..40fc5086340 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 @@ -44,6 +44,7 @@ import java.io.Reader; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.time.temporal.TemporalUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -59,6 +60,7 @@ import java.util.Optional; import java.util.OptionalDouble; import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import static java.util.Comparator.comparingInt; @@ -138,10 +140,8 @@ public class DeploymentSpecXmlReader { List<Step> steps = new ArrayList<>(); List<Endpoint> applicationEndpoints = new ArrayList<>(); - Bcp defaultBcp; if ( ! containsTag(instanceTag, root)) { // deployment spec skipping explicit instance -> "default" instance - steps.addAll(readInstanceContent("default", root, new HashMap<>(), root)); - defaultBcp = Bcp.empty(); + steps.addAll(readInstanceContent("default", root, new HashMap<>(), root, Bcp.empty())); } else { if (XML.getChildren(root).stream().anyMatch(child -> child.getTagName().equals(prodTag))) @@ -151,14 +151,14 @@ public class DeploymentSpecXmlReader { for (Element child : XML.getChildren(root)) { String tagName = child.getTagName(); + Bcp defaultBcp = readBcp(root, Optional.empty(), List.of(), List.of(), Map.of()); if (tagName.equals(instanceTag)) { - steps.addAll(readInstanceContent(child.getAttribute(idAttribute), child, new HashMap<>(), root)); + steps.addAll(readInstanceContent(child.getAttribute(idAttribute), child, new HashMap<>(), root, defaultBcp)); } else { - steps.addAll(readNonInstanceSteps(child, new HashMap<>(), root)); // (No global service id here) + steps.addAll(readNonInstanceSteps(child, new HashMap<>(), root, defaultBcp)); // (No global service id here) } } readEndpoints(root, Optional.empty(), steps, applicationEndpoints, Map.of()); - defaultBcp = readBcp(root, Optional.empty(), steps, List.of(), Map.of()); } return new DeploymentSpec(steps, @@ -167,7 +167,6 @@ public class DeploymentSpecXmlReader { stringAttribute(athenzServiceAttribute, root).map(AthenzService::from), stringAttribute(cloudAccountAttribute, root).map(CloudAccount::from), applicationEndpoints, - defaultBcp, xmlForm, deprecatedElements); } @@ -183,7 +182,8 @@ public class DeploymentSpecXmlReader { private List<DeploymentInstanceSpec> readInstanceContent(String instanceNameString, Element instanceElement, Map<String, String> prodAttributes, - Element parentTag) { + Element parentTag, + Bcp defaultBcp) { if (instanceNameString.isBlank()) illegal("<instance> attribute 'id' must be specified, and not be blank"); @@ -211,11 +211,11 @@ public class DeploymentSpecXmlReader { Tags tags = XML.attribute(tagsTag, instanceElement).map(Tags::fromString).orElse(Tags.empty()); List<Step> steps = new ArrayList<>(); for (Element instanceChild : XML.getChildren(instanceElement)) - steps.addAll(readNonInstanceSteps(instanceChild, prodAttributes, instanceChild)); + steps.addAll(readNonInstanceSteps(instanceChild, prodAttributes, instanceChild, defaultBcp)); List<Endpoint> endpoints = new ArrayList<>(); Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints = new LinkedHashMap<>(); readEndpoints(instanceElement, Optional.of(instanceNameString), steps, endpoints, zoneEndpoints); - Bcp bcp = readBcp(instanceElement, Optional.of(instanceNameString), steps, endpoints, zoneEndpoints); + Bcp bcp = complete(readBcp(instanceElement, Optional.of(instanceNameString), steps, endpoints, zoneEndpoints).orElse(defaultBcp), steps); validateEndpoints(endpoints); // Build and return instances with these values @@ -250,16 +250,16 @@ public class DeploymentSpecXmlReader { } } - private List<Step> readSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag) { + private List<Step> readSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag, Bcp defaultBcp) { if (stepTag.getTagName().equals(instanceTag)) - return new ArrayList<>(readInstanceContent(stepTag.getAttribute(idAttribute), stepTag, prodAttributes, parentTag)); + return new ArrayList<>(readInstanceContent(stepTag.getAttribute(idAttribute), stepTag, prodAttributes, parentTag, defaultBcp)); else - return readNonInstanceSteps(stepTag, prodAttributes, parentTag); + return readNonInstanceSteps(stepTag, prodAttributes, parentTag, defaultBcp); } // Consume the given tag as 0-N steps. 0 if it is not a step, >1 if it contains multiple nested steps that should be flattened - private List<Step> readNonInstanceSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag) { + private List<Step> readNonInstanceSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag, Bcp defaultBcp) { Optional<AthenzService> athenzService = mostSpecificAttribute(stepTag, athenzServiceAttribute).map(AthenzService::from); Optional<String> testerFlavor = mostSpecificAttribute(stepTag, testerFlavorAttribute); @@ -281,7 +281,7 @@ public class DeploymentSpecXmlReader { return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag))); case prodTag: // regions, delay and parallel may be nested within, but we can flatten them return XML.getChildren(stepTag).stream() - .flatMap(child -> readNonInstanceSteps(child, prodAttributes, stepTag).stream()) + .flatMap(child -> readNonInstanceSteps(child, prodAttributes, stepTag, defaultBcp).stream()) .toList(); case delayTag: return List.of(new Delay(Duration.ofSeconds(longAttribute("hours", stepTag) * 60 * 60 + @@ -289,11 +289,11 @@ public class DeploymentSpecXmlReader { longAttribute("seconds", stepTag)))); case parallelTag: // regions and instances may be nested within return List.of(new ParallelSteps(XML.getChildren(stepTag).stream() - .flatMap(child -> readSteps(child, prodAttributes, parentTag).stream()) + .flatMap(child -> readSteps(child, prodAttributes, parentTag, defaultBcp).stream()) .toList())); case stepsTag: // regions and instances may be nested within return List.of(new Steps(XML.getChildren(stepTag).stream() - .flatMap(child -> readSteps(child, prodAttributes, parentTag).stream()) + .flatMap(child -> readSteps(child, prodAttributes, parentTag, defaultBcp).stream()) .toList())); case regionTag: return List.of(readDeclaredZone(Environment.prod, athenzService, testerFlavor, stepTag)); @@ -496,15 +496,15 @@ public class DeploymentSpecXmlReader { List<Endpoint> endpoints, Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints) { Element bcpElement = XML.getChild(parent, "bcp"); if (bcpElement == null) return Bcp.empty(); + Optional<Duration> defaultDeadline = XML.attribute("deadline", bcpElement).map(value -> toDuration(value, "deadline")); List<Bcp.Group> groups = new ArrayList<>(); Map<String, Map<RegionName, List<ZoneEndpoint>>> endpointsByZone = new LinkedHashMap<>(); for (Element groupElement : XML.getChildren(bcpElement, "group")) { List<Bcp.RegionMember> regions = new ArrayList<>(); for (Element regionElement : XML.getChildren(groupElement, "region")) { - RegionName region = RegionName.from(XML.getValue(regionElement)); double fraction = toDouble(XML.attribute("fraction", regionElement).orElse(null), "fraction").orElse(1.0); - regions.add(new Bcp.RegionMember(region, fraction)); + regions.add(new Bcp.RegionMember(RegionName.from(XML.getValue(regionElement)), fraction)); } for (Element endpointElement : XML.getChildren(groupElement, "endpoint")) { if (instance.isEmpty()) illegal("The default <bcp> element at the root cannot define endpoints"); @@ -518,11 +518,32 @@ public class DeploymentSpecXmlReader { endpoint.ifPresent(e -> endpoints.add(e)); } - Duration deadline = XML.attribute("deadline", groupElement).map(value -> toDuration(value, "deadline")).orElse(Duration.ZERO); + Duration deadline = XML.attribute("deadline", groupElement).map(value -> toDuration(value, "deadline")) + .orElse(defaultDeadline.orElse(Duration.ZERO)); groups.add(new Bcp.Group(regions, deadline)); } validateAndConsolidate(endpointsByZone, zoneEndpoints); - return new Bcp(groups); + return new Bcp(groups, defaultDeadline); + } + + /** + * A bcp instance as written and imported may either specify groups containing all regions, + * or no groups, meaning it should contain one group with all regions. + * This adds that missing implicit group when appropriate. + */ + private Bcp complete(Bcp bcp, List<Step> steps) { + if ( ! bcp.groups().isEmpty()) return bcp; // has explicit groups + var group = new Bcp.Group(prodRegions(steps).stream().map(region -> new Bcp.RegionMember(region, 1.0)).toList(), + bcp.defaultDeadline().orElse(Duration.ZERO)); + return bcp.withGroups(List.of(group)); + } + + private Set<RegionName> prodRegions(List<Step> steps) { + return steps.stream() + .flatMap(s -> s.zones().stream()) + .filter(zone -> zone.environment().isProduction()) + .flatMap(z -> z.region().stream()) + .collect(Collectors.toSet()); } static void validateAndConsolidate(Map<String, Map<RegionName, List<ZoneEndpoint>>> in, Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> out) { @@ -784,24 +805,32 @@ public class DeploymentSpecXmlReader { } /** - * Returns a string consisting of a number followed by "m" or "M" to a duration of that number of minutes, + * Returns a string consisting of a number followed by "m" or "h" to a duration given in that unit, * or zero duration if null of blank. */ - private static Duration toDuration(String minutesSpec, String sourceDescription) { + private static Duration toDuration(String durationSpec, String sourceDescription) { try { - if (minutesSpec == null || minutesSpec.isBlank()) return Duration.ZERO; - minutesSpec = minutesSpec.trim().toLowerCase(); - if ( ! minutesSpec.endsWith("m")) - throw new IllegalArgumentException("Must end by 'm'"); - try { - return Duration.ofMinutes(Integer.parseInt(minutesSpec.substring(0, minutesSpec.length() - 1))); - } - catch (NumberFormatException e) { - throw new IllegalArgumentException("Must be an integer number of minutes followed by 'm'"); - } + if (durationSpec == null || durationSpec.isBlank()) return Duration.ZERO; + durationSpec = durationSpec.trim().toLowerCase(); + var magnitude = toMagnitude(durationSpec); + return switch (durationSpec.substring(durationSpec.length() - 1)) { + case "m" -> Duration.ofMinutes(magnitude); + case "h" -> Duration.ofHours(magnitude); + case "d" -> Duration.ofDays(magnitude); + default -> throw new IllegalArgumentException("Must end by 'm', 'h' or 'd'"); + }; } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Illegal " + sourceDescription + " '" + minutesSpec + "'", e); + throw new IllegalArgumentException("Illegal " + sourceDescription + " '" + durationSpec + "'", e); + } + } + + private static int toMagnitude(String durationSpec) { + try { + return Integer.parseInt(durationSpec.substring(0, durationSpec.length() - 1)); + } + catch (NumberFormatException e) { + throw new IllegalArgumentException("Must be an integer followed by 'm' or 'h'"); } } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java index a78f53e2084..9fe1d08b904 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithBcpTest.java @@ -104,7 +104,7 @@ public class DeploymentSpecWithBcpTest { var spec = DeploymentSpec.fromXml(r); - var betaBcp = spec.requireInstance("beta").bcp().orElse(spec.bcp()); + var betaBcp = spec.requireInstance("beta").bcp(); assertEquals(1, betaBcp.groups().size()); var betaGroup = betaBcp.groups().get(0); assertEquals(2, betaGroup.members().size()); @@ -112,7 +112,7 @@ public class DeploymentSpecWithBcpTest { assertEquals(new Bcp.RegionMember(RegionName.from("us-east1"), 1.0), betaGroup.members().get(0)); assertEquals(new Bcp.RegionMember(RegionName.from("us-east2"), 1.0), betaGroup.members().get(1)); - var mainBcp = spec.requireInstance("main").bcp().orElse(spec.bcp()); + var mainBcp = spec.requireInstance("main").bcp(); assertEquals(7, mainBcp.regions().size()); assertEquals(3, mainBcp.groups().size()); @@ -214,7 +214,7 @@ public class DeploymentSpecWithBcpTest { fail(); } catch (IllegalArgumentException e) { - assertEquals("Illegal deadline 'fast': Must end by 'm'", Exceptions.toMessageString(e)); + assertEquals("Illegal deadline 'fast': Must be an integer followed by 'm' or 'h'", Exceptions.toMessageString(e)); } } @@ -403,7 +403,7 @@ public class DeploymentSpecWithBcpTest { } private void assertTwoRegions(DeploymentSpec spec) { - var bcp = spec.requireInstance("default").bcp().orElse(spec.bcp()); + var bcp = spec.requireInstance("default").bcp(); assertEquals(1, bcp.groups().size()); var group = bcp.groups().get(0); assertEquals(2, group.members().size()); |