diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2019-12-12 14:07:46 +0100 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2019-12-12 14:07:46 +0100 |
commit | 786c56ae53894a3f74412bfa05a0384da8b097b3 (patch) | |
tree | 06e96effed3323dd1e6ac4779891aed8dbb9e399 /config-model-api/src | |
parent | 053f042a5fb5e987194bf32f188f677fadf80259 (diff) |
Remove broken step completion in DeploymentSpec and handle it in orchestration instead
Diffstat (limited to 'config-model-api/src')
3 files changed, 30 insertions, 88 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 8c05b47e8e8..170547430cb 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 @@ -63,7 +63,7 @@ public class DeploymentSpec { Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, String xmlForm) { - this.steps = List.copyOf(completeSteps(steps)); + this.steps = List.copyOf(steps); this.majorVersion = majorVersion; this.athenzDomain = athenzDomain; this.athenzService = athenzService; @@ -72,50 +72,6 @@ public class DeploymentSpec { validateUpgradePoliciesOfIncreasingConservativeness(steps); } - /** 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); - - // Add staging if required and missing - if (steps.stream().anyMatch(step -> step.concerns(Environment.prod)) && - steps.stream().noneMatch(step -> step.concerns(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))) { - steps.add(new DeploymentSpec.DeclaredZone(Environment.test)); - } - - // Enforce order test, staging, prod - DeploymentSpec.DeclaredZone testStep = remove(Environment.test, steps); - if (testStep != null) - steps.add(0, testStep); - DeploymentSpec.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) { - 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; - } - } - 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(); 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 e4f52f573b0..ac605add892 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 @@ -77,9 +77,8 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.steps().size()); + assertEquals(1, spec.steps().size()); assertEquals(1, spec.requireInstance("default").steps().size()); - assertTrue(spec.steps().get(0).concerns(Environment.test)); assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.staging)); assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty())); assertTrue(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty())); @@ -101,13 +100,9 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(3, spec.steps().size()); + assertEquals(1, spec.steps().size()); assertEquals(2, spec.requireInstance("default").steps().size()); - assertTrue(spec.steps().get(0).concerns(Environment.test)); - - assertTrue(spec.steps().get(1).concerns(Environment.staging)); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(0)).active()); @@ -500,6 +495,7 @@ public class DeploymentSpecTest { public void testNestedParallelAndSteps() { StringReader r = new StringReader( "<deployment athenz-domain='domain'>" + + " <staging />" + " <instance id='instance' athenz-service='in-service'>" + " <prod>" + " <parallel>" + @@ -529,13 +525,12 @@ public class DeploymentSpecTest { 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("instance 'instance'", steps.get(2).toString()); - assertEquals(Duration.ofHours(4), steps.get(2).delay()); + assertEquals(2, steps.size()); + assertEquals("staging", steps.get(0).toString()); + assertEquals("instance 'instance'", steps.get(1).toString()); + assertEquals(Duration.ofHours(4), steps.get(1).delay()); - List<DeploymentSpec.Step> instanceSteps = steps.get(2).steps(); + List<DeploymentSpec.Step> instanceSteps = steps.get(1).steps(); assertEquals(2, instanceSteps.size()); assertEquals("4 parallel steps", instanceSteps.get(0).toString()); assertEquals("prod.us-north-7", instanceSteps.get(1).toString()); @@ -590,12 +585,10 @@ public class DeploymentSpecTest { 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()); + assertEquals(1, steps.size()); + assertEquals("2 parallel steps", steps.get(0).toString()); - List<DeploymentSpec.Step> parallelSteps = steps.get(2).steps(); + List<DeploymentSpec.Step> parallelSteps = steps.get(0).steps(); assertEquals("instance 'instance0'", parallelSteps.get(0).toString()); assertEquals("instance 'instance1'", parallelSteps.get(1).toString()); } @@ -620,12 +613,10 @@ public class DeploymentSpecTest { 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()); + assertEquals(3, steps.size()); + assertEquals("instance 'instance0'", steps.get(0).toString()); + assertEquals("delay PT12H", steps.get(1).toString()); + assertEquals("instance 'instance1'", steps.get(2).toString()); } @Test 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 index 00f0a1bfa41..50999759b77 100644 --- 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 @@ -71,9 +71,8 @@ public class DeploymentSpecWithoutInstanceTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(2, spec.steps().size()); + assertEquals(1, spec.steps().size()); assertEquals(1, spec.requireInstance("default").steps().size()); - assertTrue(spec.steps().get(0).concerns(Environment.test)); assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.staging)); assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty())); assertTrue(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty())); @@ -93,13 +92,9 @@ public class DeploymentSpecWithoutInstanceTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(3, spec.steps().size()); + assertEquals(1, spec.steps().size()); assertEquals(2, spec.requireInstance("default").steps().size()); - assertTrue(spec.steps().get(0).concerns(Environment.test)); - - assertTrue(spec.steps().get(1).concerns(Environment.staging)); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(0)).active()); @@ -352,6 +347,7 @@ public class DeploymentSpecWithoutInstanceTest { public void testNestedParallelAndSteps() { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>" + + " <staging />" + " <prod>" + " <parallel>" + " <region active='true'>us-west-1</region>" + @@ -379,18 +375,17 @@ public class DeploymentSpecWithoutInstanceTest { 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("instance 'default'", steps.get(2).toString()); - assertEquals(Duration.ofHours(4), steps.get(2).delay()); - - List<DeploymentSpec.Step> instanceSteps = steps.get(2).steps(); - assertEquals(2, instanceSteps.size()); - assertEquals("4 parallel steps", instanceSteps.get(0).toString()); - assertEquals("prod.us-north-7", instanceSteps.get(1).toString()); - - List<DeploymentSpec.Step> parallelSteps = instanceSteps.get(0).steps(); + assertEquals(1, steps.size()); + assertEquals("instance 'default'", steps.get(0).toString()); + assertEquals(Duration.ofHours(4), steps.get(0).delay()); + + List<DeploymentSpec.Step> instanceSteps = steps.get(0).steps(); + assertEquals(3, instanceSteps.size()); + assertEquals("staging", instanceSteps.get(0).toString()); + assertEquals("4 parallel steps", instanceSteps.get(1).toString()); + assertEquals("prod.us-north-7", instanceSteps.get(2).toString()); + + List<DeploymentSpec.Step> parallelSteps = instanceSteps.get(1).steps(); assertEquals(4, parallelSteps.size()); assertEquals("prod.us-west-1", parallelSteps.get(0).toString()); assertEquals("4 steps", parallelSteps.get(1).toString()); |