Fix routing control for weighted records

When the weight sum for a record group in a region is 0, we have to remove the
record itself to disable routing to that region.

2 files changed, 184 insertions, 85 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
index 2fa931f2219..e5a99c2e69d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
@@ -177,51 +177,65 @@ public class RoutingPolicies {
     private void updateGlobalDnsOf(Collection<RoutingPolicy> routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) {
         Map<RoutingId, List<RoutingPolicy>> routingTable = routingTableFrom(routingPolicies);
         for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) {
-            Map<RegionEndpoint, Set<AliasTarget>> targets = computeRegionEndpoints(routeEntry.getValue(), inactiveZones);
+            Collection<RegionEndpoint> regionEndpoints = computeRegionEndpoints(routeEntry.getValue(), inactiveZones);
             // Create a weighted ALIAS per region, pointing to all zones within the same region
-            targets.forEach(((regionEndpoint, weightedTargets) -> {
-                controller.nameServiceForwarder().createAlias(RecordName.from(regionEndpoint.dnsName), weightedTargets,
+            regionEndpoints.forEach(regionEndpoint -> {
+                controller.nameServiceForwarder().createAlias(RecordName.from(regionEndpoint.target().name().value()),
+                                                              Collections.unmodifiableSet(regionEndpoint.zoneTargets()),
                                                               Priority.normal);
-            }));
+            });
+
             // Create global latency-based ALIAS pointing to each per-region weighted ALIAS
+            Set<AliasTarget> latencyTargets = new LinkedHashSet<>();
+            Set<AliasTarget> inactiveLatencyTargets = new LinkedHashSet<>();
+            for (var regionEndpoint : regionEndpoints) {
+                if (regionEndpoint.active()) {
+                    latencyTargets.add(regionEndpoint.target());
+                } else {
+                    inactiveLatencyTargets.add(regionEndpoint.target());
+                }
+            }
+            // If all targets are configured out, all targets are set in. We do this because otherwise removing 100% of
+            // the ALIAS records would cause the global endpoint to stop resolving entirely (NXDOMAIN).
+            if (latencyTargets.isEmpty() && !inactiveLatencyTargets.isEmpty()) {
+                latencyTargets.addAll(inactiveLatencyTargets);
+                inactiveLatencyTargets.clear();
+            }
             var endpoints = controller.routing().endpointsOf(routeEntry.getKey().application())
                                       .named(routeEntry.getKey().endpointId())
                                       .not().requiresRotation();
-            Set<AliasTarget> latencyTargets = targets.keySet().stream()
-                                                     .map(regionEndpoint -> new LatencyAliasTarget(HostName.from(regionEndpoint.dnsName),
-                                                                                                   regionEndpoint.dnsZone,
-                                                                                                   regionEndpoint.zone))
-                                                     .collect(Collectors.toSet());
             endpoints.forEach(endpoint -> controller.nameServiceForwarder().createAlias(RecordName.from(endpoint.dnsName()),
                                                                                         latencyTargets, Priority.normal));
+            inactiveLatencyTargets.forEach(t -> controller.nameServiceForwarder()
+                                                          .removeRecords(Record.Type.ALIAS,
+                                                                         RecordData.fqdn(t.name().value()),
+                                                                         Priority.normal));
         }
     }
 
     /** Compute region endpoints and their targets from given policies */
-    private Map<RegionEndpoint, Set<AliasTarget>> computeRegionEndpoints(List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) {
-        Map<RegionEndpoint, Set<AliasTarget>> targets = new LinkedHashMap<>();
+    private Collection<RegionEndpoint> computeRegionEndpoints(List<RoutingPolicy> policies, Set<ZoneId> inactiveZones) {
+        Map<Endpoint, RegionEndpoint> endpoints = new LinkedHashMap<>();
         RoutingMethod routingMethod = RoutingMethod.exclusive;
         for (var policy : policies) {
             if (policy.dnsZone().isEmpty()) continue;
             if (!controller.zoneRegistry().routingMethods(policy.id().zone()).contains(routingMethod)) continue;
             Endpoint weighted = policy.weightedEndpointIn(controller.system(), routingMethod);
-            // Do not route to zone if global routing status is set out at:
-            // - zone level (ZoneRoutingPolicy)
-            // - deployment level (RoutingPolicy)
-            // - application package level (deployment.xml)
-            long weight = 1;
             var zonePolicy = db.readZoneRoutingPolicy(policy.id().zone());
+            long weight = 1;
             if (isConfiguredOut(policy, zonePolicy, inactiveZones)) {
                 weight = 0; // A record with 0 weight will not received traffic. If all records within a group have 0
                             // weight, traffic is routed to all records with equal probability.
             }
-            var regionEndpoint = new RegionEndpoint(weighted, policy.dnsZone().get(), policy.id().zone());
             var weightedTarget = new WeightedAliasTarget(policy.canonicalName(), policy.dnsZone().get(),
                                                          policy.id().zone(), weight);
-            targets.computeIfAbsent(regionEndpoint, (k) -> new LinkedHashSet<>())
-                   .add(weightedTarget);
+            endpoints.computeIfAbsent(weighted, (k) -> new RegionEndpoint(new LatencyAliasTarget(HostName.from(weighted.dnsName()),
+                                                                                                 policy.dnsZone().get(),
+                                                                                                 policy.id().zone())))
+                     .zoneTargets()
+                     .add(weightedTarget);
         }
-        return Collections.unmodifiableMap(targets);
+        return endpoints.values();
     }
 
     /** Store routing policies for given load balancers */
@@ -335,18 +349,26 @@ public class RoutingPolicies {
         return false;
     }
 
-    /** Represents a region-wide endpoint */
+    /** Represents records for a region-wide endpoint */
     private static class RegionEndpoint {
 
-        private final String dnsName;
-        private final String dnsZone;
-        private final ZoneId zone;
+        private final LatencyAliasTarget target;
+        private final Set<WeightedAliasTarget> zoneTargets = new LinkedHashSet<>();
+
+        public RegionEndpoint(LatencyAliasTarget target) {
+            this.target = Objects.requireNonNull(target);
+        }
+
+        public LatencyAliasTarget target() {
+            return target;
+        }
+
+        public Set<WeightedAliasTarget> zoneTargets() {
+            return zoneTargets;
+        }
 
-        public RegionEndpoint(Endpoint endpoint, String dnsZone, ZoneId zone) {
-            this.dnsName = Objects.requireNonNull(endpoint).dnsName();
-            this.dnsZone = Objects.requireNonNull(dnsZone);
-            this.zone = Objects.requireNonNull(zone);
-            if (endpoint.scope() != Endpoint.Scope.weighted) throw new IllegalArgumentException("Region endpoint must be weighted");
+        public boolean active() {
+            return zoneTargets.stream().anyMatch(target -> target.weight() > 0);
         }
 
         @Override
@@ -354,12 +376,12 @@ public class RoutingPolicies {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
             RegionEndpoint that = (RegionEndpoint) o;
-            return dnsName.equals(that.dnsName);
+            return target.name().equals(that.target.name());
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(dnsName);
+            return Objects.hash(target.name());
         }
 
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
index ca4bcf1a354..3ee4c6961a4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
@@ -1,6 +1,7 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.routing;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Sets;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationId;
@@ -27,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 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.api.integration.dns.WeightedAliasTarget;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
@@ -48,6 +48,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -55,6 +56,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -147,7 +149,7 @@ public class RoutingPoliciesTest {
     @Test
     public void global_routing_policies_with_duplicate_region() {
         var tester = new RoutingPoliciesTester();
-        var context1 = tester.newDeploymentContext("tenant1", "app1", "default");
+        var context = tester.newDeploymentContext("tenant1", "app1", "default");
         int clustersPerZone = 2;
         int numberOfDeployments = 3;
         var applicationPackage = applicationPackageBuilder()
@@ -157,15 +159,42 @@ public class RoutingPoliciesTest {
                 .endpoint("r0", "c0")
                 .endpoint("r1", "c1")
                 .build();
-        tester.provisionLoadBalancers(clustersPerZone, context1.instanceId(), zone1, zone3, zone4);
+        tester.provisionLoadBalancers(clustersPerZone, context.instanceId(), zone1, zone3, zone4);
 
         // Creates alias records
-        context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
-        tester.assertTargets(context1.instanceId(), EndpointId.of("r0"), 0, zone1, zone3, zone4);
-        tester.assertTargets(context1.instanceId(), EndpointId.of("r1"), 1, zone1, zone3, zone4);
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone3, zone4);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 1, zone1, zone3, zone4);
         assertEquals("Routing policy count is equal to cluster count",
                      numberOfDeployments * clustersPerZone,
-                     tester.policiesOf(context1.instance().id()).size());
+                     tester.policiesOf(context.instance().id()).size());
+
+        // A zone in shared region is set out
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone4), GlobalRouting.Status.out,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+
+        // Weight of inactive zone is set to zero
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L,
+                                                                                  zone3, 1L,
+                                                                                  zone4, 0L));
+
+        // Other zone in shared region is set out. Entire record group for the region is removed as all zones in the
+        // region are out (weight sum = 0)
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone3), GlobalRouting.Status.out,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L));
+
+        // Everything is set back in
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone3), GlobalRouting.Status.in,
+                                                        GlobalRouting.Agent.tenant);
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone4), GlobalRouting.Status.in,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L,
+                                                                                           zone3, 1L,
+                                                                                           zone4, 1L));
     }
 
     @Test
