diff options
author | Jon Bratseth <bratseth@gmail.com> | 2023-02-19 11:52:26 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2023-02-19 11:52:26 +0100 |
commit | 9d9826112b6ab3a48f79a8d1346587a7cbbbe39b (patch) | |
tree | 3c95f4973f79da93901d0fe12d90218929f15012 | |
parent | 665da0c90fc7d9a26d4307d24840267d809147e6 (diff) |
Complete bcp handling
- Add bcp to schema validation
- Read endpoints defined in bcp elements
8 files changed, 397 insertions, 134 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 af369dc2672..8f88b1ba74c 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 @@ -86,17 +86,30 @@ public class Bcp { public static class Group { - private final Duration deadline; private final List<RegionMember> members; + private final List<Endpoint> endpoints; + private final Duration deadline; public Group(List<RegionMember> members, Duration deadline) { + this(members, List.of(), deadline); + } + + public Group(List<RegionMember> members, List<Endpoint> endpoints, Duration deadline) { this.members = List.copyOf(members); + this.endpoints = endpoints; this.deadline = deadline; } public List<RegionMember> members() { return members; } /** + * Returns the endpoints defined in this. + * These will be added to instances during XML import post processing + * and should not otherwise be exposed from here. + */ + List<Endpoint> endpoints() { return endpoints; } + + /** * Returns the max time until the other regions must be able to handle the additional traffic * when a region becomes unreachable, which by default is Duration.ZERO. */ diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java index fd355d427a3..1582d03df9f 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java @@ -45,7 +45,7 @@ public class Endpoint { this.level = Objects.requireNonNull(level, "level must be non-null"); this.targets = List.copyOf(Objects.requireNonNull(targets, "targets must be non-null")); if (endpointId().length() > endpointMaxLength || !endpointPattern.matcher(endpointId()).matches()) { - throw new IllegalArgumentException("Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, " + + throw new IllegalArgumentException("Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, " + "of length 1 to 12, and begin with a character; but got '" + endpointId() + "'"); } if (targets.isEmpty()) throw new IllegalArgumentException("targets must be non-empty"); 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 be6be5566a8..04cf7981d1b 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 @@ -46,8 +46,10 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -136,8 +138,10 @@ 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(); } else { if (XML.getChildren(root).stream().anyMatch(child -> child.getTagName().equals(prodTag))) @@ -154,6 +158,7 @@ public class DeploymentSpecXmlReader { } } readEndpoints(root, Optional.empty(), steps, applicationEndpoints, Map.of()); + defaultBcp = readBcp(root, Optional.empty(), steps, List.of(), Map.of()); } return new DeploymentSpec(steps, @@ -162,7 +167,7 @@ public class DeploymentSpecXmlReader { stringAttribute(athenzServiceAttribute, root).map(AthenzService::from), stringAttribute(cloudAccountAttribute, root).map(CloudAccount::from), applicationEndpoints, - readBcp(root), + defaultBcp, xmlForm, deprecatedElements); } @@ -210,6 +215,8 @@ public class DeploymentSpecXmlReader { 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); + validateEndpoints(endpoints); // Build and return instances with these values Instant now = clock.instant(); @@ -230,11 +237,19 @@ public class DeploymentSpecXmlReader { notifications, endpoints, zoneEndpoints, - readBcp(instanceElement), + bcp, now)) .toList(); } + private void validateEndpoints(List<Endpoint> endpoints) { + Set<String> endpointIds = new HashSet<>(); + for (Endpoint endpoint : endpoints) { + if ( ! endpointIds.add(endpoint.endpointId())) + illegal("Endpoint id '" + endpoint.endpointId() + "' is specified multiple times"); + } + } + private List<Step> readSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag) { if (stepTag.getTagName().equals(instanceTag)) return new ArrayList<>(readInstanceContent(stepTag.getAttribute(idAttribute), stepTag, prodAttributes, parentTag)); @@ -329,140 +344,161 @@ public class DeploymentSpecXmlReader { if (endpointsElement == null) return; Endpoint.Level level = instance.isEmpty() ? Endpoint.Level.application : Endpoint.Level.instance; - Map<String, Endpoint> endpointsById = new LinkedHashMap<>(); Map<String, Map<RegionName, List<ZoneEndpoint>>> endpointsByZone = new LinkedHashMap<>(); - XML.getChildren(endpointsElement, endpointTag).stream() // Read zone settings first. - .sorted(comparingInt(endpoint -> getZoneEndpointType(endpoint, level).isPresent() ? 0 : 1)) - .forEach(endpointElement -> { - String containerId = requireStringAttribute("container-id", endpointElement); - Optional<String> endpointId = stringAttribute("id", endpointElement); - Optional<String> zoneEndpointType = getZoneEndpointType(endpointElement, level); - String msgPrefix = (level == Endpoint.Level.application ? "Application-level" : "Instance-level") + - " endpoint '" + endpointId.orElse(Endpoint.DEFAULT_ID) + "': "; - - if (zoneEndpointType.isPresent() && endpointId.isPresent()) - illegal(msgPrefix + "cannot declare 'id' with type 'zone' or 'private'"); - - String invalidChild = level == Endpoint.Level.application ? "region" : "instance"; - if ( ! XML.getChildren(endpointElement, invalidChild).isEmpty()) - illegal(msgPrefix + "invalid element '" + invalidChild + "'"); - - boolean enabled = XML.attribute("enabled", endpointElement) - .map(value -> { - if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("zone")) - illegal(msgPrefix + "only endpoints of type 'zone' can specify 'enabled'"); - - return switch (value) { - case "true" -> true; - case "false" -> false; - default -> throw new IllegalArgumentException(msgPrefix + "invalid 'enabled' value; must be 'true' or 'false'"); - }; - }).orElse(true); - - List<AllowedUrn> allowedUrns = new ArrayList<>(); - for (var allow : XML.getChildren(endpointElement, "allow")) { - if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("private")) - illegal(msgPrefix + "only endpoints of type 'private' can specify 'allow' children"); - - switch (requireStringAttribute("with", allow)) { - case "aws-private-link" -> allowedUrns.add(new AllowedUrn(AccessType.awsPrivateLink, requireStringAttribute("arn", allow))); - case "gcp-service-connect" -> allowedUrns.add(new AllowedUrn(AccessType.gcpServiceConnect, requireStringAttribute("project", allow))); - default -> illegal("Private endpoint for container-id '" + containerId + "': " + - "invalid attribute 'with': '" + requireStringAttribute("with", allow) + "'"); - } - } + for (Element endpointElement : XML.getChildren(endpointsElement, endpointTag).stream() // Read zone settings first. + .sorted(comparingInt(endpoint -> getZoneEndpointType(endpoint, level).isPresent() ? 0 : 1)).toList()) { + Optional<Endpoint> endpoint = readEndpoint(parent, endpointElement, level, instance, steps, List.of(), endpointsByZone); + endpoint.ifPresent(e -> endpoints.add(e)); + } + validateAndConsolidate(endpointsByZone, zoneEndpoints); + } - List<Endpoint.Target> targets = new ArrayList<>(); - if (level == Endpoint.Level.application) { - Optional<String> endpointRegion = stringAttribute("region", endpointElement); - int weightSum = 0; - for (var instanceElement : XML.getChildren(endpointElement, "instance")) { - String instanceName = instanceElement.getTextContent(); - if (instanceName == null || instanceName.isBlank()) illegal(msgPrefix + "empty 'instance' element"); - Optional<String> instanceRegion = stringAttribute("region", instanceElement); - if (endpointRegion.isPresent() == instanceRegion.isPresent()) - illegal(msgPrefix + "'region' attribute must be declared on either <endpoint> or <instance> tag"); - String weightFromAttribute = requireStringAttribute("weight", instanceElement); - int weight; - try { - weight = Integer.parseInt(weightFromAttribute); - } catch (NumberFormatException e) { - throw new IllegalArgumentException(msgPrefix + "invalid weight value '" + weightFromAttribute + "'"); - } - weightSum += weight; - targets.add(new Endpoint.Target(RegionName.from(endpointRegion.orElseGet(instanceRegion::get)), - InstanceName.from(instanceName), - weight)); - } - if (weightSum == 0) illegal(msgPrefix + "sum of all weights must be positive, got " + weightSum); - } else { - if (stringAttribute("region", endpointElement).isPresent()) illegal(msgPrefix + "invalid 'region' attribute"); - Set<RegionName> regions = new LinkedHashSet<>(); - for (var regionElement : XML.getChildren(endpointElement, "region")) { - String region = regionElement.getTextContent(); - if (region == null || region.isBlank()) - illegal(msgPrefix + "empty 'region' element"); - if ( zoneEndpointType.isEmpty() - && Stream.of(RegionName.from(region), null) - .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get) - .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream()) - .anyMatch(endpoint -> ! endpoint.isPublicEndpoint())) - illegal(msgPrefix + "targets zone endpoint in '" + region + "' with 'enabled' set to 'false'"); - if ( ! regions.add(RegionName.from(region))) - illegal(msgPrefix + "duplicate 'region' element: '" + region + "'"); - } + /** + * @param parentElement + * @param endpointElement + * @param level decide what this method is reading TODO: Split into different methods instead + * @param instance the instance this applies to, or empty if it does not apply to an instance (application endpoints) + * @param steps + * @param forRegions the regions this applies to (for bcp), or empty (otherwise) to read this from "region" subelements + * @param endpointsByZone a map containing any zone endpoints rtead by this + * @return the endpoint read, unless it is added to endspointsByZone instead *sob* + */ + static Optional<Endpoint> readEndpoint(Element parentElement, + Element endpointElement, + Endpoint.Level level, + Optional<String> instance, + List<Step> steps, + Collection<RegionName> forRegions, + Map<String, Map<RegionName, List<ZoneEndpoint>>> endpointsByZone) { + String containerId = requireStringAttribute("container-id", endpointElement); + Optional<String> endpointId = stringAttribute("id", endpointElement); + Optional<String> zoneEndpointType = getZoneEndpointType(endpointElement, level); + String msgPrefix = (level == Endpoint.Level.application ? "Application-level" : "Instance-level") + + " endpoint '" + endpointId.orElse(Endpoint.DEFAULT_ID) + "': "; + + if (zoneEndpointType.isPresent() && endpointId.isPresent()) + illegal(msgPrefix + "cannot declare 'id' with type 'zone' or 'private'"); + + String invalidChild = level == Endpoint.Level.application ? "region" : "instance"; + if ( ! XML.getChildren(endpointElement, invalidChild).isEmpty()) + illegal(msgPrefix + "invalid element '" + invalidChild + "'"); + + boolean enabled = XML.attribute("enabled", endpointElement) + .map(value -> { + if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("zone")) + illegal(msgPrefix + "only endpoints of type 'zone' can specify 'enabled'"); + + return switch (value) { + case "true" -> true; + case "false" -> false; + default -> throw new IllegalArgumentException(msgPrefix + "invalid 'enabled' value; must be 'true' or 'false'"); + }; + }).orElse(true); + + List<AllowedUrn> allowedUrns = new ArrayList<>(); + for (var allow : XML.getChildren(endpointElement, "allow")) { + if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("private")) + illegal(msgPrefix + "only endpoints of type 'private' can specify 'allow' children"); + + switch (requireStringAttribute("with", allow)) { + case "aws-private-link" -> allowedUrns.add(new AllowedUrn(AccessType.awsPrivateLink, requireStringAttribute("arn", allow))); + case "gcp-service-connect" -> allowedUrns.add(new AllowedUrn(AccessType.gcpServiceConnect, requireStringAttribute("project", allow))); + default -> illegal("Private endpoint for container-id '" + containerId + "': " + + "invalid attribute 'with': '" + requireStringAttribute("with", allow) + "'"); + } + } - if (zoneEndpointType.isPresent()) { - if (regions.isEmpty()) regions.add(null); - ZoneEndpoint endpoint = switch (zoneEndpointType.get()) { - case "zone" -> new ZoneEndpoint(enabled, false, List.of()); - case "private" -> new ZoneEndpoint(true, true, allowedUrns); // Doesn't turn off public visibility. - default -> throw new IllegalArgumentException("unsupported zone endpoint type '" + zoneEndpointType.get() + "'"); - }; - for (RegionName region : regions) endpointsByZone.computeIfAbsent(containerId, __ -> new LinkedHashMap<>()) - .computeIfAbsent(region, __ -> new ArrayList<>()) - .add(endpoint); - } - else { - if (regions.isEmpty()) { - // No explicit targets given for instance level endpoint. Include all declared, enabled regions by default - List<RegionName> declared = - steps.stream() - .filter(step -> step.concerns(Environment.prod)) - .flatMap(step -> step.zones().stream()) - .flatMap(zone -> zone.region().stream()) - .toList(); - if (declared.isEmpty()) illegal(msgPrefix + "no declared regions to target"); - - declared.stream().filter(region -> Stream.of(region, null) - .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get) - .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream()) - .allMatch(ZoneEndpoint::isPublicEndpoint)) - .forEach(regions::add); - } - if (regions.isEmpty()) illegal(msgPrefix + "all eligible zone endpoints have 'enabled' set to 'false'"); - InstanceName instanceName = instance.map(InstanceName::from).get(); - for (RegionName region : regions) targets.add(new Target(region, instanceName, 1)); + List<Endpoint.Target> targets = new ArrayList<>(); + if (level == Endpoint.Level.application) { + if ( ! forRegions.isEmpty()) throw new IllegalStateException("Illegal combination"); + Optional<String> endpointRegion = stringAttribute("region", endpointElement); + int weightSum = 0; + for (var instanceElement : XML.getChildren(endpointElement, "instance")) { + String instanceName = instanceElement.getTextContent(); + if (instanceName == null || instanceName.isBlank()) illegal(msgPrefix + "empty 'instance' element"); + Optional<String> instanceRegion = stringAttribute("region", instanceElement); + if (endpointRegion.isPresent() == instanceRegion.isPresent()) + illegal(msgPrefix + "'region' attribute must be declared on either <endpoint> or <instance> tag"); + String weightFromAttribute = requireStringAttribute("weight", instanceElement); + int weight; + try { + weight = Integer.parseInt(weightFromAttribute); + } catch (NumberFormatException e) { + throw new IllegalArgumentException(msgPrefix + "invalid weight value '" + weightFromAttribute + "'"); } + weightSum += weight; + targets.add(new Endpoint.Target(RegionName.from(endpointRegion.orElseGet(instanceRegion::get)), + InstanceName.from(instanceName), + weight)); + } + if (weightSum == 0) illegal(msgPrefix + "sum of all weights must be positive, got " + weightSum); + } else { + if (stringAttribute("region", endpointElement).isPresent()) illegal(msgPrefix + "invalid 'region' attribute"); + + Set<RegionName> regions = new LinkedHashSet<>(forRegions); + List<Element> regionElements = XML.getChildren(endpointElement, "region"); + if ( ! regions.isEmpty() && ! regionElements.isEmpty()) + illegal("Endpoints in <" + parentElement.getTagName() + "> cannot contain <region> children"); + for (var regionElement : XML.getChildren(endpointElement, "region")) { + String region = regionElement.getTextContent(); + if (region == null || region.isBlank()) + illegal(msgPrefix + "empty 'region' element"); + if ( zoneEndpointType.isEmpty() + && Stream.of(RegionName.from(region), null) + .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get) + .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream()) + .anyMatch(endpoint -> ! endpoint.isPublicEndpoint())) + illegal(msgPrefix + "targets zone endpoint in '" + region + "' with 'enabled' set to 'false'"); + if ( ! regions.add(RegionName.from(region))) + illegal(msgPrefix + "duplicate 'region' element: '" + region + "'"); } - if (zoneEndpointType.isEmpty()) { - Endpoint endpoint = new Endpoint(endpointId.orElse(Endpoint.DEFAULT_ID), containerId, level, targets); - if (endpointsById.containsKey(endpoint.endpointId())) { - illegal("Endpoint ID '" + endpoint.endpointId() + "' is specified multiple times"); + if (zoneEndpointType.isPresent()) { + if (regions.isEmpty()) regions.add(null); + ZoneEndpoint endpoint = switch (zoneEndpointType.get()) { + case "zone" -> new ZoneEndpoint(enabled, false, List.of()); + case "private" -> new ZoneEndpoint(true, true, allowedUrns); // Doesn't turn off public visibility. + default -> throw new IllegalArgumentException("unsupported zone endpoint type '" + zoneEndpointType.get() + "'"); + }; + for (RegionName region : regions) endpointsByZone.computeIfAbsent(containerId, __ -> new LinkedHashMap<>()) + .computeIfAbsent(region, __ -> new ArrayList<>()) + .add(endpoint); + } + else { + if (regions.isEmpty()) { + // No explicit targets given for instance level endpoint. Include all declared, enabled regions by default + List<RegionName> declared = + steps.stream() + .filter(step -> step.concerns(Environment.prod)) + .flatMap(step -> step.zones().stream()) + .flatMap(zone -> zone.region().stream()) + .toList(); + if (declared.isEmpty()) illegal(msgPrefix + "no declared regions to target"); + + declared.stream().filter(region -> Stream.of(region, null) + .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get) + .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream()) + .allMatch(ZoneEndpoint::isPublicEndpoint)) + .forEach(regions::add); } - endpointsById.put(endpoint.endpointId(), endpoint); + if (regions.isEmpty()) illegal(msgPrefix + "all eligible zone endpoints have 'enabled' set to 'false'"); + InstanceName instanceName = instance.map(InstanceName::from).get(); + for (RegionName region : regions) targets.add(new Target(region, instanceName, 1)); } - }); - endpoints.addAll(endpointsById.values()); - validateAndConsolidate(endpointsByZone, zoneEndpoints); + } + + if (zoneEndpointType.isEmpty()) + return Optional.of(new Endpoint(endpointId.orElse(Endpoint.DEFAULT_ID), containerId, level, targets)); + return Optional.empty(); } - static Bcp readBcp(Element element) { - Element bcpElement = XML.getChild(element, "bcp"); + static Bcp readBcp(Element parent, Optional<String> instance, List<Step> steps, + List<Endpoint> endpoints, Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints) { + Element bcpElement = XML.getChild(parent, "bcp"); if (bcpElement == null) return Bcp.empty(); 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")) { @@ -470,9 +506,22 @@ public class DeploymentSpecXmlReader { double fraction = toDouble(XML.attribute("fraction", regionElement).orElse(null), "fraction").orElse(1.0); regions.add(new Bcp.RegionMember(region, fraction)); } + for (Element endpointElement : XML.getChildren(groupElement, "endpoint")) { + if (instance.isEmpty()) illegal("The default <bcp> element at the root cannot define endpoints"); + Optional<Endpoint> endpoint = readEndpoint(groupElement, + endpointElement, + Endpoint.Level.instance, + instance, + steps, + regions.stream().map(r -> r.region()).toList(), + endpointsByZone); + endpoint.ifPresent(e -> endpoints.add(e)); + } + Duration deadline = XML.attribute("deadline", groupElement).map(value -> toDuration(value, "deadline")).orElse(Duration.ZERO); - groups.add(new Bcp.Group(regions, deadline)); + groups.add(new Bcp.Group(regions, endpoints, deadline)); } + validateAndConsolidate(endpointsByZone, zoneEndpoints); return new Bcp(groups); } 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 afa7e3e502b..89b7318739e 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 @@ -1294,22 +1294,22 @@ public class DeploymentSpecTest { @Test public void invalidEndpoints() { assertInvalidEndpoints("<endpoint id='FOO' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'FOO'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'FOO'"); assertInvalidEndpoints("<endpoint id='123' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got '123'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got '123'"); assertInvalidEndpoints("<endpoint id='foo!' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo!'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo!'"); assertInvalidEndpoints("<endpoint id='foo.bar' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo.bar'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo.bar'"); assertInvalidEndpoints("<endpoint id='foo--bar' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo--bar'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo--bar'"); assertInvalidEndpoints("<endpoint id='foo-' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo-'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo-'"); assertInvalidEndpoints("<endpoint id='foooooooooooo' container-id='qrs'/>", - "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foooooooooooo'"); + "Endpoint id must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foooooooooooo'"); assertInvalidEndpoints("<endpoint id='foo' container-id='qrs'/><endpoint id='foo' container-id='qrs'/>", - "Endpoint ID 'foo' is specified multiple times"); + "Endpoint id 'foo' is specified multiple times"); assertInvalidEndpoints("<endpoint id='default' type='zone' container-id='foo' />", "Instance-level endpoint 'default': cannot declare 'id' with type 'zone' or 'private'"); assertInvalidEndpoints("<endpoint id='default' type='private' container-id='foo' />", 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 77aadc88be8..e1ff8f012bd 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 @@ -6,6 +6,7 @@ import org.junit.Test; import java.io.StringReader; import java.time.Duration; +import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -100,7 +101,9 @@ public class DeploymentSpecWithBcpTest { </bcp> </deployment> """); + var spec = DeploymentSpec.fromXml(r); + var betaBcp = spec.requireInstance("beta").bcp().orElse(spec.bcp()); assertEquals(1, betaBcp.groups().size()); var betaGroup = betaBcp.groups().get(0); @@ -241,6 +244,164 @@ public class DeploymentSpecWithBcpTest { } } + @Test + public void endpointsDefinedInBcp() { + StringReader r = new StringReader(""" + <deployment version='1.0'> + <instance id='beta'> + <prod> + <region>us-east1</region> + <region>us-east2</region> + </prod> + <bcp> + <group> + <endpoint id="foo" container-id="bar"/> + <region>us-east1</region> + <region>us-east2</region> + </group> + </bcp> + </instance> + <instance id='main'> + <prod> + <region>us-east1</region> + <region>us-east2</region> + <region>us-central1</region> + <region>us-west1</region> + <region>us-west2</region> + </prod> + <bcp> + <group> + <endpoint id="east" container-id="bar"/> + <region>us-east1</region> + <region>us-east2</region> + <region fraction="0.3">us-central1</region> + </group> + <group> + <endpoint id="west" container-id="bar"/> + <region>us-west1</region> + <region>us-west2</region> + <region fraction="0.7">us-central1</region> + </group> + </bcp> + </instance> + </deployment> + """); + + var spec = DeploymentSpec.fromXml(r); + + var betaEndpoints = spec.requireInstance("beta").endpoints(); + assertEquals(1, betaEndpoints.size()); + assertEquals("foo", betaEndpoints.get(0).endpointId()); + assertEquals("bar", betaEndpoints.get(0).containerId()); + assertEquals(List.of(RegionName.from("us-east1"), RegionName.from("us-east2")), + betaEndpoints.get(0).regions()); + + var mainEndpoints = spec.requireInstance("main").endpoints(); + assertEquals(2, mainEndpoints.size()); + assertEquals("east", mainEndpoints.get(0).endpointId()); + assertEquals(List.of(RegionName.from("us-east1"), RegionName.from("us-east2"), RegionName.from("us-central1")), + mainEndpoints.get(0).regions()); + assertEquals("west", mainEndpoints.get(1).endpointId()); + assertEquals(List.of(RegionName.from("us-west1"), RegionName.from("us-west2"), RegionName.from("us-central1")), + mainEndpoints.get(1).regions()); + } + + @Test + public void endpointsDefinedInBcpImplicitInstance() { + StringReader r = new StringReader(""" + <deployment version='1.0'> + <prod> + <region>us-east1</region> + <region>us-east2</region> + <region>us-central1</region> + <region>us-west1</region> + <region>us-west2</region> + </prod> + <bcp> + <group> + <endpoint id="east" container-id="bar"/> + <region>us-east1</region> + <region>us-east2</region> + <region fraction="0.3">us-central1</region> + </group> + <group> + <endpoint id="west" container-id="bar"/> + <region>us-west1</region> + <region>us-west2</region> + <region fraction="0.7">us-central1</region> + </group> + </bcp> + </deployment> + """); + + var spec = DeploymentSpec.fromXml(r); + + var mainEndpoints = spec.requireInstance("default").endpoints(); + assertEquals(2, mainEndpoints.size()); + assertEquals("east", mainEndpoints.get(0).endpointId()); + assertEquals(List.of(RegionName.from("us-east1"), RegionName.from("us-east2"), RegionName.from("us-central1")), + mainEndpoints.get(0).regions()); + assertEquals("west", mainEndpoints.get(1).endpointId()); + assertEquals(List.of(RegionName.from("us-west1"), RegionName.from("us-west2"), RegionName.from("us-central1")), + mainEndpoints.get(1).regions()); + } + + @Test + public void endpontsDefinedInBcpValidation1() { + StringReader r = new StringReader(""" + <deployment version='1.0'> + <instance id='beta'> + <prod> + <region>us-east1</region> + <region>us-east2</region> + </prod> + </instance> + <bcp> + <group> + <endpoint id="foo" container-id="bar"/> + <region>us-east1</region> + <region>us-east2</region> + </group> + </bcp> + </deployment> + """); + try { + DeploymentSpec.fromXml(r); + } + catch (IllegalArgumentException e) { + assertEquals("The default <bcp> element at the root cannot define endpoints", Exceptions.toMessageString(e)); + } + } + + @Test + public void endpontsDefinedInBcpValidation2() { + StringReader r = new StringReader(""" + <deployment version='1.0'> + <instance id='beta'> + <prod> + <region>us-east1</region> + <region>us-east2</region> + </prod> + <bcp> + <group> + <region>us-east1</region> + <region>us-east2</region> + <endpoint id="foo" container-id="bar"> + <region>us-east1</region> + </endpoint> + </group> + </bcp> + </instance> + </deployment> + """); + try { + DeploymentSpec.fromXml(r); + } + catch (IllegalArgumentException e) { + assertEquals("Endpoints in <group> cannot contain <region> children", Exceptions.toMessageString(e)); + } + + } private void assertTwoRegions(DeploymentSpec spec) { var bcp = spec.requireInstance("default").bcp().orElse(spec.bcp()); assertEquals(1, bcp.groups().size()); diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index d63b8885a57..cc9db471120 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -22,6 +22,7 @@ StepExceptInstance = BlockChange* & Notifications? & Endpoints? & + Bcp? & Test? & Staging? & Dev? & @@ -170,3 +171,18 @@ Endpoint = element endpoint { Endpoints = element endpoints { Endpoint+ } + +Bcp = element bcp { + Group+ +} + +Group = element group { + attribute deadline { xsd:string }? & + Endpoint* & + MemberRegion+ +} + +MemberRegion = element region { + attribute fraction { xsd:double }? & + text +} diff --git a/config-model/src/test/schema-test-files/deployment-with-instances.xml b/config-model/src/test/schema-test-files/deployment-with-instances.xml index f37ff9f6cc6..0c3409533d1 100644 --- a/config-model/src/test/schema-test-files/deployment-with-instances.xml +++ b/config-model/src/test/schema-test-files/deployment-with-instances.xml @@ -30,6 +30,18 @@ </endpoint> <endpoint container-id="bar" /> </endpoints> + <bcp> + <group deadline="60m"> + <endpoint id="foo" container-id="baz"/> + <region>us-west-1</region> + <region fraction="0.5">us-central-1</region> + </group> + <group> + <region>us-north-1</region> + <region>us-south-2</region> + <region fraction="0.5">us-central-1</region> + </group> + </bcp> </instance> <delay hours='2'/> diff --git a/config-model/src/test/schema-test-files/deployment.xml b/config-model/src/test/schema-test-files/deployment.xml index 38145a1ac74..bc0f070d88c 100644 --- a/config-model/src/test/schema-test-files/deployment.xml +++ b/config-model/src/test/schema-test-files/deployment.xml @@ -26,4 +26,16 @@ </endpoint> <endpoint container-id="bar" /> </endpoints> + <bcp> + <group deadline="60m"> + <endpoint id="foo" container-id="baz"/> + <region>us-west-1</region> + <region fraction="0.5">us-central-1</region> + </group> + <group> + <region>us-north-1</region> + <region>us-south-2</region> + <region fraction="0.5">us-central-1</region> + </group> + </bcp> </deployment> |