From f55d830d8d56a28b6f81446a3a65c162094f4da2 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 22 Nov 2019 11:11:22 +0100 Subject: Avoid duplicating endpoint URLs When routing is configured through routing polices, one policy exists per deployment, but the global endpoint URL may naturally be the same across multiple deployments. --- .../restapi/application/ApplicationApiHandler.java | 36 +++++++++++++--------- 1 file changed, 22 insertions(+), 14 deletions(-) (limited to 'controller-server') 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(); + + // 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 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 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) { -- cgit v1.2.3 From c27f34c97006852d5185f8950630726e1e69d672 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 22 Nov 2019 11:19:26 +0100 Subject: Log stale read --- .../controller/maintenance/RoutingPolicies.java | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'controller-server') 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 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 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) -- cgit v1.2.3