Simplify and add test

3 files changed, 39 insertions, 51 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java
index b8692233b2d..4f36e713ba4 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java
@@ -69,7 +69,7 @@ public class NameServiceForwarder {
         forward(new RemoveRecords(type, data), priority);
     }
 
-    private void forward(NameServiceRequest request, NameServiceQueue.Priority priority) {
+    protected void forward(NameServiceRequest request, NameServiceQueue.Priority priority) {
         try (Lock lock = db.lockNameServiceQueue()) {
             NameServiceQueue queue = db.readNameServiceQueue();
             var queued = queue.requests().size();
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 faab177e407..d6f0ff4d85f 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
@@ -24,6 +24,7 @@ import com.yahoo.vespa.hosted.controller.application.EndpointId;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority;
+import com.yahoo.vespa.hosted.controller.dns.NameServiceRequest;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 
 import java.util.ArrayList;
@@ -262,15 +263,15 @@ public class RoutingPolicies {
         var name = RecordName.from(policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry())
                                          .dnsName());
         var data = RecordData.fqdn(policy.canonicalName().value());
-        NameUpdater nameUpdater = nameUpdaterIn(policy.id().zone());
+        NameServiceForwarder forwarder = nameServiceForwarderIn(policy.id().zone());
         if (policy.id().owner().equals(SystemApplication.configServer.id())) {
             // TODO(mpolden): Remove this after transition is complete. Before automatic provisioning of config server
             //                load balancers, the DNS records for the config server LB were of type A. It's not possible
             //                to change the type of an existing record, we therefore remove the A record before creating
             //                a CNAME.
-            nameUpdater.removeRecords(Record.Type.A, name);
+            forwarder.removeRecords(Record.Type.A, name, Priority.normal);
         }
-        nameUpdater.createCname(name, data);
+        forwarder.createCname(name, data, Priority.normal);
     }
 
     /** Remove policies and zone DNS records unreferenced by given load balancers */
@@ -284,7 +285,9 @@ public class RoutingPolicies {
                 !policy.id().zone().equals(allocation.deployment.zoneId())) continue;
 
             var dnsName = policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry()).dnsName();
-            nameUpdaterIn(allocation.deployment.zoneId()).removeRecords(Record.Type.CNAME, RecordName.from(dnsName));
+            nameServiceForwarderIn(allocation.deployment.zoneId()).removeRecords(Record.Type.CNAME,
+                                                                                 RecordName.from(dnsName),
+                                                                                 Priority.normal);
             newPolicies.remove(policy.id());
         }
         db.writeRoutingPolicies(allocation.deployment.applicationId(), newPolicies);
@@ -300,8 +303,9 @@ public class RoutingPolicies {
             var endpoints = controller.routing().endpointsOf(id.application())
                                       .not().requiresRotation()
                                       .named(id.endpointId());
-            var nameUpdater = nameUpdaterIn(allocation.deployment.zoneId());
-            endpoints.forEach(endpoint -> nameUpdater.removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName())));
+            var forwarder = nameServiceForwarderIn(allocation.deployment.zoneId());
+            endpoints.forEach(endpoint -> forwarder.removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()),
+                                                                  Priority.normal));
         }
     }
 
@@ -448,52 +452,24 @@ public class RoutingPolicies {
     }
 
     /** Returns the name updater to use for given zone */
