diff options
17 files changed, 223 insertions, 212 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 fc170db5897..a4be547fe70 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 @@ -4,6 +4,7 @@ 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; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; @@ -59,7 +60,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { private final List<DeploymentSpec.ChangeBlocker> changeBlockers; private final Optional<String> globalServiceId; private final Optional<AthenzService> athenzService; - private final Optional<CloudAccount> cloudAccount; + private final Map<CloudName, CloudAccount> cloudAccounts; private final Optional<Duration> hostTTL; private final Notifications notifications; private final List<Endpoint> endpoints; @@ -77,7 +78,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { List<DeploymentSpec.ChangeBlocker> changeBlockers, Optional<String> globalServiceId, Optional<AthenzService> athenzService, - Optional<CloudAccount> cloudAccount, + Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL, Notifications notifications, List<Endpoint> endpoints, @@ -101,7 +102,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { this.changeBlockers = Objects.requireNonNull(changeBlockers); this.globalServiceId = Objects.requireNonNull(globalServiceId); this.athenzService = Objects.requireNonNull(athenzService); - this.cloudAccount = Objects.requireNonNull(cloudAccount); + this.cloudAccounts = Map.copyOf(cloudAccounts); this.hostTTL = Objects.requireNonNull(hostTTL); this.notifications = Objects.requireNonNull(notifications); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); @@ -113,10 +114,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { validateEndpoints(globalServiceId, this.endpoints); validateChangeBlockers(changeBlockers, now); validateBcp(bcp); - hostTTL.ifPresent(ttl -> { - if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); - if (ttl.isNegative()) illegal("Host TTL cannot be negative"); - }); + hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); } public InstanceName name() { return name; } @@ -266,16 +264,16 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { .filter(zone -> zone.concerns(environment, Optional.of(region))) .findFirst() .flatMap(DeploymentSpec.DeclaredZone::athenzService) - .or(() -> this.athenzService); + .or(() -> athenzService); } - /** Returns the cloud account to use for given environment and region, if any */ - public Optional<CloudAccount> cloudAccount(Environment environment, Optional<RegionName> region) { + /** Returns the cloud accounts to use for given environment and region, if any */ + public Map<CloudName, CloudAccount> cloudAccounts(Environment environment, RegionName region) { return zones().stream() - .filter(zone -> zone.concerns(environment, region)) + .filter(zone -> zone.concerns(environment, Optional.of(region))) .findFirst() - .flatMap(DeploymentSpec.DeclaredZone::cloudAccount) - .or(() -> cloudAccount); + .map(DeploymentSpec.DeclaredZone::cloudAccounts) + .orElse(cloudAccounts); } /** Returns the host TTL to use for given environment and region, if any */ 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 ec79f9e554a..f355a61fa8a 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 @@ -7,6 +7,7 @@ import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; @@ -45,7 +46,7 @@ public class DeploymentSpec { Optional.empty(), Optional.empty(), Optional.empty(), - Optional.empty(), + Map.of(), Optional.empty(), List.of(), "<deployment version='1.0'/>", @@ -57,7 +58,7 @@ public class DeploymentSpec { private final Optional<Integer> majorVersion; private final Optional<AthenzDomain> athenzDomain; private final Optional<AthenzService> athenzService; - private final Optional<CloudAccount> cloudAccount; + private final Map<CloudName, CloudAccount> cloudAccounts; private final Optional<Duration> hostTTL; private final List<Endpoint> endpoints; private final List<DeprecatedElement> deprecatedElements; @@ -68,7 +69,7 @@ public class DeploymentSpec { Optional<Integer> majorVersion, Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, - Optional<CloudAccount> cloudAccount, + Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL, List<Endpoint> endpoints, String xmlForm, @@ -77,7 +78,7 @@ public class DeploymentSpec { this.majorVersion = Objects.requireNonNull(majorVersion); this.athenzDomain = Objects.requireNonNull(athenzDomain); this.athenzService = Objects.requireNonNull(athenzService); - this.cloudAccount = Objects.requireNonNull(cloudAccount); + this.cloudAccounts = Map.copyOf(cloudAccounts); this.hostTTL = Objects.requireNonNull(hostTTL); this.xmlForm = Objects.requireNonNull(xmlForm); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); @@ -86,10 +87,7 @@ public class DeploymentSpec { validateUpgradePoliciesOfIncreasingConservativeness(steps); validateAthenz(); validateApplicationEndpoints(); - hostTTL.ifPresent(ttl -> { - if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); - if (ttl.isNegative()) illegal("Host TTL cannot be negative"); - }); + hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); } public boolean isEmpty() { return this == empty; } @@ -189,8 +187,20 @@ public class DeploymentSpec { // to have environment, instance or region variants on those. public Optional<AthenzService> athenzService() { return athenzService; } - /** Cloud account set on the deployment root; see discussion for {@link #athenzService}. */ - public Optional<CloudAccount> cloudAccount() { return cloudAccount; } + /** The most specific Athenz service for the given arguments. */ + public Optional<AthenzService> athenzService(InstanceName instance, Environment environment, RegionName region) { + return instance(instance).flatMap(spec -> spec.athenzService(environment, region)) + .or(this::athenzService); + } + + /** The most specific Cloud account for the given arguments. */ + public CloudAccount cloudAccount(CloudName cloud, InstanceName instance, ZoneId zone) { + return instance(instance).map(spec -> spec.cloudAccounts(zone.environment(), zone.region())) + .orElse(cloudAccounts) + .getOrDefault(cloud, CloudAccount.empty); + } + + public Map<CloudName, CloudAccount> cloudAccounts() { return cloudAccounts; } /** * Additional host time-to-live for this application. Requires a custom cloud account to be set. @@ -198,6 +208,11 @@ public class DeploymentSpec { * allowed remain empty, before being deprovisioned. This is useful for applications which frequently * deploy to, e.g., test and staging zones, and want to avoid the delay of having to provision hosts. */ + public Optional<Duration> hostTTL(InstanceName instance, Environment environment, RegionName region) { + return instance(instance).flatMap(spec -> spec.hostTTL(environment, Optional.of(region))) + .or(this::hostTTL); + } + public Optional<Duration> hostTTL() { return hostTTL; } /** @@ -421,31 +436,28 @@ public class DeploymentSpec { private final boolean active; private final Optional<AthenzService> athenzService; private final Optional<String> testerFlavor; - private final Optional<CloudAccount> cloudAccount; + private final Map<CloudName, CloudAccount> cloudAccounts; private final Optional<Duration> hostTTL; public DeclaredZone(Environment environment) { - this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Map.of(), Optional.empty()); } public DeclaredZone(Environment environment, Optional<RegionName> region, boolean active, Optional<AthenzService> athenzService, Optional<String> testerFlavor, - Optional<CloudAccount> cloudAccount, Optional<Duration> hostTTL) { + Map<CloudName, CloudAccount> cloudAccounts, Optional<Duration> hostTTL) { if (environment != Environment.prod && region.isPresent()) illegal("Non-prod environments cannot specify a region"); if (environment == Environment.prod && region.isEmpty()) illegal("Prod environments must be specified with a region"); + 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.cloudAccount = Objects.requireNonNull(cloudAccount); + this.cloudAccounts = Map.copyOf(cloudAccounts); this.hostTTL = Objects.requireNonNull(hostTTL); - hostTTL.ifPresent(ttl -> { - if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); - if (ttl.isNegative()) illegal("Host TTL cannot be negative"); - }); } public Environment environment() { return environment; } @@ -458,11 +470,9 @@ public class DeploymentSpec { public Optional<String> testerFlavor() { return testerFlavor; } - public Optional<AthenzService> athenzService() { return athenzService; } + Optional<AthenzService> athenzService() { return athenzService; } - public Optional<CloudAccount> cloudAccount() { - return cloudAccount; - } + Map<CloudName, CloudAccount> cloudAccounts() { return cloudAccounts; } @Override public List<DeclaredZone> zones() { return List.of(this); } @@ -510,13 +520,10 @@ public class DeploymentSpec { private final RegionName region; private final Optional<Duration> hostTTL; - public DeclaredTest(RegionName region, Optional<CloudAccount> cloudAccount, Optional<Duration> hostTTL) { + public DeclaredTest(RegionName region, Optional<Duration> hostTTL) { this.region = Objects.requireNonNull(region); this.hostTTL = Objects.requireNonNull(hostTTL); - hostTTL.ifPresent(ttl -> { - if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); - if (ttl.isNegative()) illegal("Host TTL cannot be negative"); - }); + hostTTL.filter(Duration::isNegative).ifPresent(ttl -> illegal("Host TTL cannot be negative")); } @Override 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 43281c90efe..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 @@ -25,6 +25,7 @@ import com.yahoo.config.application.api.TimeWindow; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; @@ -165,7 +166,7 @@ public class DeploymentSpecXmlReader { optionalIntegerAttribute(majorVersionAttribute, root), stringAttribute(athenzDomainAttribute, root).map(AthenzDomain::from), stringAttribute(athenzServiceAttribute, root).map(AthenzService::from), - stringAttribute(cloudAccountAttribute, root).map(CloudAccount::from), + readCloudAccounts(root), stringAttribute(hostTTLAttribute, root).map(s -> toDuration(s, "empty host TTL")), applicationEndpoints, xmlForm, @@ -205,7 +206,7 @@ public class DeploymentSpecXmlReader { int maxIdleHours = getWithFallback(instanceElement, parentTag, upgradeTag, "max-idle-hours", Integer::parseInt, 8); List<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceElement, parentTag); Optional<AthenzService> athenzService = mostSpecificAttribute(instanceElement, athenzServiceAttribute).map(AthenzService::from); - Optional<CloudAccount> cloudAccount = readCloudAccount(instanceElement); + Map<CloudName, CloudAccount> cloudAccounts = readCloudAccounts(instanceElement); Optional<Duration> hostTTL = mostSpecificAttribute(instanceElement, hostTTLAttribute).map(s -> toDuration(s, "empty host TTL")); Notifications notifications = readNotifications(instanceElement, parentTag); @@ -235,7 +236,7 @@ public class DeploymentSpecXmlReader { changeBlockers, Optional.ofNullable(prodAttributes.get(globalServiceIdAttribute)), athenzService, - cloudAccount, + cloudAccounts, hostTTL, notifications, endpoints, @@ -277,10 +278,10 @@ public class DeploymentSpecXmlReader { case testTag: if (Stream.iterate(stepTag, Objects::nonNull, Node::getParentNode) .anyMatch(node -> prodTag.equals(node.getNodeName()))) { - return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()), readCloudAccount(stepTag), readHostTTL(stepTag))); // A production test + 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, readCloudAccount(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()) @@ -670,8 +671,13 @@ public class DeploymentSpecXmlReader { /** Returns the given non-blank attribute of tag as a string, if any */ private static Optional<String> stringAttribute(String attributeName, Element tag) { + return stringAttribute(attributeName, tag, true); + } + + /** Returns the given non-blank attribute of tag as a string, if any */ + private static Optional<String> stringAttribute(String attributeName, Element tag, boolean ignoreBlanks) { String value = tag.getAttribute(attributeName); - return Optional.of(value).filter(s -> !s.isBlank()); + return Optional.of(value).filter(s -> (tag.getAttributeNode(attributeName) != null && ! ignoreBlanks || ! s.isBlank())); } /** Returns the given non-blank attribute of tag or throw */ @@ -685,11 +691,23 @@ public class DeploymentSpecXmlReader { Optional<String> testerFlavor, Element regionTag) { return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())), readActive(regionTag), athenzService, testerFlavor, - readCloudAccount(regionTag), readHostTTL(regionTag)); - } - - private Optional<CloudAccount> readCloudAccount(Element tag) { - return mostSpecificAttribute(tag, cloudAccountAttribute).map(CloudAccount::from); + readCloudAccounts(regionTag), readHostTTL(regionTag)); + } + + private Map<CloudName, CloudAccount> readCloudAccounts(Element tag) { + return mostSpecificAttribute(tag, cloudAccountAttribute, false) + .map(value -> { + Map<CloudName, CloudAccount> accounts = new HashMap<>(); + for (String part : value.split(",")) { + CloudAccount account = CloudAccount.from(part); + accounts.merge(account.cloudName(), account, (o, n) -> { + throw illegal("both '" + o.account() + "' and '" + n.account() + "' " + + "are declared for cloud '" + o.cloudName() + "', in '" + value + "'"); + }); + } + return accounts; + }) + .orElse(Map.of()); } private Optional<Duration> readHostTTL(Element tag) { @@ -802,14 +820,19 @@ public class DeploymentSpecXmlReader { } /** Returns the given attribute from the given tag or its closest ancestor with the attribute. */ - private static Optional<String> mostSpecificAttribute(Element tag, String attributeName) { + private static Optional<String> mostSpecificAttribute(Element tag, String attributeName, boolean ignoreBlanks) { return Stream.iterate(tag, Objects::nonNull, Node::getParentNode) .filter(Element.class::isInstance) .map(Element.class::cast) - .flatMap(element -> stringAttribute(attributeName, element).stream()) + .flatMap(element -> stringAttribute(attributeName, element, ignoreBlanks).stream()) .findFirst(); } + /** Returns the given attribute from the given tag or its closest ancestor with the attribute. */ + private static Optional<String> mostSpecificAttribute(Element tag, String attributeName) { + return mostSpecificAttribute(tag, attributeName, true); + } + /** * Returns a string consisting of a number followed by "m", "h" or "d" to a duration given in that unit, * or zero duration if null or blank. @@ -851,7 +874,7 @@ public class DeploymentSpecXmlReader { } } - private static void illegal(String message) { + private static IllegalArgumentException illegal(String message) { throw new IllegalArgumentException(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 721c327e5d4..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 @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.Endpoint.Level; import com.yahoo.config.application.api.Endpoint.Target; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; @@ -25,6 +26,7 @@ import java.time.ZoneId; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -32,6 +34,8 @@ import java.util.stream.Collectors; 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.CloudName.GCP; import static com.yahoo.config.provision.Environment.dev; import static com.yahoo.config.provision.Environment.perf; import static com.yahoo.config.provision.Environment.prod; @@ -1757,18 +1761,19 @@ public class DeploymentSpecTest { public void cloudAccount() { String r = """ - <deployment version='1.0' cloud-account='100000000000'> + <deployment version='1.0' cloud-account='100000000000,gcp:foobar'> <instance id='alpha'> <prod cloud-account='800000000000'> <region>us-east-1</region> </prod> </instance> <instance id='beta' cloud-account='200000000000'> - <staging cloud-account='600000000000'/> + <staging cloud-account='gcp:barbaz'/> <perf cloud-account='700000000000'/> <prod> <region>us-west-1</region> <region cloud-account='default'>us-west-2</region> + <region cloud-account=''>us-west-3</region> </prod> </instance> <instance id='main'> @@ -1782,18 +1787,24 @@ public class DeploymentSpecTest { </deployment> """; DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(Optional.of(CloudAccount.from("100000000000")), spec.cloudAccount()); - assertCloudAccount("800000000000", spec.requireInstance("alpha"), prod, "us-east-1"); - assertCloudAccount("200000000000", spec.requireInstance("beta"), prod, "us-west-1"); - assertCloudAccount("600000000000", spec.requireInstance("beta"), staging, ""); - assertCloudAccount("700000000000", spec.requireInstance("beta"), perf, ""); - assertCloudAccount("200000000000", spec.requireInstance("beta"), dev, ""); - assertCloudAccount("300000000000", spec.requireInstance("main"), prod, "us-east-1"); - assertCloudAccount("100000000000", spec.requireInstance("main"), prod, "eu-west-1"); - assertCloudAccount("400000000000", spec.requireInstance("main"), dev, ""); - assertCloudAccount("500000000000", spec.requireInstance("main"), test, ""); - assertCloudAccount("100000000000", spec.requireInstance("main"), staging, ""); - assertCloudAccount("default", spec.requireInstance("beta"), prod, "us-west-2"); + assertEquals(Map.of(AWS, CloudAccount.from("100000000000"), + GCP, CloudAccount.from("gcp:foobar")), spec.cloudAccounts()); + assertCloudAccount("800000000000", spec, AWS, "alpha", prod, "us-east-1"); + assertCloudAccount("", spec, GCP, "alpha", prod, "us-east-1"); + assertCloudAccount("200000000000", spec, AWS, "beta", prod, "us-west-1"); + assertCloudAccount("", spec, AWS, "beta", staging, "default"); + assertCloudAccount("gcp:barbaz", spec, GCP, "beta", staging, "default"); + assertCloudAccount("700000000000", spec, AWS, "beta", perf, "default"); + assertCloudAccount("200000000000", spec, AWS, "beta", dev, "default"); + assertCloudAccount("300000000000", spec, AWS, "main", prod, "us-east-1"); + assertCloudAccount("100000000000", spec, AWS, "main", prod, "eu-west-1"); + assertCloudAccount("400000000000", spec, AWS, "main", dev, "default"); + assertCloudAccount("500000000000", spec, AWS, "main", test, "default"); + assertCloudAccount("100000000000", spec, AWS, "main", staging, "default"); + assertCloudAccount("default", spec, AWS, "beta", prod, "us-west-2"); + assertCloudAccount("", spec, GCP, "beta", prod, "us-west-2"); + assertCloudAccount("", spec, AWS, "beta", prod, "us-west-3"); + assertCloudAccount("", spec, GCP, "beta", prod, "us-west-3"); } @Test @@ -1827,7 +1838,7 @@ public class DeploymentSpecTest { </deployment> """; DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(Optional.of(CloudAccount.from("100000000000")), spec.cloudAccount()); + assertEquals(Map.of(AWS, CloudAccount.from("100000000000")), spec.cloudAccounts()); assertHostTTL(Duration.ofHours(1), spec, "alpha", test, null); assertHostTTL(Duration.ofHours(1), spec, "alpha", staging, null); @@ -1864,69 +1875,15 @@ public class DeploymentSpecTest { assertHostTTL(Duration.ofHours(1), spec, "nope", perf, null); assertHostTTL(Duration.ofHours(1), spec, "nope", prod, "us-east"); assertHostTTL(Duration.ofHours(1), spec, "nope", prod, "us-west"); - - assertEquals("Host TTL can only be specified with custom cloud accounts", - assertThrows(IllegalArgumentException.class, - () -> DeploymentSpec.fromXml(""" - <deployment version='1.0' empty-host-ttl='0m'> - <instance id='main'> - <prod> - <region>us-east-1</region> - </prod> - </instance> - </deployment> - """)) - .getMessage()); - - assertEquals("Host TTL can only be specified with custom cloud accounts", - assertThrows(IllegalArgumentException.class, - () -> DeploymentSpec.fromXml(""" - <deployment version='1.0'> - <instance id='main' empty-host-ttl='0m'> - <prod> - <region>us-east-1</region> - </prod> - </instance> - </deployment> - """)) - .getMessage()); - - assertEquals("Host TTL can only be specified with custom cloud accounts", - assertThrows(IllegalArgumentException.class, - () -> DeploymentSpec.fromXml(""" - <deployment version='1.0'> - <instance id='main'> - <prod empty-host-ttl='0m'> - <region>us-east-1</region> - </prod> - </instance> - </deployment> - """)) - .getMessage()); - - assertEquals("Host TTL can only be specified with custom cloud accounts", - assertThrows(IllegalArgumentException.class, - () -> DeploymentSpec.fromXml(""" - <deployment version='1.0'> - <instance id='main'> - <prod> - <region empty-host-ttl='0m'>us-east-1</region> - </prod> - </instance> - </deployment> - """)) - .getMessage()); - } - private void assertCloudAccount(String expected, DeploymentInstanceSpec instance, Environment environment, String region) { - assertEquals(Optional.of(expected).map(CloudAccount::from), instance.cloudAccount(environment, Optional.of(region).filter(s -> !s.isEmpty()).map(RegionName::from))); + private void assertCloudAccount(String expected, DeploymentSpec spec, CloudName cloud, String instance, Environment environment, String region) { + assertEquals(CloudAccount.from(expected), + spec.cloudAccount(cloud, InstanceName.from(instance), com.yahoo.config.provision.zone.ZoneId.from(environment, RegionName.from(region)))); } private void assertHostTTL(Duration expected, DeploymentSpec spec, String instance, Environment environment, String region) { - assertEquals(Optional.of(expected), spec.instance(InstanceName.from(instance)) - .flatMap(iSpec -> iSpec.hostTTL(environment, Optional.ofNullable(region).map(RegionName::from))) - .or(spec::hostTTL)); + assertEquals(Optional.of(expected), spec.hostTTL(InstanceName.from(instance), environment, region == null ? RegionName.defaultName() : RegionName.from(region))); } private static void assertInvalid(String deploymentSpec, String errorMessagePart) { 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 ed8074ddc61..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 @@ -18,6 +18,7 @@ import java.time.Instant; import java.time.ZoneId; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -25,6 +26,7 @@ import java.util.stream.Collectors; 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; @@ -744,10 +746,10 @@ public class DeploymentSpecWithoutInstanceTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); DeploymentInstanceSpec instance = spec.requireInstance("default"); - assertEquals(Optional.of(CloudAccount.from("012345678912")), spec.cloudAccount()); - assertEquals(Optional.of(CloudAccount.from("219876543210")), instance.cloudAccount(prod, Optional.of(RegionName.from("us-east-1")))); - assertEquals(Optional.of(CloudAccount.from("012345678912")), instance.cloudAccount(prod, Optional.of(RegionName.from("us-west-1")))); - assertEquals(Optional.of(CloudAccount.from("012345678912")), instance.cloudAccount(Environment.staging, Optional.empty())); + assertEquals(Map.of(AWS, CloudAccount.from("012345678912")), spec.cloudAccounts()); + assertEquals(Map.of(AWS, CloudAccount.from("219876543210")), instance.cloudAccounts(prod, RegionName.from("us-east-1"))); + assertEquals(Map.of(AWS, CloudAccount.from("012345678912")), instance.cloudAccounts(prod, RegionName.from("us-west-1"))); + assertEquals(Map.of(AWS, CloudAccount.from("012345678912")), instance.cloudAccounts(Environment.staging, RegionName.defaultName())); r = new StringReader( "<deployment version='1.0'>" + @@ -758,9 +760,9 @@ public class DeploymentSpecWithoutInstanceTest { "</deployment>" ); spec = DeploymentSpec.fromXml(r); - assertEquals(Optional.empty(), spec.cloudAccount()); - assertEquals(Optional.of(CloudAccount.from("219876543210")), spec.requireInstance("default").cloudAccount(prod, Optional.of(RegionName.from("us-east-1")))); - assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(prod, Optional.of(RegionName.from("us-west-1")))); + assertEquals(Map.of(), spec.cloudAccounts()); + assertEquals(Map.of(AWS, CloudAccount.from("219876543210")), spec.requireInstance("default").cloudAccounts(prod, RegionName.from("us-east-1"))); + assertEquals(Map.of(), spec.requireInstance("default").cloudAccounts(prod, RegionName.from("us-west-1"))); } @Test diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java index a9ee884e977..b4b3dccd440 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java @@ -2,6 +2,7 @@ package com.yahoo.config.model; import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.application.api.Bcp.Group; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.model.api.ModelContext; @@ -10,6 +11,7 @@ import com.yahoo.config.model.producer.AnyConfigProducer; import com.yahoo.config.model.producer.TreeConfigProducer; import com.yahoo.config.provision.ClusterInfo; import com.yahoo.config.provision.ClusterInfo.Builder; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.model.VespaModel; import java.time.Duration; @@ -76,19 +78,21 @@ public final class ConfigModelContext { /** Returns a cluster info builder pre-populated with info known in this context. */ public ClusterInfo.Builder clusterInfo() { DeploymentSpec spec = getApplicationPackage().getDeploymentSpec(); - var instance = spec.instance(properties().applicationId().instance()); ClusterInfo.Builder builder = new ClusterInfo.Builder(); - spec.hostTTL().ifPresent(builder::hostTTL); - if (instance.isEmpty()) return builder; - instance.get() - .hostTTL(deployState.zone().environment(), Optional.of(deployState.zone().region())) - .ifPresent(builder::hostTTL); - var maxDeadline = instance.get().bcp().groups().stream() - .filter(group -> group.memberRegions().contains(properties().zone().region())) - .map(group -> group.deadline()) - .min(Comparator.comparing(deadline -> deadline)) - .orElse(Duration.ofMinutes(0)); - return builder.bcpDeadline(maxDeadline); + spec.hostTTL(properties().applicationId().instance(), deployState.zone().environment(), deployState.zone().region()) + .ifPresent(ttl -> { + ZoneId zoneId = ZoneId.from(deployState.zone().environment(), deployState.zone().region()); + if (spec.cloudAccount(deployState.zone().cloud().name(), properties().applicationId().instance(), zoneId).isUnspecified()) + throw new IllegalArgumentException("deployment spec specifies host TTL for " + zoneId + + " but no cloud account is specified for this zone"); + }); + spec.instance(properties().applicationId().instance()) + .flatMap(instance -> instance.bcp().groups().stream() + .filter(group -> group.memberRegions().contains(properties().zone().region())) + .map(Group::deadline) + .min(Comparator.naturalOrder())) + .ifPresent(builder::bcpDeadline); + return builder; } /** 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 b0b9125603e..e9517e1e64d 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 @@ -1163,9 +1163,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { DeploymentSpec spec) { spec.athenzDomain() .ifPresent(domain -> { - AthenzService service = spec.instance(app.getApplicationId().instance()) - .flatMap(instanceSpec -> instanceSpec.athenzService(zone.environment(), zone.region())) - .or(spec::athenzService) + AthenzService service = spec.athenzService(app.getApplicationId().instance(), zone.environment(), zone.region()) .orElseThrow(() -> new IllegalArgumentException("Missing Athenz service configuration in instance '" + app.getApplicationId().instance() + "'")); String zoneDnsSuffix = zone.environment().value() + "-" + zone.region().value() + "." + athenzDnsSuffix; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 08a8440fbe2..eedc94c729c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -10,6 +10,7 @@ import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; @@ -694,24 +695,22 @@ public class ApplicationController { public Optional<CloudAccount> decideCloudAccountOf(DeploymentId deployment, DeploymentSpec spec) { ZoneId zoneId = deployment.zoneId(); - Optional<CloudAccount> requestedAccount = spec.instance(deployment.applicationId().instance()) - .flatMap(instanceSpec -> instanceSpec.cloudAccount(zoneId.environment(), - Optional.of(zoneId.region()))) - .or(spec::cloudAccount); - if (requestedAccount.isEmpty() || requestedAccount.get().isUnspecified()) { + CloudName cloud = controller.zoneRegistry().get(zoneId).getCloudName(); + CloudAccount requestedAccount = spec.cloudAccount(cloud, deployment.applicationId().instance(), deployment.zoneId()); + if (requestedAccount.isUnspecified()) return Optional.empty(); - } + TenantName tenant = deployment.applicationId().tenant(); Set<CloudAccount> tenantAccounts = accountsOf(tenant); - if (!tenantAccounts.contains(requestedAccount.get())) { - throw new IllegalArgumentException("Requested cloud account '" + requestedAccount.get().value() + + if ( ! tenantAccounts.contains(requestedAccount)) { + throw new IllegalArgumentException("Requested cloud account '" + requestedAccount.value() + "' is not valid for tenant '" + tenant + "'"); } - if ( ! controller.zoneRegistry().hasZone(zoneId, requestedAccount.get())) { + if ( ! controller.zoneRegistry().hasZone(zoneId, requestedAccount)) { throw new IllegalArgumentException("Zone " + zoneId + " is not configured in requested cloud account '" + - requestedAccount.get().value() + "'"); + requestedAccount.value() + "'"); } - return requestedAccount; + return Optional.of(requestedAccount); } private LockedApplication withoutDeletedDeployments(LockedApplication application, InstanceName instance) { @@ -948,7 +947,7 @@ public class ApplicationController { * @param applicationPackage application package * @param deployer principal initiating the deployment, possibly empty */ - public void verifyApplicationIdentityConfiguration(TenantName tenantName, Optional<InstanceName> instanceName, Optional<ZoneId> zoneId, ApplicationPackage applicationPackage, Optional<Principal> deployer) { + public void verifyApplicationIdentityConfiguration(TenantName tenantName, Optional<DeploymentId> deployment, ApplicationPackage applicationPackage, Optional<Principal> deployer) { Optional<AthenzDomain> identityDomain = applicationPackage.deploymentSpec().athenzDomain() .map(domain -> new AthenzDomain(domain.value())); if (identityDomain.isEmpty()) { @@ -969,14 +968,12 @@ public class ApplicationController { // Either the user is member of the domain admin role, or is given the "launch" privilege on the service. Optional<AthenzUser> athenzUser = getUser(deployer); if (athenzUser.isPresent()) { - // We only need to validate the root and instance in deployment.xml. Dev/perf entries are found at the instance level as well. - var zone = zoneId.orElseThrow(() -> new IllegalArgumentException("Unable to evaluate access, no zone provided in deployment")); - var serviceToLaunch = instanceName - .flatMap(instance -> applicationPackage.deploymentSpec().instance(instance)) - .flatMap(instanceSpec -> instanceSpec.athenzService(zone.environment(), zone.region())) - .or(() -> applicationPackage.deploymentSpec().athenzService()) - .map(service -> new AthenzService(identityDomain.get(), service.value())); - + // This is a direct deployment, and we need only validate what the configserver will actually launch. + DeploymentId id = deployment.orElseThrow(() -> new IllegalArgumentException("Unable to evaluate access, no zone provided in deployment")); + var serviceToLaunch = applicationPackage.deploymentSpec().athenzService(id.applicationId().instance(), + id.zoneId().environment(), + id.zoneId().region()) + .map(service -> new AthenzService(identityDomain.get(), service.value())); if (serviceToLaunch.isPresent()) { if ( ! ((AthenzFacade) accessControl).canLaunch(athenzUser.get(), serviceToLaunch.get()) && // launch privilege @@ -989,7 +986,7 @@ public class ApplicationController { } else { // This is a rare edge case where deployment.xml specifies athenz-service on each step, but not on the root. // It is undefined which service should be launched, so handle this as an error. - throw new IllegalArgumentException("Athenz domain configured, but no service defined for deployment to " + zone.value()); + throw new IllegalArgumentException("Athenz domain configured, but no service defined for deployment to " + id.zoneId().value()); } } else { // If this is a deployment pipeline, verify that the domain in deployment.xml is the same as the tenant domain. Access control is already validated before this step. 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 bd42587576d..0d8e7745f65 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 @@ -67,11 +67,10 @@ public class ApplicationPackageValidator { private void validateCloudAccounts(Application application, ApplicationPackage applicationPackage) { Set<CloudAccount> tenantAccounts = new TreeSet<>(controller.applications().accountsOf(application.id().tenant())); - Set<CloudAccount> declaredAccounts = new TreeSet<>(); - applicationPackage.deploymentSpec().cloudAccount().ifPresent(declaredAccounts::add); + Set<CloudAccount> declaredAccounts = new TreeSet<>(applicationPackage.deploymentSpec().cloudAccounts().values()); for (DeploymentInstanceSpec instance : applicationPackage.deploymentSpec().instances()) for (ZoneId zone : controller.zoneRegistry().zones().controllerUpgraded().ids()) - instance.cloudAccount(zone.environment(), Optional.of(zone.region())).ifPresent(declaredAccounts::add); + declaredAccounts.addAll(instance.cloudAccounts(zone.environment(), zone.region()).values()); declaredAccounts.removeIf(tenantAccounts::contains); declaredAccounts.removeIf(CloudAccount::isUnspecified); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java index 7d4cd6b8747..eceaae80cef 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec.Step; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; @@ -50,6 +51,7 @@ import java.util.jar.JarInputStream; import java.util.jar.Manifest; import java.util.regex.Pattern; +import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.of; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.production; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.staging; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.staging_setup; @@ -78,7 +80,7 @@ public class TestPackage { private final ApplicationPackageStream applicationPackageStream; private final X509Certificate certificate; - public TestPackage(Supplier<InputStream> inZip, boolean isPublicSystem, RunId id, Testerapp testerApp, + public TestPackage(Supplier<InputStream> inZip, boolean isPublicSystem, CloudName cloud, RunId id, Testerapp testerApp, DeploymentSpec spec, Instant certificateValidFrom, Duration certificateValidDuration) { KeyPair keyPair; if (certificateValidFrom != null) { @@ -130,7 +132,7 @@ public class TestPackage { testerApp))); entries.put(deploymentFile, - __ -> new ByteArrayInputStream(deploymentXml(id.tester(), id.application().instance(), id.type().zone(), spec))); + __ -> new ByteArrayInputStream(deploymentXml(id.tester(), id.application().instance(), cloud, id.type().zone(), spec))); if (certificate != null) { entries.put("artifacts/key", __ -> new ByteArrayInputStream(KeyUtils.toPem(keyPair.getPrivate()).getBytes(UTF_8))); @@ -296,17 +298,18 @@ public class TestPackage { } /** Returns a dummy deployment xml which sets up the service identity for the tester, if present. */ - static byte[] deploymentXml(TesterId id, InstanceName instance, ZoneId zone, DeploymentSpec original) { + static byte[] deploymentXml(TesterId id, InstanceName instance, CloudName cloud, ZoneId zone, DeploymentSpec original) { Optional<AthenzDomain> athenzDomain = original.athenzDomain(); Optional<AthenzService> athenzService = original.requireInstance(instance) .athenzService(zone.environment(), zone.region()); - Optional<CloudAccount> cloudAccount = original.requireInstance(instance) - .cloudAccount(zone.environment(), Optional.of(zone.region())); - Optional<Duration> hostTTL = zone.environment().isProduction() - ? original.requireInstance(instance) - .steps().stream().filter(step -> step.isTest() && step.concerns(zone.environment(), Optional.of(zone.region()))) - .findFirst().flatMap(Step::hostTTL) - : original.requireInstance(instance).hostTTL(zone.environment(), Optional.of(zone.region())); + Optional<CloudAccount> cloudAccount = Optional.of(original.cloudAccount(cloud, instance, zone)) + .filter(account -> ! account.isUnspecified()); + Optional<Duration> hostTTL = (zone.environment().isProduction() + ? original.requireInstance(instance) + .steps().stream().filter(step -> step.isTest() && step.concerns(zone.environment(), Optional.of(zone.region()))) + .findFirst().flatMap(Step::hostTTL) + : original.requireInstance(instance).hostTTL(zone.environment(), Optional.of(zone.region()))) + .filter(__ -> cloudAccount.isPresent()); String deploymentSpec = "<?xml version='1.0' encoding='UTF-8'?>\n" + "<deployment version='1.0'" + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 1441cc7a44e..59ebe0d07d3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -912,6 +912,7 @@ public class InternalStepRunner implements StepRunner { TestPackage testPackage = new TestPackage(() -> controller.applications().applicationStore().streamTester(id.application().tenant(), id.application().application(), revision), controller.system().isPublic(), + controller.zoneRegistry().get(id.type().zone()).getCloudName(), id, controller.controllerConfig().steprunner().testerapp(), spec, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 28b048613ef..4324b98acba 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -2436,8 +2436,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get(APPLICATION_ZIP)); controller.applications().verifyApplicationIdentityConfiguration(id.tenant(), - Optional.of(id.instance()), - Optional.of(type.zone()), + Optional.of(new DeploymentId(id, type.zone())), applicationPackage, Optional.of(requireUserPrincipal(request))); @@ -3079,7 +3078,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { controller.applications().verifyApplicationIdentityConfiguration(tenantName, Optional.empty(), - Optional.empty(), applicationPackage, Optional.of(requireUserPrincipal(request))); 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 8e3ccca0dd1..9efdee28063 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 @@ -72,7 +72,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static com.yahoo.config.provision.SystemName.main; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.devAwsUsEast2a; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.devUsEast1; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionAwsUsEast1a; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsEast3; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsWest1; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.stagingTest; @@ -1466,8 +1468,8 @@ public class ControllerTest { @Test void testCloudAccount() { DeploymentContext context = tester.newDeploymentContext(); - ZoneId devZone = devUsEast1.zone(); - ZoneId prodZone = productionUsWest1.zone(); + ZoneId devZone = devAwsUsEast2a.zone(); + ZoneId prodZone = productionAwsUsEast1a.zone(); String cloudAccount = "aws:012345678912"; var applicationPackage = new ApplicationPackageBuilder() .cloudAccount(cloudAccount) @@ -1481,25 +1483,24 @@ public class ControllerTest { .getMessage()); assertEquals("cloud accounts [aws:012345678912] are not valid for tenant tenant", assertThrows(IllegalArgumentException.class, - () -> context.runJob(devUsEast1, applicationPackage)) + () -> context.runJob(devZone, applicationPackage)) .getMessage()); // Deployment fails because zone is not configured in requested cloud account tester.controllerTester().flagSource().withListFlag(PermanentFlags.CLOUD_ACCOUNTS.id(), List.of(cloudAccount), String.class); - assertEquals("Zone test.us-east-1 is not configured in requested cloud account 'aws:012345678912'", + assertEquals("Zone prod.aws-us-east-1a is not configured in requested cloud account 'aws:012345678912'", assertThrows(IllegalArgumentException.class, () -> context.submit(applicationPackage)) .getMessage()); - assertEquals("Zone dev.us-east-1 is not configured in requested cloud account 'aws:012345678912'", + + context.runJob(devUsEast1, applicationPackage); // OK, because no special account is used. + assertEquals("Zone dev.aws-us-east-2a is not configured in requested cloud account 'aws:012345678912'", assertThrows(IllegalArgumentException.class, - () -> context.runJob(devUsEast1, applicationPackage)) + () -> context.runJob(devZone, applicationPackage)) .getMessage()); // Deployment to prod succeeds once all zones are configured in requested account - tester.controllerTester().zoneRegistry().configureCloudAccount(CloudAccount.from(cloudAccount), - systemTest.zone(), - stagingTest.zone(), - prodZone); + tester.controllerTester().zoneRegistry().configureCloudAccount(CloudAccount.from(cloudAccount), prodZone); context.submit(applicationPackage).deploy(); // Dev zone is added as a configured zone and deployment succeeds @@ -1507,17 +1508,22 @@ public class ControllerTest { context.runJob(devZone, applicationPackage); // All deployments use the custom account - for (var zoneId : List.of(systemTest.zone(), stagingTest.zone(), devZone, prodZone)) { + for (var zoneId : List.of(devZone, prodZone)) { assertEquals(cloudAccount, tester.controllerTester().configServer() .cloudAccount(context.deploymentIdIn(zoneId)) .get().value()); } + // Tests are run in the default cloud, however, where the default cloud account is used + for (var zoneId : List.of(systemTest.zone(), stagingTest.zone())) { + assertEquals(Optional.empty(), tester.controllerTester().configServer() + .cloudAccount(context.deploymentIdIn(zoneId))); + } } @Test void testCloudAccountWithDefaultOverride() { var context = tester.newDeploymentContext(); - var prodZone1 = productionUsEast3.zone(); + var prodZone1 = productionAwsUsEast1a.zone(); var prodZone2 = productionUsWest1.zone(); var cloudAccount = "aws:012345678912"; var application = new ApplicationPackageBuilder() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java index cfe740069db..e7109b551ed 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.application.pkg; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.zone.ZoneId; @@ -24,6 +25,9 @@ import java.util.Set; import java.util.jar.JarOutputStream; import java.util.zip.ZipEntry; +import static com.yahoo.config.provision.CloudName.AWS; +import static com.yahoo.config.provision.CloudName.DEFAULT; +import static com.yahoo.config.provision.CloudName.GCP; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.production; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.staging; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.staging_setup; @@ -126,6 +130,7 @@ public class TestPackageTest { "artifacts/key", new byte[0])); TestPackage bundleTests = new TestPackage(() -> new ByteArrayInputStream(bundleZip), false, + CloudName.DEFAULT, new RunId(ApplicationId.defaultId(), JobType.dev("abc"), 123), new Testerapp.Builder().tenantCdBundle("foo").runtimeProviderClass("bar").build(), DeploymentSpec.fromXml(""" @@ -150,7 +155,7 @@ public class TestPackageTest { @Test void generates_correct_deployment_spec() { DeploymentSpec spec = DeploymentSpec.fromXml(""" - <deployment version='1.0' athenz-domain='domain' athenz-service='service' cloud-account='123123123123' empty-host-ttl='1h'> + <deployment version='1.0' athenz-domain='domain' athenz-service='service' cloud-account='123123123123,gcp:foobar' empty-host-ttl='1h'> <test empty-host-ttl='1d' /> <staging cloud-account='aws:321321321321'/> <prod> @@ -163,19 +168,31 @@ public class TestPackageTest { </prod> </deployment> """); - verifyAttributes("aws:123123123123", 1440, ZoneId.from("test", "us-east-1"), spec); - verifyAttributes("aws:321321321321", 60, ZoneId.from("staging", "us-east-2"), spec); - verifyAttributes("aws:123123123123", 60, ZoneId.from("prod", "us-east-3"), spec); - verifyAttributes("aws:123123123123", 0, ZoneId.from("prod", "us-west-1"), spec); - verifyAttributes("aws:123123123123", 60, ZoneId.from("prod", "us-central-1"), spec); + verifyAttributes("", 0, DEFAULT, ZoneId.from("test", "us-east-1"), spec); + verifyAttributes("", 0, DEFAULT, ZoneId.from("staging", "us-east-2"), spec); + verifyAttributes("", 0, DEFAULT, ZoneId.from("prod", "us-east-3"), spec); + verifyAttributes("", 0, DEFAULT, ZoneId.from("prod", "us-west-1"), spec); + verifyAttributes("", 0, DEFAULT, ZoneId.from("prod", "us-central-1"), spec); + + verifyAttributes("aws:123123123123", 1440, AWS, ZoneId.from("test", "us-east-1"), spec); + verifyAttributes("aws:321321321321", 60, AWS, ZoneId.from("staging", "us-east-2"), spec); + verifyAttributes("aws:123123123123", 60, AWS, ZoneId.from("prod", "us-east-3"), spec); + verifyAttributes("aws:123123123123", 0, AWS, ZoneId.from("prod", "us-west-1"), spec); + verifyAttributes("aws:123123123123", 60, AWS, ZoneId.from("prod", "us-central-1"), spec); + + verifyAttributes("gcp:foobar", 1440, GCP, ZoneId.from("test", "us-east-1"), spec); + verifyAttributes("", 0, GCP, ZoneId.from("staging", "us-east-2"), spec); + verifyAttributes("gcp:foobar", 60, GCP, ZoneId.from("prod", "us-east-3"), spec); + verifyAttributes("gcp:foobar", 0, GCP, ZoneId.from("prod", "us-west-1"), spec); + verifyAttributes("gcp:foobar", 60, GCP, ZoneId.from("prod", "us-central-1"), spec); } - private void verifyAttributes(String expectedAccount, int expectedTTL, ZoneId zone, DeploymentSpec spec) { + private void verifyAttributes(String expectedAccount, int expectedTTL, CloudName cloud, ZoneId zone, DeploymentSpec spec) { assertEquals("<?xml version='1.0' encoding='UTF-8'?>\n" + - "<deployment version='1.0' athenz-domain='domain' athenz-service='service' " + - "cloud-account='" + expectedAccount + "' empty-host-ttl='" + expectedTTL + "m'> " + + "<deployment version='1.0' athenz-domain='domain' athenz-service='service'" + + (expectedAccount.isEmpty() ? "" : " cloud-account='" + expectedAccount + "' empty-host-ttl='" + expectedTTL + "m'") + "> " + "<instance id='default-t' /></deployment>", - new String(TestPackage.deploymentXml(TesterId.of(ApplicationId.defaultId()), InstanceName.defaultName(), zone, spec))); + new String(TestPackage.deploymentXml(TesterId.of(ApplicationId.defaultId()), InstanceName.defaultName(), cloud, zone, spec))); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index 76cf7c9a75c..e6a9014df94 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -71,7 +71,7 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry this.zones = List.of(ZoneApiMock.fromId("test.us-east-1"), ZoneApiMock.fromId("staging.us-east-3"), ZoneApiMock.fromId("dev.us-east-1"), - ZoneApiMock.fromId("dev.aws-us-east-2a"), + ZoneApiMock.newBuilder().withId("dev.aws-us-east-2a").withCloud("aws").build(), ZoneApiMock.fromId("perf.us-east-3"), ZoneApiMock.newBuilder().withId("prod.aws-us-east-1a").withCloud("aws").build(), ZoneApiMock.newBuilder().withId("prod.aws-us-east-1b").withCloud("aws").build(), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/responses/recursion/environment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/responses/recursion/environment.json index 2d4553978c6..3b085162393 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/responses/recursion/environment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/responses/recursion/environment.json @@ -25,11 +25,11 @@ "changedAt": 0 }, { - "routingMethod": "sharedLayer4", + "routingMethod": "exclusive", "environment": "dev", "region": "aws-us-east-2a", "status": "in", - "agent": "operator", + "agent": "system", "changedAt": 0 }, { diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp index 77e8532f0d2..e8534e3f299 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.cpp +++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp @@ -36,6 +36,7 @@ MessageTracker::MessageTracker(const framework::MilliSecTimer & timer, ThrottleToken throttle_token) : MessageTracker(timer, env, replySender, true, std::move(bucketLock), std::move(msg), std::move(throttle_token)) {} + MessageTracker::MessageTracker(const framework::MilliSecTimer & timer, const PersistenceUtil & env, MessageSender & replySender, @@ -90,7 +91,7 @@ MessageTracker::sendReply() { if (count_result_as_failure()) { _env._metrics.failedOperations.inc(); } - vespalib::duration duration = vespalib::from_s(_timer.getElapsedTimeAsDouble()/1000.0); + vespalib::duration duration = _timer.getElapsedTime(); if (duration >= WARN_ON_SLOW_OPERATIONS) { LOGBT(warning, _msg->getType().toString(), "Slow processing of message %s. Processing time: %1.1f s (>=%1.1f s)", |