Create ALIAS records for DeploymentSpec endpoints

ALIAS records (global endpoints for directly routed zones) will now be created
from endpoints declared in `DeploymentSpec`.

4 files changed, 126 insertions, 81 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java
index 9f686570da1..1b0c0b9d982 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java
@@ -25,6 +25,8 @@ public class LoadBalancer {
     private final Optional<String> dnsZone;
     private final Set<RotationName> rotations;
 
+    // TODO(mpolden): Kept for API compatibility with internal code. This constructor can be removed when all usages are
+    // TODO(mpolden): removed
     public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, HostName hostname,
                         Optional<String> dnsZone, Set<RotationName> rotations) {
         this.id = Objects.requireNonNull(id, "id must be non-null");
@@ -35,6 +37,16 @@ public class LoadBalancer {
         this.rotations = ImmutableSortedSet.copyOf(Objects.requireNonNull(rotations, "rotations must be non-null"));
     }
 
+    public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, HostName hostname,
+                        Optional<String> dnsZone) {
+        this.id = Objects.requireNonNull(id, "id must be non-null");
+        this.application = Objects.requireNonNull(application, "application must be non-null");
+        this.cluster = Objects.requireNonNull(cluster, "cluster must be non-null");
+        this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null");
+        this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null");
+        this.rotations = Set.of();
+    }
+
     public String id() {
         return id;
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 7406795d0e3..65ef270ffd5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -470,7 +470,7 @@ public class ApplicationController {
         } finally {
             // Even if prepare fails, a load balancer may have been provisioned. Always refresh routing policies so that
             // any DNS updates can be propagated as early as possible.
-            routingPolicies.refresh(application, zone);
+            routingPolicies.refresh(application, applicationPackage.deploymentSpec(), zone);
         }
     }
 
