diff options
13 files changed, 304 insertions, 128 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index d08cda06e5d..affc94e2bcb 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -373,9 +373,10 @@ "public java.lang.String endpointId()", "public java.lang.String containerId()", "public java.util.Set regions()", + "public com.yahoo.config.application.api.Endpoint withRegions(java.util.Set)", "public boolean equals(java.lang.Object)", "public int hashCode()", - "public com.yahoo.config.application.api.Endpoint withRegions(java.util.Set)" + "public java.lang.String toString()" ], "fields": [] }, @@ -518,7 +519,8 @@ "public static final enum com.yahoo.config.application.api.ValidationId configModelVersionMismatch", "public static final enum com.yahoo.config.application.api.ValidationId skipOldConfigModels", "public static final enum com.yahoo.config.application.api.ValidationId forceAutomaticTenantUpgradeTests", - "public static final enum com.yahoo.config.application.api.ValidationId accessControl" + "public static final enum com.yahoo.config.application.api.ValidationId accessControl", + "public static final enum com.yahoo.config.application.api.ValidationId globalEndpointChange" ] }, "com.yahoo.config.application.api.ValidationOverrides$Allow": { diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java index e47dcd78219..99cb07f3104 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java @@ -60,6 +60,10 @@ public class Endpoint { return regions; } + public Endpoint withRegions(Set<String> regions) { + return new Endpoint(endpointId, containerId, regions); + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -75,7 +79,10 @@ public class Endpoint { return Objects.hash(endpointId, containerId, regions); } - public Endpoint withRegions(Set<String> regions) { - return new Endpoint(endpointId, containerId, regions); + @Override + public String toString() { + return "endpoint '" + endpointId() + "' (cluster " + containerId + ") -> " + + regions.stream().map(RegionName::value).sorted().collect(Collectors.joining(", ")); } + } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java index 65dc264eb8a..35ece71a72e 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java @@ -22,7 +22,8 @@ public enum ValidationId { configModelVersionMismatch("config-model-version-mismatch"), // Internal use skipOldConfigModels("skip-old-config-models"), // Internal use forceAutomaticTenantUpgradeTests("force-automatic-tenant-upgrade-test"), // Internal use - accessControl("access-control"); // Internal use, used in zones where there should be no access-control + accessControl("access-control"), // Internal use, used in zones where there should be no access-control + globalEndpointChange("global-endpoint-change"); // Changing global endpoints private final String id; 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 d118239d6a7..703fc71dc31 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 @@ -49,9 +49,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.application.ApplicationPackageValidator; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; -import com.yahoo.vespa.hosted.controller.application.DeploymentSpecValidator; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.JobList; @@ -130,7 +130,7 @@ public class ApplicationController { private final Clock clock; private final DeploymentTrigger deploymentTrigger; private final BooleanFlag provisionApplicationCertificate; - private final DeploymentSpecValidator deploymentSpecValidator; + private final ApplicationPackageValidator applicationPackageValidator; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, RotationsConfig rotationsConfig, @@ -148,7 +148,7 @@ public class ApplicationController { rotationRepository = new RotationRepository(rotationsConfig, this, curator); deploymentTrigger = new DeploymentTrigger(controller, controller.serviceRegistry().buildService(), clock); provisionApplicationCertificate = Flags.PROVISION_APPLICATION_CERTIFICATE.bindTo(controller.flagSource()); - deploymentSpecValidator = new DeploymentSpecValidator(controller); + applicationPackageValidator = new ApplicationPackageValidator(controller); // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { @@ -385,11 +385,6 @@ public class ApplicationController { validateRun(application.get(), instance, zone, platformVersion, applicationVersion); } - if (zone.environment().isProduction()) // Assign and register endpoints - application = withRotation(applicationPackage.deploymentSpec(), application, instance); - - endpoints = registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone); - if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { // Provisions a new certificate if missing applicationCertificate = getApplicationCertificate(application.get().require(instance)); @@ -401,8 +396,13 @@ public class ApplicationController { if ( ! preferOldestVersion && ! application.get().internal() && ! zone.environment().isManuallyDeployed()) { - storeWithUpdatedConfig(application, applicationPackage); + application = storeWithUpdatedConfig(application, applicationPackage); } + + if (zone.environment().isProduction()) // Assign and register endpoints + application = withRotation(applicationPackage.deploymentSpec(), application, instance); + + endpoints = registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone); } // Release application lock while doing the deployment, which is a lengthy task. // Carry out deployment without holding the application lock. @@ -454,8 +454,8 @@ public class ApplicationController { } /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */ - public void storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { - deploymentSpecValidator.validate(applicationPackage.deploymentSpec()); + public LockedApplication storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { + applicationPackageValidator.validate(application.get(), applicationPackage, clock.instant()); application = application.with(applicationPackage.deploymentSpec()); application = application.with(applicationPackage.validationOverrides()); @@ -471,6 +471,7 @@ public class ApplicationController { application = application.with(instanceName, instance -> withoutUnreferencedDeploymentJobs(deploymentSpec, instance)); } store(application); + return application; } /** Deploy a system application to given zone */ @@ -598,7 +599,7 @@ public class ApplicationController { // as the certificate provider requires the first CN to be < 64 characters long. endpointDnsNames.add(Endpoint.createHashedCn(applicationId, controller.system())); - var globalDefaultEndpoint = Endpoint.of(applicationId).named(EndpointId.default_()); + var globalDefaultEndpoint = Endpoint.of(applicationId).named(EndpointId.defaultId()); var rotationEndpoints = Endpoint.of(applicationId).wildcard(); var zoneLocalEndpoints = controller.zoneRegistry().zones().directlyRouted().zones().stream().flatMap(zone -> Stream.of( diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java new file mode 100644 index 00000000000..5ee269f8448 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java @@ -0,0 +1,150 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application; + +import com.yahoo.config.application.api.DeploymentInstanceSpec; +import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.application.api.Endpoint; +import com.yahoo.config.application.api.ValidationId; +import com.yahoo.config.application.api.ValidationOverrides; +import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * This contains validators for a {@link ApplicationPackage} that depend on a {@link Controller} to perform validation. + * + * @author mpolden + */ +public class ApplicationPackageValidator { + + private final Controller controller; + + public ApplicationPackageValidator(Controller controller) { + this.controller = Objects.requireNonNull(controller, "controller must be non-null"); + } + + /** + * Validate the given application package + * + * @throws IllegalArgumentException if any validations fail + */ + public void validate(Application application, ApplicationPackage applicationPackage, Instant instant) { + validateSteps(applicationPackage.deploymentSpec()); + validateEndpointRegions(applicationPackage.deploymentSpec()); + validateEndpointChange(application, applicationPackage, instant); + } + + /** Verify that each of the production zones listed in the deployment spec exist in this system */ + private void validateSteps(DeploymentSpec deploymentSpec) { + new DeploymentSteps(deploymentSpec, controller::system).jobs(); + deploymentSpec.instances().stream().flatMap(instance -> instance.zones().stream()) + .filter(zone -> zone.environment() == Environment.prod) + .forEach(zone -> { + if ( ! controller.zoneRegistry().hasZone(ZoneId.from(zone.environment(), + zone.region().orElse(null)))) { + throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); + } + }); + } + + /** Verify that no single endpoint contains regions in different clouds */ + private void validateEndpointRegions(DeploymentSpec deploymentSpec) { + for (var instance : deploymentSpec.instances()) { + for (var endpoint : instance.endpoints()) { + var clouds = new HashSet<CloudName>(); + for (var region : endpoint.regions()) { + for (ZoneApi zone : controller.zoneRegistry().zones().all().in(region).zones()) { + clouds.add(zone.getCloudName()); + } + } + if (clouds.size() != 1) { + throw new IllegalArgumentException("Endpoint '" + endpoint.endpointId() + "' in " + instance + + " cannot contain regions in different clouds: " + + endpoint.regions().stream().sorted().collect(Collectors.toList())); + } + } + } + } + + /** Verify endpoint configuration of given application package */ + private void validateEndpointChange(Application application, ApplicationPackage applicationPackage, Instant instant) { + applicationPackage.deploymentSpec().instances().forEach(instance -> validateEndpointChange(application, + instance.name(), + applicationPackage, + instant)); + } + + /** Verify changes to endpoint configuration by comparing given application package to the existing one, if any */ + private void validateEndpointChange(Application application, InstanceName instanceName, ApplicationPackage applicationPackage, Instant instant) { + var validationId = ValidationId.globalEndpointChange; + if (applicationPackage.validationOverrides().allows(validationId, instant)) return; + + var endpoints = application.deploymentSpec().instance(instanceName) + .map(ApplicationPackageValidator::allEndpointsOf) + .orElseGet(List::of); + var newEndpoints = allEndpointsOf(applicationPackage.deploymentSpec().requireInstance(instanceName)); + + if (newEndpoints.containsAll(endpoints)) return; // Adding new endpoints is fine + if (containsAllRegions(newEndpoints, endpoints)) return; // Adding regions to endpoints is fine + + var removedEndpoints = new ArrayList<>(endpoints); + removedEndpoints.removeAll(newEndpoints); + newEndpoints.removeAll(endpoints); + throw new IllegalArgumentException(validationId.value() + ": application '" + application.id() + + (instanceName.isDefault() ? "" : "." + instanceName.value()) + + "' has endpoints " + endpoints + + ", but does not include all of these in deployment.xml. Deploying given " + + "deployment.xml will remove " + removedEndpoints + + (newEndpoints.isEmpty() ? "" : " and add " + newEndpoints) + + ". " + ValidationOverrides.toAllowMessage(validationId)); + } + + /** Returns whether endpoint regions in newEndpoints contains all regions of corresponding endpoint in endpoints */ + private static boolean containsAllRegions(List<Endpoint> newEndpoints, List<Endpoint> endpoints) { + var containsAllRegions = true; + for (var endpoint : endpoints) { + var endpointContainsAllRegions = false; + for (var newEndpoint : newEndpoints) { + if (endpoint.endpointId().equals(newEndpoint.endpointId())) { + endpointContainsAllRegions = newEndpoint.regions().containsAll(endpoint.regions()); + } + } + containsAllRegions &= endpointContainsAllRegions; + } + return containsAllRegions; + } + + /** Returns all configued endpoints of given deployment instance spec */ + private static List<Endpoint> allEndpointsOf(DeploymentInstanceSpec deploymentInstanceSpec) { + var endpoints = new ArrayList<>(deploymentInstanceSpec.endpoints()); + legacyEndpoint(deploymentInstanceSpec).ifPresent(endpoints::add); + return endpoints; + } + + /** Returns global service ID as a endpoint, if any global service ID is set */ + private static Optional<Endpoint> legacyEndpoint(DeploymentInstanceSpec instance) { + return instance.globalServiceId().map(globalServiceId -> { + var regions = instance.zones().stream() + .filter(zone -> zone.environment().isProduction()) + .map(zone -> zone.region().get()) + .map(RegionName::value) + .collect(Collectors.toSet()); + return new Endpoint(Optional.of(EndpointId.defaultId().id()), instance.globalServiceId().get(), regions); + }); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java deleted file mode 100644 index 5c4d5874e53..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.application; - -import com.yahoo.config.application.api.DeploymentSpec; -import com.yahoo.config.provision.CloudName; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.zone.ZoneApi; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; - -import java.util.HashSet; -import java.util.Objects; -import java.util.stream.Collectors; - -/** - * This contains validators for a {@link DeploymentSpec} that depend on a {@link Controller} to perform validation. - * - * @author mpolden - */ -public class DeploymentSpecValidator { - - private final Controller controller; - - public DeploymentSpecValidator(Controller controller) { - this.controller = Objects.requireNonNull(controller, "controller must be non-null"); - } - - /** - * Validate the given deploymentSpec - * - * @throws IllegalArgumentException if any validations fail - */ - public void validate(DeploymentSpec deploymentSpec) { - validateSteps(deploymentSpec); - validateEndpoints(deploymentSpec); - } - - /** Verify that each of the production zones listed in the deployment spec exist in this system */ - private void validateSteps(DeploymentSpec deploymentSpec) { - new DeploymentSteps(deploymentSpec, controller::system).jobs(); - deploymentSpec.instances().stream().flatMap(instance -> instance.zones().stream()) - .filter(zone -> zone.environment() == Environment.prod) - .forEach(zone -> { - if ( ! controller.zoneRegistry().hasZone(ZoneId.from(zone.environment(), - zone.region().orElse(null)))) { - throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); - } - }); - } - - /** Verify that no single endpoint contains regions in different clouds */ - private void validateEndpoints(DeploymentSpec deploymentSpec) { - for (var instance : deploymentSpec.instances()) { - for (var endpoint : instance.endpoints()) { - var clouds = new HashSet<CloudName>(); - for (var region : endpoint.regions()) { - for (ZoneApi zone : controller.zoneRegistry().zones().all().in(region).zones()) { - clouds.add(zone.getCloudName()); - } - } - if (clouds.size() != 1) { - throw new IllegalArgumentException("Endpoint '" + endpoint.endpointId() + "' in " + instance + - " cannot contain regions in different clouds: " + - endpoint.regions().stream().sorted().collect(Collectors.toList())); - } - } - } - } - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java index 7c88b94a2ae..09fd0b1720a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointId.java @@ -50,7 +50,7 @@ public class EndpointId implements Comparable<EndpointId> { return input; } - public static EndpointId default_() { return DEFAULT; } + public static EndpointId defaultId() { return DEFAULT; } public static EndpointId of(String id) { return new EndpointId(id); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java index 1d475c985a4..e3d8dd092e5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java @@ -121,7 +121,7 @@ public class RotationRepository { return List.of( new AssignedRotation( new ClusterSpec.Id(deploymentSpec.requireInstance(instance.name()).globalServiceId().get()), - EndpointId.default_(), + EndpointId.defaultId(), rotation.id(), regions ) 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 1558656b086..e38f6d25cc8 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 @@ -402,49 +402,130 @@ public class ControllerTest { } @Test - public void testDnsAliasRegistrationWithChangingZones() { + public void testDnsAliasRegistrationWithChangingEndpoints() { Application application = tester.createApplication("app1", "tenant1", 1, 1L); + var west = ZoneId.from("prod", "us-west-1"); + var central = ZoneId.from("prod", "us-central-1"); + var east = ZoneId.from("prod", "us-east-3"); + var buildNumber = BuildJob.defaultBuildNumber; + // Application is deployed with endpoint pointing to 2/3 zones ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .endpoint("default", "qrs", "us-west-1", "us-central-1") - .region("us-west-1") - .region("us-central-1") + .endpoint("default", "qrs", west.region().value(), central.region().value()) + .region(west.region().value()) + .region(central.region().value()) + .region(east.region().value()) .build(); + tester.deployCompletely(application, applicationPackage, ++buildNumber); + + for (var zone : List.of(west, central)) { + assertEquals( + "Zone " + zone + " is a member of global endpoint", + Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud"), + tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), zone)) + ); + } - tester.deployCompletely(application, applicationPackage); - - assertEquals( - Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud"), - tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), ZoneId.from("prod", "us-west-1"))) - ); - + // Application is deployed with an additional endpoint + ApplicationPackage applicationPackage2 = new ApplicationPackageBuilder() + .environment(Environment.prod) + .endpoint("default", "qrs", west.region().value(), central.region().value()) + .endpoint("east", "qrs", east.region().value()) + .region(west.region().value()) + .region(central.region().value()) + .region(east.region().value()) + .build(); + tester.deployCompletely(application, applicationPackage2, ++buildNumber); + + for (var zone : List.of(west, central)) { + assertEquals( + "Zone " + zone + " is a member of global endpoint", + Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud"), + tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), zone)) + ); + } assertEquals( - Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud"), - tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), ZoneId.from("prod", "us-central-1"))) + "Zone " + east + " is a member of global endpoint", + Set.of("rotation-id-02", "east--app1--tenant1.global.vespa.oath.cloud"), + tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), east)) ); - - ApplicationPackage applicationPackage2 = new ApplicationPackageBuilder() + // Application is deployed with default endpoint pointing to 3/3 zones + ApplicationPackage applicationPackage3 = new ApplicationPackageBuilder() .environment(Environment.prod) - .endpoint("default", "qrs", "us-west-1") - .region("us-west-1") - .region("us-central-1") + .endpoint("default", "qrs", west.region().value(), central.region().value(), east.region().value()) + .endpoint("east", "qrs", east.region().value()) + .region(west.region().value()) + .region(central.region().value()) + .region(east.region().value()) .build(); + tester.deployCompletely(application, applicationPackage3, ++buildNumber); + for (var zone : List.of(west, central, east)) { + assertEquals( + "Zone " + zone + " is a member of global endpoint", + zone.equals(east) + ? Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud", + "rotation-id-02", "east--app1--tenant1.global.vespa.oath.cloud") + : Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud"), + tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), zone)) + ); + } - tester.deployCompletely(application, applicationPackage2, BuildJob.defaultBuildNumber + 1); - - assertEquals( - Set.of("rotation-id-01", "app1--tenant1.global.vespa.oath.cloud"), - tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), ZoneId.from("prod", "us-west-1"))) - ); + // Region is removed from an endpoint without override + ApplicationPackage applicationPackage4 = new ApplicationPackageBuilder() + .environment(Environment.prod) + .endpoint("default", "qrs", west.region().value(), central.region().value()) + .endpoint("east", "qrs", east.region().value()) + .region(west.region().value()) + .region(central.region().value()) + .region(east.region().value()) + .build(); + try { + tester.deployCompletely(application, applicationPackage4, ++buildNumber); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertEquals("global-endpoint-change: application 'tenant1.app1' has endpoints " + + "[endpoint 'default' (cluster qrs) -> us-central-1, us-east-3, us-west-1, endpoint 'east' (cluster qrs) -> us-east-3], " + + "but does not include all of these in deployment.xml. Deploying given deployment.xml " + + "will remove [endpoint 'default' (cluster qrs) -> us-central-1, us-east-3, us-west-1] " + + "and add [endpoint 'default' (cluster qrs) -> us-central-1, us-west-1]. " + + ValidationOverrides.toAllowMessage(ValidationId.globalEndpointChange), e.getMessage()); + } finally { + tester.buildService().clear(); + } - assertEquals( - Set.of(), - tester.configServer().rotationNames().get(new DeploymentId(application.id().defaultInstance(), ZoneId.from("prod", "us-central-1"))) - ); + // Entire endpoint is removed without override + ApplicationPackage applicationPackage5 = new ApplicationPackageBuilder() + .environment(Environment.prod) + .endpoint("east", "qrs", east.region().value()) + .region(west.region().value()) + .region(central.region().value()) + .region(east.region().value()) + .build(); + try { + tester.deployCompletely(application, applicationPackage5, ++buildNumber); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertEquals("global-endpoint-change: application 'tenant1.app1' has endpoints " + + "[endpoint 'default' (cluster qrs) -> us-central-1, us-east-3, us-west-1, endpoint 'east' (cluster qrs) -> us-east-3], " + + "but does not include all of these in deployment.xml. Deploying given deployment.xml " + + "will remove [endpoint 'default' (cluster qrs) -> us-central-1, us-east-3, us-west-1]. " + + ValidationOverrides.toAllowMessage(ValidationId.globalEndpointChange), e.getMessage()); + } finally { + tester.buildService().clear(); + } - assertEquals(Set.of(RegionName.from("us-west-1")), tester.defaultInstance(application.id()).rotations().get(0).regions()); + // ... override is added + ApplicationPackage applicationPackage6 = new ApplicationPackageBuilder() + .environment(Environment.prod) + .endpoint("east", "qrs", east.region().value()) + .region(west.region().value()) + .region(central.region().value()) + .region(east.region().value()) + .allow(ValidationId.globalEndpointChange) + .build(); + tester.deployCompletely(application, applicationPackage6, ++buildNumber); } @Test @@ -464,6 +545,7 @@ public class ControllerTest { .environment(Environment.prod) .region("us-west-1") .region("us-central-1") // Two deployments should result in each DNS alias being registered once + .allow(ValidationId.globalEndpointChange) .build(); tester.deployCompletely(application, applicationPackage2, BuildJob.defaultBuildNumber + 1); @@ -504,6 +586,7 @@ public class ControllerTest { applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .allow(ValidationId.deploymentRemoval) + .allow(ValidationId.globalEndpointChange) .build(); tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(tester.defaultInstance(app1.id()).id(), Optional.of(applicationPackage), true, systemTest); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java index fa41f5a6ac3..3e96d2f6972 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java @@ -22,7 +22,7 @@ public class EndpointTest { @Test public void test_global_endpoints() { - EndpointId endpointId = EndpointId.default_(); + EndpointId endpointId = EndpointId.defaultId(); Map<String, Endpoint> tests = Map.of( // Legacy endpoint @@ -66,7 +66,7 @@ public class EndpointTest { @Test public void test_global_endpoints_with_endpoint_id() { - var endpointId = EndpointId.default_(); + var endpointId = EndpointId.defaultId(); Map<String, Endpoint> tests = Map.of( // Legacy endpoint @@ -164,7 +164,7 @@ public class EndpointTest { // Default rotation "https://a1.t1.global.public.vespa.oath.cloud/", Endpoint.of(app1) - .named(EndpointId.default_()) + .named(EndpointId.defaultId()) .directRouting() .on(Port.tls()) .in(SystemName.Public), 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 9449f2b0854..c352fc5550f 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 @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.UncheckedIOException; -import java.nio.charset.StandardCharsets; import java.security.cert.X509Certificate; import java.text.SimpleDateFormat; import java.time.Duration; @@ -20,9 +19,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Date; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.OptionalInt; import java.util.StringJoiner; import java.util.zip.ZipEntry; @@ -80,14 +77,14 @@ public class ApplicationPackageBuilder { } public ApplicationPackageBuilder endpoint(String endpointId, String containerId, String... regions) { - endpointsBody.append(" <endpoint"); + endpointsBody.append(" <endpoint"); endpointsBody.append(" id='").append(endpointId).append("'"); endpointsBody.append(" container-id='").append(containerId).append("'"); endpointsBody.append(">\n"); for (var region : regions) { - endpointsBody.append(" <region>").append(region).append("</region>\n"); + endpointsBody.append(" <region>").append(region).append("</region>\n"); } - endpointsBody.append(" </endpoint>\n"); + endpointsBody.append(" </endpoint>\n"); return this; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java index 78a9be9f1d3..5e10bc3e5db 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java @@ -1,6 +1,7 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; @@ -127,6 +128,7 @@ public class RoutingPoliciesTest { .region(zone1.region()) .region(zone2.region()) .region(zone3.region()) + .allow(ValidationId.globalEndpointChange) .build(); tester.deployCompletely(app1, applicationPackage4, ++buildNumber); assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint1)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index ea96a839959..6ffe2fb83fc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -5,6 +5,7 @@ import ai.vespa.hosted.api.MultiPartStreamer; import ai.vespa.hosted.api.Signatures; import com.yahoo.application.container.handler.Request; import com.yahoo.component.Version; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.ClusterSpec; @@ -325,8 +326,10 @@ public class ApplicationApiTest extends ControllerContainerTest { // POST (create) another application ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .instances("instance1") + .globalServiceId("foo") .environment(Environment.prod) .region("us-west-1") + .allow(ValidationId.globalEndpointChange) .build(); tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/instance/default", POST) @@ -626,6 +629,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Third attempt finally has a service under the domain of the tenant, and succeeds. ApplicationPackage packageWithService = new ApplicationPackageBuilder() .instances("instance1") + .globalServiceId("foo") .environment(Environment.prod) .athenzIdentity(com.yahoo.config.provision.AthenzDomain.from(ATHENZ_TENANT_DOMAIN.getName()), AthenzService.from("service")) .region("us-west-1") |