Merge pull request #11338 from vespa-engine/mpolden/routing-policy-for-empty-deployment-spec

Create routing policy for empty deployment spec MERGEOK

2 files changed, 87 insertions, 42 deletions

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 98483763a0d..70134d5a86d 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,7 +1,6 @@
 // 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.DeploymentInstanceSpec;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.zone.ZoneId;
@@ -75,10 +74,11 @@ public class RoutingPolicies {
      */
     public void refresh(ApplicationId application, DeploymentSpec deploymentSpec, ZoneId zone) {
         if (!controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) return;
-        var lbs = new AllocatedLoadBalancers(application, zone, controller.serviceRegistry().configServer().getLoadBalancers(application, zone));
+        var lbs = new AllocatedLoadBalancers(application, zone, controller.serviceRegistry().configServer().getLoadBalancers(application, zone),
+                                             deploymentSpec);
         try (var lock = db.lockRoutingPolicies()) {
-            removeObsoleteEndpointsFromDns(lbs, deploymentSpec, lock);
-            storePoliciesOf(lbs, deploymentSpec, lock);
+            removeObsoleteEndpointsFromDns(lbs, lock);
+            storePoliciesOf(lbs, lock);
             removeObsoletePolicies(lbs, lock);
             registerEndpointsInDns(lbs, lock);
         }
@@ -104,27 +104,25 @@ public class RoutingPolicies {
     }
 
     /** Store routing policies for given route */
-    private void storePoliciesOf(AllocatedLoadBalancers loadBalancers, DeploymentSpec spec, @SuppressWarnings("unused") Lock lock) {
-        Set<RoutingPolicy> policies = new LinkedHashSet<>(get(loadBalancers.application));
+    private void storePoliciesOf(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) {
+        var policies = new LinkedHashSet<>(get(loadBalancers.application));
         for (LoadBalancer loadBalancer : loadBalancers.list) {
-            spec.instance(loadBalancer.application().instance()).ifPresent(instanceSpec -> {
-                RoutingPolicy policy = createPolicy(loadBalancers.application, instanceSpec, loadBalancers.zone, loadBalancer);
-                if (!policies.add(policy)) {
-                    policies.remove(policy);
-                    policies.add(policy);
-                }
-            });
+            var endpointIds = loadBalancers.endpointIdsOf(loadBalancer);
+            var policy = createPolicy(loadBalancers.application, loadBalancers.zone, loadBalancer, endpointIds);
+            if (!policies.add(policy)) {
+                // Update existing policy
+                policies.remove(policy);
+                policies.add(policy);
+            }
         }
         db.writeRoutingPolicies(loadBalancers.application, policies);
     }
 
     /** Create a policy for given load balancer and register a CNAME for it */
-    private RoutingPolicy createPolicy(ApplicationId application, DeploymentInstanceSpec instanceSpec, ZoneId zone,
-                                       LoadBalancer loadBalancer) {
-        var endpoints = endpointIdsOf(loadBalancer, zone, instanceSpec);
-        var routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone,
-                                              loadBalancer.hostname(), loadBalancer.dnsZone(),
-                                              endpoints);
+    private RoutingPolicy createPolicy(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer,
+                                       Set<EndpointId> endpointIds) {
+        var routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, loadBalancer.hostname(),
+                                              loadBalancer.dnsZone(), endpointIds);
         var name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName());
         var data = RecordData.fqdn(loadBalancer.hostname().value());
         controller.nameServiceForwarder().createCname(name, data, Priority.normal);
@@ -150,26 +148,24 @@ public class RoutingPolicies {
     }
 
     /** Remove unreferenced global endpoints for given route from DNS */
-    private void removeObsoleteEndpointsFromDns(AllocatedLoadBalancers loadBalancers, DeploymentSpec deploymentSpec, @SuppressWarnings("unused") Lock lock) {
+    private void removeObsoleteEndpointsFromDns(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) {
         var zonePolicies = get(loadBalancers.application, loadBalancers.zone);
         var removalCandidates = routingTableFrom(zonePolicies).keySet();
-        var activeRoutingIds = routingIdsFrom(loadBalancers, deploymentSpec);
+        var activeRoutingIds = routingIdsFrom(loadBalancers);
         removalCandidates.removeAll(activeRoutingIds);
         for (var id : removalCandidates) {
-            Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system());
+            var endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system());
             controller.nameServiceForwarder().removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()), Priority.normal);
         }
     }
 
     /** Compute routing IDs from given load balancers */