@@ -781,7 +781,7 @@ public class ApplicationController {
         } catch (NotFoundException ignored) {
             // ok; already gone
         } finally {
-            routingPolicies.refresh(application.get().id(), zone);
+            routingPolicies.refresh(application.get().id(), application.get().deploymentSpec(), zone);
         }
         return application.withoutDeploymentIn(zone);
     }
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 d23ae913889..020203c6548 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
@@ -1,8 +1,8 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 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.RotationName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -72,14 +72,13 @@ public class RoutingPolicies {
      * Refresh routing policies for application in given zone. This is idempotent and changes will only be performed if
      * load balancers for given application have changed.
      */
-    public void refresh(ApplicationId application, ZoneId zone) {
-        // TODO: Use this to decide how apply routing policies for shared routing layer
+    public void refresh(ApplicationId application, DeploymentSpec deploymentSpec, ZoneId zone) {
         if (!controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) return;
         var lbs = new LoadBalancers(application, zone, controller.applications().configServer()
                                                                  .getLoadBalancers(application, zone));
         try (var lock = db.lockRoutingPolicies()) {
-            removeObsoleteEndpointsFromDns(lbs, lock);
-            storePoliciesOf(lbs, lock);
+            removeObsoleteEndpointsFromDns(lbs, deploymentSpec, lock);
+            storePoliciesOf(lbs, deploymentSpec, lock);
             removeObsoletePolicies(lbs, lock);
             registerEndpointsInDns(lbs, lock);
         }
@@ -105,10 +104,10 @@ public class RoutingPolicies {
     }
 
     /** Store routing policies for given route */
-    private void storePoliciesOf(LoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) {
+    private void storePoliciesOf(LoadBalancers loadBalancers, DeploymentSpec spec, @SuppressWarnings("unused") Lock lock) {
         Set<RoutingPolicy> policies = new LinkedHashSet<>(get(loadBalancers.application));
         for (LoadBalancer loadBalancer : loadBalancers.list) {
-            RoutingPolicy policy = createPolicy(loadBalancers.application, loadBalancers.zone, loadBalancer);
+            RoutingPolicy policy = createPolicy(loadBalancers.application, spec, loadBalancers.zone, loadBalancer);
             if (!policies.add(policy)) {
                 policies.remove(policy);
                 policies.add(policy);
@@ -118,17 +117,14 @@ public class RoutingPolicies {
     }
 
     /** Create a policy for given load balancer and register a CNAME for it */
-    private RoutingPolicy createPolicy(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer) {
-        // TODO(mpolden): Remove rotations from LoadBalancer. Use endpoints from deployment spec instead
-        Set<EndpointId> endpoints = loadBalancer.rotations().stream()
-                                                .map(RotationName::value)
-                                                .map(EndpointId::of)
-                                                .collect(Collectors.toSet());
-        RoutingPolicy routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone,
-                                                        loadBalancer.hostname(), loadBalancer.dnsZone(),
-                                                        endpoints);
-        RecordName name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName());
-        RecordData data = RecordData.fqdn(loadBalancer.hostname().value());
+    private RoutingPolicy createPolicy(ApplicationId application, DeploymentSpec deploymentSpec, ZoneId zone,
+                                       LoadBalancer loadBalancer) {
+        var endpoints = endpointIdsOf(loadBalancer, zone, deploymentSpec);
+        var routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone,
+                                              loadBalancer.hostname(), loadBalancer.dnsZone(),
+                                              endpoints);
+        var name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName());
+        var data = RecordData.fqdn(loadBalancer.hostname().value());
         controller.nameServiceForwarder().createCname(name, data, Priority.normal);
         return routingPolicy;
     }
@@ -152,10 +148,10 @@ public class RoutingPolicies {
     }
 
     /** Remove unreferenced global endpoints for given route from DNS */
-    private void removeObsoleteEndpointsFromDns(LoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) {
+    private void removeObsoleteEndpointsFromDns(LoadBalancers loadBalancers, DeploymentSpec deploymentSpec, @SuppressWarnings("unused") Lock lock) {
         var zonePolicies = get(loadBalancers.application, loadBalancers.zone);
         var removalCandidates = routingTableFrom(zonePolicies).keySet();
-        var activeRoutingIds = routingIdsFrom(loadBalancers.list);
+        var activeRoutingIds = routingIdsFrom(loadBalancers, deploymentSpec);
         removalCandidates.removeAll(activeRoutingIds);
         for (var id : removalCandidates) {
             Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system());
@@ -164,11 +160,11 @@ public class RoutingPolicies {
     }
 
     /** Compute routing IDs from given load balancers */
-    private static Set<RoutingId> routingIdsFrom(List<LoadBalancer> loadBalancers) {
+    private static Set<RoutingId> routingIdsFrom(LoadBalancers loadBalancers, DeploymentSpec spec) {
         Set<RoutingId> routingIds = new LinkedHashSet<>();
-        for (var loadBalancer : loadBalancers) {
-            for (var rotation : loadBalancer.rotations()) {
-                routingIds.add(new RoutingId(loadBalancer.application(), EndpointId.of(rotation.value())));
+        for (var loadBalancer : loadBalancers.list) {
+            for (var endpointId : endpointIdsOf(loadBalancer, loadBalancers.zone, spec)) {
+                routingIds.add(new RoutingId(loadBalancer.application(), endpointId));
             }
         }
         return Collections.unmodifiableSet(routingIds);
@@ -187,6 +183,16 @@ public class RoutingPolicies {
         return routingTable;
     }
 
+    /** Compute all endpoint IDs of given load balancer */
+    private static Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer, ZoneId zone, DeploymentSpec spec) {
+        return spec.endpoints().stream()
+                   .filter(endpoint -> endpoint.containerId().equals(loadBalancer.cluster().value()))
+                   .filter(endpoint -> endpoint.regions().contains(zone.region()))
+                   .map(com.yahoo.config.application.api.Endpoint::endpointId)
+                   .map(EndpointId::of)
+                   .collect(Collectors.toSet());
+    }
+
     /** Load balancers for a particular deployment */
     private static class LoadBalancers {
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java
index 600fca4f45e..e6387ee3e0c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java
@@ -3,19 +3,18 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.RotationName;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.RoutingPolicy;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
+import com.yahoo.vespa.hosted.controller.deployment.BuildJob;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import org.junit.Test;
 
@@ -26,7 +25,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
@@ -54,59 +52,88 @@ public class RoutingPoliciesTest {
 
     @Test
     public void maintains_global_routing_policies() {
-        int buildNumber = 42;
+        long buildNumber = BuildJob.defaultBuildNumber;
         int clustersPerZone = 2;
-        // Cluster 0 is member of 2 global rotations
-        Map<Integer, Set<RotationName>> rotations = Map.of(0, Set.of(RotationName.from("r0"), RotationName.from("r1")));
-        provisionLoadBalancers(clustersPerZone, rotations, app1.id(), zone1, zone2);
+        int numberOfDeployments = 2;
+        var applicationPackage = new ApplicationPackageBuilder()
+                .region(zone1.region())
+                .region(zone2.region())
+                .endpoint("r0", "c0")
+                .endpoint("r1", "c0", "us-west-1")
+                .endpoint("r2", "c1")
+                .build();
+        provisionLoadBalancers(clustersPerZone, app1.id(), zone1, zone2);
 
-        // Creates alias records for cluster0
+        // Creates alias records
         tester.deployCompletely(app1, applicationPackage, ++buildNumber);
-        Supplier<List<Record>> records1 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r0.app1.tenant1.global.vespa.oath.cloud"));
-        Supplier<List<Record>> records2 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r1.app1.tenant1.global.vespa.oath.cloud"));
-        assertEquals(2, records1.get().size());
-        assertEquals(records1.get().size(), records2.get().size());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records1.get().get(0).data().asString());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records1.get().get(1).data().asString());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records2.get().get(0).data().asString());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records2.get().get(1).data().asString());
-        assertEquals(2, tester.controller().applications().routingPolicies().get(app1.id()).iterator().next()
-                              .rotationEndpointsIn(SystemName.main).asList().size());
+        var endpoint1 = "r0.app1.tenant1.global.vespa.oath.cloud";
+        var endpoint2 = "r1.app1.tenant1.global.vespa.oath.cloud";
+        var endpoint3 = "r2.app1.tenant1.global.vespa.oath.cloud";
+
+        assertEquals(endpoint1 + " points to c0 in all regions",
+                     List.of("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1",
+                             "lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"),
+                     aliasDataOf(endpoint1));
+        assertEquals(endpoint2 + " points to c0 us-west-1",
+                     List.of("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"),
+                     aliasDataOf(endpoint2));
+        assertEquals(endpoint3 + " points to c1 in all regions",
+                     List.of("lb-1--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1",
+                             "lb-1--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"),
+                     aliasDataOf(endpoint3));
+        assertEquals("Routing policy count is equal to cluster count",
+                     numberOfDeployments * clustersPerZone,
+                     tester.controller().applications().routingPolicies().get(app1.id()).size());
 
         // Applications gains a new deployment
-        ApplicationPackage updatedApplicationPackage = new ApplicationPackageBuilder()
-                .environment(Environment.prod)
+        ApplicationPackage applicationPackage2 = new ApplicationPackageBuilder()
+                .region(zone1.region())
+                .region(zone2.region())
+                .region(zone3.region())
+                .endpoint("r0", "c0")
+                .endpoint("r1", "c0", "us-west-1")
+                .endpoint("r2", "c1")
+                .build();
+        numberOfDeployments++;
+        provisionLoadBalancers(clustersPerZone, app1.id(), zone3);
+        tester.deployCompletely(app1, applicationPackage2, ++buildNumber);
+
+        // Endpoint is updated to contain cluster in new deployment
+        assertEquals(endpoint1 + " points to c0 in all regions",
+                     List.of("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1",
+                             "lb-0--tenant1:app1:default--prod.us-east-3/dns-zone-1/prod.us-east-3",
+                             "lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"),
+                     aliasDataOf(endpoint1));
+
+        // Another application is deployed with a single cluster and global endpoint
+        var endpoint4 = "r0.app2.tenant1.global.vespa.oath.cloud";
+        provisionLoadBalancers(1, app2.id(), zone1, zone2);
+        var applicationPackage3 = new ApplicationPackageBuilder()
+                .region(zone1.region())
+                .region(zone2.region())
+                .endpoint("r0", "c0")
+                .build();
+        tester.deployCompletely(app2, applicationPackage3);
+        assertEquals(endpoint4 + " points to c0 in all regions",
+                     List.of("lb-0--tenant1:app2:default--prod.us-central-1/dns-zone-1/prod.us-central-1",
+                             "lb-0--tenant1:app2:default--prod.us-west-1/dns-zone-1/prod.us-west-1"),
+                     aliasDataOf(endpoint4));
+
+        // All endpoints for app1 are removed
+        ApplicationPackage applicationPackage4 = new ApplicationPackageBuilder()
                 .region(zone1.region())
                 .region(zone2.region())
                 .region(zone3.region())
                 .build();
-        int numberOfDeployments = 3;
-        provisionLoadBalancers(clustersPerZone, rotations, app1.id(), zone3);
-        tester.deployCompletely(app1, updatedApplicationPackage, ++buildNumber);
-
-        // Cluster in new deployment is added to the rotation
-        assertEquals(numberOfDeployments, records1.get().size());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records1.get().get(0).data().asString());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-east-3/dns-zone-1/prod.us-east-3", records1.get().get(1).data().asString());
-        assertEquals("lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records1.get().get(2).data().asString());
-
-        // Another application is deployed
-        Supplier<List<Record>> records3 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r0.app2.tenant1.global.vespa.oath.cloud"));
-        provisionLoadBalancers(1, Map.of(0, Set.of(RotationName.from("r0"))), app2.id(), zone1, zone2);
-        tester.deployCompletely(app2, applicationPackage);
-        assertEquals(2, records3.get().size());
-        assertEquals("lb-0--tenant1:app2:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records3.get().get(0).data().asString());
-        assertEquals("lb-0--tenant1:app2:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records3.get().get(1).data().asString());
-
-        // All rotations for app1 are removed
-        provisionLoadBalancers(clustersPerZone, Map.of(), app1.id(), zone1, zone2, zone3);
-        tester.deployCompletely(app1, updatedApplicationPackage, ++buildNumber);
-        assertEquals(List.of(), records1.get());
+        tester.deployCompletely(app1, applicationPackage4, ++buildNumber);
+        assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint1));
+        assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint2));
+        assertEquals("DNS records are removed", List.of(), aliasDataOf(endpoint3));
         Set<RoutingPolicy> policies = tester.controller().curator().readRoutingPolicies(app1.id());
         assertEquals(clustersPerZone * numberOfDeployments, policies.size());
         assertTrue("Rotation membership is removed from all policies",
                    policies.stream().allMatch(policy -> policy.endpoints().isEmpty()));
-        assertEquals("Rotations for " + app2 + " are not removed", 2, records3.get().size());
+        assertEquals("Rotations for " + app2 + " are not removed", 2, aliasDataOf(endpoint4).size());
     }
 
     @Test
@@ -222,30 +249,30 @@ public class RoutingPoliciesTest {
                      .collect(Collectors.toSet());
     }
 
-    private void provisionLoadBalancers(int clustersPerZone, Map<Integer, Set<RotationName>> clusterRotations, ApplicationId application, ZoneId... zones) {
-        for (ZoneId zone : zones) {
-            tester.configServer().removeLoadBalancers(application, zone);
-            tester.configServer().addLoadBalancers(zone, createLoadBalancers(zone, application, clustersPerZone, clusterRotations));
-        }
+    private List<String> aliasDataOf(String name) {
+        return tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from(name)).stream()
+                     .map(Record::data)
+                     .map(RecordData::asString)
+                     .collect(Collectors.toList());
     }
 
     private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, ZoneId... zones) {
-        provisionLoadBalancers(clustersPerZone, Map.of(), application, zones);
+        for (ZoneId zone : zones) {
+            tester.configServer().removeLoadBalancers(application, zone);
+            tester.configServer().addLoadBalancers(zone, createLoadBalancers(zone, application, clustersPerZone));
+        }
     }
 
-    private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count,
-                                                          Map<Integer, Set<RotationName>> clusterRotations) {
+    private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count) {
         List<LoadBalancer> loadBalancers = new ArrayList<>();
         for (int i = 0; i < count; i++) {
-            Set<RotationName> rotations = clusterRotations.getOrDefault(i, Collections.emptySet());
             loadBalancers.add(
                     new LoadBalancer("LB-" + i + "-Z-" + zone.value(),
                                      application,
                                      ClusterSpec.Id.from("c" + i),
                                      HostName.from("lb-" + i + "--" + application.serializedForm() +
                                                    "--" + zone.value()),
-                                     Optional.of("dns-zone-1"),
-                                     rotations));
+                                     Optional.of("dns-zone-1")));
         }
         return loadBalancers;
     }