From 7c5d04b0eb540d1933713f34a53f6cb3d31e8517 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 19 Nov 2021 13:34:56 +0100 Subject: Only create ALIAS records when using exclusive routing --- .../vespa/hosted/controller/routing/RoutingPolicies.java | 13 ++++++++++--- .../com/yahoo/vespa/hosted/controller/ControllerTest.java | 8 ++++---- 2 files changed, 14 insertions(+), 7 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 2a39ed08014..634d76c8449 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -215,7 +215,7 @@ public class RoutingPolicies { Application application = controller.applications().requireApplication(routingTable.keySet().iterator().next().application()); Map> targetWeights = targetWeights(application); - Map> targetsByEndpoint = new LinkedHashMap<>(); + Map> targetsByEndpoint = new LinkedHashMap<>(); for (Map.Entry> routeEntry : routingTable.entrySet()) { RoutingId routingId = routeEntry.getKey(); EndpointList endpoints = controller.routing().declaredEndpointsOf(application) @@ -236,13 +236,20 @@ public class RoutingPolicies { } WeightedAliasTarget weightedAliasTarget = new WeightedAliasTarget(policy.canonicalName(), policy.dnsZone().get(), target.deployment().zoneId(), weight); - targetsByEndpoint.computeIfAbsent(endpoint.dnsName(), (k) -> new LinkedHashSet<>()) + targetsByEndpoint.computeIfAbsent(endpoint, (k) -> new LinkedHashSet<>()) .add(weightedAliasTarget); } } } targetsByEndpoint.forEach((applicationEndpoint, targets) -> { - controller.nameServiceForwarder().createAlias(RecordName.from(applicationEndpoint), targets, Priority.normal); + ZoneId targetZone = applicationEndpoint.targets().stream() + .map(Endpoint.Target::deployment) + .map(DeploymentId::zoneId) + .findFirst() + .get(); + nameServiceForwarderIn(targetZone).createAlias(RecordName.from(applicationEndpoint.dnsName()), + targets, + Priority.normal); }); } 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 9472801ef2c..58d034582cd 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 @@ -625,13 +625,13 @@ public class ControllerTest { .instances("beta,main") .region("us-west-1") .region("us-east-3") - .applicationEndpoint("a", "qrs", "us-west-1", + .applicationEndpoint("a", "default", "us-west-1", Map.of(InstanceName.from("beta"), 2, InstanceName.from("main"), 8)) - .applicationEndpoint("b", "qrs", "us-west-1", + .applicationEndpoint("b", "default", "us-west-1", Map.of(InstanceName.from("beta"), 1, InstanceName.from("main"), 1)) - .applicationEndpoint("c", "qrs", "us-east-3", + .applicationEndpoint("c", "default", "us-east-3", Map.of(InstanceName.from("beta"), 4, InstanceName.from("main"), 6)) .build(); @@ -644,7 +644,7 @@ public class ControllerTest { usEast, List.of("c--app1--tenant1.us-east-3-r.vespa.oath.cloud")); deploymentEndpoints.forEach((zone, endpointNames) -> { assertEquals("Endpoint names are passed to config server in " + zone, - Set.of(new ContainerEndpoint("qrs", "application", + Set.of(new ContainerEndpoint("default", "application", endpointNames)), tester.configServer().containerEndpoints().get(zone)); }); -- cgit v1.2.3 From 2747db0142641bf35d165d7d023450577e165775 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 19 Nov 2021 13:38:10 +0100 Subject: Disallow application endpoints with shared routing method --- .../vespa/hosted/controller/RoutingController.java | 25 +++++++++++++--------- .../hosted/controller/application/Endpoint.java | 7 +++++- .../vespa/hosted/controller/ControllerTest.java | 16 +++++++------- .../restapi/application/responses/deployment.json | 4 ++-- .../application/responses/prod-us-central-1.json | 4 ++-- 5 files changed, 33 insertions(+), 23 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 3794b69c023..2f5b92ca4c1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -145,16 +145,17 @@ public class RoutingController { .collect(Collectors.toMap(t -> new DeploymentId(application.id().instance(t.instance()), ZoneId.from(Environment.prod, t.region())), t -> t.weight())); - List availableRoutingMethods = routingMethodsOfAll(deployments.keySet(), deploymentSpec); - for (var routingMethod : availableRoutingMethods) { - endpoints.add(Endpoint.of(application.id()) - .targetApplication(EndpointId.of(declaredEndpoint.endpointId()), - ClusterSpec.Id.from(declaredEndpoint.containerId()), - deployments) - .routingMethod(routingMethod) - .on(Port.fromRoutingMethod(routingMethod)) - .in(controller.system())); - } + // An application endpoint can only target a single zone, so we just pick the zone of any deployment target + ZoneId zone = deployments.keySet().iterator().next().zoneId(); + // Application endpoints are only supported when using direct routing methods + RoutingMethod routingMethod = usesSharedRouting(zone) ? RoutingMethod.sharedLayer4 : RoutingMethod.exclusive; + endpoints.add(Endpoint.of(application.id()) + .targetApplication(EndpointId.of(declaredEndpoint.endpointId()), + ClusterSpec.Id.from(declaredEndpoint.containerId()), + deployments) + .routingMethod(routingMethod) + .on(Port.fromRoutingMethod(routingMethod)) + .in(controller.system())); } return EndpointList.copyOf(endpoints); } @@ -354,6 +355,10 @@ public class RoutingController { Priority.normal)); } + private boolean usesSharedRouting(ZoneId zone) { + return controller.zoneRegistry().routingMethods(zone).stream().anyMatch(RoutingMethod::isShared); + } + /** Returns the routing methods that are available across all given deployments */ private List routingMethodsOfAll(Collection deployments, DeploymentSpec deploymentSpec) { var deploymentsByMethod = new HashMap>(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index c736863a57e..aee7c1052be 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -60,7 +60,7 @@ public class Endpoint { this.instance = requireInstance(instanceName, scope); this.url = url; this.targets = List.copyOf(requireTargets(targets, application, instanceName, scope, certificateName)); - this.scope = scope; + this.scope = requireScope(scope, routingMethod); this.legacy = legacy; this.routingMethod = routingMethod; this.tls = port.tls; @@ -329,6 +329,11 @@ public class Endpoint { return instanceName; } + private static Scope requireScope(Scope scope, RoutingMethod routingMethod) { + if (scope == Scope.application && !routingMethod.isDirect()) throw new IllegalArgumentException("Routing method " + routingMethod + " does not support " + scope + "-scoped endpoints"); + return scope; + } + private static List requireTargets(List targets, TenantAndApplicationId application, Optional instanceName, Scope scope, boolean certificateName) { if (!certificateName && targets.isEmpty()) throw new IllegalArgumentException("At least one target must be given for " + scope + " endpoints"); if (scope == Scope.zone && targets.size() != 1) throw new IllegalArgumentException("Exactly one target must be given for " + scope + " endpoints"); 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 58d034582cd..be180f27af6 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 @@ -640,8 +640,8 @@ public class ControllerTest { // Endpoint names are passed to each deployment DeploymentId usWest = context.deploymentIdIn(ZoneId.from("prod", "us-west-1")); DeploymentId usEast = context.deploymentIdIn(ZoneId.from("prod", "us-east-3")); - Map> deploymentEndpoints = Map.of(usWest, List.of("a--app1--tenant1.us-west-1-r.vespa.oath.cloud", "b--app1--tenant1.us-west-1-r.vespa.oath.cloud"), - usEast, List.of("c--app1--tenant1.us-east-3-r.vespa.oath.cloud")); + Map> deploymentEndpoints = Map.of(usWest, List.of("a.app1.tenant1.us-west-1-r.vespa.oath.cloud", "b.app1.tenant1.us-west-1-r.vespa.oath.cloud"), + usEast, List.of("c.app1.tenant1.us-east-3-r.vespa.oath.cloud")); deploymentEndpoints.forEach((zone, endpointNames) -> { assertEquals("Endpoint names are passed to config server in " + zone, Set.of(new ContainerEndpoint("default", "application", @@ -653,21 +653,21 @@ public class ControllerTest { // DNS records are created for each endpoint Set records = tester.controllerTester().nameService().records(); assertEquals(Set.of(new Record(Record.Type.CNAME, - RecordName.from("a--app1--tenant1.us-west-1-r.vespa.oath.cloud"), + RecordName.from("a.app1.tenant1.us-west-1-r.vespa.oath.cloud"), RecordData.from("vip.prod.us-west-1.")), new Record(Record.Type.CNAME, - RecordName.from("b--app1--tenant1.us-west-1-r.vespa.oath.cloud"), + RecordName.from("b.app1.tenant1.us-west-1-r.vespa.oath.cloud"), RecordData.from("vip.prod.us-west-1.")), new Record(Record.Type.CNAME, - RecordName.from("c--app1--tenant1.us-east-3-r.vespa.oath.cloud"), + RecordName.from("c.app1.tenant1.us-east-3-r.vespa.oath.cloud"), RecordData.from("vip.prod.us-east-3."))), records); List endpointDnsNames = tester.controller().routing().declaredEndpointsOf(context.application()) .scope(Endpoint.Scope.application) .mapToList(Endpoint::dnsName); - assertEquals(List.of("a--app1--tenant1.us-west-1-r.vespa.oath.cloud", - "b--app1--tenant1.us-west-1-r.vespa.oath.cloud", - "c--app1--tenant1.us-east-3-r.vespa.oath.cloud"), + assertEquals(List.of("a.app1.tenant1.us-west-1-r.vespa.oath.cloud", + "b.app1.tenant1.us-west-1-r.vespa.oath.cloud", + "c.app1.tenant1.us-east-3-r.vespa.oath.cloud"), endpointDnsNames); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json index fb6088f54b8..ab2a3bf945c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json @@ -24,9 +24,9 @@ { "cluster": "foo", "tls": true, - "url": "https://a0--application1--tenant1.us-central-1-r.vespa.oath.cloud:4443/", + "url": "https://a0.application1.tenant1.us-central-1-r.vespa.oath.cloud/", "scope": "application", - "routingMethod": "shared", + "routingMethod": "sharedLayer4", "legacy": false } ], diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json index 409e97b063c..62ad3a2db7e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json @@ -27,9 +27,9 @@ { "cluster": "foo", "tls": true, - "url": "https://a0--application1--tenant1.us-central-1-r.vespa.oath.cloud:4443/", + "url": "https://a0.application1.tenant1.us-central-1-r.vespa.oath.cloud/", "scope": "application", - "routingMethod": "shared", + "routingMethod": "sharedLayer4", "legacy": false } ], -- cgit v1.2.3