summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMartin Polden <mpolden@mpolden.no>2022-09-07 11:54:54 +0200
committerMartin Polden <mpolden@mpolden.no>2022-09-12 09:27:42 +0200
commit89dd1b69ed6d036715969bbe2d1c28d20c731d43 (patch)
tree1fdc5c966907496746c1394d825cc549d9f40858
parent939eb860022dde6689c1dabae11ff640d529e9a1 (diff)
Allow cloud-account attribute on non-production elements
-rw-r--r--config-model-api/abi-spec.json2
-rw-r--r--config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java5
-rw-r--r--config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java6
-rw-r--r--config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java15
-rw-r--r--config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java20
-rw-r--r--config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java10
-rw-r--r--config-model/src/main/resources/schema/deployment.rnc10
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java3
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java4
9 files changed, 51 insertions, 24 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json
index 672ad4071d6..7e17cd0f600 100644
--- a/config-model-api/abi-spec.json
+++ b/config-model-api/abi-spec.json
@@ -208,7 +208,7 @@
"public boolean canUpgradeAt(java.time.Instant)",
"public boolean canChangeRevisionAt(java.time.Instant)",
"public java.util.Optional athenzService(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)",
- "public java.util.Optional cloudAccount(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)",
+ "public java.util.Optional cloudAccount(com.yahoo.config.provision.Environment, java.util.Optional)",
"public com.yahoo.config.application.api.Notifications notifications()",
"public java.util.List endpoints()",
"public boolean deploysTo(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)",
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 8cb70d50a59..cd20b5b8910 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
@@ -226,10 +226,9 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
}
/** Returns the cloud account to use for given environment and region, if any */
- public Optional<CloudAccount> cloudAccount(Environment environment, RegionName region) {
- if (!environment.isProduction()) return Optional.empty();
+ public Optional<CloudAccount> cloudAccount(Environment environment, Optional<RegionName> region) {
return zones().stream()
- .filter(zone -> zone.concerns(environment, Optional.of(region)))
+ .filter(zone -> zone.concerns(environment, region))
.findFirst()
.flatMap(DeploymentSpec.DeclaredZone::cloudAccount)
.or(() -> cloudAccount);
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 c4fec6e668e..ffdceaf7567 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
@@ -376,8 +376,6 @@ public class DeploymentSpec {
illegal("Non-prod environments cannot specify a region");
if (environment == Environment.prod && region.isEmpty())
illegal("Prod environments must be specified with a region");
- if (environment != Environment.prod && cloudAccount.isPresent())
- illegal("Non-prod environments cannot specify cloud account");
this.environment = Objects.requireNonNull(environment);
this.region = Objects.requireNonNull(region);
this.active = active;
@@ -403,7 +401,7 @@ public class DeploymentSpec {
}
@Override
- public List<DeclaredZone> zones() { return Collections.singletonList(this); }
+ public List<DeclaredZone> zones() { return List.of(this); }
@Override
public boolean concerns(Environment environment, Optional<RegionName> region) {
@@ -492,7 +490,7 @@ public class DeploymentSpec {
public List<DeclaredZone> zones() {
return steps.stream()
.flatMap(step -> step.zones().stream())
- .collect(Collectors.toUnmodifiableList());
+ .toList();
}
@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 d08b6c46943..c98e429a52b 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
@@ -57,6 +57,8 @@ public class DeploymentSpecXmlReader {
private static final String instanceTag = "instance";
private static final String testTag = "test";
private static final String stagingTag = "staging";
+ private static final String devTag = "dev";
+ private static final String perfTag = "perf";
private static final String upgradeTag = "upgrade";
private static final String blockChangeTag = "block-change";
private static final String prodTag = "prod";
@@ -234,11 +236,12 @@ public class DeploymentSpecXmlReader {
case testTag:
if (Stream.iterate(stepTag, Objects::nonNull, Node::getParentNode)
.anyMatch(node -> prodTag.equals(node.getNodeName()))) {
+ // A production test
return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim())));
}
- return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, Optional.empty()));
- case stagingTag:
- return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, Optional.empty()));
+ return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag)));
+ case devTag, perfTag, stagingTag:
+ return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(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).stream())
@@ -433,7 +436,11 @@ public class DeploymentSpecXmlReader {
Optional<String> testerFlavor, Element regionTag) {
return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())),
readActive(regionTag), athenzService, testerFlavor,
- stringAttribute(cloudAccountAttribute, regionTag).map(CloudAccount::new));
+ readCloudAccount(regionTag));
+ }
+
+ private Optional<CloudAccount> readCloudAccount(Element tag) {
+ return stringAttribute(cloudAccountAttribute, tag).map(CloudAccount::new);
}
private Optional<String> readGlobalServiceId(Element environmentTag) {
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 7f0e9b4cae8..6ffb4d2f1e0 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
@@ -1515,11 +1515,15 @@ public class DeploymentSpecTest {
StringReader r = new StringReader(
"<deployment version='1.0' cloud-account='100000000000'>" +
" <instance id='beta' cloud-account='200000000000'>" +
+ " <staging cloud-account='600000000000'/>" +
+ " <perf cloud-account='700000000000'/>" +
" <prod>" +
" <region>us-west-1</region>" +
" </prod>" +
" </instance>" +
" <instance id='main'>" +
+ " <test cloud-account='500000000000'/>" +
+ " <dev cloud-account='400000000000'/>" +
" <prod>" +
" <region cloud-account='300000000000'>us-east-1</region>" +
" <region>eu-west-1</region>" +
@@ -1528,9 +1532,19 @@ public class DeploymentSpecTest {
"</deployment>"
);
DeploymentSpec spec = DeploymentSpec.fromXml(r);
- assertEquals(Optional.of(new CloudAccount("200000000000")), spec.requireInstance("beta").cloudAccount(Environment.prod, RegionName.from("us-west-1")));
- assertEquals(Optional.of(new CloudAccount("300000000000")), spec.requireInstance("main").cloudAccount(Environment.prod, RegionName.from("us-east-1")));
- assertEquals(Optional.of(new CloudAccount("100000000000")), spec.requireInstance("main").cloudAccount(Environment.prod, RegionName.from("eu-west-1")));
+ assertCloudAccount("200000000000", spec.requireInstance("beta"), Environment.prod, "us-west-1");
+ assertCloudAccount("600000000000", spec.requireInstance("beta"), Environment.staging, "");
+ assertCloudAccount("700000000000", spec.requireInstance("beta"), Environment.perf, "");
+ assertCloudAccount("200000000000", spec.requireInstance("beta"), Environment.dev, "");
+ assertCloudAccount("300000000000", spec.requireInstance("main"), Environment.prod, "us-east-1");
+ assertCloudAccount("100000000000", spec.requireInstance("main"), Environment.prod, "eu-west-1");
+ assertCloudAccount("400000000000", spec.requireInstance("main"), Environment.dev, "");
+ assertCloudAccount("500000000000", spec.requireInstance("main"), Environment.test, "");
+ assertCloudAccount("100000000000", spec.requireInstance("main"), Environment.staging, "");
+ }
+
+ private void assertCloudAccount(String expected, DeploymentInstanceSpec instance, Environment environment, String region) {
+ assertEquals(Optional.of(expected).map(CloudAccount::new), instance.cloudAccount(environment, Optional.of(region).filter(s -> !s.isEmpty()).map(RegionName::from)));
}
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 1232f700fee..4acf0c692fb 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
@@ -715,9 +715,9 @@ public class DeploymentSpecWithoutInstanceTest {
);
DeploymentSpec spec = DeploymentSpec.fromXml(r);
DeploymentInstanceSpec instance = spec.requireInstance("default");
- assertEquals(Optional.of(new CloudAccount("219876543210")), instance.cloudAccount(Environment.prod, RegionName.from("us-east-1")));
- assertEquals(Optional.of(new CloudAccount("012345678912")), instance.cloudAccount(Environment.prod, RegionName.from("us-west-1")));
- assertEquals(Optional.empty(), instance.cloudAccount(Environment.staging, RegionName.defaultName()));
+ assertEquals(Optional.of(new CloudAccount("219876543210")), instance.cloudAccount(Environment.prod, Optional.of(RegionName.from("us-east-1"))));
+ assertEquals(Optional.of(new CloudAccount("012345678912")), instance.cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1"))));
+ assertEquals(Optional.of(new CloudAccount("012345678912")), instance.cloudAccount(Environment.staging, Optional.empty()));
r = new StringReader(
"<deployment version='1.0'>" +
@@ -728,8 +728,8 @@ public class DeploymentSpecWithoutInstanceTest {
"</deployment>"
);
spec = DeploymentSpec.fromXml(r);
- assertEquals(Optional.of(new CloudAccount("219876543210")), spec.requireInstance("default").cloudAccount(Environment.prod, RegionName.from("us-east-1")));
- assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(Environment.prod, RegionName.from("us-west-1")));
+ assertEquals(Optional.of(new CloudAccount("219876543210")), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-east-1"))));
+ assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1"))));
}
private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) {
diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc
index 251134c91e8..b95c9539750 100644
--- a/config-model/src/main/resources/schema/deployment.rnc
+++ b/config-model/src/main/resources/schema/deployment.rnc
@@ -24,6 +24,8 @@ StepExceptInstance =
Endpoints? &
Test? &
Staging? &
+ Dev? &
+ Perf? &
Prod*
PrimitiveStep =
@@ -101,6 +103,14 @@ Staging = element staging {
text
}
+Dev = element dev {
+ attribute cloud-account { xsd:string }?
+}
+
+Perf = element perf {
+ attribute cloud-account { xsd:string }?
+}
+
Prod = element prod {
attribute global-service-id { text }? &
attribute athenz-service { xsd:string }? &
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 cb5bef81890..64e2c20e68c 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
@@ -59,7 +59,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValid
import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade;
import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificates;
import com.yahoo.vespa.hosted.controller.concurrent.Once;
-import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus;
import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
import com.yahoo.vespa.hosted.controller.deployment.JobStatus;
import com.yahoo.vespa.hosted.controller.deployment.Run;
@@ -632,7 +631,7 @@ public class ApplicationController {
.collect(toList());
Optional<CloudAccount> cloudAccount = applicationPackage.deploymentSpec()
.instance(application.instance())
- .flatMap(spec -> spec.cloudAccount(zone.environment(), zone.region()));
+ .flatMap(spec -> spec.cloudAccount(zone.environment(), Optional.of(zone.region())));
ConfigServer.PreparedApplication preparedApplication =
configServer.deploy(new DeploymentData(application, zone, applicationPackage.zippedContent(), platform,
endpoints, endpointCertificateMetadata, dockerImageRepo, domain,
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 5a131ba4a29..cb48b4aa621 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
@@ -98,7 +98,7 @@ public class ApplicationPackageValidator {
if (!controller.zoneRegistry().hasZone(ZoneId.from(environment, region))) {
throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!");
}
- Optional<CloudAccount> cloudAccount = spec.cloudAccount(environment, region);
+ Optional<CloudAccount> cloudAccount = spec.cloudAccount(environment, zone.region());
if (cloudAccount.isPresent() && !controller.zoneRegistry().hasZone(ZoneId.from(environment, region), cloudAccount.get())) {
throw new IllegalArgumentException("Zone " + zone + " in deployment spec is not configured for " +
"use in cloud account '" + cloudAccount.get().value() +
@@ -195,7 +195,7 @@ public class ApplicationPackageValidator {
for (var spec : deploymentSpec.instances()) {
for (var zone : spec.zones()) {
if (!zone.environment().isProduction()) continue;
- Optional<CloudAccount> cloudAccount = spec.cloudAccount(zone.environment(), zone.region().get());
+ Optional<CloudAccount> cloudAccount = spec.cloudAccount(zone.environment(), zone.region());
if (cloudAccount.isEmpty()) continue;
if (validAccounts.contains(cloudAccount.get())) continue;
throw new IllegalArgumentException("Cloud account '" + cloudAccount.get().value() +