diff options
| author | Jon Bratseth <bratseth@oath.com> | 2019-10-07 17:30:35 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-10-07 17:30:35 +0200 |
| commit | 6474f43ba04731e8bd38d2613ad5098e3cfce90d (patch) | |
| tree | d637f62fa5c6b481181db818d3a83a5e4a307bab | |
| parent | 6ff782a0eb830f2382185a1efd7d0830b3208fae (diff) | |
| parent | fc0c6d5e3af1bd091e398968db4b7a394fb490e0 (diff) | |
Merge pull request #10909 from vespa-engine/revert-10891-bratseth/instances-in-deployment-xml-rebased
Revert "Bratseth/instances in deployment xml rebased"
24 files changed, 674 insertions, 2500 deletions
diff --git a/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java b/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java index c5452ecdde3..a09e23bbe9e 100644 --- a/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java +++ b/abi-check-plugin/src/main/java/com/yahoo/abicheck/mojo/AbiCheck.java @@ -192,7 +192,7 @@ public class AbiCheck extends AbstractMojo { } else { Map<String, JavaClassSignature> abiSpec = readSpec(specFile); if (!compareSignatures(abiSpec, signatures, getLog())) { - throw new MojoFailureException("ABI spec mismatch. To update run 'mvn package -Dabicheck.writeSpec'"); + throw new MojoFailureException("ABI spec mismatch"); } } } catch (IOException e) { diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 315b03c301a..32c9e433157 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -187,35 +187,6 @@ ], "fields": [] }, - "com.yahoo.config.application.api.DeploymentInstanceSpec": { - "superClass": "com.yahoo.config.application.api.DeploymentSpec$Step", - "interfaces": [], - "attributes": [ - "public" - ], - "methods": [ - "public void <init>(com.yahoo.config.provision.InstanceName, java.util.List, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.List, java.util.Optional, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)", - "public com.yahoo.config.provision.InstanceName name()", - "public java.time.Duration delay()", - "public java.util.List steps()", - "public com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy upgradePolicy()", - "public java.util.List changeBlocker()", - "public java.util.Optional globalServiceId()", - "public boolean canUpgradeAt(java.time.Instant)", - "public boolean canChangeRevisionAt(java.time.Instant)", - "public java.util.List zones()", - "public boolean deploysTo(com.yahoo.config.provision.Environment, java.util.Optional)", - "public java.util.Optional athenzDomain()", - "public java.util.Optional athenzService(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", - "public com.yahoo.config.application.api.Notifications notifications()", - "public java.util.List endpoints()", - "public boolean includes(com.yahoo.config.provision.Environment, java.util.Optional)", - "public boolean equals(java.lang.Object)", - "public int hashCode()", - "public java.lang.String toString()" - ], - "fields": [] - }, "com.yahoo.config.application.api.DeploymentSpec$ChangeBlocker": { "superClass": "java.lang.Object", "interfaces": [], @@ -226,8 +197,7 @@ "public void <init>(boolean, boolean, com.yahoo.config.application.api.TimeWindow)", "public boolean blocksRevisions()", "public boolean blocksVersions()", - "public com.yahoo.config.application.api.TimeWindow window()", - "public java.lang.String toString()" + "public com.yahoo.config.application.api.TimeWindow window()" ], "fields": [] }, @@ -264,9 +234,7 @@ "methods": [ "public void <init>(java.time.Duration)", "public java.time.Duration duration()", - "public java.time.Duration delay()", - "public boolean deploysTo(com.yahoo.config.provision.Environment, java.util.Optional)", - "public java.lang.String toString()" + "public boolean deploysTo(com.yahoo.config.provision.Environment, java.util.Optional)" ], "fields": [] }, @@ -279,11 +247,9 @@ "methods": [ "public void <init>(java.util.List)", "public java.util.List zones()", - "public java.util.List steps()", "public boolean deploysTo(com.yahoo.config.provision.Environment, java.util.Optional)", "public boolean equals(java.lang.Object)", - "public int hashCode()", - "public java.lang.String toString()" + "public int hashCode()" ], "fields": [] }, @@ -298,9 +264,7 @@ "public void <init>()", "public final boolean deploysTo(com.yahoo.config.provision.Environment)", "public abstract boolean deploysTo(com.yahoo.config.provision.Environment, java.util.Optional)", - "public java.util.List zones()", - "public java.time.Duration delay()", - "public java.util.List steps()" + "public java.util.List zones()" ], "fields": [] }, @@ -329,7 +293,6 @@ "public" ], "methods": [ - "public void <init>(java.util.List, java.util.Optional, java.lang.String)", "public void <init>(java.util.Optional, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.Optional, java.util.List, java.util.List, java.lang.String, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)", "public java.util.Optional globalServiceId()", "public com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy upgradePolicy()", @@ -339,21 +302,16 @@ "public java.util.List changeBlocker()", "public java.util.List steps()", "public java.util.List zones()", - "public java.util.Optional athenzDomain()", - "public java.util.Optional athenzService(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", "public com.yahoo.config.application.api.Notifications notifications()", "public java.util.List endpoints()", "public java.lang.String xmlForm()", "public boolean includes(com.yahoo.config.provision.Environment, java.util.Optional)", - "public com.yahoo.config.application.api.DeploymentInstanceSpec instance(java.lang.String)", - "public com.yahoo.config.application.api.DeploymentInstanceSpec instance(com.yahoo.config.provision.InstanceName)", - "public com.yahoo.config.application.api.DeploymentInstanceSpec requireInstance(java.lang.String)", - "public com.yahoo.config.application.api.DeploymentInstanceSpec requireInstance(com.yahoo.config.provision.InstanceName)", - "public java.util.List instances()", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.io.Reader)", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.lang.String)", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.lang.String, boolean)", "public static java.lang.String toMessageString(java.lang.Throwable)", + "public java.util.Optional athenzDomain()", + "public java.util.Optional athenzService(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", "public boolean equals(java.lang.Object)", "public int hashCode()", "public static void main(java.lang.String[])" 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 deleted file mode 100644 index df611d66b87..00000000000 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ /dev/null @@ -1,254 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.config.application.api; - -import com.yahoo.config.provision.AthenzDomain; -import com.yahoo.config.provision.AthenzService; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.RegionName; - -import java.time.Duration; -import java.time.Instant; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -/** - * The deployment spec for an application instance - * - * @author bratseth - */ -public class DeploymentInstanceSpec extends DeploymentSpec.Step { - - /** The name of the instance this step deploys */ - private final InstanceName name; - - private final List<DeploymentSpec.Step> steps; - private final DeploymentSpec.UpgradePolicy upgradePolicy; - private final List<DeploymentSpec.ChangeBlocker> changeBlockers; - private final Optional<String> globalServiceId; - private final Optional<AthenzDomain> athenzDomain; - private final Optional<AthenzService> athenzService; - private final Notifications notifications; - private final List<Endpoint> endpoints; - - public DeploymentInstanceSpec(InstanceName name, - List<DeploymentSpec.Step> steps, - DeploymentSpec.UpgradePolicy upgradePolicy, - List<DeploymentSpec.ChangeBlocker> changeBlockers, - Optional<String> globalServiceId, - Optional<AthenzDomain> athenzDomain, - Optional<AthenzService> athenzService, - Notifications notifications, - List<Endpoint> endpoints) { - this.name = name; - this.steps = steps; - this.upgradePolicy = upgradePolicy; - this.changeBlockers = changeBlockers; - this.globalServiceId = globalServiceId; - this.athenzDomain = athenzDomain; - this.athenzService = athenzService; - this.notifications = notifications; - this.endpoints = List.copyOf(validateEndpoints(endpoints, this.steps)); - validateZones(this.steps); - validateEndpoints(this.steps, globalServiceId, this.endpoints); - validateAthenz(); - } - - public InstanceName name() { return name; } - - /** Throw an IllegalArgumentException if any production zone is declared multiple times */ - private void validateZones(List<DeploymentSpec.Step> steps) { - Set<DeploymentSpec.DeclaredZone> zones = new HashSet<>(); - - for (DeploymentSpec.Step step : steps) - for (DeploymentSpec.DeclaredZone zone : step.zones()) - ensureUnique(zone, zones); - } - - private void ensureUnique(DeploymentSpec.DeclaredZone zone, Set<DeploymentSpec.DeclaredZone> zones) { - if ( ! zones.add(zone)) - throw new IllegalArgumentException(zone + " is listed twice in deployment.xml"); - } - - /** Validates the endpoints and makes sure default values are respected */ - private List<Endpoint> validateEndpoints(List<Endpoint> endpoints, List<DeploymentSpec.Step> steps) { - Objects.requireNonNull(endpoints, "Missing endpoints parameter"); - - var productionRegions = steps.stream() - .filter(step -> step.deploysTo(Environment.prod)) - .flatMap(step -> step.zones().stream()) - .flatMap(zone -> zone.region().stream()) - .map(RegionName::value) - .collect(Collectors.toSet()); - - var rebuiltEndpointsList = new ArrayList<Endpoint>(); - - for (var endpoint : endpoints) { - if (endpoint.regions().isEmpty()) { - var rebuiltEndpoint = endpoint.withRegions(productionRegions); - rebuiltEndpointsList.add(rebuiltEndpoint); - } else { - rebuiltEndpointsList.add(endpoint); - } - } - - return List.copyOf(rebuiltEndpointsList); - } - - /** Throw an IllegalArgumentException if an endpoint refers to a region that is not declared in 'prod' */ - private void validateEndpoints(List<DeploymentSpec.Step> steps, Optional<String> globalServiceId, List<Endpoint> endpoints) { - if (globalServiceId.isPresent() && ! endpoints.isEmpty()) { - throw new IllegalArgumentException("Providing both 'endpoints' and 'global-service-id'. Use only 'endpoints'."); - } - - var stepZones = steps.stream() - .flatMap(s -> s.zones().stream()) - .flatMap(z -> z.region().stream()) - .collect(Collectors.toSet()); - - for (var endpoint : endpoints){ - for (var endpointRegion : endpoint.regions()) { - if (! stepZones.contains(endpointRegion)) { - throw new IllegalArgumentException("Region used in endpoint that is not declared in 'prod': " + endpointRegion); - } - } - } - } - - /** - * Throw an IllegalArgumentException if Athenz configuration violates: - * domain not configured -> no zone can configure service - * domain configured -> all zones must configure service - */ - private void validateAthenz() { - // If athenz domain is not set, athenz service cannot be set on any level - if (athenzDomain.isEmpty()) { - for (DeploymentSpec.DeclaredZone zone : zones()) { - if(zone.athenzService().isPresent()) { - throw new IllegalArgumentException("Athenz service configured for zone: " + zone + ", but Athenz domain is not configured"); - } - } - // if athenz domain is not set, athenz service must be set implicitly or directly on all zones. - } else if (athenzService.isEmpty()) { - for (DeploymentSpec.DeclaredZone zone : zones()) { - if (zone.athenzService().isEmpty()) { - throw new IllegalArgumentException("Athenz domain is configured, but Athenz service not configured for zone: " + zone); - } - } - } - } - - @Override - public Duration delay() { - return Duration.ofSeconds(steps.stream().mapToLong(step -> (step.delay().getSeconds())).sum()); - } - - /** Returns the deployment steps inside this in the order they will be performed */ - @Override - public List<DeploymentSpec.Step> steps() { return steps; } - - /** Returns the upgrade policy of this, which is defaultPolicy if none is specified */ - public DeploymentSpec.UpgradePolicy upgradePolicy() { return upgradePolicy; } - - /** Returns time windows where upgrades are disallowed for these instances */ - public List<DeploymentSpec.ChangeBlocker> changeBlocker() { return changeBlockers; } - - /** Returns the ID of the service to expose through global routing, if present */ - public Optional<String> globalServiceId() { return globalServiceId; } - - /** Returns whether the instances in this step can upgrade at the given instant */ - public boolean canUpgradeAt(Instant instant) { - return changeBlockers.stream().filter(block -> block.blocksVersions()) - .noneMatch(block -> block.window().includes(instant)); - } - - /** Returns whether an application revision change for these instances can occur at the given instant */ - public boolean canChangeRevisionAt(Instant instant) { - return changeBlockers.stream().filter(block -> block.blocksRevisions()) - .noneMatch(block -> block.window().includes(instant)); - } - - /** Returns all the deployment steps which are zones in the order they are declared */ - public List<DeploymentSpec.DeclaredZone> zones() { - return steps.stream() - .flatMap(step -> step.zones().stream()) - .collect(Collectors.toList()); - } - - /** Returns whether this deployment spec specifies the given zone, either implicitly or explicitly */ - @Override - public boolean deploysTo(Environment environment, Optional<RegionName> region) { - for (DeploymentSpec.Step step : steps) - if (step.deploysTo(environment, region)) return true; - return false; - } - - /** Returns the athenz domain if configured */ - public Optional<AthenzDomain> athenzDomain() { return athenzDomain; } - - /** Returns the athenz service for environment/region if configured */ - public Optional<AthenzService> athenzService(Environment environment, RegionName region) { - AthenzService athenzService = zones().stream() - .filter(zone -> zone.deploysTo(environment, Optional.of(region))) - .findFirst() - .flatMap(DeploymentSpec.DeclaredZone::athenzService) - .orElse(this.athenzService.orElse(null)); - return Optional.ofNullable(athenzService); - } - - /** Returns the notification configuration of these instances */ - public Notifications notifications() { return notifications; } - - /** Returns the rotations configuration of these instances */ - public List<Endpoint> endpoints() { return endpoints; } - - /** Returns whether this instances deployment specifies the given zone, either implicitly or explicitly */ - public boolean includes(Environment environment, Optional<RegionName> region) { - for (DeploymentSpec.Step step : steps) - if (step.deploysTo(environment, region)) return true; - return false; - } - - DeploymentInstanceSpec withSteps(List<DeploymentSpec.Step> steps) { - return new DeploymentInstanceSpec(name, - steps, - upgradePolicy, - changeBlockers, - globalServiceId, - athenzDomain, - athenzService, - notifications, - endpoints); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - DeploymentInstanceSpec other = (DeploymentInstanceSpec) o; - return globalServiceId.equals(other.globalServiceId) && - upgradePolicy == other.upgradePolicy && - changeBlockers.equals(other.changeBlockers) && - steps.equals(other.steps) && - athenzDomain.equals(other.athenzDomain) && - athenzService.equals(other.athenzService) && - notifications.equals(other.notifications) && - endpoints.equals(other.endpoints); - } - - @Override - public int hashCode() { - return Objects.hash(globalServiceId, upgradePolicy, changeBlockers, steps, athenzDomain, athenzService, notifications, endpoints); - } - - @Override - public String toString() { - return "instance '" + name + "'"; - } - -} 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 71af050174b..efe75d191b8 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 @@ -5,7 +5,6 @@ import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import java.io.BufferedReader; @@ -15,9 +14,11 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; /** @@ -45,195 +46,218 @@ public class DeploymentSpec { Optional.empty(), Notifications.none(), List.of()); - - private final List<Step> steps; + + private final Optional<String> globalServiceId; + private final UpgradePolicy upgradePolicy; private final Optional<Integer> majorVersion; + private final List<ChangeBlocker> changeBlockers; + private final List<Step> steps; private final String xmlForm; + private final Optional<AthenzDomain> athenzDomain; + private final Optional<AthenzService> athenzService; + private final Notifications notifications; + private final List<Endpoint> endpoints; - public DeploymentSpec(List<Step> steps, - Optional<Integer> majorVersion, - String xmlForm) { - if (singleInstance(steps)) { // TODO: Remove this clause after November 2019 - var singleInstance = (DeploymentInstanceSpec)steps.get(0); - this.steps = List.of(singleInstance.withSteps(completeSteps(singleInstance.steps()))); - } - else { - this.steps = List.copyOf(completeSteps(steps)); - } + public DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, Optional<Integer> majorVersion, + List<ChangeBlocker> changeBlockers, List<Step> steps, String xmlForm, + Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, Notifications notifications, + List<Endpoint> endpoints) { + validateTotalDelay(steps); + this.globalServiceId = globalServiceId; + this.upgradePolicy = upgradePolicy; this.majorVersion = majorVersion; + this.changeBlockers = changeBlockers; + this.steps = List.copyOf(completeSteps(new ArrayList<>(steps))); this.xmlForm = xmlForm; - validateTotalDelay(steps); + this.athenzDomain = athenzDomain; + this.athenzService = athenzService; + this.notifications = notifications; + this.endpoints = List.copyOf(validateEndpoints(endpoints, this.steps)); + validateZones(this.steps); + validateAthenz(); + validateEndpoints(this.steps, globalServiceId, this.endpoints); } - // TODO: Remove after October 2019 - public DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, Optional<Integer> majorVersion, - List<ChangeBlocker> changeBlockers, List<Step> steps, String xmlForm, - Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, - Notifications notifications, - List<Endpoint> endpoints) { - this(List.of(new DeploymentInstanceSpec(InstanceName.from("default"), - steps, - upgradePolicy, - changeBlockers, - globalServiceId, - athenzDomain, - athenzService, - notifications, - endpoints)), - majorVersion, - xmlForm); + /** Validates the endpoints and makes sure default values are respected */ + private List<Endpoint> validateEndpoints(List<Endpoint> endpoints, List<Step> steps) { + Objects.requireNonNull(endpoints, "Missing endpoints parameter"); + + var productionRegions = steps.stream() + .filter(step -> step.deploysTo(Environment.prod)) + .flatMap(step -> step.zones().stream()) + .flatMap(zone -> zone.region().stream()) + .map(RegionName::value) + .collect(Collectors.toSet()); + + var rebuiltEndpointsList = new ArrayList<Endpoint>(); + + for (var endpoint : endpoints) { + if (endpoint.regions().isEmpty()) { + var rebuiltEndpoint = endpoint.withRegions(productionRegions); + rebuiltEndpointsList.add(rebuiltEndpoint); + } else { + rebuiltEndpointsList.add(endpoint); + } + } + + return List.copyOf(rebuiltEndpointsList); + } + + /** Throw an IllegalArgumentException if the total delay exceeds 24 hours */ + private void validateTotalDelay(List<Step> steps) { + long totalDelaySeconds = steps.stream().filter(step -> step instanceof Delay) + .mapToLong(delay -> ((Delay)delay).duration().getSeconds()) + .sum(); + if (totalDelaySeconds > Duration.ofHours(24).getSeconds()) + throw new IllegalArgumentException("The total delay specified is " + Duration.ofSeconds(totalDelaySeconds) + + " but max 24 hours is allowed"); } - /** Adds missing required steps and reorders steps to a permissible order */ - private static List<DeploymentSpec.Step> completeSteps(List<DeploymentSpec.Step> inputSteps) { - List<Step> steps = new ArrayList<>(inputSteps); + /** Throw an IllegalArgumentException if any production zone is declared multiple times */ + private void validateZones(List<Step> steps) { + Set<DeclaredZone> zones = new HashSet<>(); + + for (Step step : steps) + for (DeclaredZone zone : step.zones()) + ensureUnique(zone, zones); + } + + /** Throw an IllegalArgumentException if an endpoint refers to a region that is not declared in 'prod' */ + private void validateEndpoints(List<Step> steps, Optional<String> globalServiceId, List<Endpoint> endpoints) { + if (globalServiceId.isPresent() && ! endpoints.isEmpty()) { + throw new IllegalArgumentException("Providing both 'endpoints' and 'global-service-id'. Use only 'endpoints'."); + } + + var stepZones = steps.stream() + .flatMap(s -> s.zones().stream()) + .flatMap(z -> z.region.stream()) + .collect(Collectors.toSet()); + for (var endpoint : endpoints){ + for (var endpointRegion : endpoint.regions()) { + if (! stepZones.contains(endpointRegion)) { + throw new IllegalArgumentException("Region used in endpoint that is not declared in 'prod': " + endpointRegion); + } + } + } + } + + /* + * Throw an IllegalArgumentException if Athenz configuration violates: + * domain not configured -> no zone can configure service + * domain configured -> all zones must configure service + */ + private void validateAthenz() { + // If athenz domain is not set, athenz service cannot be set on any level + if (athenzDomain.isEmpty()) { + for (DeclaredZone zone : zones()) { + if(zone.athenzService().isPresent()) { + throw new IllegalArgumentException("Athenz service configured for zone: " + zone + ", but Athenz domain is not configured"); + } + } + // if athenz domain is not set, athenz service must be set implicitly or directly on all zones. + } else if (athenzService.isEmpty()) { + for (DeclaredZone zone : zones()) { + if (zone.athenzService().isEmpty()) { + throw new IllegalArgumentException("Athenz domain is configured, but Athenz service not configured for zone: " + zone); + } + } + } + } + + private void ensureUnique(DeclaredZone zone, Set<DeclaredZone> zones) { + if ( ! zones.add(zone)) + throw new IllegalArgumentException(zone + " is listed twice in deployment.xml"); + } + + /** Adds missing required steps and reorders steps to a permissible order */ + private static List<Step> completeSteps(List<Step> steps) { // Add staging if required and missing if (steps.stream().anyMatch(step -> step.deploysTo(Environment.prod)) && steps.stream().noneMatch(step -> step.deploysTo(Environment.staging))) { - steps.add(new DeploymentSpec.DeclaredZone(Environment.staging)); + steps.add(new DeclaredZone(Environment.staging)); } - + // Add test if required and missing if (steps.stream().anyMatch(step -> step.deploysTo(Environment.staging)) && steps.stream().noneMatch(step -> step.deploysTo(Environment.test))) { - steps.add(new DeploymentSpec.DeclaredZone(Environment.test)); + steps.add(new DeclaredZone(Environment.test)); } - + // Enforce order test, staging, prod - DeploymentSpec.DeclaredZone testStep = remove(Environment.test, steps); + DeclaredZone testStep = remove(Environment.test, steps); if (testStep != null) steps.add(0, testStep); - DeploymentSpec.DeclaredZone stagingStep = remove(Environment.staging, steps); + DeclaredZone stagingStep = remove(Environment.staging, steps); if (stagingStep != null) steps.add(1, stagingStep); - + return steps; } - /** + /** * Removes the first occurrence of a deployment step to the given environment and returns it. - * + * * @return the removed step, or null if it is not present */ - private static DeploymentSpec.DeclaredZone remove(Environment environment, List<DeploymentSpec.Step> steps) { + private static DeclaredZone remove(Environment environment, List<Step> steps) { for (int i = 0; i < steps.size(); i++) { - if ( ! (steps.get(i) instanceof DeploymentSpec.DeclaredZone)) continue; - DeploymentSpec.DeclaredZone zoneStep = (DeploymentSpec.DeclaredZone)steps.get(i); - if (zoneStep.environment() == environment) { - steps.remove(i); - return zoneStep; - } + if (steps.get(i).deploysTo(environment)) + return (DeclaredZone)steps.remove(i); } return null; } - /** Throw an IllegalArgumentException if the total delay exceeds 24 hours */ - private void validateTotalDelay(List<Step> steps) { - long totalDelaySeconds = steps.stream().mapToLong(step -> (step.delay().getSeconds())).sum(); - if (totalDelaySeconds > Duration.ofHours(24).getSeconds()) - throw new IllegalArgumentException("The total delay specified is " + Duration.ofSeconds(totalDelaySeconds) + - " but max 24 hours is allowed"); + /** Returns the ID of the service to expose through global routing, if present */ + public Optional<String> globalServiceId() { + return globalServiceId; } - // TODO: Remove after October 2019 - private DeploymentInstanceSpec defaultInstance() { - if (singleInstance(steps)) return (DeploymentInstanceSpec)steps.get(0); - throw new IllegalArgumentException("This deployment spec does not support the legacy API " + - "as it has multiple instances: " + - instances().stream().map(Step::toString).collect(Collectors.joining(","))); - } - - // TODO: Remove after October 2019 - public Optional<String> globalServiceId() { return defaultInstance().globalServiceId(); } - - // TODO: Remove after October 2019 - public UpgradePolicy upgradePolicy() { return defaultInstance().upgradePolicy(); } + /** Returns the upgrade policy of this, which is defaultPolicy if none is specified */ + public UpgradePolicy upgradePolicy() { return upgradePolicy; } /** Returns the major version this application is pinned to, or empty (default) to allow all major versions */ public Optional<Integer> majorVersion() { return majorVersion; } - // TODO: Remove after November 2019 - public boolean canUpgradeAt(Instant instant) { return defaultInstance().canUpgradeAt(instant); } + /** Returns whether upgrade can occur at the given instant */ + public boolean canUpgradeAt(Instant instant) { + return changeBlockers.stream().filter(block -> block.blocksVersions()) + .noneMatch(block -> block.window().includes(instant)); + } - // TODO: Remove after November 2019 - public boolean canChangeRevisionAt(Instant instant) { return defaultInstance().canChangeRevisionAt(instant); } + /** Returns whether an application revision change can occur at the given instant */ + public boolean canChangeRevisionAt(Instant instant) { + return changeBlockers.stream().filter(block -> block.blocksRevisions()) + .noneMatch(block -> block.window().includes(instant)); + } - // TODO: Remove after November 2019 - public List<ChangeBlocker> changeBlocker() { return defaultInstance().changeBlocker(); } + /** Returns time windows where upgrades are disallowed */ + public List<ChangeBlocker> changeBlocker() { return changeBlockers; } /** Returns the deployment steps of this in the order they will be performed */ - public List<Step> steps() { - if (singleInstance(steps)) return defaultInstance().steps(); // TODO: Remove line after November 2019 - return steps; - } + public List<Step> steps() { return steps; } - // TODO: Remove after November 2019 + /** Returns all the DeclaredZone deployment steps in the order they are declared */ public List<DeclaredZone> zones() { - return defaultInstance().steps().stream() - .flatMap(step -> step.zones().stream()) - .collect(Collectors.toList()); - } - - // TODO: Remove after November 2019 - public Optional<AthenzDomain> athenzDomain() { return defaultInstance().athenzDomain(); } - - // TODO: Remove after November 2019 - public Optional<AthenzService> athenzService(Environment environment, RegionName region) { - return defaultInstance().athenzService(environment, region); + return steps.stream() + .flatMap(step -> step.zones().stream()) + .collect(Collectors.toList()); } - // TODO: Remove after November 2019 - public Notifications notifications() { return defaultInstance().notifications(); } + /** Returns the notification configuration */ + public Notifications notifications() { return notifications; } - // TODO: Remove after November 2019 - public List<Endpoint> endpoints() { return defaultInstance().endpoints(); } + /** Returns the rotations configuration */ + public List<Endpoint> endpoints() { return endpoints; } /** Returns the XML form of this spec, or null if it was not created by fromXml, nor is empty */ public String xmlForm() { return xmlForm; } - // TODO: Remove after November 2019 + /** Returns whether this deployment spec specifies the given zone, either implicitly or explicitly */ public boolean includes(Environment environment, Optional<RegionName> region) { - return defaultInstance().deploysTo(environment, region); - } - - // TODO: Remove after November 2019 - private static boolean singleInstance(List<DeploymentSpec.Step> steps) { - return steps.size() == 1 && steps.get(0) instanceof DeploymentInstanceSpec; - } - - /** Returns the instance step containing the given instance name, or null if not present */ - public DeploymentInstanceSpec instance(String name) { - return instance(InstanceName.from(name)); - } - - /** Returns the instance step containing the given instance name, or null if not present */ - public DeploymentInstanceSpec instance(InstanceName name) { - for (DeploymentInstanceSpec instance : instances()) { - if (instance.name().equals(name)) - return instance; - } - return null; - } - - /** Returns the instance step containing the given instance name, or throws an IllegalArgumentException if not present */ - public DeploymentInstanceSpec requireInstance(String name) { - return requireInstance(InstanceName.from(name)); - } - - public DeploymentInstanceSpec requireInstance(InstanceName name) { - DeploymentInstanceSpec instance = instance(name); - if (instance == null) - throw new IllegalArgumentException("No instance '" + name + "' in deployment.xml'. Instances: " + - instances().stream().map(spec -> spec.name().toString()).collect(Collectors.joining(","))); - return instance; - } - - /** Returns the steps of this which are instances */ - public List<DeploymentInstanceSpec> instances() { - return steps.stream() - .filter(step -> step instanceof DeploymentInstanceSpec).map(DeploymentInstanceSpec.class::cast) - .collect(Collectors.toList()); + for (Step step : steps) + if (step.deploysTo(environment, region)) return true; + return false; } /** @@ -280,19 +304,40 @@ public class DeploymentSpec { return b.toString(); } + /** Returns the athenz domain if configured */ + public Optional<AthenzDomain> athenzDomain() { + return athenzDomain; + } + + /** Returns the athenz service for environment/region if configured */ + public Optional<AthenzService> athenzService(Environment environment, RegionName region) { + AthenzService athenzService = zones().stream() + .filter(zone -> zone.deploysTo(environment, Optional.of(region))) + .findFirst() + .flatMap(DeclaredZone::athenzService) + .orElse(this.athenzService.orElse(null)); + return Optional.ofNullable(athenzService); + } + @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - DeploymentSpec other = (DeploymentSpec) o; - return majorVersion.equals(other.majorVersion) && - steps.equals(other.steps) && - xmlForm.equals(other.xmlForm); + DeploymentSpec that = (DeploymentSpec) o; + return globalServiceId.equals(that.globalServiceId) && + upgradePolicy == that.upgradePolicy && + majorVersion.equals(that.majorVersion) && + changeBlockers.equals(that.changeBlockers) && + steps.equals(that.steps) && + xmlForm.equals(that.xmlForm) && + athenzDomain.equals(that.athenzDomain) && + athenzService.equals(that.athenzService) && + notifications.equals(that.notifications); } @Override public int hashCode() { - return Objects.hash(majorVersion, steps, xmlForm); + return Objects.hash(globalServiceId, upgradePolicy, majorVersion, changeBlockers, steps, xmlForm, athenzDomain, athenzService, notifications); } /** This may be invoked by a continuous build */ @@ -320,7 +365,7 @@ public class DeploymentSpec { /** A deployment step */ public abstract static class Step { - + /** Returns whether this step deploys to the given region */ public final boolean deploysTo(Environment environment) { return deploysTo(environment, Optional.empty()); @@ -332,12 +377,6 @@ public class DeploymentSpec { /** Returns the zones deployed to in this step */ public List<DeclaredZone> zones() { return Collections.emptyList(); } - /** The delay introduced by this step (beyond the time it takes to execute the step). Default is zero. */ - public Duration delay() { return Duration.ZERO; } - - /** Returns all the steps nested in this. This default implementatiino returns an empty list. */ - public List<Step> steps() { return List.of(); } - } /** A deployment step which is to wait for some time before progressing to the next step */ @@ -348,21 +387,12 @@ public class DeploymentSpec { public Delay(Duration duration) { this.duration = duration; } - - // TODO: Remove after October 2019 + public Duration duration() { return duration; } @Override - public Duration delay() { return duration; } - - @Override public boolean deploysTo(Environment environment, Optional<RegionName> region) { return false; } - @Override - public String toString() { - return "delay " + duration; - } - } /** A deployment step which is to run deployment in a particular zone */ @@ -443,31 +473,21 @@ public class DeploymentSpec { } - /** A deployment step which is to run multiple steps (zones or instances) in parallel */ + /** A deployment step which is to run deployment to multiple zones in parallel */ public static class ParallelZones extends Step { - private final List<Step> steps; - - public ParallelZones(List<Step> steps) { - this.steps = List.copyOf(steps); - } + private final List<DeclaredZone> zones; - /** Returns the steps inside this which are zones */ - @Override - public List<DeclaredZone> zones() { - return this.steps.stream() - .filter(step -> step instanceof DeclaredZone) - .map(DeclaredZone.class::cast) - .collect(Collectors.toList()); + public ParallelZones(List<DeclaredZone> zones) { + this.zones = List.copyOf(zones); } - /** Returns all the steps nested in this */ @Override - public List<Step> steps() { return steps; } + public List<DeclaredZone> zones() { return this.zones; } @Override public boolean deploysTo(Environment environment, Optional<RegionName> region) { - return steps().stream().anyMatch(zone -> zone.deploysTo(environment, region)); + return zones.stream().anyMatch(zone -> zone.deploysTo(environment, region)); } @Override @@ -475,19 +495,13 @@ public class DeploymentSpec { if (this == o) return true; if (!(o instanceof ParallelZones)) return false; ParallelZones that = (ParallelZones) o; - return Objects.equals(steps, that.steps); + return Objects.equals(zones, that.zones); } @Override public int hashCode() { - return Objects.hash(steps); + return Objects.hash(zones); } - - @Override - public String toString() { - return steps.size() + " parallel steps"; - } - } /** Controls when this application will be upgraded to new Vespa versions */ @@ -516,11 +530,6 @@ public class DeploymentSpec { public boolean blocksRevisions() { return revision; } public boolean blocksVersions() { return version; } public TimeWindow window() { return window; } - - @Override - public String toString() { - return "change blocker revision=" + revision + " version=" + version + " window=" + window; - } } 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 cb645813290..72a806bb7be 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 @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.application.api.xml; -import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; import com.yahoo.config.application.api.DeploymentSpec.Delay; @@ -15,7 +14,6 @@ import com.yahoo.config.application.api.TimeWindow; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.io.IOUtils; import com.yahoo.text.XML; @@ -40,36 +38,27 @@ import java.util.stream.Collectors; */ public class DeploymentSpecXmlReader { - private static final String instanceTag = "instance"; private static final String majorVersionTag = "major-version"; private static final String testTag = "test"; private static final String stagingTag = "staging"; - private static final String upgradeTag = "upgrade"; private static final String blockChangeTag = "block-change"; private static final String prodTag = "prod"; - private static final String regionTag = "region"; - private static final String delayTag = "delay"; - private static final String parallelTag = "parallel"; private static final String endpointsTag = "endpoints"; private static final String endpointTag = "endpoint"; - private static final String notificationsTag = "notifications"; - - private static final String idAttribute = "id"; - private static final String athenzServiceAttribute = "athenz-service"; - private static final String athenzDomainAttribute = "athenz-domain"; - private static final String testerFlavorAttribute = "tester-flavor"; private final boolean validate; - /** Creates a validating reader */ + /** + * Creates a validating reader + */ public DeploymentSpecXmlReader() { this(true); } /** - * Creates a deployment spec reader + * Creates a reader * - * @param validate true to validate the input, false to accept any input which can be unambiguously parsed + * @param validate true to validate the input, false to accept any input which can be unabiguously parsed */ public DeploymentSpecXmlReader(boolean validate) { this.validate = validate; @@ -84,135 +73,67 @@ public class DeploymentSpecXmlReader { } } - /** Reads a deployment spec from XML */ - public DeploymentSpec read(String xmlForm) { - Element root = XML.getDocument(xmlForm).getDocumentElement(); - - List<Step> steps = new ArrayList<>(); - if ( ! containsTag(instanceTag, root)) { // deployment spec skipping explicit instance -> "default" instance - steps.addAll(readInstanceContent("default", root, new MutableOptional<>(), root)); - } - else { - if (XML.getChildren(root).stream().anyMatch(child -> child.getTagName().equals(prodTag))) - throw new IllegalArgumentException("A deployment spec cannot have both a <prod> tag and an " + - "<instance> tag under the root: " + - "Wrap the prod tags inside the appropriate instance"); - - for (Element topLevelTag : XML.getChildren(root)) { - if (topLevelTag.getTagName().equals(instanceTag)) - steps.addAll(readInstanceContent(topLevelTag.getAttribute(idAttribute), topLevelTag, new MutableOptional<>(), root)); - else - steps.addAll(readNonInstanceSteps(topLevelTag, new MutableOptional<>(), topLevelTag)); // (No global service id here) - } - } - - return new DeploymentSpec(steps, - optionalIntegerAttribute(majorVersionTag, root), - xmlForm); - } - /** - * Reads the content of an (implicit or explicit) instance tag producing an instances step - * - * @param instanceNameString a comma-separated list of the names of the instances this is for - * @param instanceTag the element having the content of this instance - * @param parentTag the parent of instanceTag (or the same, if this instances is implicitly defined which means instanceTag is the root) - * @return the instances specified, one for each instance name element + * Reads a deployment spec from XML */ - private List<DeploymentInstanceSpec> readInstanceContent(String instanceNameString, - Element instanceTag, - MutableOptional<String> globalServiceId, - Element parentTag) { - if (validate) - validateTagOrder(instanceTag); - - // Values where the parent may provide a default - DeploymentSpec.UpgradePolicy upgradePolicy = readUpgradePolicy(instanceTag, parentTag); - List<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceTag, parentTag); - Optional<AthenzDomain> athenzDomain = stringAttribute(athenzDomainAttribute, instanceTag) - .or(() -> stringAttribute(athenzDomainAttribute, parentTag)) - .map(AthenzDomain::from); - Optional<AthenzService> athenzService = stringAttribute(athenzServiceAttribute, instanceTag) - .or(() -> stringAttribute(athenzServiceAttribute, parentTag)) - .map(AthenzService::from); - Notifications notifications = readNotifications(instanceTag, parentTag); - - // Values where there is no default + public DeploymentSpec read(String xmlForm) { List<Step> steps = new ArrayList<>(); - for (Element instanceChild : XML.getChildren(instanceTag)) - steps.addAll(readNonInstanceSteps(instanceChild, globalServiceId, instanceChild)); - List<Endpoint> endpoints = readEndpoints(instanceTag); - - // Build and return instances with these values - return Arrays.stream(instanceNameString.split(",")) - .map(name -> name.trim()) - .map(name -> new DeploymentInstanceSpec(InstanceName.from(name), - steps, - upgradePolicy, - changeBlockers, - globalServiceId.asOptional(), - athenzDomain, - athenzService, - notifications, - endpoints)) - .collect(Collectors.toList()); - } - - private List<Step> readSteps(Element stepTag, MutableOptional<String> globalServiceId, Element parentTag) { - if (stepTag.getTagName().equals(instanceTag)) - return new ArrayList<>(readInstanceContent(stepTag.getAttribute(idAttribute), stepTag, globalServiceId, parentTag)); - else - return readNonInstanceSteps(stepTag, globalServiceId, parentTag); - - } + Optional<String> globalServiceId = Optional.empty(); + Element root = XML.getDocument(xmlForm).getDocumentElement(); + if (validate) + validateTagOrder(root); + for (Element environmentTag : XML.getChildren(root)) { + if (!isEnvironmentName(environmentTag.getTagName())) continue; + + Environment environment = Environment.from(environmentTag.getTagName()); + Optional<AthenzService> athenzService = stringAttribute("athenz-service", environmentTag).map(AthenzService::from); + Optional<String> testerFlavor = stringAttribute("tester-flavor", environmentTag); + + if (environment == Environment.prod) { + for (Element stepTag : XML.getChildren(environmentTag)) { + if (stepTag.getTagName().equals("delay")) { + steps.add(new Delay(Duration.ofSeconds(longAttribute("hours", stepTag) * 60 * 60 + + longAttribute("minutes", stepTag) * 60 + + longAttribute("seconds", stepTag)))); + } + else if (stepTag.getTagName().equals("parallel")) { + List<DeclaredZone> zones = new ArrayList<>(); + for (Element regionTag : XML.getChildren(stepTag)) { + zones.add(readDeclaredZone(environment, athenzService, testerFlavor, regionTag)); + } + steps.add(new ParallelZones(zones)); + } + else { // a region: deploy step + steps.add(readDeclaredZone(environment, athenzService, testerFlavor, stepTag)); + } + } + } + else { + steps.add(new DeclaredZone(environment, Optional.empty(), false, athenzService, testerFlavor)); + } - // 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, MutableOptional<String> globalServiceId, Element parentTag) { - Optional<AthenzService> athenzService = stringAttribute(athenzServiceAttribute, stepTag) - .or(() -> stringAttribute(athenzServiceAttribute, parentTag)) - .map(AthenzService::from); - Optional<String> testerFlavor = stringAttribute(testerFlavorAttribute, stepTag) - .or(() -> stringAttribute(testerFlavorAttribute, parentTag)); - - if (prodTag.equals(stepTag.getTagName())) - globalServiceId.set(readGlobalServiceId(stepTag)); - else if (readGlobalServiceId(stepTag).isPresent()) - throw new IllegalArgumentException("Attribute 'global-service-id' is only valid on 'prod' tag."); - - switch (stepTag.getTagName()) { - case testTag: case stagingTag: - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor)); - case prodTag: // regions, delay and parallel may be nested within, but we can flatten them - return XML.getChildren(stepTag).stream() - .flatMap(child -> readNonInstanceSteps(child, globalServiceId, stepTag).stream()) - .collect(Collectors.toList()); - case delayTag: - return List.of(new Delay(Duration.ofSeconds(longAttribute("hours", stepTag) * 60 * 60 + - longAttribute("minutes", stepTag) * 60 + - longAttribute("seconds", stepTag)))); - case parallelTag: // regions and instances may be nested within - return List.of(new ParallelZones(XML.getChildren(stepTag).stream() - .flatMap(child -> readSteps(child, globalServiceId, stepTag).stream()) - .collect(Collectors.toList()))); - case regionTag: - return List.of(readDeclaredZone(Environment.prod, athenzService, testerFlavor, stepTag)); - default: - return List.of(); - } - } + if (environment == Environment.prod) + globalServiceId = readGlobalServiceId(environmentTag); + else if (readGlobalServiceId(environmentTag).isPresent()) + throw new IllegalArgumentException("Attribute 'global-service-id' is only valid on 'prod' tag."); - private boolean containsTag(String childTagName, Element parent) { - for (Element child : XML.getChildren(parent)) { - if (child.getTagName().equals(childTagName) || containsTag(childTagName, child)) - return true; } - return false; + Optional<AthenzDomain> athenzDomain = stringAttribute("athenz-domain", root).map(AthenzDomain::from); + Optional<AthenzService> athenzService = stringAttribute("athenz-service", root).map(AthenzService::from); + return new DeploymentSpec(globalServiceId, + readUpgradePolicy(root), + optionalIntegerAttribute(majorVersionTag, root), + readChangeBlockers(root), + steps, + xmlForm, + athenzDomain, + athenzService, + readNotifications(root), + readEndpoints(root)); } - private Notifications readNotifications(Element parent, Element fallbackParent) { - Element notificationsElement = XML.getChild(parent, notificationsTag); - if (notificationsElement == null) - notificationsElement = XML.getChild(fallbackParent, notificationsTag); + private Notifications readNotifications(Element root) { + Element notificationsElement = XML.getChild(root, "notifications"); if (notificationsElement == null) return Notifications.none(); @@ -237,17 +158,16 @@ public class DeploymentSpecXmlReader { return Notifications.of(emailAddresses, emailRoles); } - private List<Endpoint> readEndpoints(Element parent) { - var endpointsElement = XML.getChild(parent, endpointsTag); - if (endpointsElement == null) - return Collections.emptyList(); + private List<Endpoint> readEndpoints(Element root) { + final var endpointsElement = XML.getChild(root, endpointsTag); + if (endpointsElement == null) { return Collections.emptyList(); } - var endpoints = new LinkedHashMap<String, Endpoint>(); + final var endpoints = new LinkedHashMap<String, Endpoint>(); for (var endpointElement : XML.getChildren(endpointsElement, endpointTag)) { - Optional<String> rotationId = stringAttribute("id", endpointElement); - Optional<String> containerId = stringAttribute("container-id", endpointElement); - var regions = new HashSet<String>(); + final Optional<String> rotationId = stringAttribute("id", endpointElement); + final Optional<String> containerId = stringAttribute("container-id", endpointElement); + final var regions = new HashSet<String>(); if (containerId.isEmpty()) { throw new IllegalArgumentException("Missing 'container-id' from 'endpoint' tag."); @@ -335,6 +255,10 @@ public class DeploymentSpecXmlReader { return Optional.ofNullable(value).filter(s -> !s.equals("")); } + private boolean isEnvironmentName(String tagName) { + return tagName.equals(testTag) || tagName.equals(stagingTag) || tagName.equals(prodTag); + } + private DeclaredZone readDeclaredZone(Environment environment, Optional<AthenzService> athenzService, Optional<String> testerFlavor, Element regionTag) { return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())), @@ -343,44 +267,44 @@ public class DeploymentSpecXmlReader { private Optional<String> readGlobalServiceId(Element environmentTag) { String globalServiceId = environmentTag.getAttribute("global-service-id"); - if (globalServiceId == null || globalServiceId.isEmpty()) return Optional.empty(); - return Optional.of(globalServiceId); + if (globalServiceId == null || globalServiceId.isEmpty()) { + return Optional.empty(); + } + else { + return Optional.of(globalServiceId); + } } - private List<DeploymentSpec.ChangeBlocker> readChangeBlockers(Element parent, Element globalBlockersParent) { + private List<DeploymentSpec.ChangeBlocker> readChangeBlockers(Element root) { List<DeploymentSpec.ChangeBlocker> changeBlockers = new ArrayList<>(); - if (globalBlockersParent != parent) { - for (Element tag : XML.getChildren(globalBlockersParent, blockChangeTag)) - changeBlockers.add(readChangeBlocker(tag)); - } - for (Element tag : XML.getChildren(parent, blockChangeTag)) - changeBlockers.add(readChangeBlocker(tag)); - return Collections.unmodifiableList(changeBlockers); - } + for (Element tag : XML.getChildren(root)) { + if (!blockChangeTag.equals(tag.getTagName())) continue; - private DeploymentSpec.ChangeBlocker readChangeBlocker(Element tag) { - boolean blockVersions = trueOrMissing(tag.getAttribute("version")); - boolean blockRevisions = trueOrMissing(tag.getAttribute("revision")); + boolean blockVersions = trueOrMissing(tag.getAttribute("version")); + boolean blockRevisions = trueOrMissing(tag.getAttribute("revision")); - String daySpec = tag.getAttribute("days"); - String hourSpec = tag.getAttribute("hours"); - String zoneSpec = tag.getAttribute("time-zone"); - if (zoneSpec.isEmpty()) zoneSpec = "UTC"; // default - return new DeploymentSpec.ChangeBlocker(blockRevisions, blockVersions, - TimeWindow.from(daySpec, hourSpec, zoneSpec)); + String daySpec = tag.getAttribute("days"); + String hourSpec = tag.getAttribute("hours"); + String zoneSpec = tag.getAttribute("time-zone"); + if (zoneSpec.isEmpty()) { // Default to UTC time zone + zoneSpec = "UTC"; + } + changeBlockers.add(new DeploymentSpec.ChangeBlocker(blockRevisions, blockVersions, + TimeWindow.from(daySpec, hourSpec, zoneSpec))); + } + return Collections.unmodifiableList(changeBlockers); } - /** Returns true if the given value is "true", or if it is missing */ + /** + * Returns true if the given value is "true", or if it is missing + */ private boolean trueOrMissing(String value) { return value == null || value.isEmpty() || value.equals("true"); } - private DeploymentSpec.UpgradePolicy readUpgradePolicy(Element parent, Element fallbackParent) { - Element upgradeElement = XML.getChild(parent, upgradeTag); - if (upgradeElement == null) - upgradeElement = XML.getChild(fallbackParent, upgradeTag); - if (upgradeElement == null) - return DeploymentSpec.UpgradePolicy.defaultPolicy; + private DeploymentSpec.UpgradePolicy readUpgradePolicy(Element root) { + Element upgradeElement = XML.getChild(root, "upgrade"); + if (upgradeElement == null) return DeploymentSpec.UpgradePolicy.defaultPolicy; String policy = upgradeElement.getAttribute("policy"); switch (policy) { @@ -400,14 +324,4 @@ public class DeploymentSpecXmlReader { "to control whether the region should receive production traffic"); } - private static class MutableOptional<T> { - - private Optional<T> value = Optional.empty(); - - public void set(Optional<T> value) { this.value = value; } - - public Optional<T> asOptional() { return value; } - - } - } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java deleted file mode 100644 index dabdd0c4a69..00000000000 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java +++ /dev/null @@ -1,572 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.config.application.api; - -import com.google.common.collect.ImmutableSet; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; -import org.junit.Test; - -import java.io.StringReader; -import java.time.Instant; -import java.time.ZoneId; -import java.util.Collections; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -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 org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -/** - * @author bratseth - */ -// TODO: Remove after October 2019 -public class DeploymentSpecDeprecatedAPITest { - - @Test - public void testSpec() { - String specXml = "<deployment version='1.0'>" + - " <test/>" + - "</deployment>"; - - StringReader r = new StringReader(specXml); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.steps().size()); - assertFalse(spec.majorVersion().isPresent()); - assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.includes(Environment.test, Optional.empty())); - assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertFalse(spec.includes(Environment.staging, Optional.empty())); - assertFalse(spec.includes(Environment.prod, Optional.empty())); - assertFalse(spec.globalServiceId().isPresent()); - } - - @Test - public void testSpecPinningMajorVersion() { - String specXml = "<deployment version='1.0' major-version='6'>" + - " <test/>" + - "</deployment>"; - - StringReader r = new StringReader(specXml); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.steps().size()); - assertTrue(spec.majorVersion().isPresent()); - assertEquals(6, (int)spec.majorVersion().get()); - } - - @Test - public void stagingSpec() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <staging/>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.steps().size()); - assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - assertTrue(spec.includes(Environment.test, Optional.empty())); - assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.includes(Environment.staging, Optional.empty())); - assertFalse(spec.includes(Environment.prod, Optional.empty())); - assertFalse(spec.globalServiceId().isPresent()); - } - - @Test - public void minimalProductionSpec() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(4, spec.steps().size()); - - assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - - assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - - assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.steps().get(2)).active()); - - assertTrue(spec.steps().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.steps().get(3)).active()); - - assertTrue(spec.includes(Environment.test, Optional.empty())); - assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.includes(Environment.staging, Optional.empty())); - assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.globalServiceId().isPresent()); - - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.upgradePolicy()); - } - - @Test - public void maximalProductionSpec() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <test/>" + - " <staging/>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(5, spec.steps().size()); - assertEquals(4, spec.zones().size()); - - assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - - assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - - assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.steps().get(2)).active()); - - assertTrue(spec.steps().get(3) instanceof DeploymentSpec.Delay); - assertEquals(3 * 60 * 60 + 30 * 60, ((DeploymentSpec.Delay)spec.steps().get(3)).duration().getSeconds()); - - assertTrue(spec.steps().get(4).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.steps().get(4)).active()); - - assertTrue(spec.includes(Environment.test, Optional.empty())); - assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.includes(Environment.staging, Optional.empty())); - assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.globalServiceId().isPresent()); - } - - @Test - public void productionSpecWithGlobalServiceId() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <prod global-service-id='query'>" + - " <region active='true'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.globalServiceId(), Optional.of("query")); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInTest() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <test global-service-id='query' />" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInStaging() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <staging global-service-id='query' />" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test - public void productionSpecWithGlobalServiceIdBeforeStaging() { - StringReader r = new StringReader( - "<deployment>" + - " <test/>" + - " <prod global-service-id='qrs'>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " <staging/>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("qrs", spec.globalServiceId().get()); - } - - @Test - public void productionSpecWithUpgradePolicy() { - StringReader r = new StringReader( - "<deployment>" + - " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("canary", spec.upgradePolicy().toString()); - } - - @Test - public void maxDelayExceeded() { - try { - StringReader r = new StringReader( - "<deployment>" + - " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <delay hours='23'/>" + - " <region active='true'>us-central-1</region>" + - " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - fail("Expected exception due to exceeding the max total delay"); - } - catch (IllegalArgumentException e) { - // success - assertEquals("The total delay specified is PT24H1S but max 24 hours is allowed", e.getMessage()); - } - } - - @Test - public void testEmpty() { - assertFalse(DeploymentSpec.empty.globalServiceId().isPresent()); - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, DeploymentSpec.empty.upgradePolicy()); - assertTrue(DeploymentSpec.empty.steps().isEmpty()); - assertEquals("<deployment version='1.0'/>", DeploymentSpec.empty.xmlForm()); - } - - @Test - public void productionSpecWithParallelDeployments() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod> \n" + - " <region active='true'>us-west-1</region>\n" + - " <parallel>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + - " </parallel>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - DeploymentSpec.ParallelZones parallelZones = ((DeploymentSpec.ParallelZones) spec.steps().get(3)); - assertEquals(2, parallelZones.zones().size()); - assertEquals(RegionName.from("us-central-1"), parallelZones.zones().get(0).region().get()); - assertEquals(RegionName.from("us-east-3"), parallelZones.zones().get(1).region().get()); - } - - @Test - public void productionSpecWithDuplicateRegions() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " <parallel>\n" + - " <region active='true'>us-west-1</region>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + - " </parallel>\n" + - " </prod>\n" + - "</deployment>" - ); - try { - DeploymentSpec.fromXml(r); - fail("Expected exception"); - } catch (IllegalArgumentException e) { - assertEquals("prod.us-west-1 is listed twice in deployment.xml", e.getMessage()); - } - } - - @Test(expected = IllegalArgumentException.class) - public void deploymentSpecWithIllegallyOrderedDeploymentSpec1() { - StringReader r = new StringReader( - "<deployment>\n" + - " <block-change days='sat' hours='10' time-zone='CET'/>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - " <block-change days='mon,tue' hours='15-16'/>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test(expected = IllegalArgumentException.class) - public void deploymentSpecWithIllegallyOrderedDeploymentSpec2() { - StringReader r = new StringReader( - "<deployment>\n" + - " <block-change days='sat' hours='10' time-zone='CET'/>\n" + - " <test/>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test - public void deploymentSpecWithChangeBlocker() { - StringReader r = new StringReader( - "<deployment>\n" + - " <block-change revision='false' days='mon,tue' hours='15-16'/>\n" + - " <block-change days='sat' hours='10' time-zone='CET'/>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.changeBlocker().size()); - assertTrue(spec.changeBlocker().get(0).blocksVersions()); - assertFalse(spec.changeBlocker().get(0).blocksRevisions()); - assertEquals(ZoneId.of("UTC"), spec.changeBlocker().get(0).window().zone()); - - assertTrue(spec.changeBlocker().get(1).blocksVersions()); - assertTrue(spec.changeBlocker().get(1).blocksRevisions()); - assertEquals(ZoneId.of("CET"), spec.changeBlocker().get(1).window().zone()); - - assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-18T14:15:30.00Z"))); - assertFalse(spec.canUpgradeAt(Instant.parse("2017-09-18T15:15:30.00Z"))); - assertFalse(spec.canUpgradeAt(Instant.parse("2017-09-18T16:15:30.00Z"))); - assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-18T17:15:30.00Z"))); - - assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-23T09:15:30.00Z"))); - assertFalse(spec.canUpgradeAt(Instant.parse("2017-09-23T08:15:30.00Z"))); // 10 in CET - assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-23T10:15:30.00Z"))); - } - - @Test - public void athenz_config_is_read_from_deployment() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.athenzDomain().get().value(), "domain"); - assertEquals(spec.athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); - } - - @Test - public void athenz_service_is_overridden_from_environment() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>\n" + - " <test/>\n" + - " <prod athenz-service='prod-service'>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.athenzDomain().get().value(), "domain"); - assertEquals(spec.athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); - } - - @Test(expected = IllegalArgumentException.class) - public void it_fails_when_athenz_service_is_not_defined() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain'>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test(expected = IllegalArgumentException.class) - public void it_fails_when_athenz_service_is_configured_but_not_athenz_domain() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod athenz-service='service'>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test - public void noNotifications() { - assertEquals(Notifications.none(), - DeploymentSpec.fromXml("<deployment />").notifications()); - } - - @Test - public void emptyNotifications() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <notifications />" + - "</deployment>"); - assertEquals(Notifications.none(), - spec.notifications()); - } - - @Test - public void someNotifications() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <notifications when=\"failing\">\n" + - " <email role=\"author\"/>\n" + - " <email address=\"john@dev\" when=\"failing-commit\"/>\n" + - " <email address=\"jane@dev\"/>\n" + - " </notifications>\n" + - "</deployment>"); - 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)); - } - - @Test - public void customTesterFlavor() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <test tester-flavor=\"d-1-4-20\" />\n" + - " <prod tester-flavor=\"d-2-8-50\">\n" + - " <region active=\"false\">us-north-7</region>\n" + - " </prod>\n" + - "</deployment>"); - assertEquals(Optional.of("d-1-4-20"), spec.steps().get(0).zones().get(0).testerFlavor()); - assertEquals(Optional.empty(), spec.steps().get(1).zones().get(0).testerFlavor()); - assertEquals(Optional.of("d-2-8-50"), spec.steps().get(2).zones().get(0).testerFlavor()); - } - - @Test - public void noEndpoints() { - assertEquals(Collections.emptyList(), DeploymentSpec.fromXml("<deployment />").endpoints()); - } - - @Test - public void emptyEndpoints() { - final var spec = DeploymentSpec.fromXml("<deployment><endpoints/></deployment>"); - assertEquals(Collections.emptyList(), spec.endpoints()); - } - - @Test - public void someEndpoints() { - final var spec = DeploymentSpec.fromXml("" + - "<deployment>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint id=\"foo\" container-id=\"bar\">" + - " <region>us-east</region>" + - " </endpoint>" + - " <endpoint id=\"nalle\" container-id=\"frosk\" />" + - " <endpoint container-id=\"quux\" />" + - " </endpoints>" + - "</deployment>"); - - assertEquals( - List.of("foo", "nalle", "default"), - spec.endpoints().stream().map(Endpoint::endpointId).collect(Collectors.toList()) - ); - - assertEquals( - List.of("bar", "frosk", "quux"), - spec.endpoints().stream().map(Endpoint::containerId).collect(Collectors.toList()) - ); - - assertEquals(Set.of(RegionName.from("us-east")), spec.endpoints().get(0).regions()); - } - @Test - public void invalidEndpoints() { - assertInvalid("<endpoint id='FOO' container-id='qrs'/>"); // Uppercase - assertInvalid("<endpoint id='123' container-id='qrs'/>"); // Starting with non-character - assertInvalid("<endpoint id='foo!' container-id='qrs'/>"); // Non-alphanumeric - assertInvalid("<endpoint id='foo.bar' container-id='qrs'/>"); - assertInvalid("<endpoint id='foo--bar' container-id='qrs'/>"); // Multiple consecutive dashes - assertInvalid("<endpoint id='foo-' container-id='qrs'/>"); // Trailing dash - assertInvalid("<endpoint id='foooooooooooo' container-id='qrs'/>"); // Too long - assertInvalid("<endpoint id='foo' container-id='qrs'/><endpoint id='foo' container-id='qrs'/>"); // Duplicate - } - - @Test - public void validEndpoints() { - assertEquals(List.of("default"), endpointIds("<endpoint container-id='qrs'/>")); - assertEquals(List.of("default"), endpointIds("<endpoint id='' container-id='qrs'/>")); - assertEquals(List.of("f"), endpointIds("<endpoint id='f' container-id='qrs'/>")); - assertEquals(List.of("foo"), endpointIds("<endpoint id='foo' container-id='qrs'/>")); - assertEquals(List.of("foo-bar"), endpointIds("<endpoint id='foo-bar' container-id='qrs'/>")); - assertEquals(List.of("foo", "bar"), endpointIds("<endpoint id='foo' container-id='qrs'/><endpoint id='bar' container-id='qrs'/>")); - assertEquals(List.of("fooooooooooo"), endpointIds("<endpoint id='fooooooooooo' container-id='qrs'/>")); - } - - @Test - public void endpointDefaultRegions() { - var spec = DeploymentSpec.fromXml("" + - "<deployment>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " <region active=\"true\">us-west</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint id=\"foo\" container-id=\"bar\">" + - " <region>us-east</region>" + - " </endpoint>" + - " <endpoint id=\"nalle\" container-id=\"frosk\" />" + - " <endpoint container-id=\"quux\" />" + - " </endpoints>" + - "</deployment>"); - - assertEquals(Set.of("us-east"), endpointRegions("foo", spec)); - assertEquals(Set.of("us-east", "us-west"), endpointRegions("nalle", spec)); - assertEquals(Set.of("us-east", "us-west"), endpointRegions("default", spec)); - } - - private static void assertInvalid(String endpointTag) { - try { - endpointIds(endpointTag); - fail("Expected exception for input '" + endpointTag + "'"); - } catch (IllegalArgumentException ignored) {} - } - - private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) { - return spec.endpoints().stream() - .filter(endpoint -> endpoint.endpointId().equals(endpointId)) - .flatMap(endpoint -> endpoint.regions().stream()) - .map(RegionName::value) - .collect(Collectors.toSet()); - } - - private static List<String> endpointIds(String endpointTag) { - var xml = "<deployment>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " </prod>" + - " <endpoints>" + - endpointTag + - " </endpoints>" + - "</deployment>"; - - return DeploymentSpec.fromXml(xml).endpoints().stream() - .map(Endpoint::endpointId) - .collect(Collectors.toList()); - } - -} 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 b75801de7ea..47eaf7a515a 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 @@ -14,6 +14,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; 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; @@ -31,36 +32,32 @@ public class DeploymentSpecTest { @Test public void testSpec() { String specXml = "<deployment version='1.0'>" + - " <instance id='default'>" + - " <test/>" + - " </instance>" + + " <test/>" + "</deployment>"; StringReader r = new StringReader(specXml); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.instance("default").steps().size()); + assertEquals(1, spec.steps().size()); assertFalse(spec.majorVersion().isPresent()); - assertTrue(spec.instance("default").steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertFalse(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.empty())); - assertFalse(spec.instance("default").globalServiceId().isPresent()); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); + assertTrue(spec.includes(Environment.test, Optional.empty())); + assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); + assertFalse(spec.includes(Environment.staging, Optional.empty())); + assertFalse(spec.includes(Environment.prod, Optional.empty())); + assertFalse(spec.globalServiceId().isPresent()); } @Test public void testSpecPinningMajorVersion() { String specXml = "<deployment version='1.0' major-version='6'>" + - " <instance id='default'>" + - " <test/>" + - " </instance>" + + " <test/>" + "</deployment>"; StringReader r = new StringReader(specXml); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.instance("default").steps().size()); + assertEquals(1, spec.steps().size()); assertTrue(spec.majorVersion().isPresent()); assertEquals(6, (int)spec.majorVersion().get()); } @@ -69,256 +66,164 @@ public class DeploymentSpecTest { public void stagingSpec() { StringReader r = new StringReader( "<deployment version='1.0'>" + - " <instance id='default'>" + - " <staging/>" + - " </instance>" + + " <staging/>" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.instance("default").steps().size()); - assertTrue(spec.instance("default").steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.instance("default").steps().get(1).deploysTo(Environment.staging)); - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.empty())); - assertFalse(spec.instance("default").globalServiceId().isPresent()); + assertEquals(2, spec.steps().size()); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); + assertTrue(spec.includes(Environment.test, Optional.empty())); + assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); + assertTrue(spec.includes(Environment.staging, Optional.empty())); + assertFalse(spec.includes(Environment.prod, Optional.empty())); + assertFalse(spec.globalServiceId().isPresent()); } @Test public void minimalProductionSpec() { StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='default'>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" + "<deployment version='1.0'>" + + " <prod>" + + " <region active='false'>us-east1</region>" + + " <region active='true'>us-west1</region>" + + " </prod>" + + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(4, spec.instance("default").steps().size()); - - assertTrue(spec.instance("default").steps().get(0).deploysTo(Environment.test)); + assertEquals(4, spec.steps().size()); - assertTrue(spec.instance("default").steps().get(1).deploysTo(Environment.staging)); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.instance("default").steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.instance("default").steps().get(2)).active()); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - assertTrue(spec.instance("default").steps().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.instance("default").steps().get(3)).active()); + assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertFalse(((DeploymentSpec.DeclaredZone)spec.steps().get(2)).active()); - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertTrue(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.instance("default").globalServiceId().isPresent()); + assertTrue(spec.steps().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(((DeploymentSpec.DeclaredZone)spec.steps().get(3)).active()); - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.instance("default").upgradePolicy()); + assertTrue(spec.includes(Environment.test, Optional.empty())); + assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); + assertTrue(spec.includes(Environment.staging, Optional.empty())); + assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertFalse(spec.includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); + assertFalse(spec.globalServiceId().isPresent()); + + assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.upgradePolicy()); } @Test public void maximalProductionSpec() { StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='default'>" + // The block checked by assertCorrectFirstInstance - " <test/>" + - " <staging/>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertCorrectFirstInstance(spec.instance("default")); - } - - @Test - public void maximalProductionSpecMultipleInstances() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='instance1'>" + // The block checked by assertCorrectFirstInstance - " <test/>" + - " <staging/>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - " </instance>" + - " <instance id='instance2'>" + - " <prod>" + - " <region active='true'>us-central1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - - assertCorrectFirstInstance(spec.instance("instance1")); - - DeploymentInstanceSpec instance2 = spec.instance("instance2"); - assertEquals(1, instance2.steps().size()); - assertEquals(1, instance2.zones().size()); - - assertTrue(instance2.steps().get(0).deploysTo(Environment.prod, Optional.of(RegionName.from("us-central1")))); - } - - @Test - public void testMultipleInstancesShortForm() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='instance1, instance2'>" + // The block checked by assertCorrectFirstInstance - " <test/>" + - " <staging/>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" + "<deployment version='1.0'>" + + " <test/>" + + " <staging/>" + + " <prod>" + + " <region active='false'>us-east1</region>" + + " <delay hours='3' minutes='30'/>" + + " <region active='true'>us-west1</region>" + + " </prod>" + + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals(5, spec.steps().size()); + assertEquals(4, spec.zones().size()); - assertCorrectFirstInstance(spec.instance("instance1")); - assertCorrectFirstInstance(spec.instance("instance2")); - } - - private void assertCorrectFirstInstance(DeploymentInstanceSpec instance) { - assertEquals(5, instance.steps().size()); - assertEquals(4, instance.zones().size()); - - assertTrue(instance.steps().get(0).deploysTo(Environment.test)); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(instance.steps().get(1).deploysTo(Environment.staging)); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - assertTrue(instance.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)instance.steps().get(2)).active()); + assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertFalse(((DeploymentSpec.DeclaredZone)spec.steps().get(2)).active()); - assertTrue(instance.steps().get(3) instanceof DeploymentSpec.Delay); - assertEquals(3 * 60 * 60 + 30 * 60, instance.steps().get(3).delay().getSeconds()); + assertTrue(spec.steps().get(3) instanceof DeploymentSpec.Delay); + assertEquals(3 * 60 * 60 + 30 * 60, ((DeploymentSpec.Delay)spec.steps().get(3)).duration().getSeconds()); - assertTrue(instance.steps().get(4).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)instance.steps().get(4)).active()); + assertTrue(spec.steps().get(4).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(((DeploymentSpec.DeclaredZone)spec.steps().get(4)).active()); - assertTrue(instance.includes(Environment.test, Optional.empty())); - assertFalse(instance.includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(instance.includes(Environment.staging, Optional.empty())); - assertTrue(instance.includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(instance.includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(instance.includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(instance.globalServiceId().isPresent()); + assertTrue(spec.includes(Environment.test, Optional.empty())); + assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); + assertTrue(spec.includes(Environment.staging, Optional.empty())); + assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertFalse(spec.includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); + assertFalse(spec.globalServiceId().isPresent()); } @Test public void productionSpecWithGlobalServiceId() { StringReader r = new StringReader( "<deployment version='1.0'>" + - " <instance id='default'>" + - " <prod global-service-id='query'>" + - " <region active='true'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + + " <prod global-service-id='query'>" + + " <region active='true'>us-east-1</region>" + + " <region active='true'>us-west-1</region>" + + " </prod>" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("default").globalServiceId(), Optional.of("query")); + assertEquals(spec.globalServiceId(), Optional.of("query")); } @Test(expected=IllegalArgumentException.class) public void globalServiceIdInTest() { StringReader r = new StringReader( "<deployment version='1.0'>" + - " <instance id='default'>" + - " <test global-service-id='query' />" + - " </instance>" + + " <test global-service-id='query' />" + "</deployment>" ); - DeploymentSpec.fromXml(r); + DeploymentSpec spec = DeploymentSpec.fromXml(r); } @Test(expected=IllegalArgumentException.class) public void globalServiceIdInStaging() { StringReader r = new StringReader( "<deployment version='1.0'>" + - " <instance id='default'>" + - " <staging global-service-id='query' />" + - " </instance>" + + " <staging global-service-id='query' />" + "</deployment>" ); - DeploymentSpec.fromXml(r); + DeploymentSpec spec = DeploymentSpec.fromXml(r); } @Test public void productionSpecWithGlobalServiceIdBeforeStaging() { StringReader r = new StringReader( "<deployment>" + - " <instance id='default'>" + - " <test/>" + - " <prod global-service-id='qrs'>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " <staging/>" + - " </instance>" + + " <test/>" + + " <prod global-service-id='qrs'>" + + " <region active='true'>us-west-1</region>" + + " <region active='true'>us-central-1</region>" + + " <region active='true'>us-east-3</region>" + + " </prod>" + + " <staging/>" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("qrs", spec.instance("default").globalServiceId().get()); + assertEquals("qrs", spec.globalServiceId().get()); } @Test public void productionSpecWithUpgradePolicy() { StringReader r = new StringReader( "<deployment>" + - " <instance id='default'>" + - " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " </instance>" + + " <upgrade policy='canary'/>" + + " <prod>" + + " <region active='true'>us-west-1</region>" + + " <region active='true'>us-central-1</region>" + + " <region active='true'>us-east-3</region>" + + " </prod>" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("canary", spec.instance("default").upgradePolicy().toString()); - } - - @Test - public void upgradePolicyDefault() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <upgrade policy='canary'/>" + - " <instance id='instance1'>" + - " <upgrade policy='conservative'/>" + - " </instance>" + - " <instance id='instance2'>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("conservative", spec.instance("instance1").upgradePolicy().toString()); - assertEquals("canary", spec.instance("instance2").upgradePolicy().toString()); + assertEquals("canary", spec.upgradePolicy().toString()); } @Test @@ -326,16 +231,14 @@ public class DeploymentSpecTest { try { StringReader r = new StringReader( "<deployment>" + - " <instance id='default'>" + - " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <delay hours='23'/>" + - " <region active='true'>us-central-1</region>" + - " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " </instance>" + + " <upgrade policy='canary'/>" + + " <prod>" + + " <region active='true'>us-west-1</region>" + + " <delay hours='23'/>" + + " <region active='true'>us-central-1</region>" + + " <delay minutes='59' seconds='61'/>" + + " <region active='true'>us-east-3</region>" + + " </prod>" + "</deployment>" ); DeploymentSpec.fromXml(r); @@ -349,7 +252,7 @@ public class DeploymentSpecTest { @Test public void testEmpty() { - assertFalse(DeploymentSpec.empty.instance("default").globalServiceId().isPresent()); + assertFalse(DeploymentSpec.empty.globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, DeploymentSpec.empty.upgradePolicy()); assertTrue(DeploymentSpec.empty.steps().isEmpty()); assertEquals("<deployment version='1.0'/>", DeploymentSpec.empty.xmlForm()); @@ -358,139 +261,36 @@ public class DeploymentSpecTest { @Test public void productionSpecWithParallelDeployments() { StringReader r = new StringReader( - "<deployment>" + - " <instance id='default'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <parallel>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </parallel>" + - " </prod>" + - " </instance>" + - "</deployment>" + "<deployment>\n" + + " <prod> \n" + + " <region active='true'>us-west-1</region>\n" + + " <parallel>\n" + + " <region active='true'>us-central-1</region>\n" + + " <region active='true'>us-east-3</region>\n" + + " </parallel>\n" + + " </prod>\n" + + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - DeploymentSpec.ParallelZones parallelZones = ((DeploymentSpec.ParallelZones) spec.instance("default").steps().get(3)); + DeploymentSpec.ParallelZones parallelZones = ((DeploymentSpec.ParallelZones) spec.steps().get(3)); assertEquals(2, parallelZones.zones().size()); assertEquals(RegionName.from("us-central-1"), parallelZones.zones().get(0).region().get()); assertEquals(RegionName.from("us-east-3"), parallelZones.zones().get(1).region().get()); } @Test - public void testTestAndStagingOutsideAndInsideInstance() { - StringReader r = new StringReader( - "<deployment>" + - " <test/>" + - " <staging/>" + - " <instance id='instance0'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - " <instance id='instance1'>" + - " <test/>" + - " <staging/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - List<DeploymentSpec.Step> steps = spec.steps(); - assertEquals(4, steps.size()); - assertEquals("test", steps.get(0).toString()); - assertEquals("staging", steps.get(1).toString()); - assertEquals("instance 'instance0'", steps.get(2).toString()); - assertEquals("instance 'instance1'", steps.get(3).toString()); - - List<DeploymentSpec.Step> instance0Steps = ((DeploymentInstanceSpec)steps.get(2)).steps(); - assertEquals(1, instance0Steps.size()); - assertEquals("prod.us-west-1", instance0Steps.get(0).toString()); - - List<DeploymentSpec.Step> instance1Steps = ((DeploymentInstanceSpec)steps.get(3)).steps(); - assertEquals(3, instance1Steps.size()); - assertEquals("test", instance1Steps.get(0).toString()); - assertEquals("staging", instance1Steps.get(1).toString()); - assertEquals("prod.us-west-1", instance1Steps.get(2).toString()); - } - - @Test - public void testParallelInstances() { - StringReader r = new StringReader( - "<deployment>" + - " <parallel>" + - " <instance id='instance0'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - " <instance id='instance1'>" + - " <prod>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " </instance>" + - " </parallel>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - List<DeploymentSpec.Step> steps = spec.steps(); - assertEquals(3, steps.size()); - assertEquals("test", steps.get(0).toString()); - assertEquals("staging", steps.get(1).toString()); - assertEquals("2 parallel steps", steps.get(2).toString()); - - List<DeploymentSpec.Step> parallelSteps = steps.get(2).steps(); - assertEquals("instance 'instance0'", parallelSteps.get(0).toString()); - assertEquals("instance 'instance1'", parallelSteps.get(1).toString()); - } - - @Test - public void testInstancesWithDelay() { - StringReader r = new StringReader( - "<deployment>" + - " <instance id='instance0'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - " <delay hours='12'/>" + - " <instance id='instance1'>" + - " <prod>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - List<DeploymentSpec.Step> steps = spec.steps(); - assertEquals(5, steps.size()); - assertEquals("test", steps.get(0).toString()); - assertEquals("staging", steps.get(1).toString()); - assertEquals("instance 'instance0'", steps.get(2).toString()); - assertEquals("delay PT12H", steps.get(3).toString()); - assertEquals("instance 'instance1'", steps.get(4).toString()); - } - - @Test public void productionSpecWithDuplicateRegions() { StringReader r = new StringReader( - "<deployment>" + - " <instance id='default'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </parallel>" + - " </prod>" + - " </instance>" + - "</deployment>" + "<deployment>\n" + + " <prod>\n" + + " <region active='true'>us-west-1</region>\n" + + " <parallel>\n" + + " <region active='true'>us-west-1</region>\n" + + " <region active='true'>us-central-1</region>\n" + + " <region active='true'>us-east-3</region>\n" + + " </parallel>\n" + + " </prod>\n" + + "</deployment>" ); try { DeploymentSpec.fromXml(r); @@ -503,328 +303,197 @@ public class DeploymentSpecTest { @Test(expected = IllegalArgumentException.class) public void deploymentSpecWithIllegallyOrderedDeploymentSpec1() { StringReader r = new StringReader( - "<deployment>" + - " <instance id='default'>" + - " <block-change days='sat' hours='10' time-zone='CET'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " <block-change days='mon,tue' hours='15-16'/>" + - " </instance>" + + "<deployment>\n" + + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + + " <prod>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + + " <block-change days='mon,tue' hours='15-16'/>\n" + "</deployment>" ); - DeploymentSpec.fromXml(r); + DeploymentSpec spec = DeploymentSpec.fromXml(r); } @Test(expected = IllegalArgumentException.class) public void deploymentSpecWithIllegallyOrderedDeploymentSpec2() { StringReader r = new StringReader( "<deployment>\n" + - " <instance id='default'>" + - " <block-change days='sat' hours='10' time-zone='CET'/>" + - " <test/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test - public void deploymentSpecWithChangeBlocker() { - StringReader r = new StringReader( - "<deployment>" + - " <instance id='default'>" + - " <block-change revision='false' days='mon,tue' hours='15-16'/>" + - " <block-change days='sat' hours='10' time-zone='CET'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + + " <test/>\n" + + " <prod>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.instance("default").changeBlocker().size()); - assertTrue(spec.instance("default").changeBlocker().get(0).blocksVersions()); - assertFalse(spec.instance("default").changeBlocker().get(0).blocksRevisions()); - assertEquals(ZoneId.of("UTC"), spec.instance("default").changeBlocker().get(0).window().zone()); - - assertTrue(spec.instance("default").changeBlocker().get(1).blocksVersions()); - assertTrue(spec.instance("default").changeBlocker().get(1).blocksRevisions()); - assertEquals(ZoneId.of("CET"), spec.instance("default").changeBlocker().get(1).window().zone()); - - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T14:15:30.00Z"))); - assertFalse(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T15:15:30.00Z"))); - assertFalse(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T16:15:30.00Z"))); - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T17:15:30.00Z"))); - - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-23T09:15:30.00Z"))); - assertFalse(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-23T08:15:30.00Z"))); // 10 in CET - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-23T10:15:30.00Z"))); } @Test - public void testChangeBlockerInheritance() { + public void deploymentSpecWithChangeBlocker() { StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <block-change revision='false' days='mon,tue' hours='15-16'/>" + - " <instance id='instance1'>" + - " <block-change days='sat' hours='10' time-zone='CET'/>" + - " </instance>" + - " <instance id='instance2'>" + - " </instance>" + + "<deployment>\n" + + " <block-change revision='false' days='mon,tue' hours='15-16'/>\n" + + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + + " <prod>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + "</deployment>" ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals(2, spec.changeBlocker().size()); + assertTrue(spec.changeBlocker().get(0).blocksVersions()); + assertFalse(spec.changeBlocker().get(0).blocksRevisions()); + assertEquals(ZoneId.of("UTC"), spec.changeBlocker().get(0).window().zone()); - String inheritedChangeBlocker = "change blocker revision=false version=true window=time window for hour(s) [15, 16] on [monday, tuesday] in UTC"; + assertTrue(spec.changeBlocker().get(1).blocksVersions()); + assertTrue(spec.changeBlocker().get(1).blocksRevisions()); + assertEquals(ZoneId.of("CET"), spec.changeBlocker().get(1).window().zone()); - assertEquals(2, spec.instance("instance1").changeBlocker().size()); - assertEquals(inheritedChangeBlocker, spec.instance("instance1").changeBlocker().get(0).toString()); - assertEquals("change blocker revision=true version=true window=time window for hour(s) [10] on [saturday] in CET", - spec.instance("instance1").changeBlocker().get(1).toString()); + assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-18T14:15:30.00Z"))); + assertFalse(spec.canUpgradeAt(Instant.parse("2017-09-18T15:15:30.00Z"))); + assertFalse(spec.canUpgradeAt(Instant.parse("2017-09-18T16:15:30.00Z"))); + assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-18T17:15:30.00Z"))); - assertEquals(1, spec.instance("instance2").changeBlocker().size()); - assertEquals(inheritedChangeBlocker, spec.instance("instance2").changeBlocker().get(0).toString()); + assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-23T09:15:30.00Z"))); + assertFalse(spec.canUpgradeAt(Instant.parse("2017-09-23T08:15:30.00Z"))); // 10 in CET + assertTrue(spec.canUpgradeAt(Instant.parse("2017-09-23T10:15:30.00Z"))); } @Test public void athenz_config_is_read_from_deployment() { StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>" + - " <instance id='instance1'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + + "<deployment athenz-domain='domain' athenz-service='service'>\n" + + " <prod>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("instance1").athenzDomain().get().value(), "domain"); - assertEquals(spec.instance("instance1").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); - } - - @Test - public void athenz_config_is_read_from_instance() { - StringReader r = new StringReader( - "<deployment>" + - " <instance id='default' athenz-domain='domain' athenz-service='service'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("default").athenzDomain().get().value(), "domain"); - assertEquals(spec.instance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); + assertEquals(spec.athenzDomain().get().value(), "domain"); + assertEquals(spec.athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); } @Test public void athenz_service_is_overridden_from_environment() { StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>" + - " <instance id='default' athenz-domain='domain' athenz-service='service'>" + - " <test/>" + - " <prod athenz-service='prod-service'>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + + "<deployment athenz-domain='domain' athenz-service='service'>\n" + + " <test/>\n" + + " <prod athenz-service='prod-service'>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("default").athenzDomain().get().value(), "domain"); - assertEquals(spec.instance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); + assertEquals(spec.athenzDomain().get().value(), "domain"); + assertEquals(spec.athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); } @Test(expected = IllegalArgumentException.class) public void it_fails_when_athenz_service_is_not_defined() { StringReader r = new StringReader( - "<deployment>" + - " <instance id='default' athenz-domain='domain'>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + + "<deployment athenz-domain='domain'>\n" + + " <prod>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + "</deployment>" ); - DeploymentSpec.fromXml(r); + DeploymentSpec spec = DeploymentSpec.fromXml(r); } @Test(expected = IllegalArgumentException.class) public void it_fails_when_athenz_service_is_configured_but_not_athenz_domain() { StringReader r = new StringReader( - "<deployment>" + - " <instance id='default'>" + - " <prod athenz-service='service'>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + + "<deployment>\n" + + " <prod athenz-service='service'>\n" + + " <region active='true'>us-west-1</region>\n" + + " </prod>\n" + "</deployment>" ); - DeploymentSpec.fromXml(r); + DeploymentSpec spec = DeploymentSpec.fromXml(r); } @Test public void noNotifications() { assertEquals(Notifications.none(), - DeploymentSpec.fromXml("<deployment>" + - " <instance id='default'/>" + - "</deployment>").instance("default").notifications()); + DeploymentSpec.fromXml("<deployment />").notifications()); } @Test public void emptyNotifications() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>" + - " <instance id='default'>" + - " <notifications/>" + - " </instance>" + + DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + + " <notifications />" + "</deployment>"); - assertEquals(Notifications.none(), spec.instance("default").notifications()); + assertEquals(Notifications.none(), + spec.notifications()); } @Test public void someNotifications() { DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <instance id='default'>" + - " <notifications when=\"failing\">" + - " <email role=\"author\"/>" + - " <email address=\"john@dev\" when=\"failing-commit\"/>" + - " <email address=\"jane@dev\"/>" + - " </notifications>" + - " </instance>" + + " <notifications when=\"failing\">\n" + + " <email role=\"author\"/>\n" + + " <email address=\"john@dev\" when=\"failing-commit\"/>\n" + + " <email address=\"jane@dev\"/>\n" + + " </notifications>\n" + "</deployment>"); - assertEquals(ImmutableSet.of(author), spec.instance("default").notifications().emailRolesFor(failing)); - assertEquals(ImmutableSet.of(author), spec.instance("default").notifications().emailRolesFor(failingCommit)); - assertEquals(ImmutableSet.of("john@dev", "jane@dev"), spec.instance("default").notifications().emailAddressesFor(failingCommit)); - assertEquals(ImmutableSet.of("jane@dev"), spec.instance("default").notifications().emailAddressesFor(failing)); - } - - @Test - public void notificationsWithMultipleInstances() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='instance1'>" + - " <notifications when=\"failing\">" + - " <email role=\"author\"/>" + - " <email address=\"john@operator\"/>" + - " </notifications>" + - " </instance>" + - " <instance id='instance2'>" + - " <notifications when=\"failing-commit\">" + - " <email role=\"author\"/>" + - " <email address=\"mary@dev\"/>" + - " </notifications>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - DeploymentInstanceSpec instance1 = spec.instance("instance1"); - assertEquals(Set.of(author), instance1.notifications().emailRolesFor(failing)); - assertEquals(Set.of("john@operator"), instance1.notifications().emailAddressesFor(failing)); - - DeploymentInstanceSpec instance2 = spec.instance("instance2"); - assertEquals(Set.of(author), instance2.notifications().emailRolesFor(failingCommit)); - assertEquals(Set.of("mary@dev"), instance2.notifications().emailAddressesFor(failingCommit)); - } - - @Test - public void notificationsDefault() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <notifications when=\"failing-commit\">" + - " <email role=\"author\"/>" + - " <email address=\"mary@dev\"/>" + - " </notifications>" + - " <instance id='instance1'>" + - " <notifications when=\"failing\">" + - " <email role=\"author\"/>" + - " <email address=\"john@operator\"/>" + - " </notifications>" + - " </instance>" + - " <instance id='instance2'>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - DeploymentInstanceSpec instance1 = spec.instance("instance1"); - assertEquals(Set.of(author), instance1.notifications().emailRolesFor(failing)); - assertEquals(Set.of("john@operator"), instance1.notifications().emailAddressesFor(failing)); - - DeploymentInstanceSpec instance2 = spec.instance("instance2"); - assertEquals(Set.of(author), instance2.notifications().emailRolesFor(failingCommit)); - assertEquals(Set.of("mary@dev"), instance2.notifications().emailAddressesFor(failingCommit)); + 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)); } @Test public void customTesterFlavor() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>" + - " <instance id='default'>" + - " <test tester-flavor=\"d-1-4-20\" />" + - " <prod tester-flavor=\"d-2-8-50\">" + - " <region active=\"false\">us-north-7</region>" + - " </prod>" + - " </instance>" + + DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + + " <test tester-flavor=\"d-1-4-20\" />\n" + + " <prod tester-flavor=\"d-2-8-50\">\n" + + " <region active=\"false\">us-north-7</region>\n" + + " </prod>\n" + "</deployment>"); - assertEquals(Optional.of("d-1-4-20"), spec.instance("default").steps().get(0).zones().get(0).testerFlavor()); - assertEquals(Optional.empty(), spec.instance("default").steps().get(1).zones().get(0).testerFlavor()); - assertEquals(Optional.of("d-2-8-50"), spec.instance("default").steps().get(2).zones().get(0).testerFlavor()); + assertEquals(Optional.of("d-1-4-20"), spec.steps().get(0).zones().get(0).testerFlavor()); + assertEquals(Optional.empty(), spec.steps().get(1).zones().get(0).testerFlavor()); + assertEquals(Optional.of("d-2-8-50"), spec.steps().get(2).zones().get(0).testerFlavor()); } @Test public void noEndpoints() { - assertEquals(Collections.emptyList(), - DeploymentSpec.fromXml("<deployment>" + - " <instance id='default'/>" + - "</deployment>").instance("default").endpoints()); + assertEquals(Collections.emptyList(), DeploymentSpec.fromXml("<deployment />").endpoints()); } @Test public void emptyEndpoints() { - var spec = DeploymentSpec.fromXml("<deployment>" + - " <instance id='default'>" + - " <endpoints/>" + - " </instance>" + - "</deployment>"); - assertEquals(Collections.emptyList(), spec.instance("default").endpoints()); + final var spec = DeploymentSpec.fromXml("<deployment><endpoints/></deployment>"); + assertEquals(Collections.emptyList(), spec.endpoints()); } @Test public void someEndpoints() { - var spec = DeploymentSpec.fromXml("" + - "<deployment>" + - " <instance id='default'>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint id=\"foo\" container-id=\"bar\">" + - " <region>us-east</region>" + - " </endpoint>" + - " <endpoint id=\"nalle\" container-id=\"frosk\" />" + - " <endpoint container-id=\"quux\" />" + - " </endpoints>" + - " </instance>" + - "</deployment>"); + final var spec = DeploymentSpec.fromXml("" + + "<deployment>" + + " <prod>" + + " <region active=\"true\">us-east</region>" + + " </prod>" + + " <endpoints>" + + " <endpoint id=\"foo\" container-id=\"bar\">" + + " <region>us-east</region>" + + " </endpoint>" + + " <endpoint id=\"nalle\" container-id=\"frosk\" />" + + " <endpoint container-id=\"quux\" />" + + " </endpoints>" + + "</deployment>"); assertEquals( List.of("foo", "nalle", "default"), - spec.instance("default").endpoints().stream().map(Endpoint::endpointId).collect(Collectors.toList()) + spec.endpoints().stream().map(Endpoint::endpointId).collect(Collectors.toList()) ); assertEquals( List.of("bar", "frosk", "quux"), - spec.instance("default").endpoints().stream().map(Endpoint::containerId).collect(Collectors.toList()) + spec.endpoints().stream().map(Endpoint::containerId).collect(Collectors.toList()) ); - assertEquals(Set.of(RegionName.from("us-east")), spec.instance("default").endpoints().get(0).regions()); + assertEquals(Set.of(RegionName.from("us-east")), spec.endpoints().get(0).regions()); } - @Test public void invalidEndpoints() { assertInvalid("<endpoint id='FOO' container-id='qrs'/>"); // Uppercase @@ -851,21 +520,19 @@ public class DeploymentSpecTest { @Test public void endpointDefaultRegions() { var spec = DeploymentSpec.fromXml("" + - "<deployment>" + - " <instance id='default'>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " <region active=\"true\">us-west</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint id=\"foo\" container-id=\"bar\">" + - " <region>us-east</region>" + - " </endpoint>" + - " <endpoint id=\"nalle\" container-id=\"frosk\" />" + - " <endpoint container-id=\"quux\" />" + - " </endpoints>" + - " </instance>" + - "</deployment>"); + "<deployment>" + + " <prod>" + + " <region active=\"true\">us-east</region>" + + " <region active=\"true\">us-west</region>" + + " </prod>" + + " <endpoints>" + + " <endpoint id=\"foo\" container-id=\"bar\">" + + " <region>us-east</region>" + + " </endpoint>" + + " <endpoint id=\"nalle\" container-id=\"frosk\" />" + + " <endpoint container-id=\"quux\" />" + + " </endpoints>" + + "</deployment>"); assertEquals(Set.of("us-east"), endpointRegions("foo", spec)); assertEquals(Set.of("us-east", "us-west"), endpointRegions("nalle", spec)); @@ -880,7 +547,7 @@ public class DeploymentSpecTest { } private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) { - return spec.instance("default").endpoints().stream() + return spec.endpoints().stream() .filter(endpoint -> endpoint.endpointId().equals(endpointId)) .flatMap(endpoint -> endpoint.regions().stream()) .map(RegionName::value) @@ -889,17 +556,15 @@ public class DeploymentSpecTest { private static List<String> endpointIds(String endpointTag) { var xml = "<deployment>" + - " <instance id='default'>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " </prod>" + - " <endpoints>" + + " <prod>" + + " <region active=\"true\">us-east</region>" + + " </prod>" + + " <endpoints>" + endpointTag + - " </endpoints>" + - " </instance>" + + " </endpoints>" + "</deployment>"; - return DeploymentSpec.fromXml(xml).instance("default").endpoints().stream() + return DeploymentSpec.fromXml(xml).endpoints().stream() .map(Endpoint::endpointId) .collect(Collectors.toList()); } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java deleted file mode 100644 index 33ef3f4bea8..00000000000 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ /dev/null @@ -1,526 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.config.application.api; - -import com.google.common.collect.ImmutableSet; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; -import org.junit.Test; - -import java.io.StringReader; -import java.time.Instant; -import java.time.ZoneId; -import java.util.Collections; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -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 org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -/** - * @author bratseth - */ -public class DeploymentSpecWithoutInstanceTest { - - @Test - public void testSpec() { - String specXml = "<deployment version='1.0'>" + - " <test/>" + - "</deployment>"; - - StringReader r = new StringReader(specXml); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.steps().size()); - assertFalse(spec.majorVersion().isPresent()); - assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertFalse(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.empty())); - assertFalse(spec.instance("default").globalServiceId().isPresent()); - } - - @Test - public void testSpecPinningMajorVersion() { - String specXml = "<deployment version='1.0' major-version='6'>" + - " <test/>" + - "</deployment>"; - - StringReader r = new StringReader(specXml); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.steps().size()); - assertTrue(spec.majorVersion().isPresent()); - assertEquals(6, (int)spec.majorVersion().get()); - } - - @Test - public void stagingSpec() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <staging/>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.steps().size()); - assertTrue(spec.instance("default").steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.instance("default").steps().get(1).deploysTo(Environment.staging)); - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.empty())); - assertFalse(spec.instance("default").globalServiceId().isPresent()); - } - - @Test - public void minimalProductionSpec() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(4, spec.instance("default").steps().size()); - - assertTrue(spec.instance("default").steps().get(0).deploysTo(Environment.test)); - - assertTrue(spec.instance("default").steps().get(1).deploysTo(Environment.staging)); - - assertTrue(spec.instance("default").steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.instance("default").steps().get(2)).active()); - - assertTrue(spec.instance("default").steps().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.instance("default").steps().get(3)).active()); - - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertTrue(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.instance("default").globalServiceId().isPresent()); - - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.instance("default").upgradePolicy()); - } - - @Test - public void maximalProductionSpec() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <test/>" + - " <staging/>" + - " <prod>" + - " <region active='false'>us-east1</region>" + - " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(5, spec.instance("default").steps().size()); - assertEquals(4, spec.instance("default").zones().size()); - - assertTrue(spec.instance("default").steps().get(0).deploysTo(Environment.test)); - - assertTrue(spec.instance("default").steps().get(1).deploysTo(Environment.staging)); - - assertTrue(spec.instance("default").steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.instance("default").steps().get(2)).active()); - - assertTrue(spec.instance("default").steps().get(3) instanceof DeploymentSpec.Delay); - assertEquals(3 * 60 * 60 + 30 * 60, spec.instance("default").steps().get(3).delay().getSeconds()); - - assertTrue(spec.instance("default").steps().get(4).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.instance("default").steps().get(4)).active()); - - assertTrue(spec.instance("default").includes(Environment.test, Optional.empty())); - assertFalse(spec.instance("default").includes(Environment.test, Optional.of(RegionName.from("region1")))); - assertTrue(spec.instance("default").includes(Environment.staging, Optional.empty())); - assertTrue(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.instance("default").includes(Environment.prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.instance("default").globalServiceId().isPresent()); - } - - @Test - public void productionSpecWithGlobalServiceId() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <prod global-service-id='query'>" + - " <region active='true'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("default").globalServiceId(), Optional.of("query")); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInTest() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <test global-service-id='query' />" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInStaging() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <staging global-service-id='query' />" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test - public void productionSpecWithGlobalServiceIdBeforeStaging() { - StringReader r = new StringReader( - "<deployment>" + - " <test/>" + - " <prod global-service-id='qrs'>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - " <staging/>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("qrs", spec.instance("default").globalServiceId().get()); - } - - @Test - public void productionSpecWithUpgradePolicy() { - StringReader r = new StringReader( - "<deployment>" + - " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("canary", spec.instance("default").upgradePolicy().toString()); - } - - @Test - public void maxDelayExceeded() { - try { - StringReader r = new StringReader( - "<deployment>" + - " <upgrade policy='canary'/>" + - " <prod>" + - " <region active='true'>us-west-1</region>" + - " <delay hours='23'/>" + - " <region active='true'>us-central-1</region>" + - " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + - " </prod>" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - fail("Expected exception due to exceeding the max total delay"); - } - catch (IllegalArgumentException e) { - // success - assertEquals("The total delay specified is PT24H1S but max 24 hours is allowed", e.getMessage()); - } - } - - @Test - public void testEmpty() { - assertFalse(DeploymentSpec.empty.instance("default").globalServiceId().isPresent()); - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, DeploymentSpec.empty.upgradePolicy()); - assertTrue(DeploymentSpec.empty.steps().isEmpty()); - assertEquals("<deployment version='1.0'/>", DeploymentSpec.empty.xmlForm()); - } - - @Test - public void productionSpecWithParallelDeployments() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod> \n" + - " <region active='true'>us-west-1</region>\n" + - " <parallel>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + - " </parallel>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - DeploymentSpec.ParallelZones parallelZones = ((DeploymentSpec.ParallelZones) spec.instance("default").steps().get(3)); - assertEquals(2, parallelZones.zones().size()); - assertEquals(RegionName.from("us-central-1"), parallelZones.zones().get(0).region().get()); - assertEquals(RegionName.from("us-east-3"), parallelZones.zones().get(1).region().get()); - } - - @Test - public void productionSpecWithDuplicateRegions() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " <parallel>\n" + - " <region active='true'>us-west-1</region>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + - " </parallel>\n" + - " </prod>\n" + - "</deployment>" - ); - try { - DeploymentSpec.fromXml(r); - fail("Expected exception"); - } catch (IllegalArgumentException e) { - assertEquals("prod.us-west-1 is listed twice in deployment.xml", e.getMessage()); - } - } - - @Test(expected = IllegalArgumentException.class) - public void deploymentSpecWithIllegallyOrderedDeploymentSpec1() { - StringReader r = new StringReader( - "<deployment>\n" + - " <block-change days='sat' hours='10' time-zone='CET'/>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - " <block-change days='mon,tue' hours='15-16'/>\n" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test(expected = IllegalArgumentException.class) - public void deploymentSpecWithIllegallyOrderedDeploymentSpec2() { - StringReader r = new StringReader( - "<deployment>\n" + - " <block-change days='sat' hours='10' time-zone='CET'/>\n" + - " <test/>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test - public void deploymentSpecWithChangeBlocker() { - StringReader r = new StringReader( - "<deployment>\n" + - " <block-change revision='false' days='mon,tue' hours='15-16'/>\n" + - " <block-change days='sat' hours='10' time-zone='CET'/>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.instance("default").changeBlocker().size()); - assertTrue(spec.instance("default").changeBlocker().get(0).blocksVersions()); - assertFalse(spec.instance("default").changeBlocker().get(0).blocksRevisions()); - assertEquals(ZoneId.of("UTC"), spec.instance("default").changeBlocker().get(0).window().zone()); - - assertTrue(spec.instance("default").changeBlocker().get(1).blocksVersions()); - assertTrue(spec.instance("default").changeBlocker().get(1).blocksRevisions()); - assertEquals(ZoneId.of("CET"), spec.instance("default").changeBlocker().get(1).window().zone()); - - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T14:15:30.00Z"))); - assertFalse(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T15:15:30.00Z"))); - assertFalse(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T16:15:30.00Z"))); - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-18T17:15:30.00Z"))); - - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-23T09:15:30.00Z"))); - assertFalse(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-23T08:15:30.00Z"))); // 10 in CET - assertTrue(spec.instance("default").canUpgradeAt(Instant.parse("2017-09-23T10:15:30.00Z"))); - } - - @Test - public void athenz_config_is_read_from_deployment() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("default").athenzDomain().get().value(), "domain"); - assertEquals(spec.instance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); - } - - @Test - public void athenz_service_is_overridden_from_environment() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>\n" + - " <test/>\n" + - " <prod athenz-service='prod-service'>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.instance("default").athenzDomain().get().value(), "domain"); - assertEquals(spec.instance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); - } - - @Test(expected = IllegalArgumentException.class) - public void it_fails_when_athenz_service_is_not_defined() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain'>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test(expected = IllegalArgumentException.class) - public void it_fails_when_athenz_service_is_configured_but_not_athenz_domain() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod athenz-service='service'>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test - public void noNotifications() { - assertEquals(Notifications.none(), - DeploymentSpec.fromXml("<deployment />").instance("default").notifications()); - } - - @Test - public void emptyNotifications() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <notifications />" + - "</deployment>"); - assertEquals(Notifications.none(), spec.instance("default").notifications()); - } - - @Test - public void someNotifications() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <notifications when=\"failing\">\n" + - " <email role=\"author\"/>\n" + - " <email address=\"john@dev\" when=\"failing-commit\"/>\n" + - " <email address=\"jane@dev\"/>\n" + - " </notifications>\n" + - "</deployment>"); - assertEquals(ImmutableSet.of(author), spec.instance("default").notifications().emailRolesFor(failing)); - assertEquals(ImmutableSet.of(author), spec.instance("default").notifications().emailRolesFor(failingCommit)); - assertEquals(ImmutableSet.of("john@dev", "jane@dev"), spec.instance("default").notifications().emailAddressesFor(failingCommit)); - assertEquals(ImmutableSet.of("jane@dev"), spec.instance("default").notifications().emailAddressesFor(failing)); - } - - @Test - public void customTesterFlavor() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment>\n" + - " <test tester-flavor=\"d-1-4-20\" />\n" + - " <prod tester-flavor=\"d-2-8-50\">\n" + - " <region active=\"false\">us-north-7</region>\n" + - " </prod>\n" + - "</deployment>"); - assertEquals(Optional.of("d-1-4-20"), spec.instance("default").steps().get(0).zones().get(0).testerFlavor()); - assertEquals(Optional.empty(), spec.instance("default").steps().get(1).zones().get(0).testerFlavor()); - assertEquals(Optional.of("d-2-8-50"), spec.instance("default").steps().get(2).zones().get(0).testerFlavor()); - } - - @Test - public void noEndpoints() { - assertEquals(Collections.emptyList(), DeploymentSpec.fromXml("<deployment />").instance("default").endpoints()); - } - - @Test - public void emptyEndpoints() { - var spec = DeploymentSpec.fromXml("<deployment><endpoints/></deployment>"); - assertEquals(Collections.emptyList(), spec.instance("default").endpoints()); - } - - @Test - public void someEndpoints() { - var spec = DeploymentSpec.fromXml("" + - "<deployment>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint id=\"foo\" container-id=\"bar\">" + - " <region>us-east</region>" + - " </endpoint>" + - " <endpoint id=\"nalle\" container-id=\"frosk\" />" + - " <endpoint container-id=\"quux\" />" + - " </endpoints>" + - "</deployment>"); - - assertEquals( - List.of("foo", "nalle", "default"), - spec.instance("default").endpoints().stream().map(Endpoint::endpointId).collect(Collectors.toList()) - ); - - assertEquals( - List.of("bar", "frosk", "quux"), - spec.instance("default").endpoints().stream().map(Endpoint::containerId).collect(Collectors.toList()) - ); - - assertEquals(Set.of(RegionName.from("us-east")), spec.instance("default").endpoints().get(0).regions()); - } - - @Test - public void endpointDefaultRegions() { - var spec = DeploymentSpec.fromXml("" + - "<deployment>" + - " <prod>" + - " <region active=\"true\">us-east</region>" + - " <region active=\"true\">us-west</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint id=\"foo\" container-id=\"bar\">" + - " <region>us-east</region>" + - " </endpoint>" + - " <endpoint id=\"nalle\" container-id=\"frosk\" />" + - " <endpoint container-id=\"quux\" />" + - " </endpoints>" + - "</deployment>"); - - assertEquals(Set.of("us-east"), endpointRegions("foo", spec)); - assertEquals(Set.of("us-east", "us-west"), endpointRegions("nalle", spec)); - assertEquals(Set.of("us-east", "us-west"), endpointRegions("default", spec)); - } - - private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) { - return spec.instance("default").endpoints().stream() - .filter(endpoint -> endpoint.endpointId().equals(endpointId)) - .flatMap(endpoint -> endpoint.regions().stream()) - .map(RegionName::value) - .collect(Collectors.toSet()); - } - -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidator.java new file mode 100644 index 00000000000..7757a8d4748 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidator.java @@ -0,0 +1,40 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.container.ContainerCluster; +import com.yahoo.vespa.model.container.ContainerModel; + +import java.io.Reader; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Validates that deployment file (deployment.xml) has valid values (for now + * only global-service-id is validated) + * + * @author hmusum + */ +public class DeploymentFileValidator extends Validator { + + @Override + public void validate(VespaModel model, DeployState deployState) { + Optional<Reader> deployment = deployState.getApplicationPackage().getDeployment(); + + if (deployment.isPresent()) { + Reader deploymentReader = deployment.get(); + DeploymentSpec deploymentSpec = DeploymentSpec.fromXml(deploymentReader); + final Optional<String> globalServiceId = deploymentSpec.globalServiceId(); + if (globalServiceId.isPresent()) { + Set<ContainerCluster> containerClusters = model.getRoot().configModelRepo().getModels(ContainerModel.class).stream(). + map(ContainerModel::getCluster).filter(cc -> cc.getName().equals(globalServiceId.get())).collect(Collectors.toSet()); + if (containerClusters.size() != 1) { + throw new IllegalArgumentException("global-service-id '" + globalServiceId.get() + "' specified in deployment.xml does not match any container cluster id"); + } + } + } + } +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java deleted file mode 100644 index ac38336a405..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation; - -import com.yahoo.config.application.api.DeploymentInstanceSpec; -import com.yahoo.config.application.api.DeploymentSpec; -import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.container.ContainerModel; - -import java.io.Reader; -import java.util.List; -import java.util.Optional; - -/** - * Validates that deployment spec (deployment.xml) has valid values (for now - * only global-service-id is validated) - * - * @author hmusum - * @author bratseth - */ -public class DeploymentSpecValidator extends Validator { - - @Override - public void validate(VespaModel model, DeployState deployState) { - Optional<Reader> deployment = deployState.getApplicationPackage().getDeployment(); - if ( deployment.isEmpty()) return; - - Reader deploymentReader = deployment.get(); - DeploymentSpec deploymentSpec = DeploymentSpec.fromXml(deploymentReader); - List<ContainerModel> containers = model.getRoot().configModelRepo().getModels(ContainerModel.class); - for (DeploymentInstanceSpec instance : deploymentSpec.instances()) { - instance.globalServiceId().ifPresent(globalServiceId -> { - if ( containers.stream().noneMatch(container -> container.getCluster().getName().equals(globalServiceId))) - throw new IllegalArgumentException("The global-service-id in " + instance + ", '" + globalServiceId + - "' specified in deployment.xml does not match any container cluster id"); - }); - } - } - -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index 7d0d068f9d6..042c7cc867c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -53,7 +53,7 @@ public class Validation { new StreamingValidator().validate(model, deployState); new RankSetupValidator(validationParameters.ignoreValidationErrors()).validate(model, deployState); new NoPrefixForIndexes().validate(model, deployState); - new DeploymentSpecValidator().validate(model, deployState); + new DeploymentFileValidator().validate(model, deployState); new RankingConstantsValidator().validate(model, deployState); new SecretStoreValidator().validate(model, deployState); new TlsSecretsValidator().validate(model, deployState); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index caf84d88cf4..f4c7f49a9a0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -197,6 +197,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { addClientProviders(deployState, spec, cluster); addServerProviders(deployState, spec, cluster); + addAthensCopperArgos(cluster, context); // Must be added after nodes. } @@ -227,15 +228,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } private void addRotationProperties(ApplicationContainerCluster cluster, Zone zone, Set<Rotation> rotations, Set<ContainerEndpoint> endpoints, DeploymentSpec spec) { - Optional<String> globalServiceId = spec.requireInstance(app.getApplicationId().instance()).globalServiceId(); cluster.getContainers().forEach(container -> { - setRotations(container, rotations, endpoints, globalServiceId, cluster.getName()); + setRotations(container, rotations, endpoints, spec.globalServiceId(), cluster.getName()); container.setProp("activeRotation", Boolean.toString(zoneHasActiveRotation(zone, spec))); }); } private boolean zoneHasActiveRotation(Zone zone, DeploymentSpec spec) { - return spec.requireInstance(app.getApplicationId().instance()).zones().stream() + return spec.zones().stream() .anyMatch(declaredZone -> declaredZone.deploysTo(zone.environment(), Optional.of(zone.region())) && declaredZone.active()); } @@ -893,8 +893,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { Zone zone, DeploymentSpec spec) { spec.athenzDomain().ifPresent(domain -> { - AthenzService service = spec.requireInstance(app.getApplicationId().instance()).athenzService(zone.environment(), zone.region()) - .orElseThrow(() -> new RuntimeException("Missing Athenz service configuration in instance '" + app.getApplicationId().instance() + "'")); + AthenzService service = spec.athenzService(zone.environment(), zone.region()) + .orElseThrow(() -> new RuntimeException("Missing Athenz service configuration")); String zoneDnsSuffix = zone.environment().value() + "-" + zone.region().value() + "." + athenzDnsSuffix; IdentityProvider identityProvider = new IdentityProvider(domain, service, getLoadBalancerName(loadBalancerName, configServerSpecs), ztsUrl, zoneDnsSuffix, zone); cluster.addComponent(identityProvider); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidatorTest.java index c6d56455d44..5fc3f815b09 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentFileValidatorTest.java @@ -18,7 +18,7 @@ import static org.junit.Assert.fail; /** * @author hmusum */ -public class DeploymentSpecValidatorTest { +public class DeploymentFileValidatorTest { @Test public void testDeploymentWithNonExistentGlobalId() throws IOException, SAXException { @@ -58,7 +58,7 @@ public class DeploymentSpecValidatorTest { try { final DeployState deployState = builder.build(); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - new DeploymentSpecValidator().validate(model, deployState); + new DeploymentFileValidator().validate(model, deployState); fail("Did not get expected exception"); } catch (IllegalArgumentException e) { assertThat(e.getMessage(), containsString("specified in deployment.xml does not match any container cluster id")); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java index 012f246a227..b9573b21199 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java @@ -5,6 +5,7 @@ package com.yahoo.config.provision; * Environments in hosted Vespa. * * @author bratseth + * @since 5.11 */ public enum Environment { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index 96df067843d..54c96c0461d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -9,7 +9,6 @@ import com.yahoo.component.Version; import com.yahoo.component.Vtag; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.FileRegistry; import com.yahoo.config.model.api.ConfigDefinitionRepo; @@ -122,8 +121,7 @@ public class SessionPreparer { preparation.writeTlsZK(); var globalServiceId = context.getApplicationPackage().getDeployment() .map(DeploymentSpec::fromXml) - .map(spec -> spec.requireInstance(context.getApplicationPackage().getApplicationId().instance())) - .flatMap(DeploymentInstanceSpec::globalServiceId); + .flatMap(DeploymentSpec::globalServiceId); preparation.writeContainerEndpointsZK(globalServiceId); preparation.distribute(); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java index f565111c363..8b8be1a27d7 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java @@ -81,7 +81,7 @@ public class ZKApplicationPackageTest { assertThat(readInfo.getHosts().iterator().next().flavor(), is(TEST_FLAVOR)); assertEquals("6.0.1", readInfo.getHosts().iterator().next().version().get().toString()); assertTrue(zkApp.getDeployment().isPresent()); - assertEquals("mydisc", DeploymentSpec.fromXml(zkApp.getDeployment().get()).instance("default").globalServiceId().get()); + assertThat(DeploymentSpec.fromXml(zkApp.getDeployment().get()).globalServiceId().get(), is("mydisc")); } private void feed(ConfigCurator zk, File dirToFeed) throws IOException { diff --git a/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java b/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java index eceffb379aa..a37255436ca 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java +++ b/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java @@ -31,12 +31,16 @@ import com.yahoo.vespa.defaults.Defaults; */ public final class VipStatusHandler extends ThreadedHttpRequestHandler { + private static final Logger log = Logger.getLogger(VipStatusHandler.class.getName()); + private static final String NUM_REQUESTS_METRIC = "jdisc.http.requests.status"; private final boolean accessDisk; private final File statusFile; private final VipStatus vipStatus; + private volatile boolean previouslyInRotation = true; + // belongs in the response, but that's not a static class static final String OK_MESSAGE = "<title>OK</title>\n"; static final byte[] VIP_OK = Utf8.toBytes(OK_MESSAGE); @@ -158,7 +162,6 @@ public final class VipStatusHandler extends ThreadedHttpRequestHandler { * out of capacity. This is the default behavior. */ @Inject - @SuppressWarnings("unused") // injected public VipStatusHandler(VipStatusConfig vipConfig, Metric metric, VipStatus vipStatus) { // One thread should be enough for status handling - otherwise something else is completely wrong, // in which case this will eventually start returning a 503 (due to work rejection) as the bounded 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 0cf0f59102e..190a529f101 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 @@ -385,9 +385,10 @@ public class ApplicationController { } if (zone.environment().isProduction()) // Assign and register endpoints - application = withRotation(applicationPackage.deploymentSpec(), application, instance); + application = withRotation(application, instance); + + endpoints = registerEndpointsInDns(application.get().deploymentSpec(), application.get().require(instanceId.instance()), zone); - endpoints = registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone); if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { // Provisions a new certificate if missing @@ -517,9 +518,9 @@ public class ApplicationController { } /** Makes sure the application has a global rotation, if eligible. */ - private LockedApplication withRotation(DeploymentSpec deploymentSpec, LockedApplication application, InstanceName instanceName) { + private LockedApplication withRotation(LockedApplication application, InstanceName instanceName) { try (RotationLock rotationLock = rotationRepository.lock()) { - var rotations = rotationRepository.getOrAssignRotations(deploymentSpec, + var rotations = rotationRepository.getOrAssignRotations(application.get().deploymentSpec(), application.get().require(instanceName), rotationLock); application = application.with(instanceName, instance -> instance.with(rotations)); @@ -535,7 +536,7 @@ public class ApplicationController { */ private Set<ContainerEndpoint> registerEndpointsInDns(DeploymentSpec deploymentSpec, Instance instance, ZoneId zone) { var containerEndpoints = new HashSet<ContainerEndpoint>(); - var registerLegacyNames = deploymentSpec.requireInstance(instance.name()).globalServiceId().isPresent(); + var registerLegacyNames = deploymentSpec.globalServiceId().isPresent(); for (var assignedRotation : instance.rotations()) { var names = new ArrayList<String>(); var endpoints = instance.endpointsIn(controller.system(), assignedRotation.endpointId()) @@ -627,8 +628,8 @@ public class ApplicationController { private LockedApplication withoutDeletedDeployments(LockedApplication application, InstanceName instance) { DeploymentSpec deploymentSpec = application.get().deploymentSpec(); List<Deployment> deploymentsToRemove = application.get().require(instance).productionDeployments().values().stream() - .filter(deployment -> ! deploymentSpec.requireInstance(instance).includes(deployment.zone().environment(), - Optional.of(deployment.zone().region()))) + .filter(deployment -> ! deploymentSpec.includes(deployment.zone().environment(), + Optional.of(deployment.zone().region()))) .collect(Collectors.toList()); if (deploymentsToRemove.isEmpty()) return application; @@ -652,7 +653,7 @@ public class ApplicationController { private Instance withoutUnreferencedDeploymentJobs(DeploymentSpec deploymentSpec, Instance instance) { for (JobType job : JobList.from(instance).production().mapToList(JobStatus::type)) { ZoneId zone = job.zone(controller.system()); - if (deploymentSpec.requireInstance(instance.name()).includes(zone.environment(), Optional.of(zone.region()))) + if (deploymentSpec.includes(zone.environment(), Optional.of(zone.region()))) continue; instance = instance.withoutDeploymentJob(job); } @@ -910,9 +911,9 @@ public class ApplicationController { * 2. If the principal is given, verify that the principal is tenant admin or admin of the tenant domain * 3. If the principal is not given, verify that the Athenz domain of the tenant equals Athenz domain given in deployment.xml * - * @param tenantName tenant where application should be deployed - * @param applicationPackage application package - * @param deployer principal initiating the deployment, possibly empty + * @param tenantName Tenant where application should be deployed + * @param applicationPackage Application package + * @param deployer Principal initiating the deployment, possibly empty */ public void verifyApplicationIdentityConfiguration(TenantName tenantName, ApplicationPackage applicationPackage, Optional<Principal> deployer) { verifyAllowedLaunchAthenzService(applicationPackage.deploymentSpec()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java index 5c4d5874e53..ce7904dc829 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java @@ -39,7 +39,7 @@ public class DeploymentSpecValidator { /** Verify that each of the production zones listed in the deployment spec exist in this system */ private void validateSteps(DeploymentSpec deploymentSpec) { new DeploymentSteps(deploymentSpec, controller::system).jobs(); - deploymentSpec.instances().stream().flatMap(instance -> instance.zones().stream()) + deploymentSpec.zones().stream() .filter(zone -> zone.environment() == Environment.prod) .forEach(zone -> { if ( ! controller.zoneRegistry().hasZone(ZoneId.from(zone.environment(), @@ -51,20 +51,17 @@ public class DeploymentSpecValidator { /** Verify that no single endpoint contains regions in different clouds */ private void validateEndpoints(DeploymentSpec deploymentSpec) { - for (var instance : deploymentSpec.instances()) { - for (var endpoint : instance.endpoints()) { - var clouds = new HashSet<CloudName>(); - for (var region : endpoint.regions()) { - for (ZoneApi zone : controller.zoneRegistry().zones().all().in(region).zones()) { - clouds.add(zone.getCloudName()); - } - } - if (clouds.size() != 1) { - throw new IllegalArgumentException("Endpoint '" + endpoint.endpointId() + "' in " + instance + - " cannot contain regions in different clouds: " + - endpoint.regions().stream().sorted().collect(Collectors.toList())); + for (var endpoint : deploymentSpec.endpoints()) { + var clouds = new HashSet<CloudName>(); + for (var region : endpoint.regions()) { + for (ZoneApi zone : controller.zoneRegistry().zones().all().in(region).zones()) { + clouds.add(zone.getCloudName()); } } + if (clouds.size() != 1) { + throw new IllegalArgumentException("Endpoint '" + endpoint.endpointId() + "' cannot contain regions in different clouds: " + + endpoint.regions().stream().sorted().collect(Collectors.toList())); + } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 3df889d7a88..376048143d9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -372,8 +372,9 @@ public class DeploymentTrigger { } else { // All jobs are complete; find the time of completion of this step. if (stepJobs.isEmpty()) { // No jobs means this is a delay step. - completedAt = completedAt.map(at -> at.plus(step.delay())).filter(at -> !at.isAfter(clock.instant())); - reason += " after a delay of " + step.delay(); + Duration delay = ((DeploymentSpec.Delay) step).duration(); + completedAt = completedAt.map(at -> at.plus(delay)).filter(at -> !at.isAfter(clock.instant())); + reason += " after a delay of " + delay; } else { completedAt = stepJobs.stream().map(job -> instance.deploymentJobs().statusOf(job).get().lastCompleted().get().at()).max(naturalOrder()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java index 9f6bbcd2a5a..a16ca5cb201 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java @@ -77,25 +77,22 @@ public class RotationRepository { * If a rotation is already assigned to the application, that rotation will be returned. * If no rotation is assigned, return an available rotation. The caller is responsible for assigning the rotation. * - * @param deploymentSpec the deployment spec for the application - * @param instance the instance requesting a rotation - * @param lock lock which must be acquired by the caller + * @param deploymentSpec The deployment spec for the application + * @param instance The instance requesting a rotation + * @param lock Lock which must be acquired by the caller */ public Rotation getOrAssignRotation(DeploymentSpec deploymentSpec, Instance instance, RotationLock lock) { if ( ! instance.rotations().isEmpty()) { return allRotations.get(instance.rotations().get(0).rotationId()); } - - if (deploymentSpec.requireInstance(instance.name()).globalServiceId().isEmpty()) { - throw new IllegalArgumentException("global-service-id is not set in deployment spec for instance '" + - instance.name() + "'"); + if (deploymentSpec.globalServiceId().isEmpty()) { + throw new IllegalArgumentException("global-service-id is not set in deployment spec"); } - long productionZones = deploymentSpec.requireInstance(instance.name()).zones().stream() - .filter(zone -> zone.deploysTo(Environment.prod)) - .count(); + long productionZones = deploymentSpec.zones().stream() + .filter(zone -> zone.deploysTo(Environment.prod)) + .count(); if (productionZones < 2) { - throw new IllegalArgumentException("global-service-id is set but less than 2 prod zones are defined " + - "in instance '" + instance.name() + "'"); + throw new IllegalArgumentException("global-service-id is set but less than 2 prod zones are defined"); } return findAvailableRotation(instance.id(), lock); } @@ -113,23 +110,22 @@ public class RotationRepository { * @return List of rotation assignments - either new or existing */ public List<AssignedRotation> getOrAssignRotations(DeploymentSpec deploymentSpec, Instance instance, RotationLock lock) { - if (deploymentSpec.requireInstance(instance.name()).globalServiceId().isPresent() - && ! deploymentSpec.requireInstance(instance.name()).endpoints().isEmpty()) { + if (deploymentSpec.globalServiceId().isPresent() && ! deploymentSpec.endpoints().isEmpty()) { throw new IllegalArgumentException("Cannot provision rotations with both global-service-id and 'endpoints'"); } // Support the older case of setting global-service-id - if (deploymentSpec.requireInstance(instance.name()).globalServiceId().isPresent()) { - var regions = deploymentSpec.requireInstance(instance.name()).zones().stream() - .filter(zone -> zone.environment().isProduction()) - .flatMap(zone -> zone.region().stream()) - .collect(Collectors.toSet()); + if (deploymentSpec.globalServiceId().isPresent()) { + final var regions = deploymentSpec.zones().stream() + .filter(zone -> zone.environment().isProduction()) + .flatMap(zone -> zone.region().stream()) + .collect(Collectors.toSet()); - var rotation = getOrAssignRotation(deploymentSpec, instance, lock); + final var rotation = getOrAssignRotation(deploymentSpec, instance, lock); return List.of( new AssignedRotation( - new ClusterSpec.Id(deploymentSpec.requireInstance(instance.name()).globalServiceId().get()), + new ClusterSpec.Id(deploymentSpec.globalServiceId().get()), EndpointId.default_(), rotation.id(), regions @@ -137,8 +133,8 @@ public class RotationRepository { ); } - Map<EndpointId, AssignedRotation> existingAssignments = existingEndpointAssignments(deploymentSpec, instance); - Map<EndpointId, AssignedRotation> updatedAssignments = assignRotationsToEndpoints(deploymentSpec, existingAssignments, lock); + final Map<EndpointId, AssignedRotation> existingAssignments = existingEndpointAssignments(deploymentSpec, instance); + final Map<EndpointId, AssignedRotation> updatedAssignments = assignRotationsToEndpoints(deploymentSpec, existingAssignments, lock); existingAssignments.putAll(updatedAssignments); @@ -146,11 +142,11 @@ public class RotationRepository { } private Map<EndpointId, AssignedRotation> assignRotationsToEndpoints(DeploymentSpec deploymentSpec, Map<EndpointId, AssignedRotation> existingAssignments, RotationLock lock) { - var availableRotations = new ArrayList<>(availableRotations(lock).values()); + final var availableRotations = new ArrayList<>(availableRotations(lock).values()); - var neededRotations = deploymentSpec.endpoints().stream() - .filter(Predicate.not(endpoint -> existingAssignments.containsKey(EndpointId.of(endpoint.endpointId())))) - .collect(Collectors.toSet()); + final var neededRotations = deploymentSpec.endpoints().stream() + .filter(Predicate.not(endpoint -> existingAssignments.containsKey(EndpointId.of(endpoint.endpointId())))) + .collect(Collectors.toSet()); if (neededRotations.size() > availableRotations.size()) { throw new IllegalStateException("Hosted Vespa ran out of rotations, unable to assign rotation: need " + neededRotations.size() + ", have " + availableRotations.size()); @@ -176,26 +172,34 @@ public class RotationRepository { } private Map<EndpointId, AssignedRotation> existingEndpointAssignments(DeploymentSpec deploymentSpec, Instance instance) { + // // Get the regions that has been configured for an endpoint. Empty set if the endpoint // is no longer mentioned in the configuration file. - Function<EndpointId, Set<RegionName>> configuredRegionsForEndpoint = endpointId -> - deploymentSpec.requireInstance(instance.name()).endpoints().stream() + // + final Function<EndpointId, Set<RegionName>> configuredRegionsForEndpoint = endpointId -> { + return deploymentSpec.endpoints().stream() .filter(endpoint -> endpointId.id().equals(endpoint.endpointId())) .map(Endpoint::regions) .findFirst() .orElse(Set.of()); + }; + // // Build a new AssignedRotation instance where we update set of regions from the configuration instead - // of using the one already mentioned in the assignment. This allows us to overwrite the set of regions. - Function<AssignedRotation, AssignedRotation> assignedRotationWithConfiguredRegions = assignedRotation -> - new AssignedRotation( + // of using the one already mentioned in the assignment. This allows us to overwrite the set of regions + // when + final Function<AssignedRotation, AssignedRotation> assignedRotationWithConfiguredRegions = assignedRotation -> { + return new AssignedRotation( assignedRotation.clusterId(), assignedRotation.endpointId(), assignedRotation.rotationId(), - configuredRegionsForEndpoint.apply(assignedRotation.endpointId())); + configuredRegionsForEndpoint.apply(assignedRotation.endpointId()) + ); + }; return instance.rotations().stream() - .collect(Collectors.toMap( + .collect( + Collectors.toMap( AssignedRotation::endpointId, assignedRotationWithConfiguredRegions, (a, b) -> { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index e3682a78b7d..ebf80eb9daa 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -72,6 +72,7 @@ public class ControllerTest { @Test public void testDeployment() { // Setup system + ApplicationController applications = tester.controller().applications(); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-west-1") @@ -752,7 +753,7 @@ public class ControllerTest { tester.deployCompletely(application, applicationPackage); fail("Expected exception"); } catch (IllegalArgumentException e) { - assertEquals("Endpoint 'default' in instance 'default' cannot contain regions in different clouds: [aws-us-east-1, us-west-1]", e.getMessage()); + assertEquals("Endpoint 'default' cannot contain regions in different clouds: [aws-us-east-1, us-west-1]", e.getMessage()); } var applicationPackage2 = new ApplicationPackageBuilder() @@ -765,7 +766,7 @@ public class ControllerTest { tester.deployCompletely(application, applicationPackage2); fail("Expected exception"); } catch (IllegalArgumentException e) { - assertEquals("Endpoint 'foo' in instance 'default' cannot contain regions in different clouds: [aws-us-east-1, us-west-1]", e.getMessage()); + assertEquals("Endpoint 'foo' cannot contain regions in different clouds: [aws-us-east-1, us-west-1]", e.getMessage()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index 9449f2b0854..25e562ed046 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -47,7 +47,6 @@ public class ApplicationPackageBuilder { private final List<X509Certificate> trustedCertificates = new ArrayList<>(); private OptionalInt majorVersion = OptionalInt.empty(); - private String instances = "default"; private String upgradePolicy = null; private Environment environment = Environment.prod; private String globalServiceId = null; @@ -59,11 +58,6 @@ public class ApplicationPackageBuilder { return this; } - public ApplicationPackageBuilder instances(String instances) { - this.instances = instances; - return this; - } - public ApplicationPackageBuilder upgradePolicy(String upgradePolicy) { this.upgradePolicy = upgradePolicy; return this; @@ -96,7 +90,7 @@ public class ApplicationPackageBuilder { } public ApplicationPackageBuilder region(String regionName) { - environmentBody.append(" <region active='true'>"); + environmentBody.append(" <region active='true'>"); environmentBody.append(regionName); environmentBody.append("</region>\n"); return this; @@ -118,7 +112,7 @@ public class ApplicationPackageBuilder { public ApplicationPackageBuilder blockChange(boolean revision, boolean version, String daySpec, String hourSpec, String zoneSpec) { - blockChange.append(" <block-change"); + blockChange.append(" <block-change"); blockChange.append(" revision='").append(revision).append("'"); blockChange.append(" version='").append(version).append("'"); blockChange.append(" days='").append(daySpec).append("'"); @@ -172,15 +166,14 @@ public class ApplicationPackageBuilder { xml.append(athenzIdentityAttributes); } xml.append(">\n"); - xml.append(" <instance id='").append(instances).append("'>\n"); if (upgradePolicy != null) { - xml.append(" <upgrade policy='"); + xml.append("<upgrade policy='"); xml.append(upgradePolicy); xml.append("'/>\n"); } xml.append(notifications); xml.append(blockChange); - xml.append(" <"); + xml.append(" <"); xml.append(environment.value()); if (globalServiceId != null) { xml.append(" global-service-id='"); @@ -189,14 +182,13 @@ public class ApplicationPackageBuilder { } xml.append(">\n"); xml.append(environmentBody); - xml.append(" </"); + xml.append(" </"); xml.append(environment.value()); xml.append(">\n"); - xml.append(" <endpoints>\n"); + xml.append(" <endpoints>\n"); xml.append(endpointsBody); - xml.append(" </endpoints>\n"); - xml.append(" </instance>\n"); - xml.append("</deployment>\n"); + xml.append(" </endpoints>\n"); + xml.append("</deployment>"); return xml.toString().getBytes(UTF_8); } 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 9c957785606..7061af3904f 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 @@ -113,18 +113,7 @@ public class ApplicationApiTest extends ControllerContainerTest { "-----END PUBLIC KEY-----\n"; private static final String quotedPemPublicKey = pemPublicKey.replaceAll("\\n", "\\\\n"); - private static final ApplicationPackage applicationPackageDefault = new ApplicationPackageBuilder() - .instances("default") - .environment(Environment.prod) - .globalServiceId("foo") - .region("us-central-1") - .region("us-east-3") - .region("us-west-1") - .blockChange(false, true, "mon-fri", "0-8", "UTC") - .build(); - - private static final ApplicationPackage applicationPackageInstance1 = new ApplicationPackageBuilder() - .instances("instance1") + private static final ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .globalServiceId("foo") .region("us-central-1") @@ -232,7 +221,7 @@ public class ApplicationApiTest extends ControllerContainerTest { addUserToHostedOperatorRole(HostedAthenzIdentities.from(HOSTED_VESPA_OPERATOR)); // POST (deploy) an application to a zone - manual user deployment (includes a content hash for verification) - MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true); + MultiPartStreamer entity = createApplicationDeployData(applicationPackage, true); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/instance1/deploy", POST) .data(entity) .header("X-Content-Hash", Base64.getEncoder().encodeToString(Signatures.sha256Digest(entity::data))) @@ -252,7 +241,7 @@ public class ApplicationApiTest extends ControllerContainerTest { controllerTester.jobCompletion(JobType.component) .application(id) .projectId(screwdriverProjectId) - .uploadArtifact(applicationPackageInstance1) + .uploadArtifact(applicationPackage) .submit(); // ... systemtest @@ -316,7 +305,6 @@ public class ApplicationApiTest extends ControllerContainerTest { // POST (create) another application ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .instances("instance1") .environment(Environment.prod) .region("us-west-1") .build(); @@ -593,7 +581,6 @@ public class ApplicationApiTest extends ControllerContainerTest { // Second attempt has a service under a different domain than the tenant of the application, and fails. ApplicationPackage packageWithServiceForWrongDomain = new ApplicationPackageBuilder() - .instances("instance1") .environment(Environment.prod) .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from(ATHENZ_TENANT_DOMAIN_2.getName()), AthenzService.from("service")) .region("us-west-1") @@ -606,7 +593,6 @@ public class ApplicationApiTest extends ControllerContainerTest { // Third attempt finally has a service under the domain of the tenant, and succeeds. ApplicationPackage packageWithService = new ApplicationPackageBuilder() - .instances("instance1") .environment(Environment.prod) .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from(ATHENZ_TENANT_DOMAIN.getName()), AthenzService.from("service")) .region("us-west-1") @@ -720,7 +706,6 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.computeVersionStatus(); createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .instances("instance1") .globalServiceId("foo") .region("us-west-1") .region("us-east-3") @@ -729,7 +714,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Create tenant and deploy ApplicationId id = createTenantAndApplication(); long projectId = 1; - MultiPartStreamer deployData = createApplicationDeployData(Optional.of(applicationPackage), false); + MultiPartStreamer deployData = createApplicationDeployData(Optional.empty(), false); startAndTestChange(controllerTester, id, projectId, applicationPackage, deployData, 100); // us-west-1 @@ -792,7 +777,6 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.computeVersionStatus(); createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .instances("instance1") .region("us-west-1") .region("us-east-3") .region("eu-west-1") @@ -869,7 +853,7 @@ public class ApplicationApiTest extends ControllerContainerTest { new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId("application1")); // POST (deploy) an application to a prod zone - allowed when project ID is not specified - MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true); + MultiPartStreamer entity = createApplicationDeployData(applicationPackage, true); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/deploy", POST) .data(entity) .screwdriverIdentity(SCREWDRIVER_ID), @@ -901,7 +885,6 @@ public class ApplicationApiTest extends ControllerContainerTest { // Deploy ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .instances("instance1") .region("us-east-3") .build(); ApplicationId id = createTenantAndApplication(); @@ -921,7 +904,6 @@ public class ApplicationApiTest extends ControllerContainerTest { // New zone is added before us-east-3 applicationPackage = new ApplicationPackageBuilder() - .instances("instance1") .globalServiceId("foo") // These decides the ordering of deploymentJobs and instances in the response .region("us-west-1") @@ -1074,7 +1056,7 @@ public class ApplicationApiTest extends ControllerContainerTest { configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to prepare application", ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, null)); // POST (deploy) an application with an invalid application package - MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true); + MultiPartStreamer entity = createApplicationDeployData(applicationPackage, true); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/instance1/deploy", POST) .data(entity) .userIdentity(USER_ID), @@ -1194,7 +1176,7 @@ public class ApplicationApiTest extends ControllerContainerTest { 200); // Deploy to an authorized zone by a user tenant is disallowed - MultiPartStreamer entity = createApplicationDeployData(applicationPackageDefault, true); + MultiPartStreamer entity = createApplicationDeployData(applicationPackage, true); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/deploy", POST) .data(entity) .userIdentity(USER_ID), @@ -1607,7 +1589,7 @@ public class ApplicationApiTest extends ControllerContainerTest { } private MultiPartStreamer createApplicationDeployData(Optional<ApplicationPackage> applicationPackage, - Optional<ApplicationVersion> applicationVersion, boolean deployDirectly) { + Optional<ApplicationVersion> applicationVersion, boolean deployDirectly) { MultiPartStreamer streamer = new MultiPartStreamer(); streamer.addJson("deployOptions", deployOptions(deployDirectly, applicationVersion)); applicationPackage.ifPresent(ap -> streamer.addBytes("applicationZip", ap.zippedContent())); |