From 56831ac34779629b96b40e5ced6de9a69c8b939c Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 25 Apr 2019 13:02:11 +0200 Subject: Persist cluster of routing policy instead of DNS alias --- .../controller/application/RoutingPolicy.java | 37 ++++++++++------------ .../maintenance/RoutingPolicyMaintainer.java | 10 +++--- .../persistence/RoutingPolicySerializer.java | 13 ++++++-- .../maintenance/RoutingPolicyMaintainerTest.java | 2 +- .../persistence/RoutingPolicySerializerTest.java | 35 ++++++++++++++++++-- .../restapi/application/ApplicationApiTest.java | 5 +-- 6 files changed, 68 insertions(+), 34 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java index c4b69ce5588..0676297f3bc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java @@ -23,28 +23,23 @@ import java.util.Set; public class RoutingPolicy { private final ApplicationId owner; + private final ClusterSpec.Id cluster; private final ZoneId zone; - private final HostName alias; private final HostName canonicalName; private final Optional dnsZone; private final Set rotations; /** DO NOT USE. Public for serialization purposes */ - public RoutingPolicy(ApplicationId owner, ZoneId zone, HostName alias, HostName canonicalName, + public RoutingPolicy(ApplicationId owner, ClusterSpec.Id cluster, ZoneId zone, HostName canonicalName, Optional dnsZone, Set rotations) { this.owner = Objects.requireNonNull(owner, "owner must be non-null"); + this.cluster = Objects.requireNonNull(cluster, "cluster must be non-null"); this.zone = Objects.requireNonNull(zone, "zone must be non-null"); - this.alias = Objects.requireNonNull(alias, "alias must be non-null"); this.canonicalName = Objects.requireNonNull(canonicalName, "canonicalName must be non-null"); this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null"); this.rotations = ImmutableSortedSet.copyOf(Objects.requireNonNull(rotations, "rotations must be non-null")); } - public RoutingPolicy(ApplicationId owner, ZoneId zone, ClusterSpec.Id cluster, SystemName system, HostName canonicalName, - Optional dnsZone, Set rotations) { - this(owner, zone, HostName.from(endpointOf(cluster, owner, zone, system).dnsName()), canonicalName, dnsZone, rotations); - } - /** The application owning this */ public ApplicationId owner() { return owner; @@ -55,9 +50,9 @@ public class RoutingPolicy { return zone; } - /** This alias (lhs of a CNAME or ALIAS record) */ - public HostName alias() { - return alias; + /** The cluster this applies to */ + public ClusterSpec.Id cluster() { + return cluster; } /** The canonical name for this (rhs of a CNAME or ALIAS record) */ @@ -75,6 +70,11 @@ public class RoutingPolicy { return rotations; } + /** Returns the endpoint of this */ + public Endpoint endpointIn(SystemName system) { + return Endpoint.of(owner).target(cluster, zone).on(Port.tls()).directRouting().in(system); + } + /** Endpoints for this routing policy */ public EndpointList endpointsIn(SystemName system) { return EndpointList.of(rotations.stream().map(rotation -> endpointOf(owner, rotation, system))); @@ -86,19 +86,21 @@ public class RoutingPolicy { if (o == null || getClass() != o.getClass()) return false; RoutingPolicy policy = (RoutingPolicy) o; return owner.equals(policy.owner) && + cluster.equals(policy.cluster) && zone.equals(policy.zone) && - canonicalName.equals(policy.canonicalName); + canonicalName.equals(policy.canonicalName) && + dnsZone.equals(policy.dnsZone); } @Override public int hashCode() { - return Objects.hash(owner, zone, canonicalName); + return Objects.hash(owner, cluster, zone, canonicalName, dnsZone); } @Override public String toString() { - return String.format("%s -> %s [rotations: %s%s], owned by %s, in %s", alias, canonicalName, rotations, - dnsZone.map(z -> ", DNS zone: " + z).orElse(""), owner.toShortString(), + return String.format("%s [rotations: %s%s], %s owned by %s, in %s", canonicalName, rotations, + dnsZone.map(z -> ", DNS zone: " + z).orElse(""), cluster, owner.toShortString(), zone.value()); } @@ -107,9 +109,4 @@ public class RoutingPolicy { return Endpoint.of(application).target(rotation).on(Port.tls()).directRouting().in(system); } - /** Returns the endpoint of given cluster */ - public static Endpoint endpointOf(ClusterSpec.Id cluster, ApplicationId application, ZoneId zone, SystemName system) { - return Endpoint.of(application).target(cluster, zone).on(Port.tls()).directRouting().in(system); - } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java index 417a1944ad3..03d894f9a17 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java @@ -137,11 +137,10 @@ public class RoutingPolicyMaintainer extends Maintainer { /** Register DNS alias for given load balancer */ private RoutingPolicy registerCname(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer) { - RoutingPolicy routingPolicy = new RoutingPolicy(application, zone, - loadBalancer.cluster(), controller().system(), + RoutingPolicy routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, loadBalancer.hostname(), loadBalancer.dnsZone(), loadBalancer.rotations()); - RecordName name = RecordName.from(routingPolicy.alias().value()); + RecordName name = RecordName.from(routingPolicy.endpointIn(controller().system()).dnsName()); RecordData data = RecordData.fqdn(loadBalancer.hostname().value()); List existingRecords = nameService.findRecords(Record.Type.CNAME, name); if (existingRecords.size() > 1) { @@ -170,11 +169,12 @@ public class RoutingPolicyMaintainer extends Maintainer { // Remove any active load balancers removalCandidates.removeIf(policy -> activeLoadBalancers.contains(policy.canonicalName())); for (RoutingPolicy policy : removalCandidates) { + String dnsName = policy.endpointIn(controller().system()).dnsName(); try { - List records = nameService.findRecords(Record.Type.CNAME, RecordName.from(policy.alias().value())); + List records = nameService.findRecords(Record.Type.CNAME, RecordName.from(dnsName)); nameService.removeRecords(records); } catch (Exception e) { - log.log(LogLevel.WARNING, "Failed to remove record '" + policy.alias() + + log.log(LogLevel.WARNING, "Failed to remove record '" + dnsName + "'. Retrying in " + maintenanceInterval()); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java index 722cde68c65..7c4f9a66fd3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.RotationName; import com.yahoo.slime.ArrayTraverser; @@ -21,11 +22,12 @@ import java.util.function.Function; * Serializer and deserializer for a {@link RoutingPolicy}. * * @author mortent + * @author mpolden */ public class RoutingPolicySerializer { private static final String routingPoliciesField = "routingPolicies"; - private static final String aliasField = "alias"; + private static final String clusterField = "cluster"; private static final String canonicalNameField = "canonicalName"; private static final String zoneField = "zone"; private static final String dnsZoneField = "dnsZone"; @@ -37,7 +39,7 @@ public class RoutingPolicySerializer { Cursor policyArray = root.setArray(routingPoliciesField); routingPolicies.forEach(policy -> { Cursor policyObject = policyArray.addObject(); - policyObject.setString(aliasField, policy.alias().value()); + policyObject.setString(clusterField, policy.cluster().value()); policyObject.setString(zoneField, policy.zone().value()); policyObject.setString(canonicalNameField, policy.canonicalName().value()); policy.dnsZone().ifPresent(dnsZone -> policyObject.setString(dnsZoneField, dnsZone)); @@ -57,8 +59,8 @@ public class RoutingPolicySerializer { Set rotations = new LinkedHashSet<>(); inspect.field(rotationsField).traverse((ArrayTraverser) (j, rotation) -> rotations.add(RotationName.from(rotation.asString()))); policies.add(new RoutingPolicy(owner, + clusterId(inspect.field(clusterField)), ZoneId.from(inspect.field(zoneField).asString()), - HostName.from(inspect.field(aliasField).asString()), HostName.from(inspect.field(canonicalNameField).asString()), optionalField(inspect.field(dnsZoneField), Function.identity()), rotations)); @@ -66,6 +68,11 @@ public class RoutingPolicySerializer { return Collections.unmodifiableSet(policies); } + // TODO: Remove and inline after Vespa 7.43 + private static ClusterSpec.Id clusterId(Inspector field) { + return optionalField(field, ClusterSpec.Id::from).orElseGet(() -> new ClusterSpec.Id("default")); + } + private static Optional optionalField(Inspector field, Function fieldMapper) { return Optional.of(field).filter(Inspector::valid).map(Inspector::asString).map(fieldMapper); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainerTest.java index 0541a0b05f5..b0f64eee532 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainerTest.java @@ -72,7 +72,7 @@ public class RoutingPolicyMaintainerTest { assertEquals("lb-0--tenant1:app1:default--prod.us-central-1.", records2.get().get(0).data().asString()); assertEquals("lb-0--tenant1:app1:default--prod.us-west-1.", records2.get().get(1).data().asString()); assertEquals(2, tester.controller().applications().routingPolicies(app1.id()).iterator().next() - .endpointsIn(SystemName.main).asList().size()); + .rotationEndpointsIn(SystemName.main).asList().size()); // Applications gains a new deployment ApplicationPackage updatedApplicationPackage = new ApplicationPackageBuilder() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java index 4fe465ce01e..4a4fd39ccb7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java @@ -3,9 +3,11 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.RotationName; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import org.junit.Test; @@ -19,20 +21,21 @@ import static org.junit.Assert.assertEquals; */ public class RoutingPolicySerializerTest { + private final RoutingPolicySerializer serializer = new RoutingPolicySerializer(); + @Test public void test_serialization() { - RoutingPolicySerializer serializer = new RoutingPolicySerializer(); ApplicationId owner = ApplicationId.defaultId(); Set rotations = Set.of(RotationName.from("r1"), RotationName.from("r2")); Set loadBalancers = ImmutableSet.of(new RoutingPolicy(owner, + ClusterSpec.Id.from("my-cluster1"), ZoneId.from("prod", "us-north-1"), - HostName.from("my-pretty-alias"), HostName.from("long-and-ugly-name"), Optional.of("zone1"), rotations), new RoutingPolicy(owner, + ClusterSpec.Id.from("my-cluster2"), ZoneId.from("prod", "us-north-2"), - HostName.from("my-pretty-alias-2"), HostName.from("long-and-ugly-name-2"), Optional.empty(), rotations)); @@ -40,4 +43,30 @@ public class RoutingPolicySerializerTest { assertEquals(loadBalancers, serialized); } + @Test + public void test_legacy_serialization() { // TODO: Remove after 7.43 has been released + String json = "{\n" + + " \"routingPolicies\": [\n" + + " {\n" + + " \"alias\": \"my-pretty-alias\",\n" + + " \"zone\": \"prod.us-north-1\",\n" + + " \"canonicalName\": \"long-and-ugly-name\",\n" + + " \"dnsZone\": \"zone1\",\n" + + " \"rotations\": [\n" + + " \"r1\",\n" + + " \"r2\"\n" + + " ]\n" + + " }\n" + + " ]\n" + + "}"; + ApplicationId owner = ApplicationId.defaultId(); + Set expected = Set.of(new RoutingPolicy(owner, + ClusterSpec.Id.from("default"), + ZoneId.from("prod", "us-north-1"), + HostName.from("long-and-ugly-name"), + Optional.of("zone1"), + Set.of(RotationName.from("r1"), RotationName.from("r2")))); + assertEquals(expected, serializer.fromSlime(owner, SlimeUtils.jsonToSlime(json))); + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index d3f0f423089..23b09273624 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -1346,8 +1346,9 @@ public class ApplicationApiTest extends ControllerContainerTest { @Test public void applicationWithPerClusterGlobalRotation() { Application app = controllerTester.createApplication(); - RoutingPolicy policy = new RoutingPolicy(app.id(), ZoneId.from(Environment.prod, RegionName.from("us-west-1")), - ClusterSpec.Id.from("default"), controllerTester.controller().system(), + RoutingPolicy policy = new RoutingPolicy(app.id(), + ClusterSpec.Id.from("default"), + ZoneId.from(Environment.prod, RegionName.from("us-west-1")), HostName.from("lb-0-canonical-name"), Optional.of("dns-zone-1"), Set.of(RotationName.from("c0"))); tester.controller().curator().writeRoutingPolicies(app.id(), Set.of(policy)); -- cgit v1.2.3