diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-02-11 11:25:24 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2020-02-11 12:16:33 +0100 |
commit | 0e8cb5d429339e8ad5522d663ca963c92d334cdf (patch) | |
tree | 8545404b9dcb99f87eb4570b98ba9e99bab7e09c | |
parent | b64ffc518680be2b5785dd14a247a8f20e8edde7 (diff) |
Never remove all members from global endpoints
4 files changed, 89 insertions, 17 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java index 680613e8065..b54446c071e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java @@ -45,7 +45,7 @@ public class MemoryNameService implements NameService { .map(target -> new Record(Record.Type.ALIAS, name, target.asData())) .collect(Collectors.toList()); // Satisfy idempotency contract of interface - removeRecords(findRecords(Record.Type.ALIAS, name)); + removeRecords(records); records.forEach(this::add); return records; } @@ -74,7 +74,17 @@ public class MemoryNameService implements NameService { "the FQDN name"); } return records.stream() - .filter(record -> record.type() == type && record.data().equals(data)) + .filter(record -> { + if (record.type() == type) { + if (type == Record.Type.ALIAS) { + // Unpack ALIAS record and compare FQDN of data part + return RecordData.fqdn(AliasTarget.from(record.data()).name().value()) + .equals(data); + } + return record.data().equals(data); + } + return false; + }) .collect(Collectors.toUnmodifiableList()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/GlobalRouting.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/GlobalRouting.java index 244686c85e4..7ea651c7bd5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/GlobalRouting.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/GlobalRouting.java @@ -27,7 +27,10 @@ public class GlobalRouting { this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null"); } - /** The current status of this */ + /** + * The wanted status of this. The system will try to set this status, but there are constraints that may lead to + * the effective status not matching this. See {@link RoutingPolicies}. + */ public Status status() { return status; } 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 cb45c982a0b..8657a601837 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 @@ -17,7 +17,6 @@ import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -91,13 +90,6 @@ public class RoutingPolicies { } } - /** Returns whether global DNS records should be updated for given application and zone */ - private boolean updateGlobalDnsOf(ApplicationId application, ZoneId zone) { - if (application.instance().isTester()) return false; - if (!zone.environment().isProduction()) return false; - return true; - } - /** Set the status of all global endpoints in given zone */ public void setGlobalRoutingStatus(ZoneId zone, GlobalRouting.Status status) { try (var lock = db.lockRoutingPolicies()) { @@ -141,13 +133,18 @@ public class RoutingPolicies { // - zone level (ZoneRoutingPolicy) // - deployment level (RoutingPolicy) // - application package level (deployment.xml) - if (anyOut(zonePolicy.globalRouting(), policy.status().globalRouting()) || - inactiveZones.contains(policy.id().zone())) { + if (isConfiguredOut(policy, zonePolicy, inactiveZones)) { staleTargets.add(target); } else { targets.add(target); } } + // If all targets are configured out, all targets are set in. We do this because otherwise removing 100% of + // the ALIAS records would cause the global endpoint to stop resolving entirely (NXDOMAIN). + if (targets.isEmpty() && !staleTargets.isEmpty()) { + targets.addAll(staleTargets); + staleTargets.clear(); + } if (!targets.isEmpty()) { var endpoint = RoutingPolicy.globalEndpointOf(routeEntry.getKey().application(), routeEntry.getKey().endpointId(), controller.system()); @@ -238,10 +235,15 @@ public class RoutingPolicies { return Collections.unmodifiableMap(routingTable); } - private static boolean anyOut(GlobalRouting... globalRouting) { - return Arrays.stream(globalRouting) - .map(GlobalRouting::status) - .anyMatch(status -> status == GlobalRouting.Status.out); + /** Returns whether the global routing status of given policy is configured to be {@link GlobalRouting.Status#out} */ + private static boolean isConfiguredOut(RoutingPolicy policy, ZoneRoutingPolicy zonePolicy, Set<ZoneId> inactiveZones) { + // A deployment is can be configured out at any of the following levels: + // - zone level (ZoneRoutingPolicy) + // - deployment level (RoutingPolicy) + // - application package level (deployment.xml) + return zonePolicy.globalRouting().status() == GlobalRouting.Status.out || + policy.status().globalRouting().status() == GlobalRouting.Status.out || + inactiveZones.contains(policy.id().zone()); } private static boolean isActive(LoadBalancer loadBalancer) { 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 89b7e13e392..a27b2e1084a 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 @@ -45,6 +45,7 @@ import java.util.Set; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; /** @@ -532,6 +533,62 @@ public class RoutingPoliciesTest { context.completeRollout(); tester.assertTargets(context.instanceId(), endpointId, 0, prodZone); } + + @Test + public void changing_global_routing_status_never_removes_all_members() { + var tester = new RoutingPoliciesTester(); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + + // Provision load balancers and deploy application + tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); + var applicationPackage = new ApplicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) + .build(); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + + // Global DNS record is created, pointing to all configured zones + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + + // Global routing status is overridden for one deployment + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.out, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); + + // Setting other deployment out implicitly sets all deployments in + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.out, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + + // One inactive deployment is put back in. Global DNS record now points to the only active deployment + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); + + // Setting zone (containing active deployment) out puts all deployments in + tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.out); + context.flushDnsUpdates(); + assertEquals(GlobalRouting.Status.out, tester.routingPolicies().get(zone1).globalRouting().status()); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + + // Setting zone back in removes the currently inactive deployment + tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.in); + context.flushDnsUpdates(); + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); + + // Inactive deployment is set in + tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.in, + GlobalRouting.Agent.tenant); + context.flushDnsUpdates(); + for (var policy : tester.routingPolicies().get(context.instanceId()).values()) { + assertSame(GlobalRouting.Status.in, policy.status().globalRouting().status()); + } + tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); + } private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count) { List<LoadBalancer> loadBalancers = new ArrayList<>(); |