@@ -395,9 +424,9 @@ public class RoutingPoliciesTest {
                                                         GlobalRouting.Agent.tenant);
         context.flushDnsUpdates();
 
-        // Inactive zone is given zero weight
-        tester.assertWeight(0, context.instanceId(), 0, zone1);
-        tester.assertWeight(1, context.instanceId(), 0, zone2);
+        // Inactive zone is removed from global DNS record
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone2);
 
         // Status details is stored in policy
         var policy1 = tester.routingPolicies().get(context.deploymentIdIn(zone1)).values().iterator().next();
@@ -414,15 +443,16 @@ public class RoutingPoliciesTest {
         // Next deployment does not affect status
         context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
         context.flushDnsUpdates();
-        tester.assertWeight(0, context.instanceId(), 0, zone1);
-        tester.assertWeight(1, context.instanceId(), 0, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone2);
 
         // Deployment is set back in
         tester.controllerTester().clock().advance(Duration.ofHours(1));
         changedAt = tester.controllerTester().clock().instant();
         tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in, GlobalRouting.Agent.tenant);
         context.flushDnsUpdates();
-        tester.assertWeight(1, context.instanceId(), 0, zone1, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2);
 
         policy1 = tester.routingPolicies().get(context.deploymentIdIn(zone1)).values().iterator().next();
         assertEquals(GlobalRouting.Status.in, policy1.status().globalRouting().status());
