diff options
6 files changed, 237 insertions, 78 deletions
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 3c00702bd8f..556bcb78d31 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 @@ -12,10 +12,12 @@ import java.io.BufferedReader; import java.io.FileReader; import java.io.IOException; import java.io.Reader; -import java.io.UncheckedIOException; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.Optional; /** @@ -23,6 +25,9 @@ import java.util.Optional; * This may be used both for inspection as part of an application model and to answer * queries about deployment from the command line. A main method is included for the latter usage. * + * A deployment consists of a number of steps executed in the order listed in deployment xml, as + * well as some additional settings. + * * This is immutable. * * @author bratseth @@ -37,20 +42,73 @@ public class DeploymentSpec { private final Optional<String> globalServiceId; private final UpgradePolicy upgradePolicy; - private final List<DeclaredZone> zones; + private final List<Step> steps; private final String xmlForm; - public DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, List<DeclaredZone> zones) { - this(globalServiceId, upgradePolicy, zones, null); + public DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, List<Step> steps) { + this(globalServiceId, upgradePolicy, steps, null); } private DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, - List<DeclaredZone> zones, String xmlForm) { + List<Step> steps, String xmlForm) { + validateTotalDelay(steps); this.globalServiceId = globalServiceId; this.upgradePolicy = upgradePolicy; - this.zones = ImmutableList.copyOf(zones); + this.steps = ImmutableList.copyOf(completeSteps(new ArrayList<>(steps))); this.xmlForm = xmlForm; } + + /** Throw an IllegalArgumentException if the total delay exceeds 24 hours */ + private static 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<Step> completeSteps(List<Step> steps) { + // Ensure no duplicate deployments to the same zone + steps = new ArrayList<>(new LinkedHashSet<>(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 ZoneDeployment(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 ZoneDeployment(Environment.test)); + } + + // Enforce order test, staging, prod + ZoneDeployment testStep = remove(Environment.test, steps); + if (testStep != null) + steps.add(0, testStep); + ZoneDeployment 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. + * + * @param environment + * @return the removed step, or null if it is not present + */ + private static ZoneDeployment remove(Environment environment, List<Step> steps) { + for (int i = 0; i < steps.size(); i++) { + if (steps.get(i).deploysTo(environment)) + return (ZoneDeployment)steps.remove(i); + } + return null; + } /** Returns the ID of the service to expose through global routing, if present */ public Optional<String> globalServiceId() { @@ -60,16 +118,16 @@ public class DeploymentSpec { /** Returns the upgrade policy of this, which is defaultPolicy if none is specified */ public UpgradePolicy upgradePolicy() { return upgradePolicy; } - /** Returns the zones this declares as a read-only list. */ - public List<DeclaredZone> zones() { return zones; } + /** Returns the deployment steps of this in the order they will be performed */ + public List<Step> steps() { return steps; } /** Returns the XML form of this spec, or null if it was not created by fromXml or is the empty spec */ public String xmlForm() { return xmlForm; } /** Returns whether this deployment spec specifies the given zone, either implicitly or explicitly */ public boolean includes(Environment environment, Optional<RegionName> region) { - for (DeclaredZone declaredZone : zones) - if (declaredZone.matches(environment, region)) return true; + for (Step step : steps) + if (step.deploysTo(environment, region)) return true; return false; } @@ -93,30 +151,49 @@ public class DeploymentSpec { * @throws IllegalArgumentException if the XML is invalid */ public static DeploymentSpec fromXml(String xmlForm) { - List<DeclaredZone> zones = new ArrayList<>(); + List<Step> steps = new ArrayList<>(); Optional<String> globalServiceId = Optional.empty(); Element root = XML.getDocument(xmlForm).getDocumentElement(); for (Element environmentTag : XML.getChildren(root)) { - if (!isEnvironmentName(environmentTag.getTagName())) continue; + if ( ! isEnvironmentName(environmentTag.getTagName())) continue; + Environment environment = Environment.from(environmentTag.getTagName()); - List<Element> regionTags = XML.getChildren(environmentTag, "region"); - if (regionTags.isEmpty()) { - zones.add(new DeclaredZone(environment, Optional.empty(), false)); - } else { - for (Element regionTag : regionTags) { - RegionName region = RegionName.from(XML.getValue(regionTag).trim()); - boolean active = environment == Environment.prod && readActive(regionTag); - zones.add(new DeclaredZone(environment, Optional.of(region), active)); + + 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 // a region: deploy step + steps.add(new ZoneDeployment(environment, + Optional.of(RegionName.from(XML.getValue(stepTag).trim())), + readActive(stepTag))); } } + else { + steps.add(new ZoneDeployment(environment)); + } - if (Environment.prod.equals(environment)) { + if (environment == Environment.prod) globalServiceId = readGlobalServiceId(environmentTag); - } else if (readGlobalServiceId(environmentTag).isPresent()) { + else if (readGlobalServiceId(environmentTag).isPresent()) throw new IllegalArgumentException("Attribute 'global-service-id' is only valid on 'prod' tag."); - } } - return new DeploymentSpec(globalServiceId, readUpgradePolicy(root), zones, xmlForm); + return new DeploymentSpec(globalServiceId, readUpgradePolicy(root), steps, xmlForm); + } + + /** Returns the given attribute as an integer, or 0 if it is not present */ + private static long longAttribute(String attributeName, Element tag) { + String value = tag.getAttribute(attributeName); + if (value == null || value.isEmpty()) return 0; + try { + return Long.parseLong(value); + } + catch (NumberFormatException e) { + throw new IllegalArgumentException("Expected an integer for attribute '" + attributeName + + "' but got '" + value + "'"); + } } private static boolean isEnvironmentName(String tagName) { @@ -173,7 +250,7 @@ public class DeploymentSpec { } /** This may be invoked by a continuous build */ - public static void main (String[] args) { + public static void main(String[] args) { if (args.length != 2 && args.length != 3) { System.err.println("Usage: DeploymentSpec [file] [environment] [region]?" + "Returns 0 if the specified zone matches the deployment spec, 1 otherwise"); @@ -195,7 +272,37 @@ public class DeploymentSpec { } } - public static class DeclaredZone { + /** A delpoyment 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()); + } + + /** Returns whether this step deploys to the given environment, and (if specified) region */ + public abstract boolean deploysTo(Environment environment, Optional<RegionName> region); + + } + + /** A deployment step which is to wait for some time before progressing to the next step */ + public static class Delay extends Step { + + private final Duration duration; + + public Delay(Duration duration) { + this.duration = duration; + } + + public Duration duration() { return duration; } + + @Override + public boolean deploysTo(Environment environment, Optional<RegionName> region) { return false; } + + } + + /** A deployment step which is to run deployment in a particular zone */ + public static class ZoneDeployment extends Step { private final Environment environment; @@ -203,7 +310,15 @@ public class DeploymentSpec { private final boolean active; - public DeclaredZone(Environment environment, Optional<RegionName> region, boolean active) { + public ZoneDeployment(Environment environment) { + this(environment, Optional.empty(), false); + } + + public ZoneDeployment(Environment environment, Optional<RegionName> region, boolean active) { + if (environment != Environment.prod && region.isPresent()) + throw new IllegalArgumentException("Non-prod environments cannot specify a region"); + if (environment == Environment.prod && ! region.isPresent()) + throw new IllegalArgumentException("Prod environments must be specified with a region"); this.environment = environment; this.region = region; this.active = active; @@ -217,23 +332,26 @@ public class DeploymentSpec { /** Returns whether this zone should receive production traffic */ public boolean active() { return active; } - public boolean matches(Environment environment, Optional<RegionName> region) { - if (environment.equals(this.environment) && region.equals(this.region)) return true; - if ( ! region.isPresent() && prerequisite(environment)) return true; - return false; - } - - /** - * Returns whether deployment in the given environment is a prerequisite of deployment in this environment - * - * The required progression leading to prerequisites is test, staging, prod. - */ - private boolean prerequisite(Environment environment) { - if (this.environment == Environment.prod) - return environment == Environment.staging || environment == Environment.test; - if (this.environment == Environment.staging) - return environment == Environment.test; - return false; + @Override + 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 int hashCode() { + return Objects.hash(environment, region); + } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if ( ! (o instanceof ZoneDeployment)) return false; + ZoneDeployment other = (ZoneDeployment)o; + if (this.environment != other.environment) return false; + if ( ! this.region.equals(other.region())) return false; + return true; } } 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 ffde5b21458..bc1d0c55654 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 @@ -26,8 +26,8 @@ public class DeploymentSpecTest { StringReader r = new StringReader(specXml); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(specXml, spec.xmlForm()); - assertEquals(1, spec.zones().size()); - assertEquals(Environment.test, spec.zones().get(0).environment()); + assertEquals(1, spec.steps().size()); + 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())); @@ -44,8 +44,9 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(1, spec.zones().size()); - assertEquals(Environment.staging, spec.zones().get(0).environment()); + 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())); @@ -65,15 +66,17 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.zones().size()); + assertEquals(4, spec.steps().size()); - assertEquals(Environment.prod, spec.zones().get(0).environment()); - assertEquals("us-east1", spec.zones().get(0).region().get().value()); - assertFalse(spec.zones().get(0).active()); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertEquals(Environment.prod, spec.zones().get(1).environment()); - assertEquals("us-west1", spec.zones().get(1).region().get().value()); - assertTrue(spec.zones().get(1).active()); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); + + assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertFalse(((DeploymentSpec.ZoneDeployment)spec.steps().get(2)).active()); + + assertTrue(spec.steps().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(((DeploymentSpec.ZoneDeployment)spec.steps().get(3)).active()); assertTrue(spec.includes(Environment.test, Optional.empty())); assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); @@ -91,28 +94,32 @@ public class DeploymentSpecTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <test/>" + + " <test/>" + " <staging/>" + " <prod>" + " <region active='false'>us-east1</region>" + + " <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(4, spec.zones().size()); + assertEquals(5, spec.steps().size()); + + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertEquals(Environment.test, spec.zones().get(0).environment()); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - assertEquals(Environment.staging, spec.zones().get(1).environment()); + assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertFalse(((DeploymentSpec.ZoneDeployment)spec.steps().get(2)).active()); - assertEquals(Environment.prod, spec.zones().get(2).environment()); - assertEquals("us-east1", spec.zones().get(2).region().get().value()); - assertFalse(spec.zones().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()); - assertEquals(Environment.prod, spec.zones().get(3).environment()); - assertEquals("us-west1", spec.zones().get(3).region().get().value()); - assertTrue(spec.zones().get(3).active()); + assertTrue(spec.steps().get(4).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(((DeploymentSpec.ZoneDeployment)spec.steps().get(4)).active()); assertTrue(spec.includes(Environment.test, Optional.empty())); assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); @@ -192,12 +199,36 @@ public class DeploymentSpecTest { 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.zones().isEmpty()); + assertTrue(DeploymentSpec.empty.steps().isEmpty()); assertEquals("<deployment version='1.0'/>", DeploymentSpec.empty.xmlForm()); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index addf14d5e6b..0eb1abda713 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -418,10 +418,10 @@ public final class ContainerCluster private boolean zoneHasActiveRotation(Zone zone) { return getDeploymentSpec() - .flatMap(spec -> spec.zones().stream() - .filter(dz -> dz.matches(zone.environment(), Optional.of(zone.region()))) + .flatMap(spec -> spec.steps().stream() + .filter(dz -> dz.deploysTo(zone.environment(), Optional.of(zone.region()))) .findFirst()) - .map(DeploymentSpec.DeclaredZone::active) + .map(step -> ((DeploymentSpec.ZoneDeployment)step).active()) .orElse(false); } diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index c8bd11d7184..d34255c7127 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -21,11 +21,19 @@ Staging = element staging { text } -Prod = - element prod { - attribute global-service-id { text }?, - element region { - attribute active { xsd:boolean }, - text - }* - } +Prod = element prod { + attribute global-service-id { text }? & + Region* & + Delay* +} + +Region = element region { + attribute active { xsd:boolean }, + text +} + +Delay = element delay { + attribute hours { xsd:long }? & + attribute minutes { xsd:long }? & + attribute seconds { xsd:long }? +} diff --git a/config-model/src/test/schema-test-files/deployment.xml b/config-model/src/test/schema-test-files/deployment.xml index 92d59abbe53..286466eff57 100644 --- a/config-model/src/test/schema-test-files/deployment.xml +++ b/config-model/src/test/schema-test-files/deployment.xml @@ -4,7 +4,9 @@ <staging/> <prod global-service-id='qrs'> <region active='true'>us-west-1</region> + <delay hours='3'/> <region active='true'>us-central-1</region> + <delay hours='3' minutes='7' seconds='13'/> <region active='true'>us-east-3</region> </prod> </deployment> diff --git a/vespajlib/src/main/java/com/yahoo/text/XML.java b/vespajlib/src/main/java/com/yahoo/text/XML.java index c688d5f9722..1b3d690cb2c 100644 --- a/vespajlib/src/main/java/com/yahoo/text/XML.java +++ b/vespajlib/src/main/java/com/yahoo/text/XML.java @@ -25,7 +25,7 @@ import org.xml.sax.SAXParseException; * @author Bjorn Borud * @author Vegard Havdal * @author bratseth - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class XML { /** |