diff options
16 files changed, 183 insertions, 338 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 797be652ebc..358744d40f7 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.application.api; -import ai.vespa.validation.Validation; import com.yahoo.collections.Comparables; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; @@ -434,17 +433,16 @@ public class DeploymentSpec { private final Environment environment; private final Optional<RegionName> region; - private final boolean active; private final Optional<AthenzService> athenzService; private final Optional<String> testerFlavor; private final Map<CloudName, CloudAccount> cloudAccounts; private final Optional<Duration> hostTTL; public DeclaredZone(Environment environment) { - this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); + this(environment, Optional.empty(), Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); } - public DeclaredZone(Environment environment, Optional<RegionName> region, boolean active, + public DeclaredZone(Environment environment, Optional<RegionName> region, Optional<AthenzService> athenzService, Optional<String> testerFlavor, Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL) { if (environment != Environment.prod && region.isPresent()) @@ -454,7 +452,6 @@ public class DeploymentSpec { hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); this.environment = Objects.requireNonNull(environment); this.region = Objects.requireNonNull(region); - this.active = active; this.athenzService = Objects.requireNonNull(athenzService); this.testerFlavor = Objects.requireNonNull(testerFlavor); this.cloudAccounts = Map.copyOf(cloudAccounts); @@ -466,9 +463,6 @@ 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 13bc09883fa..4bfecabaf69 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -273,7 +273,7 @@ public class DeploymentSpecXmlReader { return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()), readHostTTL(stepTag))); // A production test } case devTag, perfTag, stagingTag: // Intentional fallthrough from test tag. - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccounts(stepTag), readHostTTL(stepTag))); + return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), athenzService, testerFlavor, readCloudAccounts(stepTag), readHostTTL(stepTag))); case prodTag: // regions, delay and parallel may be nested within, but we can flatten them return XML.getChildren(stepTag).stream() .flatMap(child -> readNonInstanceSteps(child, prodAttributes, stepTag, defaultBcp).stream()) @@ -682,7 +682,7 @@ public class DeploymentSpecXmlReader { private DeclaredZone readDeclaredZone(Environment environment, Optional<AthenzService> athenzService, Optional<String> testerFlavor, Element regionTag) { return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())), - readActive(regionTag), athenzService, testerFlavor, + athenzService, testerFlavor, readCloudAccounts(regionTag), readHostTTL(regionTag)); } @@ -784,16 +784,6 @@ public class DeploymentSpecXmlReader { }; } - private boolean readActive(Element regionTag) { - String activeValue = regionTag.getAttribute("active"); - if ("".equals(activeValue)) return true; // Default to active - deprecate(regionTag, List.of("active"), 7, "See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax"); - if ("true".equals(activeValue)) return true; - if ("false".equals(activeValue)) return false; - throw new IllegalArgumentException("Value of 'active' attribute in region tag must be 'true' or 'false' " + - "to control whether this region should receive traffic from the global endpoint of this application"); - } - private void deprecate(Element element, List<String> attributes, int majorVersion, String message) { deprecatedElements.add(new DeprecatedElement(majorVersion, element.getTagName(), attributes, message)); } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index c33277dfc6f..134d45e35ab 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -116,8 +116,8 @@ public class DeploymentSpecTest { <deployment version='1.0'> <instance id='default'> <prod> - <region active='false'>us-east1</region> - <region active='true'>us-west1</region> + <region>us-east1</region> + <region>us-west1</region> </prod> </instance> </deployment> @@ -128,10 +128,8 @@ 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())); @@ -154,14 +152,14 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <instance id='a' tags='tag1 tag2'>" + " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + + " <region>us-east1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + " <instance id='b' tags='tag3'>" + " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + + " <region>us-east1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -179,9 +177,9 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -199,8 +197,8 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + + " <region>us-east-1</region>" + + " <region>us-west-1</region>" + " <delay hours='1' />" + " <test>us-west-1</test>" + " <test>us-east-1</test>" + @@ -227,7 +225,7 @@ public class DeploymentSpecTest { "<deployment version='1.0'>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " <test>us-east1</test>" + " </prod>" + @@ -244,7 +242,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <prod>" + " <test>us-east1</test>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -259,7 +257,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <prod>" + " <parallel>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " </parallel>" + " </prod>" + @@ -277,14 +275,14 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + " <instance id='instance2'>" + " <prod>" + - " <region active='true'>us-central1</region>" + + " <region>us-central1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -309,9 +307,9 @@ public class DeploymentSpecTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -332,13 +330,11 @@ public class DeploymentSpecTest { assertTrue(instance.steps().get(1).concerns(staging)); assertTrue(instance.steps().get(2).concerns(prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)instance.steps().get(2)).active()); assertTrue(instance.steps().get(3) instanceof DeploymentSpec.Delay); assertEquals(3 * 60 * 60 + 30 * 60, instance.steps().get(3).delay().getSeconds()); assertTrue(instance.steps().get(4).concerns(prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)instance.steps().get(4)).active()); assertTrue(instance.concerns(test, Optional.empty())); assertTrue(instance.concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region @@ -474,11 +470,11 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <upgrade policy='canary'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <delay hours='47'/>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -515,10 +511,10 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <parallel>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-central-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -539,14 +535,14 @@ public class DeploymentSpecTest { " <staging/>" + " <instance id='instance0'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='instance1'>" + " <test/>" + " <staging/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -579,25 +575,25 @@ public class DeploymentSpecTest { " <instance id='instance' athenz-service='in-service'>" + " <prod>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <steps>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " <delay hours='2' />" + - " <region active='true'>eu-west-1</region>" + + " <region>eu-west-1</region>" + " <delay hours='2' />" + " </steps>" + " <steps>" + " <delay hours='3' />" + - " <region active='true'>aws-us-east-1a</region>" + + " <region>aws-us-east-1a</region>" + " <parallel>" + - " <region active='true' athenz-service='no-service'>ap-northeast-1</region>" + - " <region active='true'>ap-southeast-2</region>" + + " <region athenz-service='no-service'>ap-northeast-1</region>" + + " <region>ap-southeast-2</region>" + " <test>aws-us-east-1a</test>" + " </parallel>" + " </steps>" + " <delay hours='3' minutes='30' />" + " </parallel>" + - " <region active='true'>us-north-7</region>" + + " <region>us-north-7</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -651,12 +647,12 @@ public class DeploymentSpecTest { " <parallel>" + " <instance id='instance0'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='instance1'>" + " <prod>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + " </instance>" + " </parallel>" + @@ -679,13 +675,13 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='instance0'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <delay hours='12'/>" + " <instance id='instance1'>" + " <prod>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -705,11 +701,11 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-central-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-west-1</region>" + + " <region>us-central-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -796,7 +792,7 @@ public class DeploymentSpecTest { " <instance id='default'>" + " <block-change days='sat' hours='10' time-zone='CET'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " <block-change days='mon,tue' hours='15-16'/>" + " </instance>" + @@ -813,7 +809,7 @@ public class DeploymentSpecTest { " <block-change days='sat' hours='10' time-zone='CET'/>" + " <test/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -830,7 +826,7 @@ public class DeploymentSpecTest { " <block-change days='sat' hours='10' time-zone='CET'/>" + " <block-change days='mon-sun' hours='0-23' time-zone='CET' from-date='2022-01-01' to-date='2022-01-15'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -891,7 +887,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain' athenz-service='service'>" + " <instance id='instance1'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -909,10 +905,10 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain' athenz-service='service'>" + " <instance id='instance1'>" + " <prod athenz-service='prod-service'>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-west-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + " </instance>" + @@ -939,16 +935,16 @@ public class DeploymentSpecTest { <instance id='instance1'> <prod> <parallel> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </parallel> </prod> </instance> <instance id='instance2'> <prod> <parallel> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </parallel> </prod> </instance> @@ -971,7 +967,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain'>" + " <instance id='default' athenz-service='service'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -990,7 +986,7 @@ public class DeploymentSpecTest { " <test />" + " <staging athenz-service='staging-service' />" + " <prod athenz-service='prod-service'>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1013,7 +1009,7 @@ public class DeploymentSpecTest { "<deployment athenz-domain='domain'>" + " <instance id='default'>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1027,7 +1023,7 @@ public class DeploymentSpecTest { "<deployment>" + " <instance id='default'>" + " <prod athenz-service='service'>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " </prod>" + " </instance>" + "</deployment>" @@ -1356,14 +1352,14 @@ public class DeploymentSpecTest { <deployment> <instance id="beta"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> </instance> <instance id="main"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> </instance> <endpoints> @@ -1456,14 +1452,14 @@ public class DeploymentSpecTest { <deployment> <instance id="beta"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> </instance> <instance id="main"> <prod> - <region active='true'>us-west-1</region> - <region active='true'>us-east-3</region> + <region>us-west-1</region> + <region>us-east-3</region> </prod> <endpoints> <endpoint id="glob" container-id="music"/> 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 a8c3913c498..3a8d7ae1703 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java @@ -96,8 +96,8 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <prod>" + - " <region active='false'>us-east1</region>" + - " <region active='true'>us-west1</region>" + + " <region>us-east1</region>" + + " <region>us-west1</region>" + " </prod>" + "</deployment>" ); @@ -107,10 +107,8 @@ 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())); @@ -129,9 +127,9 @@ public class DeploymentSpecWithoutInstanceTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east1</region>" + + " <region>us-east1</region>" + " <delay hours='3' minutes='30'/>" + - " <region active='true'>us-west1</region>" + + " <region>us-west1</region>" + " </prod>" + "</deployment>" ); @@ -145,13 +143,11 @@ public class DeploymentSpecWithoutInstanceTest { assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.staging)); assertTrue(spec.requireInstance("default").steps().get(2).concerns(prod, Optional.of(RegionName.from("us-east1")))); - assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(2)).active()); assertTrue(spec.requireInstance("default").steps().get(3) instanceof DeploymentSpec.Delay); assertEquals(3 * 60 * 60 + 30 * 60, spec.requireInstance("default").steps().get(3).delay().getSeconds()); assertTrue(spec.requireInstance("default").steps().get(4).concerns(prod, Optional.of(RegionName.from("us-west1")))); - assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(4)).active()); assertTrue(spec.requireInstance("default").concerns(test, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region @@ -168,8 +164,8 @@ public class DeploymentSpecWithoutInstanceTest { " <test/>" + " <staging/>" + " <prod>" + - " <region active='false'>us-east-1</region>" + - " <region active='true'>us-west-1</region>" + + " <region>us-east-1</region>" + + " <region>us-west-1</region>" + " <delay hours='1' />" + " <test>us-west-1</test>" + " <test>us-east-1</test>" + @@ -194,7 +190,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment version='1.0'>" + " <prod>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " <test>us-east1</test>" + " </prod>" + @@ -209,7 +205,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment version='1.0'>" + " <prod>" + " <test>us-east1</test>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " </prod>" + "</deployment>" ); @@ -222,7 +218,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment version='1.0'>" + " <prod>" + " <parallel>" + - " <region active='true'>us-east1</region>" + + " <region>us-east1</region>" + " <test>us-east1</test>" + " </parallel>" + " </prod>" + @@ -231,26 +227,6 @@ public class DeploymentSpecWithoutInstanceTest { DeploymentSpec.fromXml(r); } - @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 productionSpecWithUpgradeRollout() { StringReader r = new StringReader( @@ -280,11 +256,11 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment>" + " <upgrade policy='canary'/>" + " <prod>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <delay hours='47'/>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <delay minutes='59' seconds='61'/>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " </prod>" + "</deployment>" ); @@ -323,10 +299,10 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod> \n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " <parallel>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + + " <region>us-central-1</region>\n" + + " <region>us-east-3</region>\n" + " </parallel>\n" + " </prod>\n" + "</deployment>" @@ -345,25 +321,25 @@ public class DeploymentSpecWithoutInstanceTest { " <staging />" + " <prod>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + + " <region>us-west-1</region>" + " <steps>" + - " <region active='true'>us-east-3</region>" + + " <region>us-east-3</region>" + " <delay hours='2' />" + - " <region active='true'>eu-west-1</region>" + + " <region>eu-west-1</region>" + " <delay hours='2' />" + " </steps>" + " <steps>" + " <delay hours='3' />" + - " <region active='true'>aws-us-east-1a</region>" + + " <region>aws-us-east-1a</region>" + " <parallel>" + - " <region active='true' athenz-service='no-service'>ap-northeast-1</region>" + - " <region active='true'>ap-southeast-2</region>" + + " <region athenz-service='no-service'>ap-northeast-1</region>" + + " <region>ap-southeast-2</region>" + " <test>aws-us-east-1a</test>" + " </parallel>" + " </steps>" + " <delay hours='3' minutes='30' />" + " </parallel>" + - " <region active='true'>us-north-7</region>" + + " <region>us-north-7</region>" + " </prod>" + "</deployment>" ); @@ -414,11 +390,11 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " <parallel>\n" + - " <region active='true'>us-west-1</region>\n" + - " <region active='true'>us-central-1</region>\n" + - " <region active='true'>us-east-3</region>\n" + + " <region>us-west-1</region>\n" + + " <region>us-central-1</region>\n" + + " <region>us-east-3</region>\n" + " </parallel>\n" + " </prod>\n" + "</deployment>" @@ -437,7 +413,7 @@ public class DeploymentSpecWithoutInstanceTest { "<deployment>\n" + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + " <block-change days='mon,tue' hours='15-16'/>\n" + "</deployment>" @@ -452,7 +428,7 @@ public class DeploymentSpecWithoutInstanceTest { " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <test/>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -466,7 +442,7 @@ public class DeploymentSpecWithoutInstanceTest { " <block-change revision='false' days='mon,tue' hours='15-16'/>\n" + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -495,7 +471,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -509,10 +485,10 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain' athenz-service='service'>" + " <prod athenz-service='prod-service'>" + - " <region active='true'>us-central-1</region>" + + " <region>us-central-1</region>" + " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-east-3</region>" + + " <region>us-west-1</region>" + + " <region>us-east-3</region>" + " </parallel>" + " </prod>" + "</deployment>" @@ -535,7 +511,7 @@ public class DeploymentSpecWithoutInstanceTest { " <test />\n" + " <staging athenz-service='staging-service' />\n" + " <prod athenz-service='prod-service'>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -552,7 +528,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment athenz-domain='domain'>\n" + " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); @@ -564,7 +540,7 @@ public class DeploymentSpecWithoutInstanceTest { StringReader r = new StringReader( "<deployment>\n" + " <prod athenz-service='service'>\n" + - " <region active='true'>us-west-1</region>\n" + + " <region>us-west-1</region>\n" + " </prod>\n" + "</deployment>" ); 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 1036a615bb5..31f8eba48bf 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,7 +10,6 @@ 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; @@ -360,24 +359,16 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { context.getDeployState().getProperties().athenzDnsSuffix(), context.getDeployState().zone(), deploymentSpec); - addRotationProperties(cluster, context.getDeployState().zone(), context.getDeployState().getEndpoints(), deploymentSpec); + addRotationProperties(cluster, context.getDeployState().getEndpoints()); } - private void addRotationProperties(ApplicationContainerCluster cluster, Zone zone, Set<ContainerEndpoint> endpoints, DeploymentSpec spec) { + private void addRotationProperties(ApplicationContainerCluster cluster, Set<ContainerEndpoint> endpoints) { cluster.getContainers().forEach(container -> { setRotations(container, endpoints, cluster.getName()); - container.setProp("activeRotation", Boolean.toString(zoneHasActiveRotation(zone, spec))); + container.setProp("activeRotation", "true"); // TODO(mpolden): This is unused and should be removed }); } - 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)) @@ -594,7 +585,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/test/cfg/application/app_complicated_deployment_spec/deployment.xml b/config-model/src/test/cfg/application/app_complicated_deployment_spec/deployment.xml index 3d3bfd35845..e30c02cdd84 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 active='true'>us-west-1</region> + <region>us-west-1</region> <steps> - <region active='true'>us-east-3</region> + <region>us-east-3</region> <delay hours='2' /> - <region active='true'>eu-west-1</region> + <region>eu-west-1</region> <delay hours='2' /> </steps> <steps> <delay hours='3' /> - <region active='true'>aws-us-east-1a</region> + <region>aws-us-east-1a</region> <parallel> - <region active='true' athenz-service='no-service'>ap-northeast-1</region> - <region active='true'>ap-southeast-2</region> + <region athenz-service='no-service'>ap-northeast-1</region> + <region>ap-southeast-2</region> </parallel> </steps> <delay hours='3' minutes='30' /> </parallel> - <region active='true'>us-north-7</region> + <region>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 active='true'>eu-central-2</region> + <region>eu-central-2</region> </prod> <notifications when='failing'> <email role='author' /> 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 92efac74530..9ce853ad0f2 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 @@ -24,7 +24,7 @@ public class DeploymentSpecValidatorTest { "<deployment version='1.0'>" + " <test />" + " <prod>" + - " <region active='true'>us-east</region>" + + " <region>us-east</region>" + " </prod>" + " <endpoints>" + " <endpoint container-id='non-existing'/>" + 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 78269d00cf3..08877b3d3a9 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 active='true'>default</region>\n" + + " <region>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 58b9462978f..2bf50103b10 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,14 +10,12 @@ 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.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ApplicationContainer; +import com.yahoo.vespa.model.container.ApplicationContainerCluster; import org.junit.jupiter.api.Test; import org.w3c.dom.Element; -import org.xml.sax.SAXException; -import java.io.IOException; -import java.util.Arrays; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -27,43 +25,32 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class RoutingBuilderTest extends ContainerModelBuilderTestBase { @Test - void setsRotationActiveAccordingToDeploymentSpec() throws IOException, SAXException { + void setsRotationActive() { Element clusterElem = DomBuilderTest.parse( "<container id='default' version='1.0'><search /></container>"); - 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>"; + 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>"""; ApplicationPackage applicationPackage = new MockApplicationPackage.Builder() .withDeploymentSpec(deploymentSpec) .build(); - //root = new MockRoot("root", applicationPackage); - for (String region : Arrays.asList("us-north-1", "us-north-3")) { - ApplicationContainer container = getContainer(applicationPackage, region, clusterElem); + for (String region : List.of("us-north-1", "us-north-2", "us-north-3", "us-north-4")) { + 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/configserver/src/test/apps/zkapp/deployment.xml b/configserver/src/test/apps/zkapp/deployment.xml index c8b60f3aa8b..ba71f9c1e47 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 global-service-id='mydisc'> - <region active='true'>us-east</region> + <prod> + <region>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 4f52eede4e8..ca2f9da3273 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,8 +22,6 @@ 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; @@ -36,7 +34,6 @@ 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; @@ -46,12 +43,10 @@ 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( @@ -61,16 +56,6 @@ 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() { @@ -88,17 +73,9 @@ 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-2"); - LbServicesConfig conf = createModelAndGetLbServicesConfig(regionName); - assertFalse(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()); } private LbServicesConfig createModelAndGetLbServicesConfig(RegionName regionName) { @@ -116,8 +93,6 @@ 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))); @@ -150,8 +125,6 @@ public class LbServicesProducerTest { @Test public void testRoutingConfigForTesterApplication() { - assumeFalse(useGlobalServiceId); - Map<TenantName, Set<ApplicationInfo>> testModel = createTestModel(new DeployState.Builder()); // No config for tester application @@ -227,32 +200,17 @@ public class LbServicesProducerTest { " </container>" + "</services>"; - 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>"; - } - - + 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>"; return new MockApplicationPackage.Builder().withServices(services).withDeploymentSpec(deploymentInfo).build(); } diff --git a/configserver/src/test/resources/deploy/hosted-app/deployment.xml b/configserver/src/test/resources/deploy/hosted-app/deployment.xml index f1ebbf7f968..cfd5c61069b 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 global-service-id="qrs"> - <region active="true">us-north-1</region> - <region active="true">us-north-2</region> + <prod> + <region>us-north-1</region> + <region>us-north-2</region> </prod> </deployment> 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 8fae3309f2b..c8c3d057ee3 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,21 +112,20 @@ 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, inactiveZones, lock); + removeGlobalDnsUnreferencedBy(allocation, deploymentPolicies, 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), inactiveZones, owner, lock); - updateApplicationDnsOf(updatedApplicationPolicies, inactiveZones, deployment, owner, lock); + updateGlobalDnsOf(instancePolicies, Optional.of(deployment), owner, lock); + updateApplicationDnsOf(updatedApplicationPolicies, deployment, owner, lock); } } @@ -137,7 +136,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(), Set.of(), Optional.of(TenantAndApplicationId.from(instance)), lock); + updateGlobalDnsOf(policies, Optional.empty(), Optional.of(TenantAndApplicationId.from(instance)), lock); }); } } @@ -157,17 +156,16 @@ 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, Set.of(), deployment, ownerOf(deployment), lock); + updateApplicationDnsOf(effectivePolicies, 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, - Set<ZoneId> inactiveZones, Optional<TenantAndApplicationId> owner, + Optional<TenantAndApplicationId> owner, @SuppressWarnings("unused") Mutex lock) { Map<RoutingId, List<RoutingPolicy>> routingTable = instancePolicies.asInstanceRoutingTable(); for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { @@ -175,17 +173,17 @@ public class RoutingPolicies { controller.routing().readDeclaredEndpointsOf(routingId.instance()) .named(routingId.endpointId(), Endpoint.Scope.global) .not().requiresRotation() - .forEach(endpoint -> updateGlobalDnsOf(endpoint, inactiveZones, routeEntry.getValue(), deployment, owner)); + .forEach(endpoint -> updateGlobalDnsOf(endpoint, routeEntry.getValue(), deployment, owner)); } } /** Update global DNS records for given global endpoint */ - private void updateGlobalDnsOf(Endpoint endpoint, Set<ZoneId> inactiveZones, List<RoutingPolicy> policies, + private void updateGlobalDnsOf(Endpoint endpoint, 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, inactiveZones); + Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(endpoint, policies); Set<AliasTarget> latencyTargets = new LinkedHashSet<>(); Set<AliasTarget> inactiveLatencyTargets = new LinkedHashSet<>(); for (var regionEndpoint : regionEndpoints) { @@ -237,7 +235,7 @@ public class RoutingPolicies { } /** Compute region endpoints and their targets from given policies */ - private Collection<RegionEndpoint> computeRegionEndpoints(Endpoint parent, List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) { + private Collection<RegionEndpoint> computeRegionEndpoints(Endpoint parent, List<RoutingPolicy> policies) { if (!parent.scope().multiDeployment()) { throw new IllegalArgumentException(parent + " has unexpected scope"); } @@ -248,7 +246,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, inactiveZones)) { + if (isConfiguredOut(zonePolicy, policy)) { 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. } @@ -270,9 +268,8 @@ public class RoutingPolicies { } - private void updateApplicationDnsOf(RoutingPolicyList routingPolicies, Set<ZoneId> inactiveZones, - DeploymentId deployment, Optional<TenantAndApplicationId> owner, - @SuppressWarnings("unused") Mutex lock) { + private void updateApplicationDnsOf(RoutingPolicyList routingPolicies, 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 @@ -297,7 +294,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, inactiveZones)) { + if (isConfiguredOut(zonePolicy, policy)) { inactiveTargets.add(Target.weighted(policy, target)); } else { activeTargets.add(Target.weighted(policy, target)); @@ -489,7 +486,7 @@ public class RoutingPolicies { } /** Remove unreferenced instance endpoints from DNS */ - private void removeGlobalDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Mutex lock) { + private void removeGlobalDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, @SuppressWarnings("unused") Mutex lock) { Set<RoutingId> removalCandidates = new HashSet<>(deploymentPolicies.asInstanceRoutingTable().keySet()); Set<RoutingId> activeRoutingIds = instanceRoutingIds(allocation); removalCandidates.removeAll(activeRoutingIds); @@ -500,7 +497,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(), inactiveZones)) { + for (var regionEndpoint : computeRegionEndpoints(endpoint, deploymentPolicies.asList())) { Record.Type type = regionEndpoint.zoneDirectTargets().isEmpty() ? Record.Type.ALIAS : Record.Type.DIRECT; controller.nameServiceForwarder().removeRecords(type, RecordName.from(regionEndpoint.target().name().value()), @@ -571,14 +568,12 @@ public class RoutingPolicies { } /** Returns whether the endpoints of given policy are configured {@link RoutingStatus.Value#out} */ - private static boolean isConfiguredOut(ZoneRoutingPolicy zonePolicy, RoutingPolicy policy, Set<ZoneId> inactiveZones) { + private static boolean isConfiguredOut(ZoneRoutingPolicy zonePolicy, RoutingPolicy policy) { // 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 || - inactiveZones.contains(policy.id().zone()); + policy.routingStatus().value() == RoutingStatus.Value.out; } /** Represents records for a region-wide endpoint */ @@ -683,17 +678,6 @@ 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/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 a70125815b1..fb3026e1d80 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 @@ -156,15 +156,8 @@ public class ApplicationPackageBuilder { return this; } - public ApplicationPackageBuilder region(RegionName regionName) { - return region(regionName, true); - } - public ApplicationPackageBuilder region(String regionName) { - prodBody.append(" <region>") - .append(regionName) - .append("</region>\n"); - return this; + return region(RegionName.from(regionName)); } public ApplicationPackageBuilder region(String regionName, String cloudAccount) { @@ -181,10 +174,8 @@ public class ApplicationPackageBuilder { return this; } - public ApplicationPackageBuilder region(RegionName regionName, boolean active) { - prodBody.append(" <region active='") - .append(active) - .append("'>") + public ApplicationPackageBuilder region(RegionName regionName) { + prodBody.append(" <region>") .append(regionName.value()) .append("</region>\n"); return this; 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 e5a10bb5889..3fb4a040e0d 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 active='true'>us-west-1</region> + <region>us-west-1</region> <steps> - <region active='true'>us-east-3</region> + <region>us-east-3</region> <delay hours='2' /> - <region active='true'>eu-west-1</region> + <region>eu-west-1</region> <delay hours='2' /> </steps> <steps> <delay hours='3' /> - <region active='true'>us-central-1</region> + <region>us-central-1</region> <parallel> - <region active='true' athenz-service='no-service'>ap-northeast-1</region> - <region active='true'>ap-northeast-2</region> + <region athenz-service='no-service'>ap-northeast-1</region> + <region>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 active='true'>ap-southeast-1</region> + <region>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 active='true'>eu-west-1</region> + <region>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 active='true'>eu-west-1</region> + <region>eu-west-1</region> </prod> </instance> </deployment> 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 e41856b0c3d..46ec42cab8f 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 @@ -642,28 +642,6 @@ 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 |