diff options
author | Martin Polden <mpolden@mpolden.no> | 2023-07-27 14:05:56 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-27 14:05:56 +0200 |
commit | 85c1576819fe616b716d9a8543b3fa10af37d9b3 (patch) | |
tree | aa35d15ab59cac4f4d47ca438fd9c9ca226d1074 /config-model-api/src | |
parent | 23760b0dc446cdaeac5494804741aca0cca9b5ba (diff) | |
parent | 49d8077e6bf7ec88aae9f2554aefeff22452e013 (diff) |
Merge pull request #27909 from vespa-engine/mpolden/remove-global-service-id
Reapply "Remove global-service-id"
Diffstat (limited to 'config-model-api/src')
5 files changed, 113 insertions, 296 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 b6f934c8824..4ca96453ad1 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 @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.application.api; -import ai.vespa.validation.Validation; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.CloudName; @@ -58,7 +57,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { private final int maxRisk; private final int maxIdleHours; private final List<DeploymentSpec.ChangeBlocker> changeBlockers; - private final Optional<String> globalServiceId; private final Optional<AthenzService> athenzService; private final Map<CloudName, CloudAccount> cloudAccounts; private final Optional<Duration> hostTTL; @@ -76,7 +74,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { DeploymentSpec.UpgradeRollout upgradeRollout, int minRisk, int maxRisk, int maxIdleHours, List<DeploymentSpec.ChangeBlocker> changeBlockers, - Optional<String> globalServiceId, Optional<AthenzService> athenzService, Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL, @@ -100,7 +97,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { this.maxRisk = require(maxRisk >= minRisk, maxRisk, "maximum risk cannot be less than minimum risk score"); this.maxIdleHours = requireInRange(maxIdleHours, "maximum idle hours", 0, 168); this.changeBlockers = Objects.requireNonNull(changeBlockers); - this.globalServiceId = Objects.requireNonNull(globalServiceId); this.athenzService = Objects.requireNonNull(athenzService); this.cloudAccounts = Map.copyOf(cloudAccounts); this.hostTTL = Objects.requireNonNull(hostTTL); @@ -111,7 +107,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { this.zoneEndpoints = Collections.unmodifiableMap(zoneEndpointsCopy); this.bcp = Objects.requireNonNull(bcp); validateZones(new HashSet<>(), new HashSet<>(), this); - validateEndpoints(globalServiceId, this.endpoints); + validateEndpoints(this.endpoints); validateChangeBlockers(changeBlockers, now); validateBcp(bcp); hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); @@ -155,11 +151,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { } /** Throw an IllegalArgumentException if an endpoint refers to a region that is not declared in 'prod' */ - private void validateEndpoints(Optional<String> globalServiceId, List<Endpoint> endpoints) { - if (globalServiceId.isPresent() && ! endpoints.isEmpty()) { - throw new IllegalArgumentException("Providing both 'endpoints' and 'global-service-id'. Use only 'endpoints'."); - } - + private void validateEndpoints(List<Endpoint> endpoints) { var regions = prodRegions(); for (var endpoint : endpoints){ for (var endpointRegion : endpoint.regions()) { @@ -243,9 +235,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { /** Returns time windows where upgrades are disallowed for these instances */ public List<DeploymentSpec.ChangeBlocker> changeBlocker() { return changeBlockers; } - /** Returns the ID of the service to expose through global routing, if present */ - public Optional<String> globalServiceId() { return globalServiceId; } - /** Returns whether the instances in this step can upgrade at the given instant */ public boolean canUpgradeAt(Instant instant) { return changeBlockers.stream().filter(block -> block.blocksVersions()) @@ -323,8 +312,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; DeploymentInstanceSpec other = (DeploymentInstanceSpec) o; - return globalServiceId.equals(other.globalServiceId) && - upgradePolicy == other.upgradePolicy && + return upgradePolicy == other.upgradePolicy && revisionTarget == other.revisionTarget && upgradeRollout == other.upgradeRollout && changeBlockers.equals(other.changeBlockers) && @@ -339,7 +327,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { @Override public int hashCode() { - return Objects.hash(globalServiceId, upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints, zoneEndpoints, bcp, tags); + return Objects.hash(upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints, zoneEndpoints, bcp, tags); } int deployableHashCode() { @@ -349,7 +337,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { toHash[i++] = name; toHash[i++] = endpoints; toHash[i++] = zoneEndpoints; - toHash[i++] = globalServiceId; toHash[i++] = tags; toHash[i++] = bcp; toHash[i++] = cloudAccounts; 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 797be652ebc..e49619c45cf 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,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.application.api; -import ai.vespa.validation.Validation; import com.yahoo.collections.Comparables; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; @@ -434,17 +433,16 @@ public class DeploymentSpec { private final Environment environment; private final Optional<RegionName> region; - private final boolean active; private final Optional<AthenzService> athenzService; private final Optional<String> testerFlavor; private final Map<CloudName, CloudAccount> cloudAccounts; private final Optional<Duration> hostTTL; public DeclaredZone(Environment environment) { - this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); + this(environment, Optional.empty(), Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); } - public DeclaredZone(Environment environment, Optional<RegionName> region, boolean active, + public DeclaredZone(Environment environment, Optional<RegionName> region, Optional<AthenzService> athenzService, Optional<String> testerFlavor, Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL) { if (environment != Environment.prod && region.isPresent()) @@ -454,7 +452,6 @@ public class DeploymentSpec { hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); this.environment = Objects.requireNonNull(environment); this.region = Objects.requireNonNull(region); - this.active = active; this.athenzService = Objects.requireNonNull(athenzService); this.testerFlavor = Objects.requireNonNull(testerFlavor); this.cloudAccounts = Map.copyOf(cloudAccounts); @@ -466,8 +463,8 @@ public class DeploymentSpec { /** The region name, or empty if not declared */ public Optional<RegionName> region() { return region; } - /** Returns whether this zone should receive production traffic */ - public boolean active() { return active; } + // TODO(mpolden): Remove after Vespa < 8.203 is no longer in use + public boolean active() { return true; } public Optional<String> testerFlavor() { return testerFlavor; } 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 38562eefb03..4bfecabaf69 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 @@ -93,7 +93,6 @@ public class DeploymentSpecXmlReader { private static final String athenzDomainAttribute = "athenz-domain"; private static final String testerFlavorAttribute = "tester-flavor"; private static final String majorVersionAttribute = "major-version"; - private static final String globalServiceIdAttribute = "global-service-id"; private static final String cloudAccountAttribute = "cloud-account"; private static final String hostTTLAttribute = "empty-host-ttl"; @@ -234,7 +233,6 @@ public class DeploymentSpecXmlReader { upgradeRollout, minRisk, maxRisk, maxIdleHours, changeBlockers, - Optional.ofNullable(prodAttributes.get(globalServiceIdAttribute)), athenzService, cloudAccounts, hostTTL, @@ -268,12 +266,6 @@ public class DeploymentSpecXmlReader { Optional<AthenzService> athenzService = mostSpecificAttribute(stepTag, athenzServiceAttribute).map(AthenzService::from); Optional<String> testerFlavor = mostSpecificAttribute(stepTag, testerFlavorAttribute); - if (prodTag.equals(stepTag.getTagName())) { - readGlobalServiceId(stepTag).ifPresent(id -> prodAttributes.put(globalServiceIdAttribute, id)); - } else { - if (readGlobalServiceId(stepTag).isPresent()) illegal("Attribute '" + globalServiceIdAttribute + "' is only valid on 'prod' tag"); - } - switch (stepTag.getTagName()) { case testTag: if (Stream.iterate(stepTag, Objects::nonNull, Node::getParentNode) @@ -281,7 +273,7 @@ public class DeploymentSpecXmlReader { return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()), readHostTTL(stepTag))); // A production test } case devTag, perfTag, stagingTag: // Intentional fallthrough from test tag. - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccounts(stepTag), readHostTTL(stepTag))); + return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), athenzService, testerFlavor, readCloudAccounts(stepTag), readHostTTL(stepTag))); case prodTag: // regions, delay and parallel may be nested within, but we can flatten them return XML.getChildren(stepTag).stream() .flatMap(child -> readNonInstanceSteps(child, prodAttributes, stepTag, defaultBcp).stream()) @@ -690,7 +682,7 @@ public class DeploymentSpecXmlReader { private DeclaredZone readDeclaredZone(Environment environment, Optional<AthenzService> athenzService, Optional<String> testerFlavor, Element regionTag) { return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())), - readActive(regionTag), athenzService, testerFlavor, + athenzService, testerFlavor, readCloudAccounts(regionTag), readHostTTL(regionTag)); } @@ -714,13 +706,6 @@ public class DeploymentSpecXmlReader { return mostSpecificAttribute(tag, hostTTLAttribute).map(s -> toDuration(s, "empty host TTL")); } - private Optional<String> readGlobalServiceId(Element environmentTag) { - String globalServiceId = environmentTag.getAttribute(globalServiceIdAttribute); - if (globalServiceId.isEmpty()) return Optional.empty(); - deprecate(environmentTag, List.of(globalServiceIdAttribute), 7, "See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax"); - return Optional.of(globalServiceId); - } - private List<DeploymentSpec.ChangeBlocker> readChangeBlockers(Element parent, Element globalBlockersParent) { List<DeploymentSpec.ChangeBlocker> changeBlockers = new ArrayList<>(); if (globalBlockersParent != parent) { @@ -799,16 +784,6 @@ public class DeploymentSpecXmlReader { }; } - private boolean readActive(Element regionTag) { - String activeValue = regionTag.getAttribute("active"); - if ("".equals(activeValue)) return true; // Default to active - deprecate(regionTag, List.of("active"), 7, "See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax"); - if ("true".equals(activeValue)) return true; - if ("false".equals(activeValue)) return false; - throw new IllegalArgumentException("Value of 'active' attribute in region tag must be 'true' or 'false' " + - "to control whether this region should receive traffic from the global endpoint of this application"); - } - private void deprecate(Element element, List<String> attributes, int majorVersion, String message) { deprecatedElements.add(new DeprecatedElement(majorVersion, element.getTagName(), attributes, message)); } 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 d4312a0e54e..134d45e35ab 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 @@ -13,8 +13,8 @@ import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Tags; import com.yahoo.config.provision.ZoneEndpoint; -import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.test.ManualClock; import org.junit.Test; @@ -46,7 +46,6 @@ import static com.yahoo.config.provision.zone.ZoneId.from; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -74,7 +73,6 @@ public class DeploymentSpecTest { assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region assertFalse(spec.requireInstance("default").concerns(staging, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @Test @@ -110,7 +108,6 @@ public class DeploymentSpecTest { assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(staging, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @Test @@ -119,8 +116,8 @@ public class DeploymentSpecTest { <deployment version='1.0'> <instance id='default'> <prod> - <region active='false'>us-east1</region> - <region active='true'>us-west1</region> + <region>us-east1</region> + <region>us-west1</region> </prod> </instance> </deployment> @@ -131,17 +128,14 @@ public class DeploymentSpecTest { assertEquals(2, spec.requireInstance("default").steps().size()); assertTrue(spec.requireInstance("default").steps().get(0).concerns(prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(0)).active()); assertTrue(spec.requireInstance("default").steps().get(1).concerns(prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(1)).active()); assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(staging, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-east1")))); assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-west1")))); assertFalse(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); assertEquals(DeploymentSpec.RevisionTarget.latest, spec.requireInstance("default").revisionTarget()); @@ -158,14 +152,14 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <instance id='a' tags='tag1 tag2'>" + " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + + " <region>us-east1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + " <instance id='b' tags='tag3'>" + " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + + " <region>us-east1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -183,9 +177,9 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -203,8 +197,8 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + + " <region>us-east-1</region>" + + " <region>us-west-1</region>" + " <delay hours='1' />" + " <test>us-west-1</test>" + " <test>us-east-1</test>" + @@ -231,7 +225,7 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " <test>us-east1</test>" + " </prod>" + @@ -248,7 +242,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <prod>" + " <test>us-east1</test>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -263,7 +257,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <prod>" + " <parallel>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " </parallel>" + " </prod>" + @@ -281,14 +275,14 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + " <instance id='instance2'>" + " <prod>" + - " <region active='true'>us-central1</region>" + + " <region>us-central1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -313,9 +307,9 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -336,13 +330,11 @@ public class DeploymentSpecTest { assertTrue(instance.steps().get(1).concerns(staging)); assertTrue(instance.steps().get(2).concerns(prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)instance.steps().get(2)).active()); assertTrue(instance.steps().get(3) instanceof DeploymentSpec.Delay); assertEquals(3 * 60 * 60 + 30 * 60, instance.steps().get(3).delay().getSeconds()); assertTrue(instance.steps().get(4).concerns(prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)instance.steps().get(4)).active()); assertTrue(instance.concerns(test, Optional.empty())); assertTrue(instance.concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region @@ -350,68 +342,6 @@ public class DeploymentSpecTest { assertTrue(instance.concerns(prod, Optional.of(RegionName.from("us-east1")))); assertTrue(instance.concerns(prod, Optional.of(RegionName.from("us-west1")))); assertFalse(instance.concerns(prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(instance.globalServiceId().isPresent()); - } - - @Test - public void productionSpecWithGlobalServiceId() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='default'>" + - " <prod global-service-id='query'>" + - " <region active='true'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.requireInstance("default").globalServiceId(), Optional.of("query")); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInTest() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='default'>" + - " <test global-service-id='query' />" + - " </instance>" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInStaging() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <instance id='default'>" + - " <staging global-service-id='query' />" + - " </instance>" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test - public void productionSpecWithGlobalServiceIdBeforeStaging() { - StringReader r = new StringReader( - "<deployment>" + - " <instance id='default'>" + - " <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/>" + - " </instance>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("qrs", spec.requireInstance("default").globalServiceId().get()); } @Test @@ -540,11 +470,11 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <upgrade policy='canary'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <delay hours='47'/>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -581,10 +511,10 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <parallel>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-central-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -605,14 +535,14 @@ public class DeploymentSpecTest { " <staging/>" + " <instance id='instance0'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='instance1'>" + " <test/>" + " <staging/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -645,25 +575,25 @@ public class DeploymentSpecTest { " <instance id='instance' athenz-service='in-service'>" + " <prod>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <steps>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " <delay hours='2' />" + - " <region active='true'>eu-west-1</region>" + + " <region>eu-west-1</region>" + " <delay hours='2' />" + " </steps>" + " <steps>" + " <delay hours='3' />" + - " <region active='true'>aws-us-east-1a</region>" + + " <region>aws-us-east-1a</region>" + " <parallel>" + - " <region active='true' athenz-service='no-service'>ap-northeast-1</region>" + - " <region active='true'>ap-southeast-2</region>" + + " <region athenz-service='no-service'>ap-northeast-1</region>" + + " <region>ap-southeast-2</region>" + " <test>aws-us-east-1a</test>" + " </parallel>" + " </steps>" + " <delay hours='3' minutes='30' />" + " </parallel>" + - " <region active='true'>us-north-7</region>" + + " <region>us-north-7</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -717,12 +647,12 @@ public class DeploymentSpecTest { " <parallel>" + " <instance id='instance0'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='instance1'>" + " <prod>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + " </instance>" + " </parallel>" + @@ -745,13 +675,13 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='instance0'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <delay hours='12'/>" + " <instance id='instance1'>" + " <prod>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -771,11 +701,11 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-west-1</region>" + + " <region>us-central-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -862,7 +792,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <block-change days='sat' hours='10' time-zone='CET'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " <block-change days='mon,tue' hours='15-16'/>" + " </instance>" + @@ -879,7 +809,7 @@ public class DeploymentSpecTest { " <block-change days='sat' hours='10' time-zone='CET'/>" + " <test/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -896,7 +826,7 @@ public class DeploymentSpecTest { " <block-change days='sat' hours='10' time-zone='CET'/>" + " <block-change days='mon-sun' hours='0-23' time-zone='CET' from-date='2022-01-01' to-date='2022-01-15'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -957,7 +887,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain' athenz-service='service'>" + " <instance id='instance1'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -975,10 +905,10 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain' athenz-service='service'>" + " <instance id='instance1'>" + " <prod athenz-service='prod-service'>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-west-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -1005,16 +935,16 @@ public class DeploymentSpecTest { <instance id='instance1'> <prod> <parallel> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </parallel> </prod> </instance> <instance id='instance2'> <prod> <parallel> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </parallel> </prod> </instance> @@ -1037,7 +967,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain'>" + " <instance id='default' athenz-service='service'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1056,7 +986,7 @@ public class DeploymentSpecTest { " <test />" + " <staging athenz-service='staging-service' />" + " <prod athenz-service='prod-service'>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1079,7 +1009,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain'>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1093,7 +1023,7 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod athenz-service='service'>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1422,14 +1352,14 @@ public class DeploymentSpecTest { <deployment> <instance id="beta"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> </instance> <instance id="main"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> </instance> <endpoints> @@ -1522,14 +1452,14 @@ public class DeploymentSpecTest { <deployment> <instance id="beta"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> </instance> <instance id="main"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> <endpoints> <endpoint id="glob" container-id="music"/> @@ -1738,16 +1668,6 @@ public class DeploymentSpecTest { DeploymentSpec.fromXml(""" <deployment> <instance id='default'> - <prod global-service-id='service'> - <region>name</region> - </prod> - </instance> - </deployment>""").deployableHashCode()); - - assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), - DeploymentSpec.fromXml(""" - <deployment> - <instance id='default'> <prod> <region>name</region> </prod> 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 e5578723612..3a8d7ae1703 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 @@ -8,8 +8,8 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.ZoneEndpoint; -import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import org.junit.Test; import java.io.StringReader; @@ -27,7 +27,6 @@ import static com.yahoo.config.application.api.Notifications.Role.author; import static com.yahoo.config.application.api.Notifications.When.failing; import static com.yahoo.config.application.api.Notifications.When.failingCommit; import static com.yahoo.config.provision.CloudName.AWS; -import static com.yahoo.config.provision.Environment.dev; import static com.yahoo.config.provision.Environment.prod; import static com.yahoo.config.provision.Environment.test; import static com.yahoo.config.provision.zone.ZoneId.defaultId; @@ -59,7 +58,6 @@ public class DeploymentSpecWithoutInstanceTest { assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @Test @@ -91,7 +89,6 @@ public class DeploymentSpecWithoutInstanceTest { assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @Test @@ -99,8 +96,8 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + + " <region>us-east1</region>" + + " <region>us-west1</region>" + " </prod>" + "</deployment>" ); @@ -110,18 +107,15 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals(2, spec.requireInstance("default").steps().size()); assertTrue(spec.requireInstance("default").steps().get(0).concerns(prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(0)).active()); assertTrue(spec.requireInstance("default").steps().get(1).concerns(prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(1)).active()); assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-east1")))); assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-west1")))); assertFalse(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); - + assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); assertEquals(DeploymentSpec.UpgradeRollout.separate, spec.requireInstance("default").upgradeRollout()); } @@ -133,9 +127,9 @@ public class DeploymentSpecWithoutInstanceTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + "</deployment>" ); @@ -149,13 +143,11 @@ public class DeploymentSpecWithoutInstanceTest { assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.staging)); assertTrue(spec.requireInstance("default").steps().get(2).concerns(prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(2)).active()); assertTrue(spec.requireInstance("default").steps().get(3) instanceof DeploymentSpec.Delay); assertEquals(3 * 60 * 60 + 30 * 60, spec.requireInstance("default").steps().get(3).delay().getSeconds()); assertTrue(spec.requireInstance("default").steps().get(4).concerns(prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(4)).active()); assertTrue(spec.requireInstance("default").concerns(test, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region @@ -163,7 +155,6 @@ public class DeploymentSpecWithoutInstanceTest { assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-east1")))); assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-west1")))); assertFalse(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("no-such-region")))); - assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @Test @@ -173,8 +164,8 @@ public class DeploymentSpecWithoutInstanceTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + + " <region>us-east-1</region>" + + " <region>us-west-1</region>" + " <delay hours='1' />" + " <test>us-west-1</test>" + " <test>us-east-1</test>" + @@ -199,7 +190,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <prod>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " <test>us-east1</test>" + " </prod>" + @@ -214,7 +205,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment version='1.0'>" + " <prod>" + " <test>us-east1</test>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " </prod>" + "</deployment>" ); @@ -227,7 +218,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment version='1.0'>" + " <prod>" + " <parallel>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " </parallel>" + " </prod>" + @@ -237,59 +228,6 @@ public class DeploymentSpecWithoutInstanceTest { } @Test - public void productionSpecWithGlobalServiceId() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <prod global-service-id='query'>" + - " <region active='true'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + - " </prod>" + - "</deployment>" - ); - - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.requireInstance("default").globalServiceId(), Optional.of("query")); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInTest() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <test global-service-id='query' />" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @Test(expected=IllegalArgumentException.class) - public void globalServiceIdInStaging() { - StringReader r = new StringReader( - "<deployment version='1.0'>" + - " <staging global-service-id='query' />" + - "</deployment>" - ); - DeploymentSpec.fromXml(r); - } - - @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.requireInstance("default").globalServiceId().get()); - } - - @Test public void productionSpecWithUpgradeRollout() { StringReader r = new StringReader( "<deployment>" + @@ -318,11 +256,11 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment>" + " <upgrade policy='canary'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <delay hours='47'/>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + "</deployment>" ); @@ -361,10 +299,10 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod> \n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " <parallel>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + + " <region>us-central-1</region>\n" + + " <region>us-east-3</region>\n" + " </parallel>\n" + " </prod>\n" + "</deployment>" @@ -383,25 +321,25 @@ public class DeploymentSpecWithoutInstanceTest { " <staging />" + " <prod>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <steps>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " <delay hours='2' />" + - " <region active='true'>eu-west-1</region>" + + " <region>eu-west-1</region>" + " <delay hours='2' />" + " </steps>" + " <steps>" + " <delay hours='3' />" + - " <region active='true'>aws-us-east-1a</region>" + + " <region>aws-us-east-1a</region>" + " <parallel>" + - " <region active='true' athenz-service='no-service'>ap-northeast-1</region>" + - " <region active='true'>ap-southeast-2</region>" + + " <region athenz-service='no-service'>ap-northeast-1</region>" + + " <region>ap-southeast-2</region>" + " <test>aws-us-east-1a</test>" + " </parallel>" + " </steps>" + " <delay hours='3' minutes='30' />" + " </parallel>" + - " <region active='true'>us-north-7</region>" + + " <region>us-north-7</region>" + " </prod>" + "</deployment>" ); @@ -452,11 +390,11 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " <parallel>\n" + - " <region active='true'>us-west-1</region>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + + " <region>us-west-1</region>\n" + + " <region>us-central-1</region>\n" + + " <region>us-east-3</region>\n" + " </parallel>\n" + " </prod>\n" + "</deployment>" @@ -475,7 +413,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment>\n" + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + " <block-change days='mon,tue' hours='15-16'/>\n" + "</deployment>" @@ -490,7 +428,7 @@ public class DeploymentSpecWithoutInstanceTest { " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <test/>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -504,7 +442,7 @@ public class DeploymentSpecWithoutInstanceTest { " <block-change revision='false' days='mon,tue' hours='15-16'/>\n" + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -533,7 +471,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -547,10 +485,10 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>" + " <prod athenz-service='prod-service'>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-west-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + "</deployment>" @@ -573,7 +511,7 @@ public class DeploymentSpecWithoutInstanceTest { " <test />\n" + " <staging athenz-service='staging-service' />\n" + " <prod athenz-service='prod-service'>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -590,7 +528,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain'>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -602,7 +540,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod athenz-service='service'>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); |