Get load balancers once per zone

4 files changed, 42 insertions, 37 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
index ad52ba48d4e..9a75764befe 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
@@ -5,6 +5,7 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname;
+import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.serviceview.bindings.ApplicationView;
 
 import java.io.IOException;
@@ -71,6 +72,7 @@ public interface ConfigServer {
     /** Get service convergence status for given deployment */
     Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment);
 
-    List<LoadBalancer> getLoadBalancers(DeploymentId deployment);
+    /** Get all load balancers in given zone */
+    List<LoadBalancer> getLoadBalancers(ZoneId zone);
 
 }
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 c175681a784..bc684e753d1 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
@@ -5,7 +5,6 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.curator.Lock;
-import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
@@ -56,26 +55,25 @@ public class RoutingPolicyMaintainer extends Maintainer {
 
     @Override
     protected void maintain() {
-        Map<DeploymentId, List<LoadBalancer>> loadBalancers = loadBalancers(controller().applications().asList());
+        Map<DeploymentId, List<LoadBalancer>> loadBalancers = findLoadBalancers();
         updateDnsRecords(loadBalancers);
         removeObsoleteDnsRecords(loadBalancers);
     }
 
     /** Find all exclusive load balancers owned by given applications, grouped by deployment */
-    private Map<DeploymentId, List<LoadBalancer>> loadBalancers(List<Application> applications) {
+    private Map<DeploymentId, List<LoadBalancer>> findLoadBalancers() {
         Map<DeploymentId, List<LoadBalancer>> result = new LinkedHashMap<>();
-        for (Application application : applications) {
-            for (ZoneId zone : application.deployments().keySet()) {
-                DeploymentId deployment = new DeploymentId(application.id(), zone);
-                try {
-                    List<LoadBalancer> loadBalancers = findLoadBalancersIn(deployment);
-                    if (loadBalancers.isEmpty()) continue;
-                    result.put(deployment, loadBalancers);
-                } catch (Exception e) {
-                    log.log(LogLevel.WARNING,
-                            String.format("Got exception fetching load balancers for application: %s, in zone: %s. Retrying in %s",
-                                          application.id().toShortString(), zone.value(), maintenanceInterval()),  e);
-                }
+        for (ZoneId zone : controller().zoneRegistry().zones().controllerUpgraded().ids()) {
+            List<LoadBalancer> loadBalancers = findLoadBalancersIn(zone);
+            for (LoadBalancer loadBalancer : loadBalancers) {
+                DeploymentId deployment = new DeploymentId(loadBalancer.application(), zone);
+                result.compute(deployment, (k, existing) -> {
+                    if (existing == null) {
+                        existing = new ArrayList<>();
+                    }
+                    existing.add(loadBalancer);
+                    return existing;
+                });
             }
         }
         return Collections.unmodifiableMap(result);
@@ -123,15 +121,14 @@ public class RoutingPolicyMaintainer extends Maintainer {
                                  loadBalancer.rotations());
     }
 
-    /** Find all load balancers assigned to application in given zone */
-    private List<LoadBalancer> findLoadBalancersIn(DeploymentId deployment) {
+    /** Find all load balancers in given zone */
+    private List<LoadBalancer> findLoadBalancersIn(ZoneId zone) {
         try {
-            return controller().applications().configServer().getLoadBalancers(deployment);
+            return controller().applications().configServer().getLoadBalancers(zone);
         } catch (Exception e) {
             log.log(LogLevel.WARNING,
-                    String.format("Got exception fetching load balancers for application: %s, in zone: %s. Retrying in %s",
-                                  deployment.applicationId().toShortString(), deployment.zoneId().value(),
-                                  maintenanceInterval()),  e);
+                    String.format("Got exception fetching load balancers in zone: %s. Retrying in %s",
+                                  zone.value(), maintenanceInterval()),  e);
         }
         return Collections.emptyList();
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 4c60e8be677..69f348c9174 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -52,7 +52,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     private final Map<DeploymentId, ServiceConvergence> serviceStatus = new HashMap<>();
     private final Version initialVersion = new Version(6, 1, 0);
     private final Set<DeploymentId> suspendedApplications = new HashSet<>();
-    private final Map<DeploymentId, List<LoadBalancer>> loadBalancers = new HashMap<>();
+    private final Map<ZoneId, List<LoadBalancer>> loadBalancers = new HashMap<>();
 
     private Version lastPrepareVersion = null;
     private RuntimeException prepareException = null;
@@ -171,16 +171,22 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     }
 
     @Override
-    public List<LoadBalancer> getLoadBalancers(DeploymentId deployment) {
-        return loadBalancers.getOrDefault(deployment, Collections.emptyList());
+    public List<LoadBalancer> getLoadBalancers(ZoneId zone) {
+        return loadBalancers.getOrDefault(zone, Collections.emptyList());
     }
 
-    public void addLoadBalancers(ZoneId zoneId, ApplicationId applicationId, List<LoadBalancer> loadBalancers) {
-        this.loadBalancers.put(new DeploymentId(applicationId, zoneId), loadBalancers);
+    public void addLoadBalancers(ZoneId zone, List<LoadBalancer> loadBalancers) {
+        this.loadBalancers.compute(zone, (k, existing) -> {
+           if (existing == null) {
+               existing = new ArrayList<>();
+           }
+           existing.addAll(loadBalancers);
+           return existing;
+        });
     }
 
-    public void removeLoadBalancers(DeploymentId deployment) {
-        this.loadBalancers.remove(deployment);
+    public void removeLoadBalancers(ApplicationId application, ZoneId zone) {
+        getLoadBalancers(zone).removeIf(lb -> lb.application().equals(application));
     }
 
     @Override
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 654c0e803d9..addf43b7549 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
@@ -7,7 +7,6 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.RotationName;
 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.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
@@ -120,7 +119,10 @@ public class RoutingPolicyMaintainerTest {
 
         // Remove app2 completely
         tester.controller().applications().require(app2.id()).deployments().keySet()
-              .forEach(zone -> tester.controller().applications().deactivate(app2.id(), zone));
+              .forEach(zone -> {
+                  tester.controller().applications().deactivate(app2.id(), zone);
+                  tester.configServer().removeLoadBalancers(app2.id(), zone);
+              });
         maintainer.maintain();
         expectedRecords = Set.of(
                 "c0--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
@@ -142,15 +144,13 @@ public class RoutingPolicyMaintainerTest {
     }
 
     private void provisionLoadBalancers(Application application, int numberOfClustersPerZone) {
-        tester.controller().applications().get(application.id())
-              .orElseThrow(() -> new RuntimeException("No deployments"))
+        tester.controller().applications().require(application.id())
               .deployments().keySet()
-              .forEach(zone -> tester.configServer().removeLoadBalancers(new DeploymentId(application.id(), zone)));
-        tester.controller().applications().get(application.id())
-              .orElseThrow(() -> new RuntimeException("No deployments"))
+              .forEach(zone -> tester.configServer().removeLoadBalancers(application.id(), zone));
+        tester.controller().applications().require(application.id())
               .deployments().keySet()
               .forEach(zone -> tester.configServer()
-                                     .addLoadBalancers(zone, application.id(), createLoadBalancers(zone, application.id(), numberOfClustersPerZone)));
+                                     .addLoadBalancers(zone, createLoadBalancers(zone, application.id(), numberOfClustersPerZone)));
 
     }