Fix serialization TODOs

4 files changed, 29 insertions, 93 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index 759bb89de94..44b5160c61d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -55,8 +55,8 @@ public class CuratorDatabaseClient {
     
     private final Zone zone;
 
-    public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, NameResolver nameResolver) {
-        this.nodeSerializer = new NodeSerializer(flavors, nameResolver);
+    public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone) {
+        this.nodeSerializer = new NodeSerializer(flavors);
         this.zone = zone;
         jsonMapper.registerModule(new JodaModule());
         this.curatorDatabase = new CuratorDatabase(curator, root, /* useCache: */ false);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
index a9a98b6f1c6..23b770137e6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
@@ -42,8 +42,6 @@ public class NodeSerializer {
     /** The configured node flavors */
     private final NodeFlavors flavors;
 
-    private final NameResolver nameResolver;
-
     // Node fields
     private static final String hostnameKey = "hostname";
     private static final String ipAddressesKey = "ipAddresses";
@@ -81,9 +79,8 @@ public class NodeSerializer {
 
     // ---------------- Serialization ----------------------------------------------------
 
-    public NodeSerializer(NodeFlavors flavors, NameResolver nameResolver) {
+    public NodeSerializer(NodeFlavors flavors) {
         this.flavors = flavors;
-        this.nameResolver = nameResolver;
     }
 
     public byte[] toJson(Node node) {
@@ -152,7 +149,7 @@ public class NodeSerializer {
 
     private Node nodeFromSlime(Node.State state, Inspector object) {
         return new Node(object.field(openStackIdKey).asString(),
-                        ipAddressesFromResolverOrSlime(object),
+                        ipAddressesFromSlime(object),
                         object.field(hostnameKey).asString(),
                         parentHostnameFromSlime(object),
                         flavorFromSlime(object),
@@ -230,14 +227,10 @@ public class NodeSerializer {
             return Optional.empty();
     }
 
-    // TODO: Remove this and use the ipAddresses field directly after 6.55 has been deployed everywhere
-    private Set<String> ipAddressesFromResolverOrSlime(Inspector object) {
-        if (object.field(ipAddressesKey).valid()) {
-            ImmutableSet.Builder<String> ipAddresses = ImmutableSet.builder();
-            object.field(ipAddressesKey).traverse((ArrayTraverser) (i, item) -> ipAddresses.add(item.asString()));
-            return ipAddresses.build();
-        }
-        return nameResolver.getAllByNameOrThrow(object.field("hostname").asString());
+    private Set<String> ipAddressesFromSlime(Inspector object) {
+        ImmutableSet.Builder<String> ipAddresses = ImmutableSet.builder();
+        object.field(ipAddressesKey).traverse((ArrayTraverser) (i, item) -> ipAddresses.add(item.asString()));
+        return ipAddresses.build();
     }
     
     private Optional<Status.HardwareFailureType> hardwareFailureFromSlime(Inspector object) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java
index e5d67e5d789..52f84b41a71 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java
@@ -11,7 +11,6 @@ import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.testutils.FlavorConfigBuilder;
-import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
 import org.junit.Test;
 
 import java.time.Clock;
@@ -26,12 +25,12 @@ public class CuratorDatabaseClientTest {
 
     private final Curator curator = new MockCurator();
     private final CuratorDatabaseClient zkClient = new CuratorDatabaseClient(
-            FlavorConfigBuilder.createDummies("default"), curator, Clock.systemUTC(), Zone.defaultZone(),
-            new MockNameResolver().mockAnyLookup());
+            FlavorConfigBuilder.createDummies("default"), curator, Clock.systemUTC(), Zone.defaultZone()
+    );
 
     @Test
     public void can_read_stored_host_information() throws Exception {
-        String zkline = "{\"hostname\":\"oxy-oxygen-0a4ae4f1.corp.bf1.yahoo.com\",\"openStackId\":\"7951bb9d-3989-4a60-a21c-13690637c8ea\",\"flavor\":\"default\",\"created\":1421054425159, \"type\":\"host\"}";
+        String zkline = "{\"hostname\":\"oxy-oxygen-0a4ae4f1.corp.bf1.yahoo.com\",\"ipAddresses\":[\"127.0.0.1\"],\"openStackId\":\"7951bb9d-3989-4a60-a21c-13690637c8ea\",\"flavor\":\"default\",\"created\":1421054425159, \"type\":\"host\"}";
         curator.framework().create().creatingParentsIfNeeded().forPath("/provision/v1/ready/oxy-oxygen-0a4ae4f1.corp.bf1.yahoo.com", zkline.getBytes());
 
         List<Node> allocatedNodes = zkClient.getNodes(Node.State.ready);
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java
index 47fe408553a..a34e70fee7a 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java
@@ -7,6 +7,7 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.test.ManualClock;
@@ -16,11 +17,8 @@ import com.yahoo.vespa.hosted.provision.Node.State;
 import com.yahoo.vespa.hosted.provision.node.Allocation;
 import com.yahoo.vespa.hosted.provision.node.Generation;
 import com.yahoo.vespa.hosted.provision.node.History;
-import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.vespa.hosted.provision.node.Status;
 import com.yahoo.vespa.hosted.provision.testutils.FlavorConfigBuilder;
-import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.nio.charset.StandardCharsets;
@@ -33,7 +31,6 @@ import static java.util.Collections.singleton;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 /**
  * @author bratseth
@@ -42,15 +39,9 @@ import static org.junit.Assert.fail;
 public class SerializationTest {
 
     private final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", "large", "ugccloud-container");
-    private final MockNameResolver nameResolver = new MockNameResolver();
-    private final NodeSerializer nodeSerializer = new NodeSerializer(nodeFlavors, nameResolver);
+    private final NodeSerializer nodeSerializer = new NodeSerializer(nodeFlavors);
     private final ManualClock clock = new ManualClock();
 
-    @Before
-    public void before() {
-        nameResolver.reset();
-    }
-
     @Test
     public void testProvisionedNodeSerialization() {
         Node node = createNode();
@@ -116,7 +107,6 @@ public class SerializationTest {
 
     @Test
     public void testRebootAndRestartAndTypeNoCurrentValuesSerialization() {
-        nameResolver.addRecord("myHostname", "127.0.0.1");
         String nodeData = 
                 "{\n" +
                 "   \"type\" : \"tenant\",\n" +
@@ -139,7 +129,8 @@ public class SerializationTest {
                 "      \"removable\" : true\n" +
                 "   },\n" +
                 "   \"openStackId\" : \"myId\",\n" +
-                "   \"hostname\" : \"myHostname\"\n" +
+                "   \"hostname\" : \"myHostname\",\n" +
+                "   \"ipAddresses\" : [\"127.0.0.1\"]\n" +
                 "}";
 
         Node node = nodeSerializer.fromJson(Node.State.provisioned, Utf8.toBytes(nodeData));
@@ -153,38 +144,6 @@ public class SerializationTest {
         assertEquals(NodeType.tenant, node.type());
     }
 
-    // TODO: Remove when 6.31 is deployed everywhere
-    @Test
-    public void testLegacyFlavor() {
-        nameResolver.addRecord("myHostname", "127.0.0.1");
-        String nodeData =
-                "{\n" +
-                "   \"type\" : \"tenant\",\n" +
-                "   \"rebootGeneration\" : 0,\n" +
-                "   \"flavor\" : \"large\"\n," +
-                "   \"history\" : [\n" +
-                "      {\n" +
-                "         \"type\" : \"reserved\",\n" +
-                "         \"at\" : 1444391402611\n" +
-                "      }\n" +
-                "   ],\n" +
-                "   \"instance\" : {\n" +
-                "      \"applicationId\" : \"myApplication\",\n" +
-                "      \"tenantId\" : \"myTenant\",\n" +
-                "      \"instanceId\" : \"myInstance\",\n" +
-                "      \"serviceId\" : \"content/myId/0\",\n" +
-                "      \"restartGeneration\" : 0,\n" +
-                "      \"removable\" : false\n" +
-                "   },\n" +
-                "   \"openStackId\" : \"myId\",\n" +
-                "   \"hostname\" : \"myHostname\",\n" +
-                "   \"hardwareFailure\" : \"memory_mcelog\"\n" +
-                "}";
-
-        Node node = nodeSerializer.fromJson(Node.State.provisioned, Utf8.toBytes(nodeData));
-        assertEquals("large", node.flavor().canonicalName());
-    }
-
     @Test
     public void testRetiredNodeSerialization() {
         Node node = createNode();
@@ -213,8 +172,20 @@ public class SerializationTest {
 
     @Test
     public void testAssimilatedDeserialization() {
-        nameResolver.addRecord("assimilate2.vespahosted.yahoo.tld", "127.0.0.1");
-        Node node = nodeSerializer.fromJson(Node.State.active, "{\"type\":\"tenant\",\"hostname\":\"assimilate2.vespahosted.yahoo.tld\",\"openStackId\":\"\",\"flavor\":\"ugccloud-container\",\"instance\":{\"tenantId\":\"by_mortent\",\"applicationId\":\"ugc-assimilate\",\"instanceId\":\"default\",\"serviceId\":\"container/ugccloud-container/0/0\",\"restartGeneration\":0}}\n".getBytes());
+        Node node = nodeSerializer.fromJson(Node.State.active, ("{\n" +
+                "  \"type\": \"tenant\",\n" +
+                "  \"hostname\": \"assimilate2.vespahosted.yahoo.tld\",\n" +
+                "  \"ipAddresses\": [\"127.0.0.1\"],\n" +
+                "  \"openStackId\": \"\",\n" +
+                "  \"flavor\": \"ugccloud-container\",\n" +
+                "  \"instance\": {\n" +
+                "    \"tenantId\": \"by_mortent\",\n" +
+                "    \"applicationId\": \"ugc-assimilate\",\n" +
+                "    \"instanceId\": \"default\",\n" +
+                "    \"serviceId\": \"container/ugccloud-container/0/0\",\n" +
+                "    \"restartGeneration\": 0\n" +
+                "  }\n" +
+                "}\n").getBytes());
         assertEquals(0, node.history().events().size());
         assertTrue(node.allocation().isPresent());
         assertEquals("ugccloud-container", node.allocation().get().membership().cluster().id().value());
@@ -267,35 +238,8 @@ public class SerializationTest {
         assertEquals(parentHostname, deserializedNode.parentHostname().get());
     }
 
-
-    @Test
-    public void resolves_hostname_when_deserializing() throws Exception {
-        nameResolver.addRecord("node1.yahoo.tld", "127.0.0.1");
-        byte[] nodeWithoutIp = createNodeJson("node1.yahoo.tld");
-        Node deserializedNode = nodeSerializer.fromJson(State.provisioned, nodeWithoutIp);
-        assertEquals(singleton("127.0.0.1"), deserializedNode.ipAddresses());
-    }
-
-    @Test
-    public void throws_when_hostname_cannot_be_resolved() throws Exception {
-        byte[] nodeWithoutIp = createNodeJson("node2.yahoo.tld");
-        try {
-            nodeSerializer.fromJson(State.provisioned, nodeWithoutIp);
-            fail("Expected exception to be thrown");
-        } catch (RuntimeException ignored) {}
-    }
-
-    @Test
-    public void resolver_is_not_invoked_when_ip_address_is_present() throws Exception {
-        nameResolver.failIfInvoked();
-        byte[] nodeWithIp = createNodeJson("node3.yahoo.tld", "127.0.0.3");
-        Node deserializedNode = nodeSerializer.fromJson(State.provisioned, nodeWithIp);
-        assertEquals(singleton("127.0.0.3"), deserializedNode.ipAddresses());
-    }
-
     @Test
     public void serializes_multiple_ip_addresses() throws Exception {
-        nameResolver.failIfInvoked();
         byte[] nodeWithMultipleIps = createNodeJson("node4.yahoo.tld", "127.0.0.4", "::4");
         Node deserializedNode = nodeSerializer.fromJson(State.provisioned, nodeWithMultipleIps);
         assertEquals(ImmutableSet.of("127.0.0.4", "::4"), deserializedNode.ipAddresses());