diff options
4 files changed, 38 insertions, 13 deletions
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 fa8851c414d..c76616c6d2c 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 @@ -256,7 +256,7 @@ public class RoutingController { EndpointList endpoints = declaredEndpointsOf(application.get()).targets(deployment); EndpointList globalEndpoints = endpoints.scope(Endpoint.Scope.global); for (var assignedRotation : instance.rotations()) { - EndpointList rotationEndpoints = globalEndpoints.named(assignedRotation.endpointId()) + EndpointList rotationEndpoints = globalEndpoints.named(assignedRotation.endpointId(), Scope.global) .requiresRotation(); // Skip rotations which do not apply to this zone. Legacy names always point to all zones diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java index 7fe8d554998..3da9065b52d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java @@ -32,9 +32,10 @@ public class EndpointList extends AbstractFilteringList<Endpoint, EndpointList> return not().legacy().asList().stream().findFirst(); } - /** Returns the subset of endpoints named according to given ID */ - public EndpointList named(EndpointId id) { - return matching(endpoint -> endpoint.name().equals(id.id())); + /** Returns the subset of endpoints named according to given ID and scope */ + public EndpointList named(EndpointId id, Endpoint.Scope scope) { + return matching(endpoint -> endpoint.scope() == scope && // ID is only unique within a scope + endpoint.name().equals(id.id())); } /** Returns the subset of endpoints pointing to given cluster */ 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 637393d71cb..59f886bdea6 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 @@ -170,7 +170,7 @@ public class RoutingPolicies { for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { RoutingId routingId = routeEntry.getKey(); controller.routing().readDeclaredEndpointsOf(routingId.instance()) - .named(routingId.endpointId()) + .named(routingId.endpointId(), Endpoint.Scope.global) .not().requiresRotation() .forEach(endpoint -> updateGlobalDnsOf(endpoint, inactiveZones, routeEntry.getValue(), owner)); } @@ -269,8 +269,7 @@ public class RoutingPolicies { for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { RoutingId routingId = routeEntry.getKey(); EndpointList endpoints = controller.routing().declaredEndpointsOf(application) - .scope(Endpoint.Scope.application) - .named(routingId.endpointId()); + .named(routingId.endpointId(), Endpoint.Scope.application); for (Endpoint endpoint : endpoints) { for (var policy : routeEntry.getValue()) { for (var target : endpoint.targets()) { @@ -471,7 +470,7 @@ public class RoutingPolicies { for (var id : removalCandidates) { EndpointList endpoints = controller.routing().readDeclaredEndpointsOf(id.instance()) .not().requiresRotation() - .named(id.endpointId()); + .named(id.endpointId(), Endpoint.Scope.global); NameServiceForwarder forwarder = nameServiceForwarderIn(allocation.deployment.zoneId()); // This removes all ALIAS records having this DNS name. There is no attempt to delete only the entry for the // affected zone. Instead, the correct set of records is (re)created by updateGlobalDnsOf @@ -491,7 +490,7 @@ public class RoutingPolicies { TenantAndApplicationId application = TenantAndApplicationId.from(id.instance()); EndpointList endpoints = controller.routing() .readDeclaredEndpointsOf(application) - .named(id.endpointId()); + .named(id.endpointId(), Endpoint.Scope.application); List<RoutingPolicy> policies = routingTable.get(id); for (var policy : policies) { if (!policy.appliesTo(allocation.deployment)) continue; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index a90d942bd09..24d5e02240d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -846,8 +846,8 @@ public class RoutingPoliciesTest { tester.assertTargets(application, EndpointId.of("a0"), ClusterSpec.Id.from("c0"), 0, Map.of(betaZone5, 1)); assertTrue(tester.controllerTester().controller().routing() - .readDeclaredEndpointsOf(application) - .named(EndpointId.of("a1")).isEmpty(), + .readDeclaredEndpointsOf(application) + .named(EndpointId.of("a1"), Endpoint.Scope.application).isEmpty(), "Endpoint removed"); } @@ -917,6 +917,31 @@ public class RoutingPoliciesTest { mainZone2, 9)); } + @Test + public void duplicate_endpoint_ids_across_different_scopes() { + RoutingPoliciesTester tester = new RoutingPoliciesTester(); + ApplicationId instance = ApplicationId.from("t1", "a1", "i1"); + DeploymentContext context = tester.newDeploymentContext(instance); + var applicationPackage = applicationPackageBuilder() + .instances(instance.instance().value()) + .region(zone1.region()) + .region(zone2.region()) + .endpoint("default", "c0") + .applicationEndpoint("default", "c0", zone1.region().value(), + Map.of(instance.instance(), 1)) + .build(); + tester.provisionLoadBalancers(1, instance, zone1, zone2); + context.submit(applicationPackage).deploy(); + tester.assertTargets(instance, EndpointId.defaultId(), 0, zone1, zone2); + tester.assertTargets(TenantAndApplicationId.from(instance), EndpointId.defaultId(), + ClusterSpec.Id.from("c0"), 0, Map.of(context.deploymentIdIn(zone1), 1)); + + tester.controllerTester().controller().applications().deactivate(context.instanceId(), zone1); + tester.controllerTester().controller().applications().deactivate(context.instanceId(), zone2); + assertTrue(tester.controllerTester().controller().routing().policies().read(context.instanceId()).isEmpty(), + "Policies removed"); + } + /** Returns an application package builder that satisfies requirements for a directly routed endpoint */ private static ApplicationPackageBuilder applicationPackageBuilder() { return new ApplicationPackageBuilder().athenzIdentity(AthenzDomain.from("domain"), @@ -1053,7 +1078,7 @@ public class RoutingPoliciesTest { Map<String, List<DeploymentId>> deploymentsByDnsName = new HashMap<>(); for (var deployment : deploymentWeights.keySet()) { EndpointList applicationEndpoints = tester.controller().routing().readDeclaredEndpointsOf(application) - .named(endpointId) + .named(endpointId, Endpoint.Scope.application) .targets(deployment) .cluster(cluster); assertEquals(1, @@ -1103,7 +1128,7 @@ public class RoutingPoliciesTest { }); List<DeploymentId> deployments = zoneWeights.keySet().stream().map(z -> new DeploymentId(instance, z)).toList(); String globalEndpoint = tester.controller().routing().readDeclaredEndpointsOf(instance) - .named(endpointId) + .named(endpointId, Endpoint.Scope.global) .targets(deployments) .primary() .map(Endpoint::dnsName) |