@@ -437,8 +467,8 @@ public class RoutingPoliciesTest {
                 .endpoint("r1", "c0", zone1.region().value(), zone2.region().value())
                 .build();
         context.submit(applicationPackage2).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
-        tester.assertWeight(1, context.instanceId(), 0, zone1);
-        tester.assertWeight(0, context.instanceId(), 0, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1);
 
         // ... back in
         var applicationPackage3 = applicationPackageBuilder()
@@ -448,7 +478,8 @@ public class RoutingPoliciesTest {
                 .endpoint("r1", "c0", zone1.region().value(), zone2.region().value())
                 .build();
         context.submit(applicationPackage3).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
-        tester.assertWeight(1, context.instanceId(), 0, zone1, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2);
+        tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2);
     }
 
     @Test
@@ -468,14 +499,13 @@ public class RoutingPoliciesTest {
             tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2);
             context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
             tester.assertTargets(context.instanceId(), EndpointId.defaultId(), 0, zone1, zone2);
-            tester.assertWeight(1, context.instanceId(), 0, zone1, zone2);
         }
 
         // Set zone out
         tester.routingPolicies().setGlobalRoutingStatus(zone2, GlobalRouting.Status.out);
         context1.flushDnsUpdates();
-        tester.assertWeight(1, context1.instanceId(), 0, zone1);
-        tester.assertWeight(0, context1.instanceId(), 0, zone2);
+        tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1);
+        tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1);
         for (var context : contexts) {
             var policies = tester.routingPolicies().get(context.instanceId());
             assertTrue("Global routing status for policy remains " + GlobalRouting.Status.in,
@@ -494,8 +524,8 @@ public class RoutingPoliciesTest {
         // Setting status per deployment does not affect status as entire zone is out
         tester.routingPolicies().setGlobalRoutingStatus(context1.deploymentIdIn(zone2), GlobalRouting.Status.in, GlobalRouting.Agent.tenant);
         context1.flushDnsUpdates();
-        tester.assertWeight(0, context1.instanceId(), 0, zone2);
-        tester.assertWeight(0, context2.instanceId(), 0, zone2);
+        tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1);
+        tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1);
 
         // Set single deployment out
         tester.routingPolicies().setGlobalRoutingStatus(context1.deploymentIdIn(zone2), GlobalRouting.Status.out, GlobalRouting.Agent.tenant);
@@ -504,9 +534,8 @@ public class RoutingPoliciesTest {
         // Set zone back in. Deployment set explicitly out, remains out, the rest are in
         tester.routingPolicies().setGlobalRoutingStatus(zone2, GlobalRouting.Status.in);
         context1.flushDnsUpdates();
-        tester.assertWeight(1, context1.instanceId(), 0, zone1);
-        tester.assertWeight(0, context1.instanceId(), 0, zone2);
-        tester.assertWeight(1, context2.instanceId(), 0, zone1, zone2);
+        tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1);
+        tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1, zone2);
     }
 
     @Test
