diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2019-10-25 07:49:55 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2019-11-05 09:28:52 +0100 |
commit | d501b21ca23087992a27adaf1ea1edf34eba44e1 (patch) | |
tree | 1534788de6918e367ebaf467a3fd57e6613b1e8c /config-model-api | |
parent | c246d19f5faf4af1466c5ce9fc0e13c2811f1e64 (diff) |
Revert "Revert "stop using deprecated deployment spec methods""
Diffstat (limited to 'config-model-api')
5 files changed, 90 insertions, 59 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 7f4273c827b..6a41ce66456 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -331,8 +331,6 @@ "methods": [ "public void <init>(java.util.List, java.util.Optional, java.util.Optional, 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()", "public java.util.Optional majorVersion()", "public boolean canUpgradeAt(java.time.Instant)", "public boolean canChangeRevisionAt(java.time.Instant)", 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 446dc8d1fc3..7b4e91bfe4a 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 @@ -1,6 +1,7 @@ // 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.yahoo.collections.Comparables; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; @@ -72,6 +73,7 @@ public class DeploymentSpec { this.athenzService = athenzService; this.xmlForm = xmlForm; validateTotalDelay(steps); + validateUpgradePoliciesOfIncreasingConservativeness(steps); } // TODO: Remove after October 2019 @@ -147,6 +149,23 @@ public class DeploymentSpec { " but max 24 hours is allowed"); } + /** Throws an IllegalArgumentException if any instance has a looser upgrade policy than the previous */ + private void validateUpgradePoliciesOfIncreasingConservativeness(List<Step> steps) { + UpgradePolicy previous = Collections.min(List.of(UpgradePolicy.values())); + for (Step step : steps) { + UpgradePolicy strictest = previous; + List<DeploymentInstanceSpec> specs = instances(List.of(step)); + for (DeploymentInstanceSpec spec : specs) { + if (spec.upgradePolicy().compareTo(previous) < 0) + throw new IllegalArgumentException("Instance '" + spec.name() + "' cannot have a looser upgrade " + + "policy than the previous of '" + previous + "'"); + + strictest = Comparables.max(strictest, spec.upgradePolicy()); + } + previous = strictest; + } + } + // TODO: Remove after October 2019 private DeploymentInstanceSpec singleInstance() { return singleInstance(steps); @@ -161,12 +180,6 @@ public class DeploymentSpec { instances.stream().map(Step::toString).collect(Collectors.joining(","))); } - // TODO: Remove after October 2019 - public Optional<String> globalServiceId() { return singleInstance().globalServiceId(); } - - // TODO: Remove after October 2019 - public UpgradePolicy upgradePolicy() { return singleInstance().upgradePolicy(); } - /** Returns the major version this application is pinned to, or empty (default) to allow all major versions */ public Optional<Integer> majorVersion() { return majorVersion; } @@ -270,7 +283,7 @@ public class DeploymentSpec { private static List<DeploymentInstanceSpec> instances(List<DeploymentSpec.Step> steps) { return steps.stream() - .flatMap(step -> step instanceof ParallelZones ? ((ParallelZones)step).steps.stream() : List.of(step).stream()) + .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()); } 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 index 5a8358e65c3..8167ba7af11 100644 --- 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 @@ -45,7 +45,6 @@ public class DeploymentSpecDeprecatedAPITest { 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 @@ -78,7 +77,6 @@ public class DeploymentSpecDeprecatedAPITest { 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 @@ -111,9 +109,6 @@ public class DeploymentSpecDeprecatedAPITest { 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 @@ -208,7 +203,6 @@ public class DeploymentSpecDeprecatedAPITest { 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 @@ -223,7 +217,6 @@ public class DeploymentSpecDeprecatedAPITest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.globalServiceId(), Optional.of("query")); } @Test(expected=IllegalArgumentException.class) @@ -247,41 +240,6 @@ public class DeploymentSpecDeprecatedAPITest { } @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( @@ -307,8 +265,6 @@ public class DeploymentSpecDeprecatedAPITest { @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()); } @@ -366,7 +322,7 @@ public class DeploymentSpecDeprecatedAPITest { " <block-change days='mon,tue' hours='15-16'/>\n" + "</deployment>" ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); + DeploymentSpec.fromXml(r); } @Test(expected = IllegalArgumentException.class) 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 7b571417ef8..080982cb785 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 @@ -310,16 +310,16 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <upgrade policy='canary'/>" + " <instance id='instance1'>" + - " <upgrade policy='conservative'/>" + " </instance>" + " <instance id='instance2'>" + + " <upgrade policy='conservative'/>" + " </instance>" + "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("conservative", spec.requireInstance("instance1").upgradePolicy().toString()); - assertEquals("canary", spec.requireInstance("instance2").upgradePolicy().toString()); + assertEquals("canary", spec.requireInstance("instance1").upgradePolicy().toString()); + assertEquals("conservative", spec.requireInstance("instance2").upgradePolicy().toString()); } @Test @@ -351,7 +351,6 @@ public class DeploymentSpecTest { @Test public void testEmpty() { assertFalse(DeploymentSpec.empty.requireInstance("default").globalServiceId().isPresent()); - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, DeploymentSpec.empty.upgradePolicy()); assertTrue(DeploymentSpec.empty.steps().isEmpty()); assertEquals("<deployment version='1.0'/>", DeploymentSpec.empty.xmlForm()); } @@ -502,6 +501,72 @@ public class DeploymentSpecTest { } @Test(expected = IllegalArgumentException.class) + public void deploymentSpecWithIncreasinglyStrictUpgradePolicies() { + StringReader r = new StringReader( + "<deployment version='1.0'>" + + " <instance id='instance1'>" + + " <upgrade policy='conservative'/>" + + " </instance>" + + " <instance id='instance2' />" + + "</deployment>" + ); + DeploymentSpec.fromXml(r); + } + + @Test(expected = IllegalArgumentException.class) + public void deploymentSpecWithIncreasinglyStrictUpgradePoliciesInParallel() { + StringReader r = new StringReader( + "<deployment version='1.0'>" + + " <instance />" + + " <parallel>" + + " <instance id='instance1'>" + + " <upgrade policy='conservative'/>" + + " </instance>" + + " <instance id='instance2'>" + + " <upgrade policy='canary'/>" + + " </instance>" + + " </parallel>" + + "</deployment>" + ); + DeploymentSpec.fromXml(r); + } + + @Test(expected = IllegalArgumentException.class) + public void deploymentSpecWithIncreasinglyStrictUpgradePoliciesAfterParallel() { + StringReader r = new StringReader( + "<deployment version='1.0'>" + + " <parallel>" + + " <instance id='instance1'>" + + " <upgrade policy='conservative'/>" + + " </instance>" + + " <instance id='instance2'>" + + " <upgrade policy='canary'/>" + + " </instance>" + + " </parallel>" + + " <instance />" + + "</deployment>" + ); + DeploymentSpec.fromXml(r); + } + + @Test + public void deploymentSpecWithDifferentUpgradePoliciesInParallel() { + StringReader r = new StringReader( + "<deployment version='1.0'>" + + " <parallel>" + + " <instance id='instance1'>" + + " <upgrade policy='conservative'/>" + + " </instance>" + + " <instance id='instance2' />" + + " </parallel>" + + "</deployment>" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals(DeploymentSpec.UpgradePolicy.conservative, spec.requireInstance("instance1").upgradePolicy()); + assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("instance2").upgradePolicy()); + } + + @Test(expected = IllegalArgumentException.class) public void deploymentSpecWithIllegallyOrderedDeploymentSpec1() { StringReader r = new StringReader( "<deployment>" + 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 7805b73cc6a..25c37a2a240 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 @@ -253,7 +253,6 @@ public class DeploymentSpecWithoutInstanceTest { @Test public void testEmpty() { assertFalse(DeploymentSpec.empty.requireInstance("default").globalServiceId().isPresent()); - assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, DeploymentSpec.empty.upgradePolicy()); assertTrue(DeploymentSpec.empty.steps().isEmpty()); assertEquals("<deployment version='1.0'/>", DeploymentSpec.empty.xmlForm()); } |