diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2019-11-29 19:13:36 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-29 19:13:36 +0100 |
commit | 12fcbd278a5fadb085456f0cde9059755c0fe231 (patch) | |
tree | 4023bb0e6c835026ffb18f5f00580121455a906c /config-model-api/src/main | |
parent | 21620abb46bbfb18abeb6c8cc917b39fb0e316b8 (diff) |
Revert "Jvenstad/test steps in deployment spec"
Diffstat (limited to 'config-model-api/src/main')
3 files changed, 110 insertions, 193 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java index 541e52cf6e9..f96400fc9a9 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java @@ -7,6 +7,7 @@ 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; @@ -21,11 +22,12 @@ import java.util.stream.Collectors; * * @author bratseth */ -public class DeploymentInstanceSpec extends DeploymentSpec.Steps { +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; @@ -43,54 +45,34 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { Optional<AthenzService> athenzService, Notifications notifications, List<Endpoint> endpoints) { - super(steps); 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, steps())); - validateZones(new HashSet<>(), this); - validateEndpoints(steps(), globalServiceId, this.endpoints); + this.endpoints = List.copyOf(validateEndpoints(endpoints, this.steps)); + validateZones(this.steps); + validateEndpoints(this.steps, globalServiceId, this.endpoints); validateAthenz(); } public InstanceName name() { return name; } - /** - * Throws an IllegalArgumentException if any production deployment or test is declared multiple times, - * or if any production test is declared not after its corresponding deployment. - * - * @param deployments previously seen deployments - * @param step step whose members to validate - * @return all contained tests - */ - private static Set<RegionName> validateZones(Set<RegionName> deployments, DeploymentSpec.Step step) { - if ( ! step.steps().isEmpty()) { - Set<RegionName> oldDeployments = Set.copyOf(deployments); - Set<RegionName> tests = new HashSet<>(); - for (DeploymentSpec.Step nested : step.steps()) { - for (RegionName test : validateZones(deployments, nested)) { - if ( ! (step.isOrdered() ? deployments : oldDeployments).contains(test)) - throw new IllegalArgumentException("tests for prod." + test + " must be after the corresponding deployment in deployment.xml"); - - if ( ! tests.add(test)) - throw new IllegalArgumentException("tests for prod." + test + " arelisted twice in deployment.xml"); - } - } - return tests; - } - if (step.concerns(Environment.prod)) { - if (step.isTest()) - return Set.of(((DeploymentSpec.DeclaredTest) step).region()); + /** Throw an IllegalArgumentException if any production zone is declared multiple times */ + private void validateZones(List<DeploymentSpec.Step> steps) { + Set<DeploymentSpec.DeclaredZone> zones = new HashSet<>(); - RegionName region = ((DeploymentSpec.DeclaredZone) step).region().get(); - if ( ! deployments.add(region)) - throw new IllegalArgumentException("prod." + region + " is listed twice in deployment.xml"); - } - return Set.of(); + 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 */ @@ -98,7 +80,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { Objects.requireNonNull(endpoints, "Missing endpoints parameter"); var productionRegions = steps.stream() - .filter(step -> step.concerns(Environment.prod)) + .filter(step -> step.deploysTo(Environment.prod)) .flatMap(step -> step.zones().stream()) .flatMap(zone -> zone.region().stream()) .map(RegionName::value) @@ -161,6 +143,15 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { } } + @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; } @@ -182,16 +173,32 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { .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) { - return zones().stream() - .filter(zone -> zone.concerns(environment, Optional.of(region))) - .findFirst() - .flatMap(DeploymentSpec.DeclaredZone::athenzService) - .or(() -> this.athenzService); + 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 */ @@ -200,9 +207,23 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { /** Returns the rotations configuration of these instances */ public List<Endpoint> endpoints() { return endpoints; } - /** Returns whether this instance deploys to the given zone, either implicitly or explicitly */ - public boolean deploysTo(Environment environment, Optional<RegionName> region) { - return zones().stream().anyMatch(zone -> zone.concerns(environment, region)); + /** 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 @@ -213,7 +234,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { return globalServiceId.equals(other.globalServiceId) && upgradePolicy == other.upgradePolicy && changeBlockers.equals(other.changeBlockers) && - steps().equals(other.steps()) && + steps.equals(other.steps) && athenzDomain.equals(other.athenzDomain) && athenzService.equals(other.athenzService) && notifications.equals(other.notifications) && @@ -222,7 +243,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { @Override public int hashCode() { - return Objects.hash(globalServiceId, upgradePolicy, changeBlockers, steps(), athenzDomain, athenzService, notifications, endpoints); + return Objects.hash(globalServiceId, upgradePolicy, changeBlockers, steps, athenzDomain, athenzService, notifications, endpoints); } @Override 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 67b3d585881..ea7df677e0f 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 @@ -13,12 +13,10 @@ import java.io.Reader; import java.time.Duration; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; -import java.util.stream.Stream; /** * Specifies the environments and regions to which an application should be deployed. @@ -77,14 +75,14 @@ public class DeploymentSpec { List<Step> steps = new ArrayList<>(inputSteps); // Add staging if required and missing - if (steps.stream().anyMatch(step -> step.concerns(Environment.prod)) && - steps.stream().noneMatch(step -> step.concerns(Environment.staging))) { + if (steps.stream().anyMatch(step -> step.deploysTo(Environment.prod)) && + steps.stream().noneMatch(step -> step.deploysTo(Environment.staging))) { steps.add(new DeploymentSpec.DeclaredZone(Environment.staging)); } // Add test if required and missing - if (steps.stream().anyMatch(step -> step.concerns(Environment.staging)) && - steps.stream().noneMatch(step -> step.concerns(Environment.test))) { + if (steps.stream().anyMatch(step -> step.deploysTo(Environment.staging)) && + steps.stream().noneMatch(step -> step.deploysTo(Environment.test))) { steps.add(new DeploymentSpec.DeclaredZone(Environment.test)); } @@ -164,6 +162,13 @@ public class DeploymentSpec { // to have environment, instance or region variants on those. public Optional<AthenzService> athenzService() { return this.athenzService; } + // TODO remove when 7.135 is the oldest version + public Optional<AthenzService> athenzService(InstanceName instanceName, Environment environment, RegionName region) { + Optional<DeploymentInstanceSpec> instance = instance(instanceName); + if (instance.isEmpty()) return this.athenzService; + return instance.get().athenzService(environment, region).or(() -> this.athenzService); + } + /** Returns the XML form of this spec, or null if it was not created by fromXml, nor is empty */ public String xmlForm() { return xmlForm; } @@ -200,15 +205,11 @@ public class DeploymentSpec { private static List<DeploymentInstanceSpec> instances(List<DeploymentSpec.Step> steps) { return steps.stream() - .flatMap(DeploymentSpec::flatten) + .flatMap(step -> step instanceof ParallelZones ? ((ParallelZones) step).steps.stream() : List.of(step).stream()) + .filter(step -> step instanceof DeploymentInstanceSpec).map(DeploymentInstanceSpec.class::cast) .collect(Collectors.toList()); } - private static Stream<DeploymentInstanceSpec> flatten(Step step) { - if (step instanceof DeploymentInstanceSpec) return Stream.of((DeploymentInstanceSpec) step); - return step.steps().stream().flatMap(DeploymentSpec::flatten); - } - /** * Creates a deployment spec from XML. * @@ -271,31 +272,23 @@ public class DeploymentSpec { /** A deployment step */ public abstract static class Step { - /** Returns whether this step specifies the given environment. */ - public final boolean concerns(Environment environment) { - return concerns(environment, Optional.empty()); + /** Returns whether this step deploys to the given region */ + public final boolean deploysTo(Environment environment) { + return deploysTo(environment, Optional.empty()); } - /** Returns whether this step specifies the given environment, and, optionally, region. */ - public abstract boolean concerns(Environment environment, Optional<RegionName> region); + /** Returns whether this step deploys to the given environment, and (if specified) region */ + public abstract boolean deploysTo(Environment environment, Optional<RegionName> region); - /** Returns the zones deployed to in this step. */ + /** 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). */ + /** 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 any steps nested in this. */ + /** Returns all the steps nested in this. This default implementatiino returns an empty list. */ public List<Step> steps() { return List.of(); } - /** Returns whether this step is a test step. */ - public boolean isTest() { return false; } - - /** Returns whether the nested steps in this, if any, should be performed in declaration order. */ - public boolean isOrdered() { - return true; - } - } /** A deployment step which is to wait for some time before progressing to the next step */ @@ -311,7 +304,7 @@ public class DeploymentSpec { public Duration delay() { return duration; } @Override - public boolean concerns(Environment environment, Optional<RegionName> region) { return false; } + public boolean deploysTo(Environment environment, Optional<RegionName> region) { return false; } @Override public String toString() { @@ -362,16 +355,13 @@ public class DeploymentSpec { public List<DeclaredZone> zones() { return Collections.singletonList(this); } @Override - public boolean concerns(Environment environment, Optional<RegionName> region) { + public boolean deploysTo(Environment environment, Optional<RegionName> region) { if (environment != this.environment) return false; if (region.isPresent() && ! region.equals(this.region)) return false; return true; } @Override - public boolean isTest() { return environment.isTest(); } - - @Override public int hashCode() { return Objects.hash(environment, region); } @@ -393,82 +383,39 @@ public class DeploymentSpec { } - /** A declared production test */ - public static class DeclaredTest extends Step { - - private final RegionName region; - - public DeclaredTest(RegionName region) { - this.region = Objects.requireNonNull(region); - } - - @Override - public boolean concerns(Environment environment, Optional<RegionName> region) { - return region.map(this.region::equals).orElse(true) && environment == Environment.prod; - } - - @Override - public boolean isTest() { return true; } - - /** Returns the region this test is for. */ - public RegionName region() { - return region; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - DeclaredTest that = (DeclaredTest) o; - return region.equals(that.region); - } - - @Override - public int hashCode() { - return Objects.hash(region); - } - - @Override - public String toString() { - return "tests for prod." + region; - } - - } - - /** A container for several steps, by default in serial order */ - public static class Steps extends Step { + /** A deployment step which is to run multiple steps (zones or instances) in parallel */ + public static class ParallelZones extends Step { private final List<Step> steps; - public Steps(List<Step> steps) { + public ParallelZones(List<Step> steps) { this.steps = List.copyOf(steps); } + /** Returns the steps inside this which are zones */ @Override public List<DeclaredZone> zones() { - return steps.stream() - .flatMap(step -> step.zones().stream()) - .collect(Collectors.toUnmodifiableList()); + return this.steps.stream() + .filter(step -> step instanceof DeclaredZone) + .map(DeclaredZone.class::cast) + .collect(Collectors.toList()); } + /** Returns all the steps nested in this */ @Override public List<Step> steps() { return steps; } @Override - public boolean concerns(Environment environment, Optional<RegionName> region) { - return steps.stream().anyMatch(step -> step.concerns(environment, region)); - } - - @Override - public Duration delay() { - return steps.stream().map(Step::delay).reduce(Duration.ZERO, Duration::plus); + public boolean deploysTo(Environment environment, Optional<RegionName> region) { + return steps().stream().anyMatch(zone -> zone.deploysTo(environment, region)); } @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - return steps.equals(((Steps) o).steps); + if (!(o instanceof ParallelZones)) return false; + ParallelZones that = (ParallelZones) o; + return Objects.equals(steps, that.steps); } @Override @@ -478,43 +425,7 @@ public class DeploymentSpec { @Override public String toString() { - return steps.size() + " steps"; - } - - } - - /** A container for multiple other steps, which are executed in parallel */ - public static class ParallelSteps extends Steps { - - public ParallelSteps(List<Step> steps) { - super(steps); - } - - @Override - public Duration delay() { - return steps().stream().map(Step::delay).max(Comparator.naturalOrder()).orElse(Duration.ZERO); - } - - @Override - public boolean isOrdered() { - return false; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if ( ! (o instanceof ParallelSteps)) return false; - return Objects.equals(steps(), ((ParallelSteps) o).steps()); - } - - @Override - public int hashCode() { - return Objects.hash(steps()); - } - - @Override - public String toString() { - return steps().size() + " parallel steps"; + return steps.size() + " parallel steps"; } } 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 c14e6ce5966..bc17ee0cb2b 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 @@ -3,12 +3,10 @@ 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.DeclaredTest; import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; import com.yahoo.config.application.api.DeploymentSpec.Delay; -import com.yahoo.config.application.api.DeploymentSpec.ParallelSteps; +import com.yahoo.config.application.api.DeploymentSpec.ParallelZones; import com.yahoo.config.application.api.DeploymentSpec.Step; -import com.yahoo.config.application.api.DeploymentSpec.Steps; import com.yahoo.config.application.api.Endpoint; import com.yahoo.config.application.api.Notifications; import com.yahoo.config.application.api.Notifications.Role; @@ -22,7 +20,6 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.io.IOUtils; import com.yahoo.text.XML; import org.w3c.dom.Element; -import org.w3c.dom.Node; import java.io.IOException; import java.io.Reader; @@ -35,10 +32,8 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; -import java.util.stream.Stream; /** * @author bratseth @@ -55,7 +50,6 @@ public class DeploymentSpecXmlReader { private static final String regionTag = "region"; private static final String delayTag = "delay"; private static final String parallelTag = "parallel"; - private static final String stepsTag = "steps"; private static final String endpointsTag = "endpoints"; private static final String endpointTag = "endpoint"; private static final String notificationsTag = "notifications"; @@ -124,7 +118,7 @@ public class DeploymentSpecXmlReader { * * @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 instance is implicitly defined, which means instanceTag is the root) + * @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 */ private List<DeploymentInstanceSpec> readInstanceContent(String instanceNameString, @@ -175,7 +169,6 @@ public class DeploymentSpecXmlReader { } // 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 - @SuppressWarnings("fallthrough") private List<Step> readNonInstanceSteps(Element stepTag, MutableOptional<String> globalServiceId, Element parentTag) { Optional<AthenzService> athenzService = stringAttribute(athenzServiceAttribute, stepTag) .or(() -> stringAttribute(athenzServiceAttribute, parentTag)) @@ -189,11 +182,7 @@ public class DeploymentSpecXmlReader { throw new IllegalArgumentException("Attribute 'global-service-id' is only valid on 'prod' tag."); switch (stepTag.getTagName()) { - case testTag: - if (Stream.iterate(stepTag, Objects::nonNull, Node::getParentNode) - .anyMatch(node -> prodTag.equals(node.getNodeName()))) - return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()))); - case stagingTag: + 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() @@ -204,13 +193,9 @@ public class DeploymentSpecXmlReader { longAttribute("minutes", stepTag) * 60 + longAttribute("seconds", stepTag)))); case parallelTag: // regions and instances may be nested within - return List.of(new ParallelSteps(XML.getChildren(stepTag).stream() + return List.of(new ParallelZones(XML.getChildren(stepTag).stream() .flatMap(child -> readSteps(child, globalServiceId, parentTag).stream()) .collect(Collectors.toList()))); - case stepsTag: // regions and instances may be nested within - return List.of(new Steps(XML.getChildren(stepTag).stream() - .flatMap(child -> readSteps(child, globalServiceId, parentTag).stream()) - .collect(Collectors.toList()))); case regionTag: return List.of(readDeclaredZone(Environment.prod, athenzService, testerFlavor, stepTag)); default: |