diff options
author | Martin Polden <mpolden@mpolden.no> | 2019-11-22 12:17:57 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-22 12:17:57 +0100 |
commit | 0ba6bd6683c981bec57c2a96074d13472f841c70 (patch) | |
tree | 482ac52ce5b208c85080d1664804265421506378 | |
parent | 9f3974b292f5781f999fc912d66b14e86721147c (diff) | |
parent | c27f34c97006852d5185f8950630726e1e69d672 (diff) |
Merge pull request #11382 from vespa-engine/mpolden/avoid-duplicating-rotation-urls
Avoid duplicating endpoint URLs
2 files changed, 37 insertions, 19 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java index 70134d5a86d..4e70ebb971b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.log.LogLevel; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; @@ -28,6 +29,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.logging.Logger; import java.util.stream.Collectors; /** @@ -38,6 +40,8 @@ import java.util.stream.Collectors; */ public class RoutingPolicies { + private static final Logger LOGGER = Logger.getLogger(RoutingPolicies.class.getName()); + private final Controller controller; private final CuratorDb db; @@ -78,8 +82,8 @@ public class RoutingPolicies { deploymentSpec); try (var lock = db.lockRoutingPolicies()) { removeObsoleteEndpointsFromDns(lbs, lock); - storePoliciesOf(lbs, lock); - removeObsoletePolicies(lbs, lock); + var writtenPolicies = storePoliciesOf(lbs, lock); + removeObsoletePolicies(lbs, writtenPolicies, lock); registerEndpointsInDns(lbs, lock); } } @@ -103,8 +107,8 @@ public class RoutingPolicies { } } - /** Store routing policies for given route */ - private void storePoliciesOf(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { + /** Store routing policies for given route. Returns the persisted policies. */ + private Set<RoutingPolicy> storePoliciesOf(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { var policies = new LinkedHashSet<>(get(loadBalancers.application)); for (LoadBalancer loadBalancer : loadBalancers.list) { var endpointIds = loadBalancers.endpointIdsOf(loadBalancer); @@ -116,6 +120,7 @@ public class RoutingPolicies { } } db.writeRoutingPolicies(loadBalancers.application, policies); + return policies; } /** Create a policy for given load balancer and register a CNAME for it */ @@ -130,8 +135,13 @@ public class RoutingPolicies { } /** Remove obsolete policies for given route and their CNAME records */ - private void removeObsoletePolicies(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { + private void removeObsoletePolicies(AllocatedLoadBalancers loadBalancers, Set<RoutingPolicy> writtenPolicies, + @SuppressWarnings("unused") Lock lock) { var allPolicies = new LinkedHashSet<>(get(loadBalancers.application)); + if (!writtenPolicies.equals(allPolicies)) { + LOGGER.log(LogLevel.ERROR, String.format("Stale read! This should not happen. Wrote policies %s, but read %s", + writtenPolicies, allPolicies)); + } var removalCandidates = new HashSet<>(allPolicies); var activeLoadBalancers = loadBalancers.list.stream() .map(LoadBalancer::hostname) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index c8f5720327a..c406ef0bbd8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -109,6 +109,7 @@ import java.util.Arrays; import java.util.Base64; import java.util.Comparator; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -749,35 +750,42 @@ public class ApplicationApiHandler extends LoggingRequestHandler { })); } - // Rotation - Cursor globalRotationsArray = object.setArray("globalRotations"); + // Global endpoints + var globalEndpointUrls = new LinkedHashSet<String>(); + + // Add default global endpoints. These are backed by rotations. instance.endpointsIn(controller.system()) .scope(Endpoint.Scope.global) .legacy(false) // Hide legacy names .asList().stream() .map(Endpoint::url) .map(URI::toString) - .forEach(globalRotationsArray::addString); - - instance.rotations().stream() - .map(AssignedRotation::rotationId) - .findFirst() - .ifPresent(rotation -> object.setString("rotationId", rotation.asString())); + .forEach(globalEndpointUrls::add); - // Per-cluster rotations + // Per-cluster endpoints. These are backed by load balancers. Set<RoutingPolicy> routingPolicies = controller.applications().routingPolicies().get(instance.id()); - for (RoutingPolicy policy : routingPolicies) { + for (var policy : routingPolicies) { policy.rotationEndpointsIn(controller.system()).asList().stream() .map(Endpoint::url) .map(URI::toString) - .forEach(globalRotationsArray::addString); + .forEach(globalEndpointUrls::add); } + var globalRotationsArray = object.setArray("globalRotations"); + globalEndpointUrls.forEach(globalRotationsArray::addString); + + // Legacy field. Identifies the first assigned rotation, if any. + instance.rotations().stream() + .map(AssignedRotation::rotationId) + .findFirst() + .ifPresent(rotation -> object.setString("rotationId", rotation.asString())); + + // Deployments sorted according to deployment spec List<Deployment> deployments = deploymentSpec.instance(instance.name()) - .map(spec -> new DeploymentSteps(spec, controller::system)) - .map(steps -> steps.sortedDeployments(instance.deployments().values())) - .orElse(List.copyOf(instance.deployments().values())); + .map(spec -> new DeploymentSteps(spec, controller::system)) + .map(steps -> steps.sortedDeployments(instance.deployments().values())) + .orElse(List.copyOf(instance.deployments().values())); Cursor deploymentsArray = object.setArray("deployments"); for (Deployment deployment : deployments) { |