@@ -549,7 +578,64 @@ public class RoutingPoliciesTest {
 
         // Deployment completes
         context.completeRollout();
-        tester.assertTargets(context.instanceId(), endpointId, ClusterSpec.Id.from("default"), 0, prodZone);
+        tester.assertTargets(context.instanceId(), endpointId, ClusterSpec.Id.from("default"), 0, Map.of(prodZone, 1L));
+    }
+
+    @Test
+    public void changing_global_routing_status_never_removes_all_members() {
+        var tester = new RoutingPoliciesTester();
+        var context = tester.newDeploymentContext("tenant1", "app1", "default");
+
+        // Provision load balancers and deploy application
+        tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2);
+        var applicationPackage = applicationPackageBuilder()
+                .region(zone1.region())
+                .region(zone2.region())
+                .endpoint("r0", "c0", zone1.region().value(), zone2.region().value())
+                .build();
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
+
+        // Global DNS record is created, pointing to all configured zones
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2);
+
+        // Global routing status is overridden for one deployment
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.out,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2);
+
+        // Setting other deployment out implicitly sets all deployments in. Weight is set to zero, but that has no
+        // impact on routing decisions when the weight sum is zero
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.out,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 0L, zone2, 0L));
+
+        // One inactive deployment is put back in. Global DNS record now points to the only active deployment
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1);
+
+        // Setting zone (containing active deployment) out puts all deployments in
+        tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.out);
+        context.flushDnsUpdates();
+        assertEquals(GlobalRouting.Status.out, tester.routingPolicies().get(zone1).globalRouting().status());
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 0L, zone2, 0L));
+
+        // Setting zone back in removes the currently inactive deployment
+        tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.in);
+        context.flushDnsUpdates();
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1);
+
+        // Inactive deployment is set in
+        tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.in,
+                                                        GlobalRouting.Agent.tenant);
+        context.flushDnsUpdates();
+        for (var policy : tester.routingPolicies().get(context.instanceId()).values()) {
+            assertSame(GlobalRouting.Status.in, policy.status().globalRouting().status());
+        }
+        tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2);
     }
 
     @Test
