Clean up NameService interface

4 files changed, 65 insertions, 84 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
index df368c3c60c..15b238dca15 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
@@ -47,14 +47,14 @@ public class DnsMaintainer extends Maintainer {
     /** Remove DNS alias for unassigned rotation */
     private void removeDnsAlias(Rotation rotation) {
         // When looking up CNAME by data, the data must be a FQDN
-        nameService.findRecord(Record.Type.CNAME, RecordData.fqdn(rotation.name())).stream()
-                .filter(DnsMaintainer::canUpdate)
-                .forEach(record -> {
-                    log.info(String.format("Removing DNS record %s (%s) because it points to the unassigned " +
-                                           "rotation %s (%s)", record.id().asString(),
-                                           record.name().asString(), rotation.id().asString(), rotation.name()));
-                    nameService.removeRecord(record.id());
-                });
+        nameService.findRecords(Record.Type.CNAME, RecordData.fqdn(rotation.name())).stream()
+                   .filter(DnsMaintainer::canUpdate)
+                   .forEach(record -> {
+                       log.info(String.format("Removing DNS record %s (%s) because it points to the unassigned " +
+                                              "rotation %s (%s)", record.id().asString(),
+                                              record.name().asString(), rotation.id().asString(), rotation.name()));
+                       nameService.removeRecord(record.id());
+                   });
     }
 
     /** Returns whether we can update the given record */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/LoadBalancerAliasMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/LoadBalancerAliasMaintainer.java
index 6c5000e3e3d..407fdfbe466 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/LoadBalancerAliasMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/LoadBalancerAliasMaintainer.java
@@ -95,11 +95,15 @@ public class LoadBalancerAliasMaintainer extends Maintainer {
         HostName alias = HostName.from(LoadBalancerAlias.createAlias(loadBalancer.cluster(), application, zone));
         RecordName name = RecordName.from(alias.value());
         RecordData data = RecordData.fqdn(loadBalancer.hostname().value());
-        Optional<Record> existingRecord = nameService.findRecord(Record.Type.CNAME, name);
+        List<Record> existingRecords = nameService.findRecords(Record.Type.CNAME, name);
+        if (existingRecords.size() > 1) {
+            throw new IllegalStateException("Found more than 1 CNAME record for " + name.asString() + ": " + existingRecords);
+        }
+        Optional<Record> record = existingRecords.stream().findFirst();
         RecordId id;
-        if(existingRecord.isPresent()) {
-            id = existingRecord.get().id();
-            nameService.updateRecord(existingRecord.get().id(), data);
+        if (record.isPresent()) {
+            id = record.get().id();
+            nameService.updateRecord(id, data);
         } else {
             id = nameService.createCname(name, data);
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index d4dff6ecff3..06417da8157 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -280,26 +280,26 @@ public class ControllerTest {
                 .region("us-central-1") // Two deployments should result in each DNS alias being registered once
                 .build();
 
+        Function<String, Optional<Record>> findCname = (name) -> tester.controllerTester().nameService()
+                                                                       .findRecords(Record.Type.CNAME,
+                                                                                    RecordName.from(name))
+                                                                       .stream()
+                                                                       .findFirst();
+
         tester.deployCompletely(application, applicationPackage);
         assertEquals(3, tester.controllerTester().nameService().records().size());
 
-        Optional<Record> record = tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
-                                                                                    );
+        Optional<Record> record = findCname.apply("app1--tenant1.global.vespa.yahooapis.com");
         assertTrue(record.isPresent());
         assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-        record = tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.oath.cloud")
-        );
+        record = findCname.apply("app1--tenant1.global.vespa.oath.cloud");
         assertTrue(record.isPresent());
         assertEquals("app1--tenant1.global.vespa.oath.cloud", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-        record = tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
-        );
+        record = findCname.apply("app1.tenant1.global.vespa.yahooapis.com");
         assertTrue(record.isPresent());
         assertEquals("app1.tenant1.global.vespa.yahooapis.com", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
@@ -309,6 +309,12 @@ public class ControllerTest {
     public void testUpdatesExistingDnsAlias() {
         DeploymentTester tester = new DeploymentTester();
 
+        Function<String, Optional<Record>> findCname = (name) -> tester.controllerTester().nameService()
+                                                                       .findRecords(Record.Type.CNAME,
+                                                                                    RecordName.from(name))
+                                                                       .stream()
+                                                                       .findFirst();
+
         // Application 1 is deployed and deleted
         {
             Application app1 = tester.createApplication("app1", "tenant1", 1, 1L);
@@ -322,16 +328,12 @@ public class ControllerTest {
             tester.deployCompletely(app1, applicationPackage);
             assertEquals(3, tester.controllerTester().nameService().records().size());
 
-            Optional<Record> record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
-                                                                                        );
+            Optional<Record> record = findCname.apply("app1--tenant1.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
             assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString());
             assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
-            );
+            record = findCname.apply("app1.tenant1.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
             assertEquals("app1.tenant1.global.vespa.yahooapis.com", record.get().name().asString());
             assertEquals("rotation-fqdn-01.", record.get().data().asString());
@@ -353,19 +355,13 @@ public class ControllerTest {
             }
 
             // Records remain
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
-            );
+            record = findCname.apply("app1--tenant1.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.oath.cloud")
-            );
+            record = findCname.apply("app1--tenant1.global.vespa.oath.cloud");
             assertTrue(record.isPresent());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
-            );
+            record = findCname.apply("app1.tenant1.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
         }
 
@@ -381,27 +377,20 @@ public class ControllerTest {
             tester.deployCompletely(app2, applicationPackage);
             assertEquals(6, tester.controllerTester().nameService().records().size());
 
-            Optional<Record> record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app2--tenant2.global.vespa.yahooapis.com")
-                                                                                        );
+            Optional<Record> record = findCname.apply("app2--tenant2.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
             assertEquals("app2--tenant2.global.vespa.yahooapis.com", record.get().name().asString());
             assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app2--tenant2.global.vespa.oath.cloud")
-                                                                                        );
+            record = findCname.apply("app2--tenant2.global.vespa.oath.cloud");
             assertTrue(record.isPresent());
             assertEquals("app2--tenant2.global.vespa.oath.cloud", record.get().name().asString());
             assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app2.tenant2.global.vespa.yahooapis.com")
-            );
+            record = findCname.apply("app2.tenant2.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
             assertEquals("app2.tenant2.global.vespa.yahooapis.com", record.get().name().asString());
             assertEquals("rotation-fqdn-01.", record.get().data().asString());
-
         }
 
         // Application 1 is recreated, deployed and assigned a new rotation
@@ -421,24 +410,17 @@ public class ControllerTest {
             // Existing DNS records are updated to point to the newly assigned rotation
             assertEquals(6, tester.controllerTester().nameService().records().size());
 
-            Optional<Record> record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
-                                                                                        );
+            Optional<Record> record = findCname.apply("app1--tenant1.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
             assertEquals("rotation-fqdn-02.", record.get().data().asString());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.oath.cloud")
-                                                                                        );
+            record = findCname.apply("app1--tenant1.global.vespa.oath.cloud");
             assertTrue(record.isPresent());
             assertEquals("rotation-fqdn-02.", record.get().data().asString());
 
-            record = tester.controllerTester().nameService().findRecord(
-                    Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
-            );
+            record = findCname.apply("app1.tenant1.global.vespa.yahooapis.com");
             assertTrue(record.isPresent());
             assertEquals("rotation-fqdn-02.", record.get().data().asString());
-
         }
 
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
index 92f3e87f001..3d2a3ccd842 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
@@ -17,6 +17,7 @@ import org.junit.Test;
 
 import java.time.Duration;
 import java.util.Optional;
+import java.util.function.Function;
 
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.component;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest;
@@ -33,9 +34,9 @@ public class DnsMaintainerTest {
     public void removes_record_for_unassigned_rotation() {
         DeploymentTester tester = new DeploymentTester();
         Application application = tester.createApplication("app1", "tenant1", 1, 1L);
-        DnsMaintainer dnsMaintainer = new DnsMaintainer(tester.controller(), Duration.ofHours(12),
-                                                        new JobControl(new MockCuratorDb()),
-                                                        tester.controllerTester().nameService());
+        DnsMaintainer maintainer = new DnsMaintainer(tester.controller(), Duration.ofHours(12),
+                                                     new JobControl(new MockCuratorDb()),
+                                                     tester.controllerTester().nameService());
 
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)
@@ -44,39 +45,36 @@ public class DnsMaintainerTest {
                 .region("us-central-1")
                 .build();
 
+        Function<String, Optional<Record>> findCname = (name) -> tester.controllerTester().nameService()
+                                                                       .findRecords(Record.Type.CNAME,
+                                                                                    RecordName.from(name))
+                                                                       .stream()
+                                                                       .findFirst();
+
         // Deploy application
         tester.deployCompletely(application, applicationPackage);
         assertEquals(3, tester.controllerTester().nameService().records().size());
 
-        Optional<Record> record = tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
-                                                                                    );
+        Optional<Record> record = findCname.apply("app1--tenant1.global.vespa.yahooapis.com");
         assertTrue(record.isPresent());
         assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-        record = tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.oath.cloud")
-                                                                                    );
+        record = findCname.apply("app1--tenant1.global.vespa.oath.cloud");
         assertTrue(record.isPresent());
         assertEquals("app1--tenant1.global.vespa.oath.cloud", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
-        record = tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
-        );
+        record = findCname.apply("app1.tenant1.global.vespa.yahooapis.com");
         assertTrue(record.isPresent());
         assertEquals("app1.tenant1.global.vespa.yahooapis.com", record.get().name().asString());
         assertEquals("rotation-fqdn-01.", record.get().data().asString());
 
         // DnsMaintainer does nothing
-        dnsMaintainer.maintain();
-        assertTrue("DNS record is not removed", tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")).isPresent());
-        assertTrue("DNS record is not removed", tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.oath.cloud")).isPresent());
-        assertTrue("DNS record is not removed", tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")).isPresent());
+        maintainer.maintain();
+        assertTrue("DNS record is not removed", findCname.apply("app1--tenant1.global.vespa.yahooapis.com").isPresent());
+        assertTrue("DNS record is not removed", findCname.apply("app1--tenant1.global.vespa.oath.cloud").isPresent());
+        assertTrue("DNS record is not removed", findCname.apply("app1.tenant1.global.vespa.yahooapis.com").isPresent());
 
         // Remove application
         applicationPackage = new ApplicationPackageBuilder()
@@ -91,15 +89,12 @@ public class DnsMaintainerTest {
         tester.applications().deleteApplication(application.id(), Optional.of(new OktaAccessToken("okta-token")));
 
         // DnsMaintainer removes records
-        dnsMaintainer.maintain();
-        assertFalse("DNS record removed", tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")).isPresent());
-        dnsMaintainer.maintain();
-        assertFalse("DNS record removed", tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.oath.cloud")).isPresent());
-        dnsMaintainer.maintain();
-        assertFalse("DNS record removed", tester.controllerTester().nameService().findRecord(
-                Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")).isPresent());
+        maintainer.maintain();
+        assertFalse("DNS record removed", findCname.apply("app1--tenant1.global.vespa.yahooapis.com").isPresent());
+        maintainer.maintain();
+        assertFalse("DNS record removed", findCname.apply("app1--tenant1.global.vespa.oath.cloud").isPresent());
+        maintainer.maintain();
+        assertFalse("DNS record removed", findCname.apply("app1.tenant1.global.vespa.yahooapis.com").isPresent());
     }
 
 }