-    private NameUpdater nameUpdaterIn(ZoneId zone) {
+    private NameServiceForwarder nameServiceForwarderIn(ZoneId zone) {
         if (controller.zoneRegistry().routingMethods(zone).contains(RoutingMethod.exclusive)) {
-            return new NameUpdater(controller.nameServiceForwarder());
+            return controller.nameServiceForwarder();
         }
-        return new DiscardingNameUpdater();
+        return new NameServiceDiscarder(controller.curator());
     }
 
-    /** A name updater that passes name service operations to the next handler */
-    private static class NameUpdater {
+    /** A {@link NameServiceForwarder} that does nothing. Used in zones where no explicit DNS updates are needed */
+    private static class NameServiceDiscarder extends NameServiceForwarder {
 
-        private final NameServiceForwarder forwarder;
-
-        public NameUpdater(NameServiceForwarder forwarder) {
-            this.forwarder = forwarder;
-        }
-
-        public void removeRecords(Record.Type type, RecordName name) {
-            forwarder.removeRecords(type, name, Priority.normal);
-        }
-
-        public void createAlias(RecordName name, Set<AliasTarget> targets) {
-            forwarder.createAlias(name, targets, Priority.normal);
-        }
-
-        public void createCname(RecordName name, RecordData data) {
-            forwarder.createCname(name, data, Priority.normal);
-        }
-
-    }
-
-    /** A name updater that does nothing */
-    private static class DiscardingNameUpdater extends NameUpdater {
-
-        private DiscardingNameUpdater() {
-            super(null);
+        public NameServiceDiscarder(CuratorDb db) {
+            super(db);
         }
 
         @Override
-        public void removeRecords(Record.Type type, RecordName name) {}
-
-        @Override
-        public void createAlias(RecordName name, Set<AliasTarget> targets) {}
-
-        @Override
-        public void createCname(RecordName name, RecordData data) {}
-
+        protected void forward(NameServiceRequest request, Priority priority) {
+            // Ignored
+        }
     }
 
 }
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 b6e593017ff..7064c6e1362 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
@@ -315,8 +315,19 @@ public class RoutingPoliciesTest {
     }
 
     @Test
+    public void zone_routing_policies_without_dns_update() {
+        var tester = new RoutingPoliciesTester(new DeploymentTester(), false);
+        var context = tester.newDeploymentContext("tenant1", "app1", "default");
+        tester.provisionLoadBalancers(1, context.instanceId(), true, zone1, zone2);
+        context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy();
+        assertEquals(0, tester.controllerTester().controller().curator().readNameServiceQueue().requests().size());
+        assertEquals(Set.of(), tester.recordNames());
+        assertEquals(2, tester.policiesOf(context.instanceId()).size());
+    }
+
+    @Test
     public void global_routing_policies_in_rotationless_system() {
-        var tester = new RoutingPoliciesTester(new DeploymentTester(new ControllerTester(new RotationsConfig.Builder().build())));
+        var tester = new RoutingPoliciesTester(new DeploymentTester(new ControllerTester(new RotationsConfig.Builder().build())), true);
         var context = tester.newDeploymentContext("tenant1", "app1", "default");
         tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2);
 
@@ -713,7 +724,7 @@ public class RoutingPoliciesTest {
         }
 
         public RoutingPoliciesTester(SystemName system) {
-            this(new DeploymentTester(new ControllerTester(new ServiceRegistryMock(system))));
+            this(new DeploymentTester(new ControllerTester(new ServiceRegistryMock(system))), true);
         }
 
         public RoutingPolicies routingPolicies() {
@@ -728,14 +739,15 @@ public class RoutingPoliciesTest {
             return tester.controllerTester();
         }
 
-        public RoutingPoliciesTester(DeploymentTester tester) {
+        public RoutingPoliciesTester(DeploymentTester tester, boolean exclusiveRouting) {
             this.tester = tester;
             List<ZoneApi> zones = new ArrayList<>(tester.controllerTester().zoneRegistry().zones().all().zones());
             zones.add(ZoneApiMock.from(zone3));
             zones.add(ZoneApiMock.from(zone4));
-            tester.controllerTester().zoneRegistry()
-                  .setZones(zones)
-                  .exclusiveRoutingIn(zones);
+            tester.controllerTester().zoneRegistry().setZones(zones);
+            if (exclusiveRouting) {
+                  tester.controllerTester().zoneRegistry().exclusiveRoutingIn(zones);
+            }
             tester.controllerTester().configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(),
                                                                SystemApplication.all());
             ((InMemoryFlagSource) tester.controllerTester().controller().flagSource()).withBooleanFlag(Flags.WEIGHTED_DNS_PER_REGION.id(), true);