Merge pull request #8552 from vespa-engine/mpolden/remove-record-id

Remove RecordId

8 files changed, 32 insertions, 55 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..484d5043d8b 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
@@ -28,17 +28,15 @@ public class RoutingPolicy {
 
     private final ApplicationId owner;
     private final ZoneId zone;
-    private final String recordId;
     private final HostName alias;
     private final HostName canonicalName;
     private final Optional<String> dnsZone;
     private final Set<RotationName> rotations;
 
-    public RoutingPolicy(ApplicationId owner, ZoneId zone, String recordId, HostName alias, HostName canonicalName,
+    public RoutingPolicy(ApplicationId owner, ZoneId zone, HostName alias, HostName canonicalName,
                          Optional<String> dnsZone, Set<RotationName> rotations) {
         this.owner = Objects.requireNonNull(owner, "owner must be non-null");
         this.zone = Objects.requireNonNull(zone, "zone must be non-null");
-        this.recordId = Objects.requireNonNull(recordId, "recordId must be non-null");
         this.alias = Objects.requireNonNull(alias, "alias must be non-null");
         this.canonicalName = Objects.requireNonNull(canonicalName, "canonicalName must be non-null");
         this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null");
@@ -55,11 +53,6 @@ public class RoutingPolicy {
         return zone;
     }
 
-    /** The ID of the DNS record identifying this */
-    public String recordId() {
-        return recordId;
-    }
-
     /** This alias (lhs of a CNAME or ALIAS record) */
     public HostName alias() {
         return alias;
@@ -84,24 +77,23 @@ public class RoutingPolicy {
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
-        RoutingPolicy that = (RoutingPolicy) o;
-        return owner.equals(that.owner) &&
-               zone.equals(that.zone) &&
-               recordId.equals(that.recordId) &&
-               alias.equals(that.alias) &&
-               canonicalName.equals(that.canonicalName) &&
-               dnsZone.equals(that.dnsZone) &&
-               rotations.equals(that.rotations);
+        RoutingPolicy policy = (RoutingPolicy) o;
+        return owner.equals(policy.owner) &&
+               zone.equals(policy.zone) &&
+               alias.equals(policy.alias) &&
+               canonicalName.equals(policy.canonicalName) &&
+               dnsZone.equals(policy.dnsZone) &&
+               rotations.equals(policy.rotations);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(owner, zone, recordId, alias, canonicalName, dnsZone, rotations);
+        return Objects.hash(owner, zone, alias, canonicalName, dnsZone, rotations);
     }
 
     @Override
     public String toString() {
-        return String.format("%s: %s -> %s [rotations: %s%s], owned by %s, in %s", recordId, alias, canonicalName, rotations,
+        return String.format("%s -> %s [rotations: %s%s], owned by %s, in %s", alias, canonicalName, rotations,
                              dnsZone.map(z -> ", DNS zone: " + z).orElse(""), owner.toShortString(),
                              zone.value());
     }
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..6af9a6d42d7 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,17 +140,15 @@ 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(),
-                                 loadBalancer.dnsZone(), loadBalancer.rotations());
+        return new RoutingPolicy(application, zone, alias, loadBalancer.hostname(), loadBalancer.dnsZone(),
+                                 loadBalancer.rotations());
     }
 
     /** Remove all DNS records that point to non-existing load balancers */
@@ -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/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java
index ccea83caef4..86ef56a2587 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java
@@ -42,7 +42,6 @@ public class RoutingPolicySerializer {
         Cursor policyArray = root.setArray(routingPoliciesField);
         routingPolicies.forEach(policy -> {
             Cursor policyObject = policyArray.addObject();
-            policyObject.setString(recordIdField, policy.recordId());
             policyObject.setString(aliasField, policy.alias().value());
             policyObject.setString(zoneField, policy.zone().value());
             policyObject.setString(canonicalNameField, policy.canonicalName().value());
@@ -72,7 +71,6 @@ public class RoutingPolicySerializer {
             policies.add(new RoutingPolicy(owner,
                                            // TODO: Remove fallback after 7.13 has been released
                                            optionalField(inspect.field(zoneField), ZoneId::from).orElse(ZoneId.from(Environment.defaultEnvironment(), RegionName.defaultName())),
-                                           recordId.asString(),
                                            HostName.from(inspect.field(aliasField).asString()),
                                            HostName.from(inspect.field(canonicalNameField).asString()),
                                            optionalField(inspect.field(dnsZoneField), Function.identity()),
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());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java
index 99a2f6406ef..505bcce50d5 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializerTest.java
@@ -30,14 +30,12 @@ public class RoutingPolicySerializerTest {
         Set<RotationName> rotations = Set.of(RotationName.from("r1"), RotationName.from("r2"));
         Set<RoutingPolicy> loadBalancers = ImmutableSet.of(new RoutingPolicy(owner,
                                                                              ZoneId.from("prod", "us-north-1"),
-                                                                             "record-id-1",
                                                                              HostName.from("my-pretty-alias"),
                                                                              HostName.from("long-and-ugly-name"),
                                                                              Optional.of("zone1"),
                                                                              rotations),
                                                            new RoutingPolicy(owner,
                                                                              ZoneId.from("prod", "us-north-2"),
-                                                                             "record-id-2",
                                                                              HostName.from("my-pretty-alias-2"),
                                                                              HostName.from("long-and-ugly-name-2"),
                                                                              Optional.empty(),
@@ -66,14 +64,12 @@ public class RoutingPolicySerializerTest {
         ApplicationId owner = ApplicationId.defaultId();
         Set<RoutingPolicy> loadBalancers = ImmutableSet.of(new RoutingPolicy(owner,
                                                                              ZoneId.from(Environment.defaultEnvironment(), RegionName.defaultName()),
-                                                                             "record-id-1",
                                                                              HostName.from("my-pretty-alias"),
                                                                              HostName.from("long-and-ugly-name"),
                                                                              Optional.empty(),
                                                                              Collections.emptySet()),
                                                            new RoutingPolicy(owner,
                                                                              ZoneId.from(Environment.defaultEnvironment(), RegionName.defaultName()),
-                                                                             "record-id-2",
                                                                              HostName.from("my-pretty-alias-2"),
                                                                              HostName.from("long-and-ugly-name-2"),
                                                                              Optional.empty(),