Persist cluster of routing policy instead of DNS alias

6 files changed, 68 insertions, 34 deletions

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<String> dnsZone;
     private final Set<RotationName> 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<String> dnsZone, Set<RotationName> 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<String> dnsZone, Set<RotationName> 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<Record> 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<Record> records = nameService.findRecords(Record.Type.CNAME, RecordName.from(policy.alias().value()));
+                    List<Record> 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<RotationName> 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 <T> Optional<T> optionalField(Inspector field, Function<String, T> 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<RotationName> rotations = Set.of(RotationName.from("r1"), RotationName.from("r2"));
         Set<RoutingPolicy> 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<RoutingPolicy> 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));