Merge pull request #4337 from vespa-engine/mpolden/update-name-service-interface

Add removeRecord to NameService

5 files changed, 43 insertions, 24 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 10f9e18fa41..c82a3fd571d 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
@@ -2,9 +2,9 @@
 package com.yahoo.vespa.hosted.controller.api.integration.dns;
 
 
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.List;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
 
@@ -15,22 +15,28 @@ import java.util.UUID;
  */
 public class MemoryNameService implements NameService {
 
-    private final List<Record> records = new ArrayList<>();
+    private final Map<RecordId, Record> records = new HashMap<>();
 
-    public List<Record> records() {
-        return Collections.unmodifiableList(records);
+    public Map<RecordId, Record> records() {
+        return Collections.unmodifiableMap(records);
     }
 
     @Override
     public RecordId createCname(String alias, String canonicalName) {
-        records.add(new Record(Record.Type.CNAME.name(), alias, canonicalName));
-        return new RecordId(UUID.randomUUID().toString());
+        RecordId id = new RecordId(UUID.randomUUID().toString());
+        records.put(id, new Record(id, Record.Type.CNAME, alias, canonicalName));
+        return id;
     }
 
     @Override
     public Optional<Record> findRecord(Record.Type type, String name) {
-        return records.stream()
+        return records.values().stream()
                 .filter(record -> record.type() == type && record.name().equals(name))
                 .findFirst();
     }
+
+    @Override
+    public void removeRecord(RecordId id) {
+        records.remove(id);
+    }
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java
index 2ccce23b60c..9b7c527db63 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java
@@ -21,4 +21,7 @@ public interface NameService {
     /** Find record by type and name */
     Optional<Record> findRecord(Record.Type type, String name);
 
+    /** Remove record by ID */
+    void removeRecord(RecordId id);
+
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java
index 0782a82da79..f190d11cf8f 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java
@@ -4,34 +4,40 @@ package com.yahoo.vespa.hosted.controller.api.integration.dns;
 import java.util.Objects;
 
 /**
- * A basic representation of a DNS resource record, containing only the record type, name and value.
+ * A basic representation of a DNS resource record, containing the record id, type, name and value.
  *
  * @author mpolden
  */
 public class Record {
 
+    private final RecordId id;
     private final Type type;
     private final String name;
     private final String value;
 
-    public Record(Type type, String name, String value) {
+    public Record(RecordId id, Type type, String name, String value) {
+        this.id = id;
         this.type = type;
         this.name = name;
         this.value = value;
     }
 
-    public Record(String type, String name, String value) {
-        this(Type.valueOf(type), name, value);
+    /** Unique identifier for this */
+    public RecordId id() {
+        return id;
     }
 
+    /** DNS type of this */
     public Type type() {
         return type;
     }
 
+    /** Value for this, e.g. IP address for "A" record */
     public String value() {
         return value;
     }
 
+    /** Name of this, e.g. a FQDN for "A" record */
     public String name() {
         return name;
     }
@@ -51,24 +57,26 @@ public class Record {
     @Override
     public String toString() {
         return "Record{" +
-                "type=" + type +
-                ", name='" + name + '\'' +
-                ", value='" + value + '\'' +
-                '}';
+               "id=" + id +
+               ", type=" + type +
+               ", name='" + name + '\'' +
+               ", value='" + value + '\'' +
+               '}';
     }
 
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
-        if (!(o instanceof Record)) return false;
+        if (o == null || getClass() != o.getClass()) return false;
         Record record = (Record) o;
-        return type == record.type &&
-                Objects.equals(name, record.name);
+        return Objects.equals(id, record.id) &&
+               type == record.type &&
+               Objects.equals(name, record.name) &&
+               Objects.equals(value, record.value);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(type, name);
+        return Objects.hash(id, type, name, value);
     }
-
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java
index 9c47be12855..da42c38252a 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java
@@ -14,7 +14,7 @@ public class RecordId {
         this.id = id;
     }
 
-    public String id() {
+    public String asString() {
         return id;
     }
 
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 34f3ea8f060..ce4bb912e97 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
@@ -51,6 +51,7 @@ import com.yahoo.vespa.hosted.controller.rotation.Rotation;
 import com.yahoo.vespa.hosted.controller.rotation.RotationId;
 import com.yahoo.vespa.hosted.controller.rotation.RotationRepository;
 import com.yahoo.vespa.hosted.rotation.config.RotationsConfig;
+import com.yahoo.yolean.Exceptions;
 
 import java.io.IOException;
 import java.net.URI;
@@ -476,7 +477,7 @@ public class ApplicationController {
             Optional<Record> record = nameService.findRecord(Record.Type.CNAME, dnsName);
             if (!record.isPresent()) {
                 RecordId recordId = nameService.createCname(dnsName, rotation.name());
-                log.info("Registered mapping with record ID " + recordId.id() + ": " +
+                log.info("Registered mapping with record ID " + recordId.asString() + ": " +
                          dnsName + " -> " + rotation.name());
             }
         } catch (RuntimeException e) {
@@ -508,7 +509,8 @@ public class ApplicationController {
             return Optional.of(new InstanceEndpoints(endPointUrls));
         }
         catch (RuntimeException e) {
-            log.log(Level.WARNING, "Failed to get endpoint information for " + deploymentId, e);
+            log.log(Level.WARNING, "Failed to get endpoint information for " + deploymentId + ": "
+                                   + Exceptions.toMessageString(e));
             return Optional.empty();
         }
     }