diff options
5 files changed, 153 insertions, 29 deletions
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 ae8a81caa17..d015e65a5e1 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller; import com.google.common.collect.ImmutableList; +import com.yahoo.collections.ArraySet; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationId; @@ -9,6 +10,7 @@ import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.athenz.api.AthenzDomain; @@ -129,6 +131,7 @@ public class ApplicationController { private final RoutingPolicies routingPolicies; private final Clock clock; private final BooleanFlag redirectLegacyDnsFlag; + private final BooleanFlag useMultipleEndpoints; private final DeploymentTrigger deploymentTrigger; private final BooleanFlag provisionApplicationCertificate; private final ApplicationCertificateProvider applicationCertificateProvider; @@ -146,6 +149,7 @@ public class ApplicationController { this.routingPolicies = new RoutingPolicies(controller); this.clock = clock; this.redirectLegacyDnsFlag = Flags.REDIRECT_LEGACY_DNS_NAMES.bindTo(controller.flagSource()); + this.useMultipleEndpoints = Flags.MULTIPLE_GLOBAL_ENDPOINTS.bindTo(controller.flagSource()); this.artifactRepository = artifactRepository; this.applicationStore = applicationStore; @@ -296,6 +300,7 @@ public class ApplicationController { Version platformVersion; ApplicationVersion applicationVersion; ApplicationPackage applicationPackage; + Set<String> legacyRotations = new LinkedHashSet<>(); Set<ContainerEndpoint> endpoints = new LinkedHashSet<>(); ApplicationCertificate applicationCertificate; @@ -335,22 +340,34 @@ public class ApplicationController { // TODO: Remove this when all packages are validated upon submission, as in ApplicationApiHandler.submit(...). verifyApplicationIdentityConfiguration(applicationId.tenant(), applicationPackage, deployingIdentity); + // Assign global rotation - application = withRotation(application, zone); - Application app = application.get(); - // Include global DNS names - app.assignedRotations().stream() - .filter(assignedRotation -> assignedRotation.regions().contains(zone.region())) - .map(assignedRotation -> { - return new ContainerEndpoint( - assignedRotation.clusterId().value(), - Stream.concat( - app.endpointsIn(controller.system(), assignedRotation.endpointId()).legacy(false).asList().stream().map(Endpoint::dnsName), - app.rotations().stream().map(RotationId::asString) - ).collect(Collectors.toList()) - ); - }) - .forEach(endpoints::add); + if (useMultipleEndpoints.with(FetchVector.Dimension.APPLICATION_ID, application.get().id().serializedForm()).value()) { + application = withRotation(application, zone); + + // Include global DNS names + Application app = application.get(); + app.assignedRotations().stream() + .filter(assignedRotation -> assignedRotation.regions().contains(zone.region())) + .map(assignedRotation -> { + return new ContainerEndpoint( + assignedRotation.clusterId().value(), + Stream.concat( + app.endpointsIn(controller.system(), assignedRotation.endpointId()).legacy(false).asList().stream().map(Endpoint::dnsName), + app.rotations().stream().map(RotationId::asString) + ).collect(Collectors.toList()) + ); + }) + .forEach(endpoints::add); + } else { + application = withRotationLegacy(application, zone); + + // Add both the names we have in DNS for each endpoint as well as name of the rotation so healthchecks works + Application app = application.get(); + app.endpointsIn(controller.system()).asList().stream().map(Endpoint::dnsName).forEach(legacyRotations::add); + app.rotations().stream().map(RotationId::asString).forEach(legacyRotations::add); + } + // Get application certificate (provisions a new certificate if missing) application = withApplicationCertificate(application); @@ -366,7 +383,7 @@ public class ApplicationController { // Carry out deployment without holding the application lock. options = withVersion(platformVersion, options); - ActivateResult result = deploy(applicationId, applicationPackage, zone, options, endpoints, applicationCertificate); + ActivateResult result = deploy(applicationId, applicationPackage, zone, options, legacyRotations, endpoints, applicationCertificate); lockOrThrow(applicationId, application -> store(application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant(), @@ -433,7 +450,7 @@ public class ApplicationController { artifactRepository.getSystemApplicationPackage(application.id(), zone, version) ); DeployOptions options = withVersion(version, DeployOptions.none()); - return deploy(application.id(), applicationPackage, zone, options, Set.of(), /* No application cert */ null); + return deploy(application.id(), applicationPackage, zone, options, Set.of(), Set.of(), /* No application cert */ null); } else { throw new RuntimeException("This system application does not have an application package: " + application.id().toShortString()); } @@ -441,16 +458,16 @@ public class ApplicationController { /** Deploys the given tester application to the given zone. */ public ActivateResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, DeployOptions options) { - return deploy(tester.id(), applicationPackage, zone, options, Set.of(), /* No application cert for tester*/ null); + return deploy(tester.id(), applicationPackage, zone, options, Set.of(), Set.of(), /* No application cert for tester*/ null); } private ActivateResult deploy(ApplicationId application, ApplicationPackage applicationPackage, ZoneId zone, DeployOptions deployOptions, - Set<ContainerEndpoint> endpoints, ApplicationCertificate applicationCertificate) { + Set<String> legacyRotations, Set<ContainerEndpoint> endpoints, ApplicationCertificate applicationCertificate) { DeploymentId deploymentId = new DeploymentId(application, zone); try { ConfigServer.PreparedApplication preparedApplication = - configServer.deploy(deploymentId, deployOptions, Set.of(), endpoints, applicationCertificate, applicationPackage.zippedContent()); + configServer.deploy(deploymentId, deployOptions, legacyRotations, endpoints, applicationCertificate, applicationPackage.zippedContent()); return new ActivateResult(new RevisionId(applicationPackage.hash()), preparedApplication.prepareResponse(), applicationPackage.zippedContent().length); } finally { @@ -461,6 +478,53 @@ public class ApplicationController { } /** Makes sure the application has a global rotation, if eligible. */ + private LockedApplication withRotationLegacy(LockedApplication application, ZoneId zone) { + if (zone.environment() == Environment.prod && application.get().deploymentSpec().globalServiceId().isPresent()) { + try (RotationLock rotationLock = rotationRepository.lock()) { + Rotation rotation = rotationRepository.getOrAssignRotation(application.get(), rotationLock); + application = application.with(createDefaultGlobalIdRotation(application.get(), rotation)); + store(application); // store assigned rotation even if deployment fails + + boolean redirectLegacyDns = redirectLegacyDnsFlag.with(FetchVector.Dimension.APPLICATION_ID, application.get().id().serializedForm()) + .value(); + + EndpointList globalEndpoints = application.get() + .endpointsIn(controller.system()) + .scope(Endpoint.Scope.global); + + globalEndpoints.main().ifPresent(mainEndpoint -> { + registerCname(mainEndpoint.dnsName(), rotation.name()); + if (redirectLegacyDns) { + globalEndpoints.legacy(true).asList().forEach(endpoint -> registerCname(endpoint.dnsName(), mainEndpoint.dnsName())); + } else { + globalEndpoints.legacy(true).asList().forEach(endpoint -> registerCname(endpoint.dnsName(), rotation.name())); + } + }); + } + } + return application; + } + + private List<AssignedRotation> createDefaultGlobalIdRotation(Application application, Rotation rotation) { + // This is guaranteed by .withRotationLegacy, but add this to make inspections accept the use of .get() below + assert application.deploymentSpec().globalServiceId().isPresent(); + + final Set<RegionName> regions = application.deploymentSpec().zones().stream() + .filter(zone -> zone.environment().isProduction()) + .flatMap(zone -> zone.region().stream()) + .collect(Collectors.toSet()); + + final var assignment = new AssignedRotation( + ClusterSpec.Id.from(application.deploymentSpec().globalServiceId().get()), + EndpointId.default_(), + rotation.id(), + regions + ); + + return List.of(assignment); + } + + /** Makes sure the application has a global rotation, if eligible. */ private LockedApplication withRotation(LockedApplication application, ZoneId zone) { if (zone.environment() == Environment.prod) { try (RotationLock rotationLock = rotationRepository.lock()) { @@ -473,10 +537,6 @@ public class ApplicationController { return application; } - private boolean applicationNeedsRotations(DeploymentSpec spec) { - return spec.globalServiceId().isPresent() || !spec.endpoints().isEmpty(); - } - private void registerAssignedRotationCnames(Application application) { application.assignedRotations().forEach(assignedRotation -> { final var endpoints = application 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 6e7ca423444..c26f1879f6a 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 @@ -277,6 +277,8 @@ public class ControllerTest { @Test public void testDnsAliasRegistration() { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); + Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -306,7 +308,50 @@ public class ControllerTest { } @Test + public void testDnsAliasRegistrationLegacy() { + Application application = tester.createApplication("app1", "tenant1", 1, 1L); + + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .globalServiceId("foo") + .region("us-west-1") + .region("us-central-1") // Two deployments should result in each DNS alias being registered once + .build(); + + tester.deployCompletely(application, applicationPackage); + Collection<Deployment> deployments = tester.application(application.id()).deployments().values(); + assertFalse(deployments.isEmpty()); + for (Deployment deployment : deployments) { + assertEquals("Rotation names are passed to config server in " + deployment.zone(), + Set.of("rotation-id-01", + "app1--tenant1.global.vespa.oath.cloud", + "app1.tenant1.global.vespa.yahooapis.com", + "app1--tenant1.global.vespa.yahooapis.com"), + tester.configServer().rotationNames().get(new DeploymentId(application.id(), deployment.zone()))); + } + tester.flushDnsRequests(); + assertEquals(3, tester.controllerTester().nameService().records().size()); + + Optional<Record> record = tester.controllerTester().findCname("app1--tenant1.global.vespa.yahooapis.com"); + assertTrue(record.isPresent()); + assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString()); + assertEquals("rotation-fqdn-01.", record.get().data().asString()); + + record = tester.controllerTester().findCname("app1--tenant1.global.vespa.oath.cloud"); + assertTrue(record.isPresent()); + assertEquals("app1--tenant1.global.vespa.oath.cloud", record.get().name().asString()); + assertEquals("rotation-fqdn-01.", record.get().data().asString()); + + record = tester.controllerTester().findCname("app1.tenant1.global.vespa.yahooapis.com"); + assertTrue(record.isPresent()); + assertEquals("app1.tenant1.global.vespa.yahooapis.com", record.get().name().asString()); + assertEquals("rotation-fqdn-01.", record.get().data().asString()); + } + + @Test public void testDnsAliasRegistrationWithEndpoints() { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); + Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -314,7 +359,7 @@ public class ControllerTest { .endpoint("foobar", "qrs", "us-west-1", "us-central-1") .endpoint("default", "qrs", "us-west-1", "us-central-1") .region("us-west-1") - .region("us-central-1") // Two deployments should result in each DNS alias being registered once + .region("us-central-1") .build(); tester.deployCompletely(application, applicationPackage); @@ -347,13 +392,15 @@ public class ControllerTest { @Test public void testDnsAliasRegistrationWithChangingZones() { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); + Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .endpoint("default", "qrs", "us-west-1", "us-central-1") .region("us-west-1") - .region("us-central-1") // Two deployments should result in each DNS alias being registered once + .region("us-central-1") .build(); tester.deployCompletely(application, applicationPackage); @@ -373,7 +420,7 @@ public class ControllerTest { .environment(Environment.prod) .endpoint("default", "qrs", "us-west-1") .region("us-west-1") - .region("us-central-1") // Two deployments should result in each DNS alias being registered once + .region("us-central-1") .build(); tester.deployCompletely(application, applicationPackage2, BuildJob.defaultBuildNumber + 1); @@ -393,6 +440,8 @@ public class ControllerTest { @Test public void testUnassignRotations() { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); + Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -426,6 +475,8 @@ public class ControllerTest { @Test public void testUpdatesExistingDnsAlias() { + ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); + // Application 1 is deployed and deleted { Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 4841b6c0c12..a89c5988396 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -45,6 +45,7 @@ import java.util.Set; import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.IntStream; +import java.util.stream.Stream; /** * @author mortent @@ -238,7 +239,13 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer if (nodeRepository().list(deployment.zoneId(), deployment.applicationId()).isEmpty()) provision(deployment.zoneId(), deployment.applicationId()); - this.rotationNames.put(deployment, containerEndpoints.stream().flatMap(e -> e.names().stream()).collect(Collectors.toSet())); + this.rotationNames.put( + deployment, + Stream.concat( + containerEndpoints.stream().flatMap(e -> e.names().stream()), + rotationNames.stream() + ).collect(Collectors.toSet()) + ); return () -> { Application application = applications.get(deployment.applicationId()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index b994c55ce55..58f35c0ac05 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -253,7 +253,7 @@ public class MetricsReporterTest { tester.deployCompletely(application, applicationPackage); reporter.maintain(); - assertEquals("Deployment queues name services requests", 2, metrics.getMetric(MetricsReporter.NAME_SERVICE_REQUESTS_QUEUED).intValue()); + assertEquals("Deployment queues name services requests", 6, metrics.getMetric(MetricsReporter.NAME_SERVICE_REQUESTS_QUEUED).intValue()); tester.flushDnsRequests(); reporter.maintain(); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index b969e328419..5e9c0e2543c 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -151,6 +151,12 @@ public class Flags { "Takes effect on deployment through controller", APPLICATION_ID); + public static final UnboundBooleanFlag MULTIPLE_GLOBAL_ENDPOINTS = defineFeatureFlag( + "multiple-global-endpoints", false, + "Allow applications to use new endpoints syntax in deployment.xml", + "Takes effect on deployment through controller", + APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { |