Handle existing A record for config server LB

3 files changed, 36 insertions, 9 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
index b54446c071e..59324079e6f 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
@@ -5,6 +5,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.dns;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
@@ -22,11 +23,11 @@ public class MemoryNameService implements NameService {
         return Collections.unmodifiableSet(records);
     }
 
-    private void add(Record record) {
-        if (records.stream().anyMatch(r -> r.type().equals(record.type()) &&
-                                           r.name().equals(record.name()) &&
-                                           r.data().equals(record.data()))) {
-            throw new IllegalArgumentException("Record already exists: " + record);
+    public void add(Record record) {
+        Optional<Record> conflict = records.stream().filter(r -> conflicts(r, record)).findFirst();
+        if (conflict.isPresent()) {
+            throw new AssertionError("'" + record + "' conflicts with existing record '" +
+                                     conflict.get() + "'");
         }
         records.add(record);
     }
@@ -45,8 +46,9 @@ public class MemoryNameService implements NameService {
                              .map(target -> new Record(Record.Type.ALIAS, name, target.asData()))
                              .collect(Collectors.toList());
         // Satisfy idempotency contract of interface
-        removeRecords(records);
-        records.forEach(this::add);
+        records.stream()
+               .filter(r -> !this.records.contains(r))
+               .forEach(this::add);
         return records;
     }
 
@@ -108,4 +110,15 @@ public class MemoryNameService implements NameService {
         this.records.removeAll(records);
     }
 
+    /**
+     * Returns whether record r1 and r2 can co-exist in a name service. This attempts to enforce the same constraints as
+     * most real name services.
+     */
+    private static boolean conflicts(Record r1, Record r2) {
+        if (!r1.name().equals(r2.name())) return false;               // Distinct names never conflict
+        if (r1.type() == Record.Type.ALIAS && r1.type() == r2.type()) // ALIAS records only require distinct data
+            return r1.data().equals(r2.data());
+        return true;                                                  // Anything else is considered a conflict
+    }
+
 }
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 abd4770ea0d..a429c444e0b 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
@@ -14,6 +14,7 @@ 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.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.persistence.CuratorDb;
@@ -178,7 +179,15 @@ public class RoutingPolicies {
         var name = RecordName.from(policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry())
                                          .dnsName());
         var data = RecordData.fqdn(policy.canonicalName().value());
-        nameUpdaterIn(policy.id().zone()).createCname(name, data);
+        NameUpdater nameUpdater = nameUpdaterIn(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);
+        }
+        nameUpdater.createCname(name, data);
     }
 
     /** Remove policies and zone DNS records unreferenced by given load balancers */
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 56fb7194e4e..760d80d230c 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
@@ -584,12 +584,17 @@ public class RoutingPoliciesTest {
     public void config_server_routing_policy() {
         var tester = new RoutingPoliciesTester();
         var app = SystemApplication.configServer.id();
+        RecordName name = RecordName.from("cfg.prod.us-west-1.test.vip");
+        tester.controllerTester().nameService().add(new Record(Record.Type.A, name, RecordData.from("192.0.2.1")));
 
         tester.provisionLoadBalancers(1, app, zone1);
         tester.routingPolicies().refresh(app, DeploymentSpec.empty, zone1);
         new NameServiceDispatcher(tester.tester.controller(), Duration.ofDays(1), Integer.MAX_VALUE).run();
 
-        assertEquals(Set.of("cfg.prod.us-west-1.test.vip"), tester.recordNames());
+        List<Record> records = tester.controllerTester().nameService().findRecords(Record.Type.CNAME, name);
+        assertEquals(1, records.size());
+        assertEquals(RecordData.from("lb-0--hosted-vespa:zone-config-servers:default--prod.us-west-1."),
+                     records.get(0).data());
     }
 
     /** Returns an application package builder that satisfies requirements for a directly routed endpoint */