Remove RecordId

`NameService` shouldn't enforce the use of record IDs as it implementations may
not always operate on IDs. Creating synthetic IDs in such cases is more trouble
than it's worth.

6 files changed, 22 insertions, 30 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 30c99288d51..a5e952b76c1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -37,7 +37,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
 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.RecordId;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator;
@@ -498,16 +497,15 @@ public class ApplicationController {
             records.forEach(record -> {
                 // Ensure that the existing record points to the correct rotation
                 if ( ! record.data().equals(rotationName)) {
-                    nameService.updateRecord(record.id(), rotationName);
-                    log.info("Updated mapping for record ID " + record.id().asString() + ": '" + dnsName
-                            + "' -> '" + rotation.name() + "'");
+                    nameService.updateRecord(record, rotationName);
+                    log.info("Updated mapping for record '" + record + "': '" + dnsName
+                             + "' -> '" + rotation.name() + "'");
                 }
             });
 
             if (records.isEmpty()) {
-                RecordId id = nameService.createCname(RecordName.from(dnsName), rotationName);
-                log.info("Registered mapping with record ID " + id.asString() + ": '" + dnsName + "' -> '"
-                         + rotation.name() + "'");
+                Record record = nameService.createCname(RecordName.from(dnsName), rotationName);
+                log.info("Registered mapping as record  '" + record + "'");
             }
         } catch (RuntimeException e) {
             log.log(Level.WARNING, "Failed to register CNAME", e);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java
index 9924727edbc..acf4be70f50 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java
@@ -56,6 +56,7 @@ public class RoutingPolicy {
     }
 
     /** The ID of the DNS record identifying this */
+    // TODO: Remove this field as it's no longer needed
     public String recordId() {
         return recordId;
     }
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 36d83b8ee6a..4dca8bc2811 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
@@ -19,6 +19,7 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 /**
  * Performs DNS maintenance tasks such as removing DNS aliases for unassigned rotations.
@@ -53,14 +54,12 @@ 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.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());
-                   });
+        List<Record> records = nameService.findRecords(Record.Type.CNAME, RecordData.fqdn(rotation.name())).stream()
+                                          .filter(DnsMaintainer::canUpdate)
+                                          .collect(Collectors.toList());
+        log.info(String.format("Removing DNS records %s because they point to the unassigned " +
+                               "rotation %s (%s)", records, rotation.id().asString(), rotation.name()));
+        nameService.removeRecords(records);
     }
 
     /**
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 c778b103700..9455c44a2f9 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
@@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.AliasTarget;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
 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.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.GlobalDnsName;
@@ -141,16 +140,14 @@ public class RoutingPolicyMaintainer extends Maintainer {
             throw new IllegalStateException("Found more than 1 CNAME record for " + name.asString() + ": " + existingRecords);
         }
         Optional<Record> record = existingRecords.stream().findFirst();
-        RecordId id;
         if (record.isPresent()) {
-            id = record.get().id();
             if (!record.get().data().equals(data)) {
-                nameService.updateRecord(id, data);
+                nameService.updateRecord(record.get(), data);
             }
         } else {
-            id = nameService.createCname(name, data);
+            nameService.createCname(name, data);
         }
-        return new RoutingPolicy(application, zone, id.asString(), alias, loadBalancer.hostname(),
+        return new RoutingPolicy(application, zone, "fake-id" + alias, alias, loadBalancer.hostname(),
                                  loadBalancer.dnsZone(), loadBalancer.rotations());
     }
 
@@ -167,9 +164,10 @@ public class RoutingPolicyMaintainer extends Maintainer {
             removalCandidates.removeIf(policy -> activeLoadBalancers.contains(policy.canonicalName()));
             for (RoutingPolicy policy : removalCandidates) {
                 try {
-                    nameService.removeRecord(new RecordId(policy.recordId()));
+                    List<Record> records = nameService.findRecords(Record.Type.CNAME, RecordName.from(policy.alias().value()));
+                    nameService.removeRecords(records);
                 } catch (Exception e) {
-                    log.log(LogLevel.WARNING, "Failed to remove CNAME record with ID '" + policy.recordId() +
+                    log.log(LogLevel.WARNING, "Failed to remove record '" + policy.alias() +
                                               "'. Retrying in " + maintenanceInterval());
                 }
             }
@@ -186,7 +184,7 @@ public class RoutingPolicyMaintainer extends Maintainer {
                 GlobalDnsName dnsName = dnsName(id);
                 try {
                     List<Record> records = nameService.findRecords(Record.Type.ALIAS, RecordName.from(dnsName.oathDnsName()));
-                    records.stream().map(Record::id).forEach(nameService::removeRecord);
+                    nameService.removeRecords(records);
                 } catch (Exception e) {
                     log.log(LogLevel.WARNING, "Failed to remove all ALIAS records with name '" + dnsName.oathDnsName() +
                                               "'. Retrying in " + maintenanceInterval());
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 a4f9dfeb13f..578e7824913 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
@@ -9,7 +9,6 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 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.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.ApplicationPackage;
@@ -23,7 +22,6 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.time.Duration;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
@@ -131,7 +129,7 @@ public class DnsMaintainerTest {
         }
     }
 
-    private Map<RecordId, Set<Record>> records() {
+    private Set<Record> records() {
         return tester.controllerTester().nameService().records();
     }
 
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 ce691bf4de0..b2a05f5e1f8 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
@@ -21,7 +21,6 @@ import org.junit.Test;
 
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -194,8 +193,7 @@ public class RoutingPolicyMaintainerTest {
     }
 
     private Set<String> recordNames() {
-        return tester.controllerTester().nameService().records().values().stream()
-                     .flatMap(Collection::stream)
+        return tester.controllerTester().nameService().records().stream()
                      .map(Record::name)
                      .map(RecordName::asString)
                      .collect(Collectors.toSet());