diff options
author | Martin Polden <mpolden@mpolden.no> | 2023-07-27 12:21:14 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-27 12:21:14 +0200 |
commit | 2921d5bc1358b094db2e131970c969dcad481502 (patch) | |
tree | 2ffff84602408dd610cc27b56bcca024ad33ccbf | |
parent | 2b43a46817cc779dccedd82ea8460802367a448a (diff) |
Revert "Remove global-service-id"
37 files changed, 754 insertions, 241 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 4ca96453ad1..b6f934c8824 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,6 +1,7 @@ // 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; @@ -57,6 +58,7 @@ 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; @@ -74,6 +76,7 @@ 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, @@ -97,6 +100,7 @@ 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); @@ -107,7 +111,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { this.zoneEndpoints = Collections.unmodifiableMap(zoneEndpointsCopy); this.bcp = Objects.requireNonNull(bcp); validateZones(new HashSet<>(), new HashSet<>(), this); - validateEndpoints(this.endpoints); + validateEndpoints(globalServiceId, this.endpoints); validateChangeBlockers(changeBlockers, now); validateBcp(bcp); hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); @@ -151,7 +155,11 @@ 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(List<Endpoint> endpoints) { + 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'."); + } + var regions = prodRegions(); for (var endpoint : endpoints){ for (var endpointRegion : endpoint.regions()) { @@ -235,6 +243,9 @@ 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()) @@ -312,7 +323,8 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; DeploymentInstanceSpec other = (DeploymentInstanceSpec) o; - return upgradePolicy == other.upgradePolicy && + return globalServiceId.equals(other.globalServiceId) && + upgradePolicy == other.upgradePolicy && revisionTarget == other.revisionTarget && upgradeRollout == other.upgradeRollout && changeBlockers.equals(other.changeBlockers) && @@ -327,7 +339,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { @Override public int hashCode() { - return Objects.hash(upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints, zoneEndpoints, bcp, tags); + return Objects.hash(globalServiceId, upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints, zoneEndpoints, bcp, tags); } int deployableHashCode() { @@ -337,6 +349,7 @@ 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 358744d40f7..797be652ebc 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 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; @@ -433,16 +434,17 @@ 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(), Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); + this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); } - public DeclaredZone(Environment environment, Optional<RegionName> region, + public DeclaredZone(Environment environment, Optional<RegionName> region, boolean active, Optional<AthenzService> athenzService, Optional<String> testerFlavor, Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL) { if (environment != Environment.prod && region.isPresent()) @@ -452,6 +454,7 @@ 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); @@ -463,6 +466,9 @@ 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; } + public Optional<String> testerFlavor() { return testerFlavor; } Optional<AthenzService> athenzService() { return athenzService; } 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 4bfecabaf69..38562eefb03 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,6 +93,7 @@ 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"; @@ -233,6 +234,7 @@ public class DeploymentSpecXmlReader { upgradeRollout, minRisk, maxRisk, maxIdleHours, changeBlockers, + Optional.ofNullable(prodAttributes.get(globalServiceIdAttribute)), athenzService, cloudAccounts, hostTTL, @@ -266,6 +268,12 @@ 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) @@ -273,7 +281,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(), athenzService, testerFlavor, readCloudAccounts(stepTag), readHostTTL(stepTag))); + return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, 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()) @@ -682,7 +690,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())), - athenzService, testerFlavor, + readActive(regionTag), athenzService, testerFlavor, readCloudAccounts(regionTag), readHostTTL(regionTag)); } @@ -706,6 +714,13 @@ 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) { @@ -784,6 +799,16 @@ 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 134d45e35ab..d4312a0e54e 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.AccessType; import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import com.yahoo.test.ManualClock; import org.junit.Test; @@ -46,6 +46,7 @@ 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; @@ -73,6 +74,7 @@ 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 @@ -108,6 +110,7 @@ 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 @@ -116,8 +119,8 @@ public class DeploymentSpecTest { <deployment version='1.0'> <instance id='default'> <prod> - <region>us-east1</region> - <region>us-west1</region> + <region active='false'>us-east1</region> + <region active='true'>us-west1</region> </prod> </instance> </deployment> @@ -128,14 +131,17 @@ 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()); @@ -152,14 +158,14 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <instance id='a' tags='tag1 tag2'>" + " <prod>" + - " <region>us-east1</region>" + - " <region>us-west1</region>" + + " <region active='false'>us-east1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + " </instance>" + " <instance id='b' tags='tag3'>" + " <prod>" + - " <region>us-east1</region>" + - " <region>us-west1</region>" + + " <region active='false'>us-east1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -177,9 +183,9 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region>us-east1</region>" + + " <region active='false'>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region>us-west1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -197,8 +203,8 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region>us-east-1</region>" + - " <region>us-west-1</region>" + + " <region active='false'>us-east-1</region>" + + " <region active='true'>us-west-1</region>" + " <delay hours='1' />" + " <test>us-west-1</test>" + " <test>us-east-1</test>" + @@ -225,7 +231,7 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <instance id='default'>" + " <prod>" + - " <region>us-east1</region>" + + " <region active='true'>us-east1</region>" + " <test>us-east1</test>" + " <test>us-east1</test>" + " </prod>" + @@ -242,7 +248,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <prod>" + " <test>us-east1</test>" + - " <region>us-east1</region>" + + " <region active='true'>us-east1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -257,7 +263,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <prod>" + " <parallel>" + - " <region>us-east1</region>" + + " <region active='true'>us-east1</region>" + " <test>us-east1</test>" + " </parallel>" + " </prod>" + @@ -275,14 +281,14 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region>us-east1</region>" + + " <region active='false'>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region>us-west1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + " </instance>" + " <instance id='instance2'>" + " <prod>" + - " <region>us-central1</region>" + + " <region active='true'>us-central1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -307,9 +313,9 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region>us-east1</region>" + + " <region active='false'>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region>us-west1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -330,11 +336,13 @@ 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 @@ -342,6 +350,68 @@ 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 @@ -470,11 +540,11 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <upgrade policy='canary'/>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " <delay hours='47'/>" + - " <region>us-central-1</region>" + + " <region active='true'>us-central-1</region>" + " <delay minutes='59' seconds='61'/>" + - " <region>us-east-3</region>" + + " <region active='true'>us-east-3</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -511,10 +581,10 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " <parallel>" + - " <region>us-central-1</region>" + - " <region>us-east-3</region>" + + " <region active='true'>us-central-1</region>" + + " <region active='true'>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -535,14 +605,14 @@ public class DeploymentSpecTest { " <staging/>" + " <instance id='instance0'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='instance1'>" + " <test/>" + " <staging/>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -575,25 +645,25 @@ public class DeploymentSpecTest { " <instance id='instance' athenz-service='in-service'>" + " <prod>" + " <parallel>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " <steps>" + - " <region>us-east-3</region>" + + " <region active='true'>us-east-3</region>" + " <delay hours='2' />" + - " <region>eu-west-1</region>" + + " <region active='true'>eu-west-1</region>" + " <delay hours='2' />" + " </steps>" + " <steps>" + " <delay hours='3' />" + - " <region>aws-us-east-1a</region>" + + " <region active='true'>aws-us-east-1a</region>" + " <parallel>" + - " <region athenz-service='no-service'>ap-northeast-1</region>" + - " <region>ap-southeast-2</region>" + + " <region active='true' athenz-service='no-service'>ap-northeast-1</region>" + + " <region active='true'>ap-southeast-2</region>" + " <test>aws-us-east-1a</test>" + " </parallel>" + " </steps>" + " <delay hours='3' minutes='30' />" + " </parallel>" + - " <region>us-north-7</region>" + + " <region active='true'>us-north-7</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -647,12 +717,12 @@ public class DeploymentSpecTest { " <parallel>" + " <instance id='instance0'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='instance1'>" + " <prod>" + - " <region>us-east-3</region>" + + " <region active='true'>us-east-3</region>" + " </prod>" + " </instance>" + " </parallel>" + @@ -675,13 +745,13 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='instance0'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + " <delay hours='12'/>" + " <instance id='instance1'>" + " <prod>" + - " <region>us-east-3</region>" + + " <region active='true'>us-east-3</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -701,11 +771,11 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " <parallel>" + - " <region>us-west-1</region>" + - " <region>us-central-1</region>" + - " <region>us-east-3</region>" + + " <region active='true'>us-west-1</region>" + + " <region active='true'>us-central-1</region>" + + " <region active='true'>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -792,7 +862,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <block-change days='sat' hours='10' time-zone='CET'/>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " <block-change days='mon,tue' hours='15-16'/>" + " </instance>" + @@ -809,7 +879,7 @@ public class DeploymentSpecTest { " <block-change days='sat' hours='10' time-zone='CET'/>" + " <test/>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -826,7 +896,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>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -887,7 +957,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain' athenz-service='service'>" + " <instance id='instance1'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -905,10 +975,10 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain' athenz-service='service'>" + " <instance id='instance1'>" + " <prod athenz-service='prod-service'>" + - " <region>us-central-1</region>" + + " <region active='true'>us-central-1</region>" + " <parallel>" + - " <region>us-west-1</region>" + - " <region>us-east-3</region>" + + " <region active='true'>us-west-1</region>" + + " <region active='true'>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -935,16 +1005,16 @@ public class DeploymentSpecTest { <instance id='instance1'> <prod> <parallel> - <region>us-west-1</region> - <region>us-east-3</region> + <region active='true'>us-west-1</region> + <region active='true'>us-east-3</region> </parallel> </prod> </instance> <instance id='instance2'> <prod> <parallel> - <region>us-west-1</region> - <region>us-east-3</region> + <region active='true'>us-west-1</region> + <region active='true'>us-east-3</region> </parallel> </prod> </instance> @@ -967,7 +1037,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain'>" + " <instance id='default' athenz-service='service'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -986,7 +1056,7 @@ public class DeploymentSpecTest { " <test />" + " <staging athenz-service='staging-service' />" + " <prod athenz-service='prod-service'>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1009,7 +1079,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain'>" + " <instance id='default'>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1023,7 +1093,7 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod athenz-service='service'>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1352,14 +1422,14 @@ public class DeploymentSpecTest { <deployment> <instance id="beta"> <prod> - <region>us-west-1</region> - <region>us-east-3</region> + <region active='true'>us-west-1</region> + <region active='true'>us-east-3</region> </prod> </instance> <instance id="main"> <prod> - <region>us-west-1</region> - <region>us-east-3</region> + <region active='true'>us-west-1</region> + <region active='true'>us-east-3</region> </prod> </instance> <endpoints> @@ -1452,14 +1522,14 @@ public class DeploymentSpecTest { <deployment> <instance id="beta"> <prod> - <region>us-west-1</region> - <region>us-east-3</region> + <region active='true'>us-west-1</region> + <region active='true'>us-east-3</region> </prod> </instance> <instance id="main"> <prod> - <region>us-west-1</region> - <region>us-east-3</region> + <region active='true'>us-west-1</region> + <region active='true'>us-east-3</region> </prod> <endpoints> <endpoint id="glob" container-id="music"/> @@ -1668,6 +1738,16 @@ 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 3a8d7ae1703..e5578723612 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.AccessType; import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import org.junit.Test; import java.io.StringReader; @@ -27,6 +27,7 @@ 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; @@ -58,6 +59,7 @@ 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 @@ -89,6 +91,7 @@ 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 @@ -96,8 +99,8 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <prod>" + - " <region>us-east1</region>" + - " <region>us-west1</region>" + + " <region active='false'>us-east1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + "</deployment>" ); @@ -107,15 +110,18 @@ 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()); } @@ -127,9 +133,9 @@ public class DeploymentSpecWithoutInstanceTest { " <test/>" + " <staging/>" + " <prod>" + - " <region>us-east1</region>" + + " <region active='false'>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region>us-west1</region>" + + " <region active='true'>us-west1</region>" + " </prod>" + "</deployment>" ); @@ -143,11 +149,13 @@ 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 @@ -155,6 +163,7 @@ 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 @@ -164,8 +173,8 @@ public class DeploymentSpecWithoutInstanceTest { " <test/>" + " <staging/>" + " <prod>" + - " <region>us-east-1</region>" + - " <region>us-west-1</region>" + + " <region active='false'>us-east-1</region>" + + " <region active='true'>us-west-1</region>" + " <delay hours='1' />" + " <test>us-west-1</test>" + " <test>us-east-1</test>" + @@ -190,7 +199,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <prod>" + - " <region>us-east1</region>" + + " <region active='true'>us-east1</region>" + " <test>us-east1</test>" + " <test>us-east1</test>" + " </prod>" + @@ -205,7 +214,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment version='1.0'>" + " <prod>" + " <test>us-east1</test>" + - " <region>us-east1</region>" + + " <region active='true'>us-east1</region>" + " </prod>" + "</deployment>" ); @@ -218,7 +227,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment version='1.0'>" + " <prod>" + " <parallel>" + - " <region>us-east1</region>" + + " <region active='true'>us-east1</region>" + " <test>us-east1</test>" + " </parallel>" + " </prod>" + @@ -228,6 +237,59 @@ 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>" + @@ -256,11 +318,11 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment>" + " <upgrade policy='canary'/>" + " <prod>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " <delay hours='47'/>" + - " <region>us-central-1</region>" + + " <region active='true'>us-central-1</region>" + " <delay minutes='59' seconds='61'/>" + - " <region>us-east-3</region>" + + " <region active='true'>us-east-3</region>" + " </prod>" + "</deployment>" ); @@ -299,10 +361,10 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod> \n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " <parallel>\n" + - " <region>us-central-1</region>\n" + - " <region>us-east-3</region>\n" + + " <region active='true'>us-central-1</region>\n" + + " <region active='true'>us-east-3</region>\n" + " </parallel>\n" + " </prod>\n" + "</deployment>" @@ -321,25 +383,25 @@ public class DeploymentSpecWithoutInstanceTest { " <staging />" + " <prod>" + " <parallel>" + - " <region>us-west-1</region>" + + " <region active='true'>us-west-1</region>" + " <steps>" + - " <region>us-east-3</region>" + + " <region active='true'>us-east-3</region>" + " <delay hours='2' />" + - " <region>eu-west-1</region>" + + " <region active='true'>eu-west-1</region>" + " <delay hours='2' />" + " </steps>" + " <steps>" + " <delay hours='3' />" + - " <region>aws-us-east-1a</region>" + + " <region active='true'>aws-us-east-1a</region>" + " <parallel>" + - " <region athenz-service='no-service'>ap-northeast-1</region>" + - " <region>ap-southeast-2</region>" + + " <region active='true' athenz-service='no-service'>ap-northeast-1</region>" + + " <region active='true'>ap-southeast-2</region>" + " <test>aws-us-east-1a</test>" + " </parallel>" + " </steps>" + " <delay hours='3' minutes='30' />" + " </parallel>" + - " <region>us-north-7</region>" + + " <region active='true'>us-north-7</region>" + " </prod>" + "</deployment>" ); @@ -390,11 +452,11 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " <parallel>\n" + - " <region>us-west-1</region>\n" + - " <region>us-central-1</region>\n" + - " <region>us-east-3</region>\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" + " </parallel>\n" + " </prod>\n" + "</deployment>" @@ -413,7 +475,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment>\n" + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <prod>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + " <block-change days='mon,tue' hours='15-16'/>\n" + "</deployment>" @@ -428,7 +490,7 @@ public class DeploymentSpecWithoutInstanceTest { " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <test/>\n" + " <prod>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -442,7 +504,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>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -471,7 +533,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>\n" + " <prod>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -485,10 +547,10 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>" + " <prod athenz-service='prod-service'>" + - " <region>us-central-1</region>" + + " <region active='true'>us-central-1</region>" + " <parallel>" + - " <region>us-west-1</region>" + - " <region>us-east-3</region>" + + " <region active='true'>us-west-1</region>" + + " <region active='true'>us-east-3</region>" + " </parallel>" + " </prod>" + "</deployment>" @@ -511,7 +573,7 @@ public class DeploymentSpecWithoutInstanceTest { " <test />\n" + " <staging athenz-service='staging-service' />\n" + " <prod athenz-service='prod-service'>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -528,7 +590,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain'>\n" + " <prod>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -540,7 +602,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod athenz-service='service'>\n" + - " <region>us-west-1</region>\n" + + " <region active='true'>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java index d919a35c7ef..0193eacba3a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java @@ -29,6 +29,9 @@ public class DeploymentSpecValidator extends Validator { DeploymentSpec deploymentSpec = DeploymentSpec.fromXml(deploymentReader); List<ContainerModel> containers = model.getRoot().configModelRepo().getModels(ContainerModel.class); for (DeploymentInstanceSpec instance : deploymentSpec.instances()) { + instance.globalServiceId().ifPresent(globalServiceId -> { + requireClusterId(containers, instance.name(), "Attribute 'globalServiceId'", globalServiceId); + }); instance.endpoints().forEach(endpoint -> { requireClusterId(containers, instance.name(), "Endpoint '" + endpoint.endpointId() + "'", endpoint.containerId()); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 31f8eba48bf..1036a615bb5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -10,6 +10,7 @@ import com.yahoo.config.application.Xml; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.api.ApplicationClusterEndpoint; @@ -359,16 +360,24 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { context.getDeployState().getProperties().athenzDnsSuffix(), context.getDeployState().zone(), deploymentSpec); - addRotationProperties(cluster, context.getDeployState().getEndpoints()); + addRotationProperties(cluster, context.getDeployState().zone(), context.getDeployState().getEndpoints(), deploymentSpec); } - private void addRotationProperties(ApplicationContainerCluster cluster, Set<ContainerEndpoint> endpoints) { + private void addRotationProperties(ApplicationContainerCluster cluster, Zone zone, Set<ContainerEndpoint> endpoints, DeploymentSpec spec) { cluster.getContainers().forEach(container -> { setRotations(container, endpoints, cluster.getName()); - container.setProp("activeRotation", "true"); // TODO(mpolden): This is unused and should be removed + container.setProp("activeRotation", Boolean.toString(zoneHasActiveRotation(zone, spec))); }); } + private boolean zoneHasActiveRotation(Zone zone, DeploymentSpec spec) { + Optional<DeploymentInstanceSpec> instance = spec.instance(app.getApplicationId().instance()); + if (instance.isEmpty()) return false; + return instance.get().zones().stream() + .anyMatch(declaredZone -> declaredZone.concerns(zone.environment(), Optional.of(zone.region())) && + declaredZone.active()); + } + private void setRotations(Container container, Set<ContainerEndpoint> endpoints, String containerClusterName) { var rotationsProperty = endpoints.stream() .filter(endpoint -> endpoint.clusterId().equals(containerClusterName)) @@ -585,7 +594,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private void addDefaultConnectorHostedFilterBinding(ApplicationContainerCluster cluster) { cluster.getHttp().getAccessControl() - .ifPresent(accessControl -> accessControl.configureDefaultHostedConnector(cluster.getHttp())); + .ifPresent(accessControl -> accessControl.configureDefaultHostedConnector(cluster.getHttp())); ; } private void addCloudMtlsConnector(DeployState state, ApplicationContainerCluster cluster) { diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 87783c1ee20..d47ce81eaac 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -122,6 +122,7 @@ Perf = element perf { } Prod = element prod { + attribute global-service-id { text }? & attribute athenz-service { xsd:string }? & attribute tester-flavor { xsd:string }? & attribute cloud-account { xsd:string }? & diff --git a/config-model/src/test/cfg/application/app1/deployment.xml b/config-model/src/test/cfg/application/app1/deployment.xml index 25954df1c7d..a8771123ecc 100644 --- a/config-model/src/test/cfg/application/app1/deployment.xml +++ b/config-model/src/test/cfg/application/app1/deployment.xml @@ -2,8 +2,8 @@ <deployment version="1.0"> <test/> <staging/> - <prod> - <region>us-east-3</region> - <region>us-west-1</region> + <prod global-service-id="query"> + <region active="true">us-east-3</region> + <region active="false">us-west-1</region> </prod> </deployment> diff --git a/config-model/src/test/cfg/application/app_complicated_deployment_spec/deployment.xml b/config-model/src/test/cfg/application/app_complicated_deployment_spec/deployment.xml index e30c02cdd84..3d3bfd35845 100644 --- a/config-model/src/test/cfg/application/app_complicated_deployment_spec/deployment.xml +++ b/config-model/src/test/cfg/application/app_complicated_deployment_spec/deployment.xml @@ -5,24 +5,24 @@ <instance id='instance' athenz-service='in-service'> <prod> <parallel> - <region>us-west-1</region> + <region active='true'>us-west-1</region> <steps> - <region>us-east-3</region> + <region active='true'>us-east-3</region> <delay hours='2' /> - <region>eu-west-1</region> + <region active='true'>eu-west-1</region> <delay hours='2' /> </steps> <steps> <delay hours='3' /> - <region>aws-us-east-1a</region> + <region active='true'>aws-us-east-1a</region> <parallel> - <region athenz-service='no-service'>ap-northeast-1</region> - <region>ap-southeast-2</region> + <region active='true' athenz-service='no-service'>ap-northeast-1</region> + <region active='true'>ap-southeast-2</region> </parallel> </steps> <delay hours='3' minutes='30' /> </parallel> - <region>us-north-7</region> + <region active='true'>us-north-7</region> </prod> </instance> <instance id='other'> @@ -30,7 +30,7 @@ <block-change days='sat' hours='10' time-zone='CET' /> <test /> <prod> - <region>eu-central-2</region> + <region active='true'>eu-central-2</region> </prod> <notifications when='failing'> <email role='author' /> diff --git a/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml b/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml index 43f7d74ca69..738a3397aad 100644 --- a/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml +++ b/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml @@ -2,7 +2,7 @@ <deployment version="1.0"> <test/> <staging/> - <prod> + <prod global-service-id="query"> <region>us-east-3</region> <region invalid="invalid">us-west-1</region> </prod> diff --git a/config-model/src/test/cfg/application/invalid_parallel_deployment_xml/deployment.xml b/config-model/src/test/cfg/application/invalid_parallel_deployment_xml/deployment.xml index 24e1ec69507..4eaf7926078 100644 --- a/config-model/src/test/cfg/application/invalid_parallel_deployment_xml/deployment.xml +++ b/config-model/src/test/cfg/application/invalid_parallel_deployment_xml/deployment.xml @@ -2,7 +2,7 @@ <deployment version="1.0"> <test/> <staging/> - <prod> + <prod global-service-id="query"> <parallel> <instance id="hello" /> </parallel> diff --git a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java index e975421002e..c00b2846021 100644 --- a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model; +import com.google.common.io.Files; import com.yahoo.config.application.api.ApplicationMetaData; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.UnparsedConfigDefinition; @@ -34,12 +35,7 @@ import java.util.Map; import java.util.Set; import java.util.jar.JarEntry; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; public class ApplicationDeployTest { @@ -198,7 +194,7 @@ public class ApplicationDeployTest { void testThatAppWithInvalidParallelDeploymentFails() throws IOException { String expectedMessage = """ 4: <staging/> - 5: <prod> + 5: <prod global-service-id="query"> 6: <parallel> 7: <instance id="hello" /> 8: </parallel> diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java index 9ce853ad0f2..dba8205c0e7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java @@ -19,12 +19,25 @@ import static org.junit.jupiter.api.Assertions.fail; public class DeploymentSpecValidatorTest { @Test + void testDeploymentWithNonExistentGlobalId() { + var deploymentXml = "<?xml version='1.0' encoding='UTF-8'?>" + + "<deployment version='1.0'>" + + " <test />" + + " <prod global-service-id='non-existing'>" + + " <region active='true'>us-east</region>" + + " </prod>" + + "</deployment>"; + assertValidationError("Attribute 'globalServiceId' in instance default: 'non-existing' specified in " + + "deployment.xml does not match any container cluster ID", deploymentXml); + } + + @Test void testEndpointNonExistentContainerId() { var deploymentXml = "<?xml version='1.0' encoding='UTF-8'?>" + "<deployment version='1.0'>" + " <test />" + " <prod>" + - " <region>us-east</region>" + + " <region active='true'>us-east</region>" + " </prod>" + " <endpoints>" + " <endpoint container-id='non-existing'/>" + diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 94d98f526a0..f0c39ecc920 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -790,6 +790,42 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } } + @Test + void logs_deployment_spec_deprecations() throws Exception { + String containerService = joinLines("<container id='foo' version='1.0'>", + " <nodes>", + " <node hostalias='host1' />", + " </nodes>", + "</container>"); + String deploymentXml = joinLines("<deployment version='1.0'>", + " <prod global-service-id='foo'>", + " <region active='true'>us-east-1</region>", + " </prod>", + "</deployment>"); + + ApplicationPackage applicationPackage = new MockApplicationPackage.Builder() + .withServices(containerService) + .withDeploymentSpec(deploymentXml) + .build(); + + TestLogger logger = new TestLogger(); + DeployState deployState = new DeployState.Builder() + .applicationPackage(applicationPackage) + .zone(new Zone(Environment.prod, RegionName.from("us-east-1"))) + .properties(new TestProperties().setHostedVespa(true)) + .deployLogger(logger) + .build(); + + createModel(root, deployState, null, DomBuilderTest.parse(containerService)); + assertFalse(logger.msgs.isEmpty()); + assertEquals(Level.WARNING, logger.msgs.get(0).getFirst()); + assertEquals(Level.WARNING, logger.msgs.get(1).getFirst()); + assertEquals("Element 'prod' contains attribute 'global-service-id' deprecated since major version 7. See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax", + logger.msgs.get(0).getSecond()); + assertEquals("Element 'region' contains attribute 'active' deprecated since major version 7. See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax", + logger.msgs.get(1).getSecond()); + } + private void assertComponentConfigured(ApplicationContainer container, String id) { assertTrue(container.getComponents().getComponents().stream().anyMatch(component -> id.equals(component.getComponentId().getName()))); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/IdentityBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/IdentityBuilderTest.java index 08877b3d3a9..78269d00cf3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/IdentityBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/IdentityBuilderTest.java @@ -24,7 +24,7 @@ public class IdentityBuilderTest extends ContainerModelBuilderTestBase { String deploymentXml = "<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" + " <test/>\n" + " <prod>\n" + - " <region>default</region>\n" + + " <region active='true'>default</region>\n" + " </prod>\n" + "</deployment>\n"; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/RoutingBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/RoutingBuilderTest.java index 2bf50103b10..58b9462978f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/RoutingBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/RoutingBuilderTest.java @@ -10,12 +10,14 @@ import com.yahoo.config.model.test.MockRoot; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; -import com.yahoo.vespa.model.container.ApplicationContainer; import com.yahoo.vespa.model.container.ApplicationContainerCluster; +import com.yahoo.vespa.model.container.ApplicationContainer; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; +import org.xml.sax.SAXException; -import java.util.List; +import java.io.IOException; +import java.util.Arrays; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -25,32 +27,43 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class RoutingBuilderTest extends ContainerModelBuilderTestBase { @Test - void setsRotationActive() { + void setsRotationActiveAccordingToDeploymentSpec() throws IOException, SAXException { Element clusterElem = DomBuilderTest.parse( "<container id='default' version='1.0'><search /></container>"); - String deploymentSpec = """ - <deployment> - <prod> - <region>us-north-1</region> - <parallel> - <region>us-north-2</region> - <region>us-north-3</region> - </parallel> - <region>us-north-4</region> - </prod> - </deployment>"""; + String deploymentSpec = "<deployment>\n" + + " <prod> \n" + + " <region active='true'>us-north-1</region>\n" + + " <parallel>\n" + + " <region active='false'>us-north-2</region>\n" + + " <region active='true'>us-north-3</region>\n" + + " </parallel>\n" + + " <region active='false'>us-north-4</region>\n" + + " </prod>\n" + + "</deployment>"; ApplicationPackage applicationPackage = new MockApplicationPackage.Builder() .withDeploymentSpec(deploymentSpec) .build(); - - for (String region : List.of("us-north-1", "us-north-2", "us-north-3", "us-north-4")) { + //root = new MockRoot("root", applicationPackage); + for (String region : Arrays.asList("us-north-1", "us-north-3")) { ApplicationContainer container = getContainer(applicationPackage, region, clusterElem); + assertEquals("true", container.getServicePropertyString("activeRotation"), "Region " + region + " is active"); } + for (String region : Arrays.asList("us-north-2", "us-north-4")) { + ApplicationContainer container = getContainer(applicationPackage, region, clusterElem); + + assertEquals("false", + container.getServicePropertyString("activeRotation"), + "Region " + region + " is inactive"); + } + ApplicationContainer container = getContainer(applicationPackage, "unknown", clusterElem); + assertEquals("false", + container.getServicePropertyString("activeRotation"), + "Unknown region is inactive"); } diff --git a/config-model/src/test/schema-test-files/deployment-with-instances.xml b/config-model/src/test/schema-test-files/deployment-with-instances.xml index c9f3af49ac2..3b872f4c1cf 100644 --- a/config-model/src/test/schema-test-files/deployment-with-instances.xml +++ b/config-model/src/test/schema-test-files/deployment-with-instances.xml @@ -9,19 +9,19 @@ <instance id="one,two"> <block-change days="mon,tue" hours="14,15" time-zone="CET"/> - <prod athenz-service='other-service'> - <region>us-west-1</region> + <prod global-service-id='qrs' athenz-service='other-service'> + <region active='true'>us-west-1</region> <delay hours='3'/> - <region>us-central-1</region> + <region active='true'>us-central-1</region> <delay hours='3' minutes='7' seconds='13'/> - <region>us-east-3</region> + <region active='true'>us-east-3</region> <parallel> - <region>us-north-1</region> - <region>us-south-1</region> + <region active='true'>us-north-1</region> + <region active='true'>us-south-1</region> </parallel> <parallel> - <region>us-north-2</region> - <region>us-south-2</region> + <region active='true'>us-north-2</region> + <region active='true'>us-south-2</region> </parallel> </prod> <endpoints> @@ -55,7 +55,7 @@ <upgrade policy='conservative'/> <block-change days="mon,tue,wed" hours="14,15"/> <prod> - <region>us-central-1</region> + <region active='true'>us-central-1</region> </prod> <endpoints> <endpoint container-id="barz" /> diff --git a/config-model/src/test/schema-test-files/deployment.xml b/config-model/src/test/schema-test-files/deployment.xml index 8317bf4b80c..bc0f070d88c 100644 --- a/config-model/src/test/schema-test-files/deployment.xml +++ b/config-model/src/test/schema-test-files/deployment.xml @@ -5,19 +5,19 @@ <staging/> <block-change revision='true' version='false' days="mon,tue" hours="14,15"/> <block-change days="mon,tue" hours="14,15" time-zone="CET"/> - <prod athenz-service='other-service'> - <region>us-west-1</region> + <prod global-service-id='qrs' athenz-service='other-service'> + <region active='true'>us-west-1</region> <delay hours='3'/> - <region>us-central-1</region> + <region active='true'>us-central-1</region> <delay hours='3' minutes='7' seconds='13'/> - <region>us-east-3</region> + <region active='true'>us-east-3</region> <parallel> - <region>us-north-1</region> - <region>us-south-1</region> + <region active='true'>us-north-1</region> + <region active='true'>us-south-1</region> </parallel> <parallel> - <region>us-north-2</region> - <region>us-south-2</region> + <region active='true'>us-north-2</region> + <region active='true'>us-south-2</region> </parallel> </prod> <endpoints> diff --git a/configserver/src/test/apps/zkapp/deployment.xml b/configserver/src/test/apps/zkapp/deployment.xml index ba71f9c1e47..c8b60f3aa8b 100644 --- a/configserver/src/test/apps/zkapp/deployment.xml +++ b/configserver/src/test/apps/zkapp/deployment.xml @@ -2,7 +2,7 @@ <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> <deployment version='1.0'> <test/> - <prod> - <region>us-east</region> + <prod global-service-id='mydisc'> + <region active='true'>us-east</region> </prod> </deployment> diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java index ca2f9da3273..4f52eede4e8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/model/LbServicesProducerTest.java @@ -22,6 +22,8 @@ import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.model.VespaModel; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.xml.sax.SAXException; import java.io.IOException; @@ -34,6 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Random; import java.util.Set; +import java.util.stream.Collectors; import static com.yahoo.cloud.config.LbServicesConfig.Tenants.Applications.Endpoints.RoutingMethod.Enum.sharedLayer4; import static com.yahoo.cloud.config.LbServicesConfig.Tenants.Applications.Endpoints.Scope.Enum.application; @@ -43,10 +46,12 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; /** * @author Ulf Lilleengen */ +@RunWith(Parameterized.class) public class LbServicesProducerTest { private static final Set<ContainerEndpoint> endpoints = Set.of( @@ -56,6 +61,16 @@ public class LbServicesProducerTest { private static final List<String> zoneDnsSuffixes = List.of(".endpoint1.suffix", ".endpoint2.suffix"); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); + private final boolean useGlobalServiceId; + + @Parameterized.Parameters + public static Object[] useGlobalServiceId() { + return new Object[] { true, false }; + } + + public LbServicesProducerTest(boolean useGlobalServiceId) { + this.useGlobalServiceId = useGlobalServiceId; + } @Test public void testDeterministicGetConfig() { @@ -73,9 +88,17 @@ public class LbServicesProducerTest { @Test public void testConfigActiveRotation() { - RegionName regionName = RegionName.from("us-east-1"); - LbServicesConfig conf = createModelAndGetLbServicesConfig(regionName); - assertTrue(conf.tenants("foo").applications("foo:prod:" + regionName.value() + ":default").activeRotation()); + { + RegionName regionName = RegionName.from("us-east-1"); + LbServicesConfig conf = createModelAndGetLbServicesConfig(regionName); + assertTrue(conf.tenants("foo").applications("foo:prod:" + regionName.value() + ":default").activeRotation()); + } + + { + RegionName regionName = RegionName.from("us-east-2"); + LbServicesConfig conf = createModelAndGetLbServicesConfig(regionName); + assertFalse(conf.tenants("foo").applications("foo:prod:" + regionName.value() + ":default").activeRotation()); + } } private LbServicesConfig createModelAndGetLbServicesConfig(RegionName regionName) { @@ -93,6 +116,8 @@ public class LbServicesProducerTest { @Test public void testConfigAliasesWithEndpoints() { + assumeFalse(useGlobalServiceId); + Map<TenantName, Set<ApplicationInfo>> testModel = createTestModel(new DeployState.Builder() .endpoints(endpoints) .properties(new TestProperties().setHostedVespa(true))); @@ -125,6 +150,8 @@ public class LbServicesProducerTest { @Test public void testRoutingConfigForTesterApplication() { + assumeFalse(useGlobalServiceId); + Map<TenantName, Set<ApplicationInfo>> testModel = createTestModel(new DeployState.Builder()); // No config for tester application @@ -200,17 +227,32 @@ public class LbServicesProducerTest { " </container>" + "</services>"; - String deploymentInfo = "<?xml version='1.0' encoding='UTF-8'?>" + - "<deployment version='1.0'>" + - " <test />" + - " <prod>" + - " <region>us-east-1</region>" + - " <region>us-east-2</region>" + - " </prod>" + - " <endpoints>" + - " <endpoint container-id='mydisc' />" + - " </endpoints>" + - "</deployment>"; + String deploymentInfo; + + if (useGlobalServiceId) { + deploymentInfo ="<?xml version='1.0' encoding='UTF-8'?>" + + "<deployment version='1.0'>" + + " <test />" + + " <prod global-service-id='mydisc'>" + + " <region active='true'>us-east-1</region>" + + " <region active='false'>us-east-2</region>" + + " </prod>" + + "</deployment>"; + } else { + deploymentInfo ="<?xml version='1.0' encoding='UTF-8'?>" + + "<deployment version='1.0'>" + + " <test />" + + " <prod>" + + " <region active='true'>us-east-1</region>" + + " <region active='false'>us-east-2</region>" + + " </prod>" + + " <endpoints>" + + " <endpoint container-id='mydisc' />" + + " </endpoints>" + + "</deployment>"; + } + + return new MockApplicationPackage.Builder().withServices(services).withDeploymentSpec(deploymentInfo).build(); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java index 9508a67fc01..f3a2c42852f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.server.zookeeper; import com.yahoo.component.Version; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ClusterMembership; @@ -101,6 +102,8 @@ public class ZKApplicationPackageTest { assertEquals(dockerImage, readInfo.getHosts().iterator().next().dockerImageRepo().get().asString()); assertTrue(zkApp.getDeployment().isPresent()); assertFalse(zkApp.getDeploymentSpec().isEmpty()); + assertEquals("mydisc", DeploymentSpec.fromXml(zkApp.getDeployment().get()).requireInstance("default").globalServiceId().get()); + assertEquals("mydisc", zkApp.getDeploymentSpec().requireInstance("default").globalServiceId().get()); } private void feed(com.yahoo.vespa.curator.Curator zk, File dirToFeed) throws IOException { diff --git a/configserver/src/test/resources/deploy/hosted-app/deployment.xml b/configserver/src/test/resources/deploy/hosted-app/deployment.xml index cfd5c61069b..f1ebbf7f968 100644 --- a/configserver/src/test/resources/deploy/hosted-app/deployment.xml +++ b/configserver/src/test/resources/deploy/hosted-app/deployment.xml @@ -1,7 +1,7 @@ <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> <deployment version='1.0'> - <prod> - <region>us-north-1</region> - <region>us-north-2</region> + <prod global-service-id="qrs"> + <region active="true">us-north-1</region> + <region active="true">us-north-2</region> </prod> </deployment> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index bceef3fd96f..3d550973f22 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller; import com.google.common.hash.HashCode; import com.google.common.hash.Hashing; import com.google.common.io.BaseEncoding; +import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; @@ -157,6 +158,15 @@ public class RoutingController { DeploymentSpec deploymentSpec = application.deploymentSpec(); for (var spec : deploymentSpec.instances()) { ApplicationId instance = application.id().instance(spec.name()); + // Add endpoint declared with legacy syntax + spec.globalServiceId().ifPresent(clusterId -> { + List<DeploymentId> deployments = spec.zones().stream() + .filter(zone -> zone.concerns(Environment.prod)) + .map(zone -> new DeploymentId(instance, ZoneId.from(Environment.prod, zone.region().get()))) + .toList(); + RoutingId routingId = RoutingId.of(instance, EndpointId.defaultId()); + endpoints.addAll(computeGlobalEndpoints(routingId, ClusterSpec.Id.from(clusterId), deployments, generatedEndpoints)); + }); // Add endpoints declared with current syntax spec.endpoints().forEach(declaredEndpoint -> { RoutingId routingId = RoutingId.of(instance, EndpointId.of(declaredEndpoint.endpointId())); @@ -265,6 +275,7 @@ public class RoutingController { } // Add endpoints backed by a rotation, and register them in DNS if necessary + boolean registerLegacyNames = requiresLegacyNames(application.get().deploymentSpec(), instanceName); Instance instance = application.get().require(instanceName); Set<ContainerEndpoint> containerEndpoints = new HashSet<>(); DeploymentId deployment = new DeploymentId(instance.id(), zone); @@ -274,11 +285,16 @@ public class RoutingController { EndpointList rotationEndpoints = globalEndpoints.named(assignedRotation.endpointId(), Scope.global) .requiresRotation(); - // Skip rotations which do not apply to this zone - if (!assignedRotation.regions().contains(zone.region())) { + // Skip rotations which do not apply to this zone. Legacy names always point to all zones + if (!registerLegacyNames && !assignedRotation.regions().contains(zone.region())) { continue; } + // Omit legacy DNS names when assigning rotations using <endpoints/> syntax + if (!registerLegacyNames) { + rotationEndpoints = rotationEndpoints.not().legacy(); + } + // Register names in DNS Rotation rotation = rotationRepository.requireRotation(assignedRotation.rotationId()); for (var endpoint : rotationEndpoints) { @@ -464,6 +480,13 @@ public class RoutingController { return randomizedEndpoints.with(FetchVector.Dimension.APPLICATION_ID, instance.serializedForm()).value(); } + /** Whether legacy global DNS names should be available for given application */ + private static boolean requiresLegacyNames(DeploymentSpec deploymentSpec, InstanceName instanceName) { + return deploymentSpec.instance(instanceName) + .flatMap(DeploymentInstanceSpec::globalServiceId) + .isPresent(); + } + /** Create a common name based on a hash of given application. This must be less than 64 characters long. */ private static String commonNameHashOf(ApplicationId application, SystemName system) { @SuppressWarnings("deprecation") // for Hashing.sha1() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java index 0c05d710763..186e6838a71 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java @@ -21,6 +21,8 @@ import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.application.EndpointId; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import java.time.Instant; import java.util.ArrayList; @@ -29,6 +31,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.TreeSet; import java.util.stream.Collectors; @@ -176,10 +179,9 @@ public class ApplicationPackageValidator { if (applicationPackage.validationOverrides().allows(validationId, instant)) return; var endpoints = application.deploymentSpec().instance(instanceName) - .map(deploymentInstanceSpec1 -> deploymentInstanceSpec1.endpoints()) + .map(ApplicationPackageValidator::allEndpointsOf) .orElseGet(List::of); - DeploymentInstanceSpec deploymentInstanceSpec = applicationPackage.deploymentSpec().requireInstance(instanceName); - var newEndpoints = new ArrayList<>(deploymentInstanceSpec.endpoints()); + var newEndpoints = allEndpointsOf(applicationPackage.deploymentSpec().requireInstance(instanceName)); if (newEndpoints.containsAll(endpoints)) return; // Adding new endpoints is fine if (containsAllDestinationsOf(endpoints, newEndpoints)) return; // Adding destinations is fine @@ -253,6 +255,26 @@ public class ApplicationPackageValidator { return containsAllRegions && hasSameCluster; } + /** Returns all configued endpoints of given deployment instance spec */ + private static List<Endpoint> allEndpointsOf(DeploymentInstanceSpec deploymentInstanceSpec) { + var endpoints = new ArrayList<>(deploymentInstanceSpec.endpoints()); + legacyEndpoint(deploymentInstanceSpec).ifPresent(endpoints::add); + return endpoints; + } + + /** Returns global service ID as an endpoint, if any global service ID is set */ + private static Optional<Endpoint> legacyEndpoint(DeploymentInstanceSpec instance) { + return instance.globalServiceId().map(globalServiceId -> { + var targets = instance.zones().stream() + .filter(zone -> zone.environment().isProduction()) + .flatMap(zone -> zone.region().stream()) + .distinct() + .map(region -> new Endpoint.Target(region, instance.name(), 1)) + .toList(); + return new Endpoint(EndpointId.defaultId().id(), globalServiceId, Endpoint.Level.instance, targets); + }); + } + /** Returns a list of the non-compactable IDs of given instance and endpoint */ private static List<String> nonCompactableIds(InstanceName instance, Endpoint endpoint) { List<String> ids = new ArrayList<>(2); @@ -265,6 +287,16 @@ public class ApplicationPackageValidator { return ids; } - private record InstanceEndpoint(InstanceName instance, String endpointId) {} + private static class InstanceEndpoint { + + private final InstanceName instance; + private final String endpointId; + + public InstanceEndpoint(InstanceName instance, String endpointId) { + this.instance = instance; + this.endpointId = endpointId; + } + + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index c8c3d057ee3..a309afcd039 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -112,20 +112,21 @@ public class RoutingPolicies { List<LoadBalancer> loadBalancers = controller.serviceRegistry().configServer() .getLoadBalancers(instance, deployment.zoneId()); LoadBalancerAllocation allocation = new LoadBalancerAllocation(deployment, deploymentSpec, loadBalancers); + Set<ZoneId> inactiveZones = inactiveZones(instance, deploymentSpec); Optional<TenantAndApplicationId> owner = ownerOf(allocation); try (var lock = db.lockRoutingPolicies()) { RoutingPolicyList applicationPolicies = read(TenantAndApplicationId.from(instance)); RoutingPolicyList deploymentPolicies = applicationPolicies.deployment(allocation.deployment); - removeGlobalDnsUnreferencedBy(allocation, deploymentPolicies, lock); + removeGlobalDnsUnreferencedBy(allocation, deploymentPolicies, inactiveZones, lock); removeApplicationDnsUnreferencedBy(allocation, deploymentPolicies, lock); RoutingPolicyList instancePolicies = storePoliciesOf(allocation, applicationPolicies, generatedEndpoints, lock); instancePolicies = removePoliciesUnreferencedBy(allocation, instancePolicies, lock); RoutingPolicyList updatedApplicationPolicies = applicationPolicies.replace(instance, instancePolicies); - updateGlobalDnsOf(instancePolicies, Optional.of(deployment), owner, lock); - updateApplicationDnsOf(updatedApplicationPolicies, deployment, owner, lock); + updateGlobalDnsOf(instancePolicies, Optional.of(deployment), inactiveZones, owner, lock); + updateApplicationDnsOf(updatedApplicationPolicies, inactiveZones, deployment, owner, lock); } } @@ -136,7 +137,7 @@ public class RoutingPolicies { controller.clock().instant()))); Map<ApplicationId, RoutingPolicyList> allPolicies = readAll().groupingBy(policy -> policy.id().owner()); allPolicies.forEach((instance, policies) -> { - updateGlobalDnsOf(policies, Optional.empty(), Optional.of(TenantAndApplicationId.from(instance)), lock); + updateGlobalDnsOf(policies, Optional.empty(), Set.of(), Optional.of(TenantAndApplicationId.from(instance)), lock); }); } } @@ -156,16 +157,17 @@ public class RoutingPolicies { Map<ApplicationId, RoutingPolicyList> policiesByInstance = effectivePolicies.groupingBy(policy -> policy.id().owner()); policiesByInstance.forEach((ignored, instancePolicies) -> updateGlobalDnsOf(instancePolicies, Optional.of(deployment), + Set.of(), ownerOf(deployment), lock)); - updateApplicationDnsOf(effectivePolicies, deployment, ownerOf(deployment), lock); + updateApplicationDnsOf(effectivePolicies, Set.of(), deployment, ownerOf(deployment), lock); policiesByInstance.forEach((owner, instancePolicies) -> db.writeRoutingPolicies(owner, instancePolicies.asList())); } } /** Update global DNS records for given policies */ private void updateGlobalDnsOf(RoutingPolicyList instancePolicies, Optional<DeploymentId> deployment, - Optional<TenantAndApplicationId> owner, + Set<ZoneId> inactiveZones, Optional<TenantAndApplicationId> owner, @SuppressWarnings("unused") Mutex lock) { Map<RoutingId, List<RoutingPolicy>> routingTable = instancePolicies.asInstanceRoutingTable(); for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { @@ -173,17 +175,17 @@ public class RoutingPolicies { controller.routing().readDeclaredEndpointsOf(routingId.instance()) .named(routingId.endpointId(), Endpoint.Scope.global) .not().requiresRotation() - .forEach(endpoint -> updateGlobalDnsOf(endpoint, routeEntry.getValue(), deployment, owner)); + .forEach(endpoint -> updateGlobalDnsOf(endpoint, inactiveZones, routeEntry.getValue(), deployment, owner)); } } /** Update global DNS records for given global endpoint */ - private void updateGlobalDnsOf(Endpoint endpoint, List<RoutingPolicy> policies, + private void updateGlobalDnsOf(Endpoint endpoint, Set<ZoneId> inactiveZones, List<RoutingPolicy> policies, Optional<DeploymentId> deployment, Optional<TenantAndApplicationId> owner) { if (endpoint.scope() != Endpoint.Scope.global) throw new IllegalArgumentException("Endpoint " + endpoint + " is not global"); if (deployment.isPresent() && !endpoint.deployments().contains(deployment.get())) return; - Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(endpoint, policies); + Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(endpoint, policies, inactiveZones); Set<AliasTarget> latencyTargets = new LinkedHashSet<>(); Set<AliasTarget> inactiveLatencyTargets = new LinkedHashSet<>(); for (var regionEndpoint : regionEndpoints) { @@ -235,7 +237,7 @@ public class RoutingPolicies { } /** Compute region endpoints and their targets from given policies */ - private Collection<RegionEndpoint> computeRegionEndpoints(Endpoint parent, List<RoutingPolicy> policies) { + private Collection<RegionEndpoint> computeRegionEndpoints(Endpoint parent, List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) { if (!parent.scope().multiDeployment()) { throw new IllegalArgumentException(parent + " has unexpected scope"); } @@ -246,7 +248,7 @@ public class RoutingPolicies { Endpoint endpoint = policy.regionEndpointIn(controller.system(), RoutingMethod.exclusive, parent.generated()); var zonePolicy = db.readZoneRoutingPolicy(policy.id().zone()); long weight = 1; - if (isConfiguredOut(zonePolicy, policy)) { + if (isConfiguredOut(zonePolicy, policy, inactiveZones)) { weight = 0; // A record with 0 weight will not receive traffic. If all records within a group have 0 // weight, traffic is routed to all records with equal probability. } @@ -268,8 +270,9 @@ public class RoutingPolicies { } - private void updateApplicationDnsOf(RoutingPolicyList routingPolicies, DeploymentId deployment, - Optional<TenantAndApplicationId> owner, @SuppressWarnings("unused") Mutex lock) { + private void updateApplicationDnsOf(RoutingPolicyList routingPolicies, Set<ZoneId> inactiveZones, + DeploymentId deployment, Optional<TenantAndApplicationId> owner, + @SuppressWarnings("unused") Mutex lock) { // In the context of single deployment (which this is) there is only one routing policy per routing ID. I.e. // there is no scenario where more than one deployment within an instance can be a member the same // application-level endpoint. However, to allow this in the future the routing table remains @@ -294,7 +297,7 @@ public class RoutingPolicies { Set<Target> activeTargets = targetsByEndpoint.computeIfAbsent(endpoint, (k) -> new LinkedHashSet<>()); Set<Target> inactiveTargets = inactiveTargetsByEndpoint.computeIfAbsent(endpoint, (k) -> new LinkedHashSet<>()); - if (isConfiguredOut(zonePolicy, policy)) { + if (isConfiguredOut(zonePolicy, policy, inactiveZones)) { inactiveTargets.add(Target.weighted(policy, target)); } else { activeTargets.add(Target.weighted(policy, target)); @@ -486,7 +489,7 @@ public class RoutingPolicies { } /** Remove unreferenced instance endpoints from DNS */ - private void removeGlobalDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, @SuppressWarnings("unused") Mutex lock) { + private void removeGlobalDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Mutex lock) { Set<RoutingId> removalCandidates = new HashSet<>(deploymentPolicies.asInstanceRoutingTable().keySet()); Set<RoutingId> activeRoutingIds = instanceRoutingIds(allocation); removalCandidates.removeAll(activeRoutingIds); @@ -497,7 +500,7 @@ public class RoutingPolicies { // This removes all ALIAS records having this DNS name. There is no attempt to delete only the entry for the // affected zone. Instead, the correct set of records is (re)created by updateGlobalDnsOf for (var endpoint : endpoints) { - for (var regionEndpoint : computeRegionEndpoints(endpoint, deploymentPolicies.asList())) { + for (var regionEndpoint : computeRegionEndpoints(endpoint, deploymentPolicies.asList(), inactiveZones)) { Record.Type type = regionEndpoint.zoneDirectTargets().isEmpty() ? Record.Type.ALIAS : Record.Type.DIRECT; controller.nameServiceForwarder().removeRecords(type, RecordName.from(regionEndpoint.target().name().value()), @@ -568,12 +571,14 @@ public class RoutingPolicies { } /** Returns whether the endpoints of given policy are configured {@link RoutingStatus.Value#out} */ - private static boolean isConfiguredOut(ZoneRoutingPolicy zonePolicy, RoutingPolicy policy) { + private static boolean isConfiguredOut(ZoneRoutingPolicy zonePolicy, RoutingPolicy policy, Set<ZoneId> inactiveZones) { // A deployment can be configured out from endpoints at any of the following levels: // - zone level (ZoneRoutingPolicy) // - deployment level (RoutingPolicy) + // - application package level (deployment.xml) return zonePolicy.routingStatus().value() == RoutingStatus.Value.out || - policy.routingStatus().value() == RoutingStatus.Value.out; + policy.routingStatus().value() == RoutingStatus.Value.out || + inactiveZones.contains(policy.id().zone()); } /** Represents records for a region-wide endpoint */ @@ -653,6 +658,10 @@ public class RoutingPolicies { if (instanceSpec.isEmpty()) { return Set.of(); } + if (instanceSpec.get().globalServiceId().filter(id -> id.equals(loadBalancer.cluster().value())).isPresent()) { + // Legacy assignment always has the default endpoint ID + return Set.of(EndpointId.defaultId()); + } return instanceSpec.get().endpoints().stream() .filter(endpoint -> endpoint.containerId().equals(loadBalancer.cluster().value())) .filter(endpoint -> endpoint.regions().contains(deployment.zoneId().region())) @@ -678,6 +687,17 @@ public class RoutingPolicies { } + /** Returns zones where global routing is declared inactive for instance through deploymentSpec */ + private static Set<ZoneId> inactiveZones(ApplicationId instance, DeploymentSpec deploymentSpec) { + var instanceSpec = deploymentSpec.instance(instance.instance()); + if (instanceSpec.isEmpty()) return Set.of(); + return instanceSpec.get().zones().stream() + .filter(zone -> zone.environment().isProduction()) + .filter(zone -> !zone.active()) + .map(zone -> ZoneId.from(zone.environment(), zone.region().get())) + .collect(Collectors.toUnmodifiableSet()); + } + /** Returns the name updater to use for given endpoint */ private NameServiceForwarder nameServiceForwarder(Endpoint endpoint) { return switch (endpoint.routingMethod()) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepository.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepository.java index d54bdead0bd..1e9f11e0349 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepository.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepository.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.routing.rotation; +import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.Endpoint; import com.yahoo.config.provision.ApplicationId; @@ -62,10 +63,44 @@ public class RotationRepository { } /** + * Returns a single rotation for the given application. This is only used when a rotation is assigned through the + * use of a global service ID. + * + * If a rotation is already assigned to the application, that rotation will be returned. + * If no rotation is assigned, return an available rotation. The caller is responsible for assigning the rotation. + * + * @param instanceSpec the instance deployment spec + * @param instance the instance requesting a rotation + * @param lock lock which must be acquired by the caller + */ + private AssignedRotation assignRotationTo(String globalServiceId, DeploymentInstanceSpec instanceSpec, + Instance instance, RotationLock lock) { + RotationId rotation; + if (instance.rotations().isEmpty()) { + rotation = findAvailableRotation(instance.id(), lock).id(); + } else { + rotation = instance.rotations().get(0).rotationId(); + } + var productionRegions = instanceSpec.zones().stream() + .filter(zone -> zone.environment().isProduction()) + .flatMap(zone -> zone.region().stream()) + .collect(Collectors.toSet()); + if (productionRegions.size() < 2) { + throw new IllegalArgumentException("global-service-id is set but less than 2 prod zones are defined " + + "in instance '" + instance.name() + "'"); + } + return new AssignedRotation(new ClusterSpec.Id(globalServiceId), + EndpointId.defaultId(), + rotation, + productionRegions); + } + + /** * Returns rotation assignments for all endpoints in application. * * If rotations are already assigned, these will be returned. * If rotations are not assigned, a new assignment will be created taking new rotations from the repository. + * This method supports both global-service-id as well as the new endpoints tag. * * @param deploymentSpec The deployment spec of the application * @param instance The application requesting rotations @@ -77,7 +112,19 @@ public class RotationRepository { if (allRotations.isEmpty()) { return List.of(); } + + // Only allow one kind of configuration syntax var instanceSpec = deploymentSpec.requireInstance(instance.name()); + if ( instanceSpec.globalServiceId().isPresent() + && ! instanceSpec.endpoints().isEmpty()) { + throw new IllegalArgumentException("Cannot provision rotations with both global-service-id and 'endpoints'"); + } + + // Support legacy global-service-id + if (instanceSpec.globalServiceId().isPresent()) { + return List.of(assignRotationTo(instanceSpec.globalServiceId().get(), instanceSpec, instance, lock)); + } + return assignRotationsTo(instanceSpec.endpoints(), instance, lock); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index c46a28c4567..3d2a66adc81 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -384,7 +384,7 @@ public class ControllerTest { void testDnsUpdatesForGlobalEndpointLegacySyntax() { var context = tester.newDeploymentContext("tenant1", "app1", "default"); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .endpoint("default", "foo") + .globalServiceId("foo") .region("us-west-1") .region("us-central-1") // Two deployments should result in each DNS alias being registered once .build(); @@ -1532,6 +1532,22 @@ public class ControllerTest { } @Test + void testSubmitWithElementDeprecatedOnPreviousMajor() { + DeploymentContext context = tester.newDeploymentContext(); + var applicationPackage = new ApplicationPackageBuilder() + .compileVersion(Version.fromString("8.1")) + .region("us-west-1") + .globalServiceId("qrs") + .build(); + try { + context.submit(applicationPackage).deploy(); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Element 'prod' contains attribute 'global-service-id' deprecated since major version 7")); + } + } + + @Test void testDeactivateDeploymentUnknownByController() { DeploymentContext context = tester.newDeploymentContext(); DeploymentId deployment = context.deploymentIdIn(ZoneId.from("prod", "us-west-1")); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index fb3026e1d80..965201ec6da 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -65,6 +65,7 @@ public class ApplicationPackageBuilder { private String revisionTarget = "latest"; private String revisionChange = "always"; private String upgradeRollout = null; + private String globalServiceId = null; private String athenzIdentityAttributes = "athenz-domain='domain' athenz-service='service'"; private String searchDefinition = "search test { }"; private Version compileVersion = Version.fromString("6.1"); @@ -100,6 +101,11 @@ public class ApplicationPackageBuilder { return this; } + public ApplicationPackageBuilder globalServiceId(String globalServiceId) { + this.globalServiceId = globalServiceId; + return this; + } + public ApplicationPackageBuilder endpoint(String id, String containerId, String... regions) { endpointsBody.append(" <endpoint"); endpointsBody.append(" id='").append(id).append("'"); @@ -156,8 +162,15 @@ public class ApplicationPackageBuilder { return this; } + public ApplicationPackageBuilder region(RegionName regionName) { + return region(regionName, true); + } + public ApplicationPackageBuilder region(String regionName) { - return region(RegionName.from(regionName)); + prodBody.append(" <region>") + .append(regionName) + .append("</region>\n"); + return this; } public ApplicationPackageBuilder region(String regionName, String cloudAccount) { @@ -174,8 +187,10 @@ public class ApplicationPackageBuilder { return this; } - public ApplicationPackageBuilder region(RegionName regionName) { - prodBody.append(" <region>") + public ApplicationPackageBuilder region(RegionName regionName, boolean active) { + prodBody.append(" <region active='") + .append(active) + .append("'>") .append(regionName.value()) .append("</region>\n"); return this; @@ -320,7 +335,13 @@ public class ApplicationPackageBuilder { xml.append(" />\n"); }); xml.append(blockChange); - xml.append(" <prod>\n"); + xml.append(" <prod"); + if (globalServiceId != null) { + xml.append(" global-service-id='"); + xml.append(globalServiceId); + xml.append("'"); + } + xml.append(">\n"); xml.append(prodBody); xml.append(" </prod>\n"); if (endpointsBody.length() > 0) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 3fb4a040e0d..e5a10bb5889 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -1312,19 +1312,19 @@ public class DeploymentTriggerTest { <staging /> <prod> <parallel> - <region>us-west-1</region> + <region active='true'>us-west-1</region> <steps> - <region>us-east-3</region> + <region active='true'>us-east-3</region> <delay hours='2' /> - <region>eu-west-1</region> + <region active='true'>eu-west-1</region> <delay hours='2' /> </steps> <steps> <delay hours='3' /> - <region>us-central-1</region> + <region active='true'>us-central-1</region> <parallel> - <region athenz-service='no-service'>ap-northeast-1</region> - <region>ap-northeast-2</region> + <region active='true' athenz-service='no-service'>ap-northeast-1</region> + <region active='true'>ap-northeast-2</region> <test>us-central-1</test> </parallel> </steps> @@ -1335,7 +1335,7 @@ public class DeploymentTriggerTest { <test>ap-northeast-1</test> </parallel> <test>us-east-3</test> - <region>ap-southeast-1</region> + <region active='true'>ap-southeast-1</region> </prod> <endpoints> <endpoint id='foo' container-id='bar'> @@ -1350,7 +1350,7 @@ public class DeploymentTriggerTest { <test /> <block-change revision='true' version='false' days='sat' hours='0-23' time-zone='CET' /> <prod> - <region>eu-west-1</region> + <region active='true'>eu-west-1</region> <test>eu-west-1</test> </prod> <notifications when='failing'> @@ -1363,7 +1363,7 @@ public class DeploymentTriggerTest { <instance id='last'> <upgrade policy='conservative' /> <prod> - <region>eu-west-1</region> + <region active='true'>eu-west-1</region> </prod> </instance> </deployment> diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index 593d788fd7d..5df2ce234a1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -275,7 +275,7 @@ public class MetricsReporterTest { void name_service_queue_size_metric() { var tester = new DeploymentTester(); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .endpoint("default", "foo") + .globalServiceId("default") .region("us-west-1") .region("us-east-3") .build(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index f287bc52604..69b473dce87 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -78,8 +78,8 @@ public class ApplicationSerializerTest { DeploymentSpec deploymentSpec = DeploymentSpec.fromXml("<deployment version='1.0'>\n" + " <staging/>\n" + " <instance id=\"i1\">\n" + - " <prod>\n" + - " <region>us-west-1</region>\n" + + " <prod global-service-id=\"default\">\n" + + " <region active=\"true\">us-west-1</region>\n" + " </prod>\n" + " </instance>\n" + "</deployment>"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json index 32f7e8e8f5a..ec36f52c23a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json @@ -2,7 +2,7 @@ "id": "tenant1:app1", "internal": true, "deploymentIssueId": "321", - "deploymentSpecField": "<deployment version='1.0'>\n <test />\n <!--<staging />-->\n <prod>\n <region>us-east-3</region>\n <region>us-west-1</region>\n </prod>\n</deployment>\n", + "deploymentSpecField": "<deployment version='1.0'>\n <test />\n <!--<staging />-->\n <prod global-service-id=\"foo\">\n <region active=\"true\">us-east-3</region>\n <region active=\"true\">us-west-1</region>\n </prod>\n</deployment>\n", "validationOverrides": "<validation-overrides>\n <allow until=\"2016-04-28\" comment=\"Renaming content cluster\">content-cluster-removal</allow>\n <allow until=\"2016-08-22\" comment=\"Migrating us-east-3 to C-2E\">cluster-size-reduction</allow>\n <allow until=\"2017-06-30\" comment=\"Test Vespa upgrade tests\">force-automatic-tenant-upgrade-test</allow>\n</validation-overrides>\n", "projectId": 102889, "deployingField": { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index 915466dac26..3cd9d586350 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -542,7 +542,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { var applicationPackage = new ApplicationPackageBuilder() .trustDefaultCertificate() .instances("default") - .endpoint("default", "foo") + .globalServiceId("foo") .region("aws-us-east-1c") .build(); new ControllerTester(tester).upgradeSystem(new Version("6.1")); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 98775ea214d..14c279c9ef8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -130,7 +130,7 @@ public class ApplicationApiTest extends ControllerContainerTest { private static final ApplicationPackage applicationPackageDefault = new ApplicationPackageBuilder() .withoutAthenzIdentity() .instances("default") - .endpoint("default", "foo") + .globalServiceId("foo") .region("us-central-1") .region("us-east-3") .region("us-west-1") @@ -140,7 +140,7 @@ public class ApplicationApiTest extends ControllerContainerTest { private static final ApplicationPackage applicationPackageInstance1 = new ApplicationPackageBuilder() .withoutAthenzIdentity() .instances("instance1") - .endpoint("default", "foo") + .globalServiceId("foo") .region("us-central-1") .region("us-east-3") .region("us-west-1") @@ -343,7 +343,7 @@ public class ApplicationApiTest extends ControllerContainerTest { ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .withoutAthenzIdentity() .instances("instance1") - .endpoint("default", "foo") + .globalServiceId("foo") .region("us-west-1") .region("us-east-3") .allow(ValidationId.globalEndpointChange) @@ -864,7 +864,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Third attempt has a service under the domain of the tenant, and also succeeds. ApplicationPackage packageWithService = new ApplicationPackageBuilder() .instances("instance1") - .endpoint("default", "foo") + .globalServiceId("foo") .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from(ATHENZ_TENANT_DOMAIN.getName()), AthenzService.from("service")) .region("us-central-1") .parallel("us-west-1", "us-east-3") @@ -1043,7 +1043,7 @@ public class ApplicationApiTest extends ControllerContainerTest { var eastZone = ZoneId.from("prod", "us-east-3"); var applicationPackage = new ApplicationPackageBuilder() .instances("instance1") - .endpoint("default", "foo") + .globalServiceId("foo") .region(westZone.region()) .region(eastZone.region()) .build(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 46ec42cab8f..02f030aa758 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -234,6 +234,27 @@ public class RoutingPoliciesTest { } @Test + void global_routing_policies_legacy_global_service_id() { + var tester = new RoutingPoliciesTester(); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + int clustersPerZone = 2; + int numberOfDeployments = 2; + var applicationPackage = applicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .globalServiceId("c0") + .build(); + tester.provisionLoadBalancers(clustersPerZone, context.instanceId(), zone1, zone2); + + // Creates alias records + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + tester.assertTargets(context.instanceId(), EndpointId.defaultId(), 0, zone1, zone2); + assertEquals(numberOfDeployments * clustersPerZone, + tester.policiesOf(context.instance().id()).size(), + "Routing policy count is equal to cluster count"); + } + + @Test void zone_routing_policies() { zone_routing_policies(false); zone_routing_policies(true); @@ -642,6 +663,28 @@ public class RoutingPoliciesTest { assertEquals(RoutingStatus.Value.in, policy1.routingStatus().value()); assertEquals(RoutingStatus.Agent.tenant, policy1.routingStatus().agent()); assertEquals(changedAt.truncatedTo(ChronoUnit.MILLIS), policy1.routingStatus().changedAt()); + + // Deployment is set out through a new deployment.xml + var applicationPackage2 = applicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region(), false) + .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) + .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) + .build(); + context.submit(applicationPackage2).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1); + + // ... back in + var applicationPackage3 = applicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) + .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) + .build(); + context.submit(applicationPackage3).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepositoryTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepositoryTest.java index 6190680d098..168a1345c39 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepositoryTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationRepositoryTest.java @@ -41,7 +41,7 @@ public class RotationRepositoryTest { ); private static final ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .endpoint("default", "foo") + .globalServiceId("foo") .region("us-east-3") .region("us-west-1") .build(); @@ -74,7 +74,7 @@ public class RotationRepositoryTest { // Adding region updates rotation var applicationPackage = new ApplicationPackageBuilder() - .endpoint("default", "foo") + .globalServiceId("foo") .region("us-east-3") .region("us-west-1") .region("us-central-1") @@ -116,6 +116,15 @@ public class RotationRepositoryTest { } @Test + void too_few_zones() { + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .globalServiceId("foo") + .region("us-east-3") + .build(); + application.submit(applicationPackage).runJobExpectingFailure(DeploymentContext.systemTest, "less than 2 prod zones are defined"); + } + + @Test void no_rotation_assigned_for_application_without_service_id() { ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .region("us-east-3") @@ -128,7 +137,7 @@ public class RotationRepositoryTest { @Test void prefixes_system_when_not_main() { ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .endpoint("default", "foo") + .globalServiceId("foo") .region("cd-us-east-1") .region("cd-us-west-1") .build(); @@ -155,7 +164,7 @@ public class RotationRepositoryTest { .instances("instance1,instance2") .region("us-central-1") .parallel("us-west-1", "us-east-3") - .endpoint("default", "global") + .globalServiceId("global") .build(); var instance1 = tester.newDeploymentContext("tenant1", "application1", "instance1") .submit(applicationPackage) |