@@ -584,7 +670,7 @@ public class RoutingPoliciesTest {
                 lbHostname = HostName.from("shared-lb--" + zone.value());
             } else {
                 lbHostname = HostName.from("lb-" + i + "--" + application.serializedForm() +
-                                                     "--" + zone.value());
+                                           "--" + zone.value());
             }
             loadBalancers.add(
                     new LoadBalancer("LB-" + i + "-Z-" + zone.value(),
@@ -656,11 +742,11 @@ public class RoutingPoliciesTest {
                          .collect(Collectors.toSet());
         }
 
-        private List<String> aliasDataOf(String name) {
+        private Set<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());
+                         .collect(Collectors.toSet());
         }
 
         private List<String> cnameDataOf(String name) {
@@ -670,27 +756,10 @@ public class RoutingPoliciesTest {
                          .collect(Collectors.toList());
         }
 
-        private void assertWeight(long expected, ApplicationId application, int loadBalancerId, ZoneId... zones) {
-            for (var zone : zones) {
-                Endpoint weighted = tester.controller().routing().endpointsOf(new DeploymentId(application, zone))
-                                          .scope(Endpoint.Scope.weighted)
-                                          .named(EndpointId.of("c" + loadBalancerId))
-                                          .asList()
-                                          .get(0);
-                List<Record> records = tester.controllerTester().nameService().findRecords(Record.Type.ALIAS,
-                                                                                           RecordName.from(weighted.dnsName()));
-                assertEquals(1, records.size());
-                assertEquals("Record " + weighted.dnsName() + " has expected weight",
-                             expected,
-                             WeightedAliasTarget.unpack(records.get(0).data())
-                                                .weight());
-            }
-        }
-
-        private void assertTargets(ApplicationId application, EndpointId endpointId, ClusterSpec.Id clusterId, int loadBalancerId, ZoneId... zones) {
+        private void assertTargets(ApplicationId application, EndpointId endpointId, ClusterSpec.Id clusterId, int loadBalancerId, Map<ZoneId, Long> zoneWeights) {
             Set<String> latencyTargets = new HashSet<>();
             Map<String, List<ZoneId>> zonesByRegionEndpoint = new HashMap<>();
-            for (var zone : zones) {
+            for (var zone : zoneWeights.keySet()) {
                 Endpoint weighted = tester.controller().routing().endpointsOf(new DeploymentId(application, zone))
                                           .scope(Endpoint.Scope.weighted)
                                           .named(EndpointId.of(clusterId.value()))
@@ -700,11 +769,11 @@ public class RoutingPoliciesTest {
                                      .add(zone);
             }
             zonesByRegionEndpoint.forEach((regionEndpoint, zonesInRegion) -> {
-                List<String> weightedTargets = zonesInRegion.stream()
-                                                            .map(z -> "weighted/lb-" + loadBalancerId + "--" +
-                                                                      application.serializedForm() + "--" + z.value() +
-                                                                      "/dns-zone-1/" + z.value() + "/1")
-                                                            .collect(Collectors.toList());
+                Set<String> weightedTargets = zonesInRegion.stream()
+                                                           .map(z -> "weighted/lb-" + loadBalancerId + "--" +
+                                                                     application.serializedForm() + "--" + z.value() +
+                                                                     "/dns-zone-1/" + z.value() + "/" + zoneWeights.get(z))
+                                                           .collect(Collectors.toSet());
                 assertEquals("Weighted endpoint " + regionEndpoint + " points to load balancer",
                              weightedTargets,
                              aliasDataOf(regionEndpoint));
@@ -714,7 +783,7 @@ public class RoutingPoliciesTest {
             });
             String globalEndpoint = tester.controller().routing().endpointsOf(application)
                                           .named(endpointId)
-                                          .targets(List.of(zones))
+                                          .targets(List.copyOf(zoneWeights.keySet()))
                                           .primary()
                                           .map(Endpoint::dnsName)
                                           .orElse("<none>");
@@ -724,7 +793,15 @@ public class RoutingPoliciesTest {
         }
 
         private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, ZoneId... zones) {
-            assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zones);
+            Map<ZoneId, Long> zoneWeights = new LinkedHashMap<>();
+            for (var zone : zones) {
+                zoneWeights.put(zone, 1L);
+            }
+            assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zoneWeights);
+        }
+
+        private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, Map<ZoneId, Long> zoneWeights) {
+            assertTargets(application, endpointId, ClusterSpec.Id.from("c" + loadBalancerId), loadBalancerId, zoneWeights);
         }
 
     }