diff options
author | Jon Bratseth <bratseth@yahoo-inc.com> | 2017-06-01 16:56:48 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@yahoo-inc.com> | 2017-06-01 16:56:48 +0200 |
commit | 2f8e3b5a78d75427575534d9ab551048e59cbbf2 (patch) | |
tree | 967faaa06a60d71d6589a605e629292116ba88ca /config-model-api | |
parent | 9ee7ea37ba5ad9bcbc803579ec02bf2cc434ac52 (diff) |
Validate max total delay
Diffstat (limited to 'config-model-api')
-rw-r--r-- | config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java | 22 | ||||
-rw-r--r-- | config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java | 70 |
2 files changed, 66 insertions, 26 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 7687ad1313f..ff965b5a5df 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 @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; /** * Specifies the environments and regions to which an application should be deployed. @@ -50,12 +51,23 @@ public class DeploymentSpec { private DeploymentSpec(Optional<String> globalServiceId, UpgradePolicy upgradePolicy, List<Step> steps, String xmlForm) { + validateTotalDelay(steps); this.globalServiceId = globalServiceId; this.upgradePolicy = upgradePolicy; 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) { // Add staging if required and missing @@ -104,7 +116,7 @@ public class DeploymentSpec { public UpgradePolicy upgradePolicy() { return upgradePolicy; } /** Returns the deployment steps of this in the order they will be performed */ - public List<Step> zones() { return steps; } + 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; } @@ -147,9 +159,9 @@ public class DeploymentSpec { if (environment == Environment.prod) { for (Element stepTag : XML.getChildren(environmentTag)) { if (stepTag.getTagName().equals("delay")) - steps.add(new Delay(Duration.ofSeconds(intAttribute("hours", stepTag) * 60 * 60 + - intAttribute("minutes", stepTag) * 60 + - intAttribute("seconds", stepTag)))); + 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())), @@ -169,7 +181,7 @@ public class DeploymentSpec { } /** Returns the given attribute as an integer, or 0 if it is not present */ - private static int intAttribute(String attributeName, Element tag) { + private static long longAttribute(String attributeName, Element tag) { String value = tag.getAttribute(attributeName); if (value == null || value.isEmpty()) return 0; try { 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 cd34331c069..c6bff6124ba 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()); - assertTrue(spec.zones().get(0).deploysTo(Environment.test)); + 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,9 +44,9 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.zones().size()); - assertTrue(spec.zones().get(0).deploysTo(Environment.test)); - assertTrue(spec.zones().get(1).deploysTo(Environment.staging)); + 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())); @@ -66,17 +66,17 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(4, spec.zones().size()); + assertEquals(4, spec.steps().size()); - assertTrue(spec.zones().get(0).deploysTo(Environment.test)); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.zones().get(1).deploysTo(Environment.staging)); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - assertTrue(spec.zones().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.ZoneDeployment)spec.zones().get(2)).active()); + assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertFalse(((DeploymentSpec.ZoneDeployment)spec.steps().get(2)).active()); - assertTrue(spec.zones().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.ZoneDeployment)spec.zones().get(3)).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")))); @@ -97,23 +97,27 @@ public class DeploymentSpecTest { " <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(4, spec.zones().size()); + assertEquals(5, spec.steps().size()); - assertTrue(spec.zones().get(0).deploysTo(Environment.test)); + assertTrue(spec.steps().get(0).deploysTo(Environment.test)); - assertTrue(spec.zones().get(1).deploysTo(Environment.staging)); + assertTrue(spec.steps().get(1).deploysTo(Environment.staging)); - assertTrue(spec.zones().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.ZoneDeployment)spec.zones().get(2)).active()); + assertTrue(spec.steps().get(2).deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertFalse(((DeploymentSpec.ZoneDeployment)spec.steps().get(2)).active()); - assertTrue(spec.zones().get(3).deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.ZoneDeployment)spec.zones().get(3)).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.ZoneDeployment)spec.steps().get(4)).active()); assertTrue(spec.includes(Environment.test, Optional.empty())); assertFalse(spec.includes(Environment.test, Optional.of(RegionName.from("region1")))); @@ -193,12 +197,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()); } |