Merge pull request #8470 from vespa-engine/mpolden/refactor-routing-policy-maintainer

Refactor RoutingPolicyMaintainer

8 files changed, 188 insertions, 182 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/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
index 1fb22d28ac7..e4b8a1d9e84 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
@@ -10,7 +10,6 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.api.integration.MetricsService;
 import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId;
@@ -28,13 +27,11 @@ import com.yahoo.vespa.hosted.controller.rotation.RotationId;
 
 import java.time.Instant;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
-import java.util.stream.Collectors;
 
 /**
  * An application that has been locked for modification. Provides methods for modifying an application's fields.
@@ -150,8 +147,7 @@ public class LockedApplication {
                                                   previousDeployment.clusterUtils(),
                                                   previousDeployment.clusterInfo(),
                                                   previousDeployment.metrics(),
-                                                  previousDeployment.activity(),
-                                                  previousDeployment.loadBalancers());
+                                                  previousDeployment.activity());
         return with(newDeployment);
     }
 
@@ -254,13 +250,6 @@ public class LockedApplication {
                                      outstandingChange, ownershipIssueId, owner, majorVersion, metrics, rotation, rotationStatus);
     }
 
-    public LockedApplication withLoadBalancersIn(ZoneId zoneId, List<LoadBalancer> loadBalancers) {
-        Map<ClusterSpec.Id, HostName> loadBalancersByCluster = loadBalancers.stream()
-                                                                            .collect(Collectors.toUnmodifiableMap(LoadBalancer::cluster,
-                                                                                                                  LoadBalancer::hostname));
-        return with(deployments.get(zoneId).withLoadBalancers(loadBalancersByCluster));
-    }
-
     /** Don't expose non-leaf sub-objects. */
     private LockedApplication with(Deployment deployment) {
         Map<ZoneId, Deployment> deployments = new LinkedHashMap<>(this.deployments);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java
index 3fd2932c3e1..51ac25bc321 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller.application;
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ClusterSpec.Id;
-import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 
@@ -30,17 +29,16 @@ public class Deployment {
     private final Map<Id, ClusterInfo> clusterInfo;
     private final DeploymentMetrics metrics;
     private final DeploymentActivity activity;
-    private final Map<Id, HostName> loadBalancers;
 
     public Deployment(ZoneId zone, ApplicationVersion applicationVersion, Version version, Instant deployTime) {
         this(zone, applicationVersion, version, deployTime, Collections.emptyMap(), Collections.emptyMap(),
-             DeploymentMetrics.none, DeploymentActivity.none, Collections.emptyMap());
+             DeploymentMetrics.none, DeploymentActivity.none);
     }
 
     public Deployment(ZoneId zone, ApplicationVersion applicationVersion, Version version, Instant deployTime,
                       Map<Id, ClusterUtilization> clusterUtilization, Map<Id, ClusterInfo> clusterInfo,
                       DeploymentMetrics metrics,
-                      DeploymentActivity activity, Map<Id, HostName> loadBalancers) {
+                      DeploymentActivity activity) {
         this.zone = Objects.requireNonNull(zone, "zone cannot be null");
         this.applicationVersion = Objects.requireNonNull(applicationVersion, "applicationVersion cannot be null");
         this.version = Objects.requireNonNull(version, "version cannot be null");
@@ -49,7 +47,6 @@ public class Deployment {
         this.clusterInfo = ImmutableMap.copyOf(Objects.requireNonNull(clusterInfo, "clusterInfo cannot be null"));
         this.metrics = Objects.requireNonNull(metrics, "deploymentMetrics cannot be null");
         this.activity = Objects.requireNonNull(activity, "activity cannot be null");
-        this.loadBalancers = ImmutableMap.copyOf(Objects.requireNonNull(loadBalancers, "loadBalancers cannot be null"));
     }
 
     /** Returns the zone this was deployed to */
@@ -82,33 +79,24 @@ public class Deployment {
         return clusterUtilization;
     }
 
-    public Map<Id, HostName> loadBalancers() {
-        return loadBalancers;
-    }
-
     public Deployment recordActivityAt(Instant instant) {
         return new Deployment(zone, applicationVersion, version, deployTime, clusterUtilization, clusterInfo, metrics,
-                              activity.recordAt(instant, metrics), loadBalancers);
+                              activity.recordAt(instant, metrics));
     }
 
     public Deployment withClusterUtils(Map<Id, ClusterUtilization> clusterUtilization) {
         return new Deployment(zone, applicationVersion, version, deployTime, clusterUtilization, clusterInfo, metrics,
-                              activity, loadBalancers);
+                              activity);
     }
 
     public Deployment withClusterInfo(Map<Id, ClusterInfo> newClusterInfo) {
         return new Deployment(zone, applicationVersion, version, deployTime, clusterUtilization, newClusterInfo, metrics,
-                              activity, loadBalancers);
+                              activity);
     }
 
     public Deployment withMetrics(DeploymentMetrics metrics) {
         return new Deployment(zone, applicationVersion, version, deployTime, clusterUtilization, clusterInfo, metrics,
-                              activity, loadBalancers);
-    }
-
-    public Deployment withLoadBalancers(Map<Id, HostName> loadBalancers) {
-        return new Deployment(zone, applicationVersion, version, deployTime, clusterUtilization, clusterInfo, metrics,
-                              activity, loadBalancers);
+                              activity);
     }
 
     /**
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 42e8a572815..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,8 +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.ApplicationController;
 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;
@@ -16,7 +14,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordId;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
-import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.RoutingPolicy;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 
@@ -24,6 +21,7 @@ import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -33,9 +31,10 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
- * Maintains routing policies for all exclusive load balancers in this system.
+ * Maintains DNS records as defined by routing policies for all exclusive load balancers in this system.
  *
  * @author mortent
+ * @author mpolden
  */
 public class RoutingPolicyMaintainer extends Maintainer {
 
@@ -43,7 +42,6 @@ public class RoutingPolicyMaintainer extends Maintainer {
 
     private final NameService nameService;
     private final CuratorDb db;
-    private final ApplicationController applications;
 
     public RoutingPolicyMaintainer(Controller controller,
                                    Duration interval,
@@ -53,39 +51,51 @@ public class RoutingPolicyMaintainer extends Maintainer {
         super(controller, interval, jobControl);
         this.nameService = nameService;
         this.db = db;
-        this.applications = controller.applications();
     }
 
     @Override
     protected void maintain() {
-        updateDnsRecords();
-        removeObsoleteDnsRecords();
+        Map<DeploymentId, List<LoadBalancer>> loadBalancers = findLoadBalancers();
+        updateDnsRecords(loadBalancers);
+        removeObsoleteDnsRecords(loadBalancers);
     }
 
-    /** Create DNS records for all exclusive load balancers */
-    private void updateDnsRecords() {
-        for (Application application : applications.asList()) {
-            for (ZoneId zone : application.deployments().keySet()) {
-                List<LoadBalancer> loadBalancers = findLoadBalancersIn(zone, application.id());
-                if (loadBalancers.isEmpty()) continue;
-
-                applications.lockIfPresent(application.id(), (locked) -> {
-                    applications.store(locked.withLoadBalancersIn(zone, loadBalancers));
+    /** Find all exclusive load balancers owned by given applications, grouped by deployment */
+    private Map<DeploymentId, List<LoadBalancer>> findLoadBalancers() {
+        Map<DeploymentId, List<LoadBalancer>> result = new LinkedHashMap<>();
+        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);
+    }
 
-                try (Lock lock = db.lockRoutingPolicies()) {
-                    Set<RoutingPolicy> policies = new LinkedHashSet<>(db.readRoutingPolicies(application.id()));
-                    for (LoadBalancer loadBalancer : loadBalancers) {
-                        try {
-                            policies.add(registerDnsAlias(application.id(), zone, loadBalancer));
-                        } catch (Exception e) {
-                            log.log(LogLevel.WARNING, "Failed to create or update DNS record for load balancer " +
-                                                      loadBalancer.hostname() + ". Retrying in " + maintenanceInterval(),
-                                    e);
-                        }
+    /** Create DNS records for all exclusive load balancers */
+    private void updateDnsRecords(Map<DeploymentId, List<LoadBalancer>> loadBalancers) {
+        for (Map.Entry<DeploymentId, List<LoadBalancer>> entry : loadBalancers.entrySet()) {
+            ApplicationId application = entry.getKey().applicationId();
+            ZoneId zone = entry.getKey().zoneId();
+            try (Lock lock = db.lockRoutingPolicies()) {
+                Set<RoutingPolicy> policies = new LinkedHashSet<>(db.readRoutingPolicies(application));
+                for (LoadBalancer loadBalancer : entry.getValue()) {
+                    try {
+                        policies.add(registerDnsAlias(application, zone, loadBalancer));
+                    } catch (Exception e) {
+                        log.log(LogLevel.WARNING, "Failed to create or update DNS record for load balancer " +
+                                                  loadBalancer.hostname() + ". Retrying in " + maintenanceInterval(),
+                                e);
                     }
-                    db.writeRoutingPolicies(application.id(), policies);
                 }
+                db.writeRoutingPolicies(application, policies);
             }
         }
     }
@@ -111,30 +121,26 @@ public class RoutingPolicyMaintainer extends Maintainer {
                                  loadBalancer.rotations());
     }
 
-    /** Find all load balancers assigned to application in given zone */
-    private List<LoadBalancer> findLoadBalancersIn(ZoneId zone, ApplicationId application) {
+    /** Find all load balancers in given zone */
+    private List<LoadBalancer> findLoadBalancersIn(ZoneId zone) {
         try {
-            return controller().applications().configServer().getLoadBalancers(new DeploymentId(application, zone));
+            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",
-                                  application.toShortString(), zone.value(), maintenanceInterval()),  e);
+                    String.format("Got exception fetching load balancers in zone: %s. Retrying in %s",
+                                  zone.value(), maintenanceInterval()),  e);
         }
         return Collections.emptyList();
     }
 
     /** Remove all DNS records that point to non-existing load balancers */
-    private void removeObsoleteDnsRecords() {
+    private void removeObsoleteDnsRecords(Map<DeploymentId, List<LoadBalancer>> loadBalancers) {
         try (Lock lock = db.lockRoutingPolicies()) {
             List<RoutingPolicy> removalCandidates = new ArrayList<>(db.readRoutingPolicies());
-            Set<HostName> activeLoadBalancers = controller().applications().asList().stream()
-                                                            .map(Application::deployments)
-                                                            .map(Map::values)
-                                                            .flatMap(Collection::stream)
-                                                            .map(Deployment::loadBalancers)
-                                                            .map(Map::values)
-                                                            .flatMap(Collection::stream)
-                                                            .collect(Collectors.toUnmodifiableSet());
+            Set<HostName> activeLoadBalancers = loadBalancers.values().stream()
+                                                             .flatMap(Collection::stream)
+                                                             .map(LoadBalancer::hostname)
+                                                             .collect(Collectors.toSet());
 
             // Remove any active load balancers
             removalCandidates.removeIf(lb -> activeLoadBalancers.contains(lb.canonicalName()));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
index 7f052fd7574..2242b3832de 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
@@ -89,7 +89,6 @@ public class ApplicationSerializer {
     private final String lastWrittenField = "lastWritten";
     private final String lastQueriesPerSecondField = "lastQueriesPerSecond";
     private final String lastWritesPerSecondField = "lastWritesPerSecond";
-    private final String loadBalancers = "loadBalancers";
 
     // DeploymentJobs fields
     private final String projectIdField = "projectId";
@@ -181,7 +180,6 @@ public class ApplicationSerializer {
         deployment.activity().lastWritten().ifPresent(instant -> object.setLong(lastWrittenField, instant.toEpochMilli()));
         deployment.activity().lastQueriesPerSecond().ifPresent(value -> object.setDouble(lastQueriesPerSecondField, value));
         deployment.activity().lastWritesPerSecond().ifPresent(value -> object.setDouble(lastWritesPerSecondField, value));
-        loadBalancersToSlime(deployment.loadBalancers(), object);
     }
 
     private void deploymentMetricsToSlime(DeploymentMetrics metrics, Cursor object) {
@@ -304,13 +302,6 @@ public class ApplicationSerializer {
         });
     }
 
-    private void loadBalancersToSlime(Map<ClusterSpec.Id, HostName> loadBalancerMap, Cursor parentObject) {
-        Cursor root = parentObject.setObject(loadBalancers);
-        for (Map.Entry<ClusterSpec.Id, HostName> entry : loadBalancerMap.entrySet()) {
-            root.setString(entry.getKey().value(), entry.getValue().value());
-        }
-    }
-
     // ------------------ Deserialization
 
     public Application fromSlime(Slime slime) {
@@ -353,8 +344,7 @@ public class ApplicationSerializer {
                               DeploymentActivity.create(optionalInstant(deploymentObject.field(lastQueriedField)),
                                                         optionalInstant(deploymentObject.field(lastWrittenField)),
                                                         optionalDouble(deploymentObject.field(lastQueriesPerSecondField)),
-                                                        optionalDouble(deploymentObject.field(lastWritesPerSecondField))),
-                              loadBalancerMapFromSlime(deploymentObject.field(loadBalancers)));
+                                                        optionalDouble(deploymentObject.field(lastWritesPerSecondField))));
     }
 
     private DeploymentMetrics deploymentMetricsFromSlime(Inspector object) {
@@ -510,12 +500,6 @@ public class ApplicationSerializer {
         return field.valid() ? optionalString(field).map(RotationId::new) : Optional.empty();
     }
 
-    private Map<ClusterSpec.Id, HostName> loadBalancerMapFromSlime(Inspector object) {
-        Map<ClusterSpec.Id, HostName> loadBalancers = new HashMap<>();
-        object.traverse((String name, Inspector value) -> loadBalancers.put(new ClusterSpec.Id(name), HostName.from(value.asString())));
-        return loadBalancers;
-    }
-
     private OptionalLong optionalLong(Inspector field) {
         return field.valid() ? OptionalLong.of(field.asLong()) : OptionalLong.empty();
     }
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 f1c571da451..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,10 +7,7 @@ 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.dns.Record;
-import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordId;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.RoutingPolicy;
@@ -26,7 +23,7 @@ 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;
 
@@ -35,99 +32,137 @@ import static org.junit.Assert.assertEquals;
  */
 public class RoutingPolicyMaintainerTest {
 
-    @Test
-    public void maintains_routing_policies() {
-        DeploymentTester tester = new DeploymentTester();
-        Application application = tester.createApplication("app1", "tenant1", 1, 1L);
-        RoutingPolicyMaintainer maintainer = new RoutingPolicyMaintainer(tester.controller(), Duration.ofHours(12),
-                                                                         new JobControl(new MockCuratorDb()),
-                                                                         tester.controllerTester().nameService(),
-                                                                         tester.controllerTester().curator());
-
-        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-                .environment(Environment.prod)
-                .region("us-west-1")
-                .region("us-central-1")
-                .build();
-
-        int numberOfClustersPerZone = 2;
+    private final DeploymentTester tester = new DeploymentTester();
+    private final Application app1 = tester.createApplication("app1", "tenant1", 1, 1L);
+    private final RoutingPolicyMaintainer maintainer = new RoutingPolicyMaintainer(tester.controller(), Duration.ofHours(12),
+                                                                                   new JobControl(new MockCuratorDb()),
+                                                                                   tester.controllerTester().nameService(),
+                                                                                   tester.controllerTester().curator());
+    private final ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+            .environment(Environment.prod)
+            .region("us-west-1")
+            .region("us-central-1")
+            .build();
 
+    @Test
+    public void maintains_routing_policies_per_zone() {
         // Deploy application
-        tester.deployCompletely(application, applicationPackage);
-        setupClustersWithLoadBalancers(tester, application, numberOfClustersPerZone);
+        int clustersPerZone = 2;
+        tester.deployCompletely(app1, applicationPackage);
+        provisionLoadBalancers(app1, clustersPerZone);
 
+        // Creates records and policies for all clusters in all zones
         maintainer.maintain();
-        Map<RecordId, Record> records = tester.controllerTester().nameService().records();
-        Supplier<Long> recordCount = () -> records.entrySet().stream().filter(entry -> entry.getValue().data().asString().contains("loadbalancer")).count();
-        assertEquals(4, (long) recordCount.get());
-
-        Set<RoutingPolicy> policies = tester.controller().curator().readRoutingPolicies(application.id());
-        assertEquals(4, policies.size());
-
-
-        // no update
+        Set<String> expectedRecords = Set.of(
+                "c0--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c0--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-central-1.vespa.oath.cloud"
+        );
+        assertEquals(expectedRecords, records());
+        assertEquals(4, policies(app1).size());
+
+        // Next run does nothing
         maintainer.maintain();
-        Map<RecordId, Record> records2 = tester.controllerTester().nameService().records();
-        assertEquals(4, (long) recordCount.get());
-        assertEquals(records, records2);
-
-
-        // add 1 cluster per zone
-        setupClustersWithLoadBalancers(tester, application, numberOfClustersPerZone + 1);
+        assertEquals(expectedRecords, records());
+        assertEquals(4, policies(app1).size());
 
+        // Add 1 cluster in each zone
+        provisionLoadBalancers(app1, clustersPerZone + 1);
         maintainer.maintain();
-        assertEquals(6, (long) recordCount.get());
-
-        Set<RoutingPolicy> policies2 = tester.controller().curator().readRoutingPolicies(application.id());
-        assertEquals(6, policies2.size());
-
-
-        // Add application
-        Application application2 = tester.createApplication("app2", "tenant1", 1, 1L);
-        tester.deployCompletely(application2, applicationPackage);
-        setupClustersWithLoadBalancers(tester, application2, numberOfClustersPerZone);
-
+        expectedRecords = Set.of(
+                "c0--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c2--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c0--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c2--app1--tenant1.prod.us-central-1.vespa.oath.cloud"
+        );
+        assertEquals(expectedRecords, records());
+        assertEquals(6, policies(app1).size());
+
+        // Add another application
+        Application app2 = tester.createApplication("app2", "tenant1", 1, 1L);
+        tester.deployCompletely(app2, applicationPackage);
+        provisionLoadBalancers(app2, clustersPerZone);
         maintainer.maintain();
-        assertEquals(10, (long) recordCount.get());
-
-        Set<RoutingPolicy> aliases4 = tester.controller().curator().readRoutingPolicies(application2.id());
-        assertEquals(4, aliases4.size());
-
-
-        // Remove cluster in app1
-        setupClustersWithLoadBalancers(tester, application, numberOfClustersPerZone);
-
+        expectedRecords = Set.of(
+                "c0--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c2--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c0--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c2--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c0--app2--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app2--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c0--app2--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app2--tenant1.prod.us-west-1.vespa.oath.cloud"
+        );
+        assertEquals(expectedRecords, records());
+        assertEquals(4, policies(app2).size());
+
+
+        // Remove cluster from app1
+        provisionLoadBalancers(app1, clustersPerZone);
+        maintainer.maintain();
+        expectedRecords = Set.of(
+                "c0--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c0--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c0--app2--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app2--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c0--app2--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app2--tenant1.prod.us-west-1.vespa.oath.cloud"
+        );
+        assertEquals(expectedRecords, records());
+
+        // Remove app2 completely
+        tester.controller().applications().require(app2.id()).deployments().keySet()
+              .forEach(zone -> {
+                  tester.controller().applications().deactivate(app2.id(), zone);
+                  tester.configServer().removeLoadBalancers(app2.id(), zone);
+              });
         maintainer.maintain();
-        assertEquals(8, (long) recordCount.get());
+        expectedRecords = Set.of(
+                "c0--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-west-1.vespa.oath.cloud",
+                "c0--app1--tenant1.prod.us-central-1.vespa.oath.cloud",
+                "c1--app1--tenant1.prod.us-central-1.vespa.oath.cloud"
+        );
+        assertEquals(expectedRecords, records());
+    }
 
-        // Remove application app2
-        tester.controller().applications().get(application2.id())
-              .map(app -> app.deployments().keySet())
-              .orElse(Collections.emptySet())
-              .forEach(zone -> tester.controller().applications().deactivate(application2.id(), zone));
+    private Set<RoutingPolicy> policies(Application application) {
+        return tester.controller().curator().readRoutingPolicies(application.id());
+    }
 
-        maintainer.maintain();
-        assertEquals(4, (long) recordCount.get());
+    private Set<String> records() {
+        return tester.controllerTester().nameService().records().values().stream()
+                     .map(r -> r.name().asString())
+                     .collect(Collectors.toSet());
     }
 
-    private void setupClustersWithLoadBalancers(DeploymentTester tester, Application application, int numberOfClustersPerZone) {
-        tester.controller().applications().get(application.id()).orElseThrow(()->new RuntimeException("No deployments")).deployments().keySet()
-                .forEach(zone -> tester.configServer()
-                        .removeLoadBalancers(new DeploymentId(application.id(), zone)));
-        tester.controller().applications().get(application.id()).orElseThrow(()->new RuntimeException("No deployments")).deployments().keySet()
-                .forEach(zone -> tester.configServer()
-                        .addLoadBalancers(zone, application.id(), makeLoadBalancers(zone, application.id(), numberOfClustersPerZone)));
+    private void provisionLoadBalancers(Application application, int numberOfClustersPerZone) {
+        tester.controller().applications().require(application.id())
+              .deployments().keySet()
+              .forEach(zone -> tester.configServer().removeLoadBalancers(application.id(), zone));
+        tester.controller().applications().require(application.id())
+              .deployments().keySet()
+              .forEach(zone -> tester.configServer()
+                                     .addLoadBalancers(zone, createLoadBalancers(zone, application.id(), numberOfClustersPerZone)));
 
     }
 
-    private List<LoadBalancer> makeLoadBalancers(ZoneId zone, ApplicationId application, int count) {
+    private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count,
+                                                   Map<Integer, Set<RotationName>> clusterRotations) {
         List<LoadBalancer> loadBalancers = new ArrayList<>();
-        Set<RotationName> rotations = Collections.singleton(RotationName.from("r1"));
         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("cluster-" + i),
+                                     ClusterSpec.Id.from("c" + i),
                                      HostName.from("loadbalancer-" + i + "-" + application.serializedForm() +
                                                    "-zone-" + zone.value()),
                                      Optional.of("dns-zone-1"),
@@ -136,4 +171,8 @@ public class RoutingPolicyMaintainerTest {
         return loadBalancers;
     }
 
+    private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count) {
+        return createLoadBalancers(zone, application, count, Collections.emptyMap());
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
index 849b251a230..b07e983099a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
@@ -1,7 +1,6 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.persistence;
 
-import com.google.common.collect.ImmutableMap;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationOverrides;
@@ -80,7 +79,7 @@ public class ApplicationSerializerTest {
                                        createClusterUtils(3, 0.2), createClusterInfo(3, 4),
                                        new DeploymentMetrics(2, 3, 4, 5, 6, Optional.of(Instant.now().truncatedTo(ChronoUnit.MILLIS))),
                                        DeploymentActivity.create(Optional.of(activityAt), Optional.of(activityAt),
-                                                                 OptionalDouble.of(200), OptionalDouble.of(10)), createLoadBalancers("default", "foo.bar")));
+                                                                 OptionalDouble.of(200), OptionalDouble.of(10))));
 
         OptionalLong projectId = OptionalLong.of(123L);
         List<JobStatus> statusList = new ArrayList<>();
@@ -200,9 +199,6 @@ public class ApplicationSerializerTest {
             Application original6 = writable(original).withOutstandingChange(Change.of(ApplicationVersion.from(new SourceRevision("a", "b", "c"), 42))).get();
             Application serialized6 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original6));
             assertEquals(original6.outstandingChange(), serialized6.outstandingChange());
-
-            assertEquals(1, serialized.deployments().get(zone2).loadBalancers().size());
-            assertEquals(original.deployments().get(zone2).loadBalancers(), serialized.deployments().get(zone2).loadBalancers());
         }
     }
 
@@ -236,10 +232,6 @@ public class ApplicationSerializerTest {
         return result;
     }
 
-    private Map<ClusterSpec.Id, HostName> createLoadBalancers(String clusterId, String hostName) {
-        return ImmutableMap.of(ClusterSpec.Id.from(clusterId), HostName.from(hostName));
-    }
-
     @Test
     public void testCompleteApplicationDeserialization() throws Exception {
         byte[] applicationJson = Files.readAllBytes(testData.resolve("complete-application.json"));