-    private static Set<RoutingId> routingIdsFrom(AllocatedLoadBalancers loadBalancers, DeploymentSpec spec) {
+    private static Set<RoutingId> routingIdsFrom(AllocatedLoadBalancers loadBalancers) {
         Set<RoutingId> routingIds = new LinkedHashSet<>();
         for (var loadBalancer : loadBalancers.list) {
-            spec.instance(loadBalancer.application().instance()).ifPresent(instanceSpec -> {
-                for (var endpointId : endpointIdsOf(loadBalancer, loadBalancers.zone, instanceSpec)) {
-                    routingIds.add(new RoutingId(loadBalancer.application(), endpointId));
-                }
-            });
+            for (var endpointId : loadBalancers.endpointIdsOf(loadBalancer)) {
+                routingIds.add(new RoutingId(loadBalancer.application(), endpointId));
+            }
         }
         return Collections.unmodifiableSet(routingIds);
     }
@@ -187,29 +183,39 @@ public class RoutingPolicies {
         return routingTable;
     }
 
-    /** Compute all endpoint IDs of given load balancer */
-    private static Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer, ZoneId zone, DeploymentInstanceSpec 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 allocated to a deployment */
     private static class AllocatedLoadBalancers {
 
         private final ApplicationId application;
         private final ZoneId zone;
         private final List<LoadBalancer> list;
+        private final DeploymentSpec deploymentSpec;
 
-        private AllocatedLoadBalancers(ApplicationId application, ZoneId zone, List<LoadBalancer> loadBalancers) {
+        private AllocatedLoadBalancers(ApplicationId application, ZoneId zone, List<LoadBalancer> loadBalancers,
+                                       DeploymentSpec deploymentSpec) {
             this.application = application;
             this.zone = zone;
             this.list = loadBalancers.stream()
                                      .filter(AllocatedLoadBalancers::shouldUpdatePolicy)
                                      .collect(Collectors.toUnmodifiableList());
+            this.deploymentSpec = deploymentSpec;
+        }
+
+        /** Compute all endpoint IDs for given load balancer */
+        private Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer) {
+            if (zone.environment().isManuallyDeployed()) { // Manual deployments do not have any configurable endpoints
+                return Set.of();
+            }
+            var instanceSpec = deploymentSpec.instance(loadBalancer.application().instance());
+            if (instanceSpec.isEmpty()) {
+                return Set.of();
+            }
+            return instanceSpec.get().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());
         }
 
         private static boolean shouldUpdatePolicy(LoadBalancer loadBalancer) {
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 bc184715589..c9ec03bd1af 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
@@ -1,12 +1,14 @@
 // 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.application.api.ValidationId;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Instance;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 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;
@@ -16,6 +18,7 @@ import com.yahoo.vespa.hosted.controller.application.RoutingPolicy;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
 import org.junit.Test;
 
 import java.net.URI;
@@ -51,7 +54,7 @@ public class RoutingPoliciesTest {
             .build();
 
     @Test
-    public void maintains_global_routing_policies() {
+    public void global_routing_policies() {
         int clustersPerZone = 2;
         int numberOfDeployments = 2;
         var applicationPackage = new ApplicationPackageBuilder()
@@ -137,10 +140,9 @@ public class RoutingPoliciesTest {
     }
 
     @Test
-    public void maintains_routing_policies_per_zone() {
+    public void zone_routing_policies() {
         // Deploy application
         int clustersPerZone = 2;
-        int buildNumber = 42;
         provisionLoadBalancers(clustersPerZone, context1.instanceId(), zone1, zone2);
         context1.submit(applicationPackage).deploy();
 
@@ -238,6 +240,43 @@ public class RoutingPoliciesTest {
                      tester.controller().applications().clusterEndpoints(context1.deploymentIdIn(zone1)));
     }
 
+    @Test
+    public void manual_deployment_creates_routing_policy() {
+        // Empty application package is valid in manually deployed environments
+        var emptyApplicationPackage = new ApplicationPackageBuilder().build();
+        var zone = ZoneId.from("dev", "us-east-1");
+        tester.controllerTester().serviceRegistry().zoneRegistry().setZones(ZoneApiMock.from(zone.environment(), zone.region()));
+        provisionLoadBalancers(1, context1.instanceId(), zone);
+
+        // Deploy to dev
+        tester.controller().applications().deploy(context1.instanceId(), zone, Optional.of(emptyApplicationPackage), DeployOptions.none());
+        assertEquals("DeploymentSpec is not persisted", DeploymentSpec.empty, context1.application().deploymentSpec());
+        context1.flushDnsUpdates();
+
+        // Routing policy is created and DNS is updated
+        assertEquals(1, policies(context1.instance()).size());
+        assertEquals(Set.of("c0.app1.tenant1.us-east-1.dev.vespa.oath.cloud"), recordNames());
+    }
+
+    @Test
+    public void manual_deployment_creates_routing_policy_with_non_empty_spec() {
+        // Initial deployment
+        context1.submit(applicationPackage).deploy();
+        var zone = ZoneId.from("dev", "us-east-1");
+        tester.controllerTester().serviceRegistry().zoneRegistry().setZones(ZoneApiMock.from(zone.environment(), zone.region()));
+
+        // Deploy to dev under different instance
+        var devInstance = context1.application().id().instance("user");
+        provisionLoadBalancers(1, devInstance, zone);
+        tester.controller().applications().deploy(devInstance, zone, Optional.of(applicationPackage), DeployOptions.none());
+        assertEquals("DeploymentSpec is persisted", applicationPackage.deploymentSpec(), context1.application().deploymentSpec());
+        context1.flushDnsUpdates();
+
+        // Routing policy is created and DNS is updated
+        assertEquals(1, policies(tester.instance(devInstance)).size());
+        assertEquals(Set.of("c0.user.app1.tenant1.us-east-1.dev.vespa.oath.cloud"), recordNames());
+    }
+
     private Set<RoutingPolicy> policies(Instance instance) {
         return tester.controller().curator().readRoutingPolicies(instance.id());
     }