Use NameResolver from noderepo - which provides a mock instance

6 files changed, 61 insertions, 30 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index 7f595b4a541..6d04685189f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -111,7 +111,10 @@ public class NodeRepository extends AbstractComponent {
 
     /** Returns the Docker image to use for nodes in this */
     public DockerImage dockerImage() { return dockerImage; }
-    
+
+    /** @return The name resolver used to resolve hostname and ip addresses */
+    public NameResolver nameResolver() { return nameResolver; }
+
     // ---------------- Query API ----------------------------------------------------------------
 
     /**
@@ -258,6 +261,7 @@ public class NodeRepository extends AbstractComponent {
         if (ipAddresses.isEmpty()) {
             ipAddresses = nameResolver.getAllByNameOrThrow(hostname);
         }
+
         return Node.create(openStackId, ImmutableSet.copyOf(ipAddresses), additionalIpAddresses, hostname, parentHostname, flavor, type);
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/DnsNameResolver.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/DnsNameResolver.java
index 6486343150c..f0165673aa9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/DnsNameResolver.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/DnsNameResolver.java
@@ -2,14 +2,19 @@
 package com.yahoo.vespa.hosted.provision.persistence;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.net.InetAddresses;
+import com.yahoo.log.LogLevel;
 
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
 import javax.naming.directory.Attributes;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.InitialDirContext;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.Optional;
 import java.util.Set;
+import java.util.logging.Logger;
 
 /**
  * Implementation of a name resolver that always uses a DNS server to resolve the given name. The intention is to avoid
@@ -19,7 +24,8 @@ import java.util.Set;
  */
 public class DnsNameResolver implements NameResolver {
 
-    /** Resolve IP addresses for given host name */
+    private static Logger logger = Logger.getLogger(DnsNameResolver.class.getName());
+
     @Override
     public Set<String> getAllByNameOrThrow(String hostname) {
         try {
@@ -39,6 +45,18 @@ public class DnsNameResolver implements NameResolver {
         }
     }
 
+    @Override
+    public Optional<String> getHostname(String ipAddress) {
+        try {
+            String hostname = InetAddress.getByName(ipAddress).getHostName();
+            return InetAddresses.isInetAddress(hostname) ? Optional.empty() : Optional.of(hostname);
+        } catch (UnknownHostException e) {
+            // This is not an exceptional state hence the debug level
+            logger.log(LogLevel.DEBUG, "Unable to resolve ipaddress", e);
+        }
+        return Optional.empty();
+    }
+
     private Optional<String> lookupName(String name, Type type) throws NamingException {
         DirContext ctx = new InitialDirContext();
         Attributes attributes = ctx.getAttributes("dns:/" + name, new String[]{type.value});
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NameResolver.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NameResolver.java
index b079ebc1100..1b6e4628927 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NameResolver.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NameResolver.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.persistence;
 
+import java.util.Optional;
 import java.util.Set;
 
 /**
@@ -10,6 +11,20 @@ import java.util.Set;
  */
 public interface NameResolver {
 
+    /**
+     * Get IP addresses for given host name
+     *
+     * @param hostname The hostname to resolve
+     * @return A set of IPv4 or IPv6 addresses
+     */
     Set<String> getAllByNameOrThrow(String hostname);
 
+    /**
+     * Get hostname from IP address
+     *
+     * @param ipAddress The IPv4 or IPv6 address for the host
+     * @return Empty if the IP does not resolve or the hostname if it does
+     */
+    Optional<String> getHostname(String ipAddress);
+
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
index 81be03dc4ef..b30f8a8c020 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.provisioning;
 
-import com.google.common.net.InetAddresses;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Flavor;
@@ -9,9 +8,8 @@ import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.persistence.NameResolver;
 
-import java.net.InetAddress;
-import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -21,8 +19,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
@@ -41,7 +37,7 @@ public class NodePrioritizer {
     private final NodeSpec requestedNodes;
     private final ApplicationId appId;
     private final ClusterSpec clusterSpec;
-
+    private final NameResolver nameResolver;
     private final boolean isDocker;
     private final boolean isAllocatingForReplacement;
     private final Set<Node> spareHosts;
@@ -50,12 +46,12 @@ public class NodePrioritizer {
     /** This is before we mock DNS to resolve on test IP that we use in tests */
     public static boolean unitTesting = false;
 
-    NodePrioritizer(List<Node> allNodes, ApplicationId appId, ClusterSpec clusterSpec, NodeSpec nodeSpec, NodeFlavors nodeFlavors, int spares) {
+    NodePrioritizer(List<Node> allNodes, ApplicationId appId, ClusterSpec clusterSpec, NodeSpec nodeSpec, NodeFlavors nodeFlavors, int spares, NameResolver nameResolver) {
         this.allNodes = Collections.unmodifiableList(allNodes);
         this.requestedNodes = nodeSpec;
         this.clusterSpec = clusterSpec;
         this.appId = appId;
-
+        this.nameResolver = nameResolver;
         this.spareHosts = findSpareHosts(allNodes, spares);
         this.headroomHosts = findHeadroomHosts(allNodes, spareHosts, nodeFlavors);
 
@@ -79,22 +75,6 @@ public class NodePrioritizer {
     }
 
     /**
-     * From ipAddress - get hostname
-     *
-     * @return hostname or null if not able to do the lookup
-     */
-    static String lookupHostname(String ipAddress) {
-        try {
-            String hostname = InetAddress.getByName(ipAddress).getHostName();
-            if (unitTesting) return hostname;
-            return InetAddresses.isInetAddress(hostname) ? null : hostname;
-        } catch (UnknownHostException e) {
-            Logger.getLogger(NodePrioritizer.class.getName()).log(Level.FINER, "Unable to resolve ipaddress", e);
-        }
-        return null;
-    }
-
-    /**
      * Spare hosts are the two hosts in the system with the most free capacity.
      *
      * We do not count retired or inactive nodes as used capacity (as they could have been
@@ -213,10 +193,10 @@ public class NodePrioritizer {
                     Set<String> ipAddresses = DockerHostCapacity.findFreeIps(node, allNodes);
                     if (ipAddresses.isEmpty()) continue;
                     String ipAddress = ipAddresses.stream().findFirst().get();
-                    String hostname = lookupHostname(ipAddress);
-                    if (hostname == null) continue;
+                    Optional<String> hostname = nameResolver.getHostname(ipAddress);
+                    if (!hostname.isPresent()) continue;
                     Node newNode = Node.createDockerNode("fake-" + hostname, Collections.singleton(ipAddress),
-                            Collections.emptySet(), hostname, Optional.of(node.hostname()), getFlavor(requestedNodes), NodeType.tenant);
+                            Collections.emptySet(), hostname.get(), Optional.of(node.hostname()), getFlavor(requestedNodes), NodeType.tenant);
                     PrioritizableNode nodePri = toNodePriority(newNode, false, true);
                     if (!nodePri.violatesSpares || isAllocatingForReplacement) {
                         nodes.put(newNode, nodePri);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNameResolver.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNameResolver.java
index b5c10bb7755..842fd78ed2c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNameResolver.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNameResolver.java
@@ -10,6 +10,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ThreadLocalRandom;
 
@@ -56,11 +57,20 @@ public class MockNameResolver implements NameResolver {
             return records.get(hostname);
         }
         if (mockAnyLookup) {
-            return Collections.singleton(randomIpAddress());
+            Set<String> ipAddresses = Collections.singleton(randomIpAddress());
+            records.put(hostname, ipAddresses);
         }
         throw new RuntimeException(new UnknownHostException("Could not resolve: " + hostname));
     }
 
+    @Override
+    public Optional<String> getHostname(String ipAddress) {
+        for (String host : records.keySet()) {
+            if (records.get(host).contains(ipAddress)) return Optional.of(host);
+        }
+        return Optional.empty();
+    }
+
     private static String randomIpAddress() {
         // Generate a random IP in 127/8 (loopback block)
         ThreadLocalRandom random = ThreadLocalRandom.current();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
index 0c62fd5062c..80472487b2d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java
@@ -62,6 +62,10 @@ public class MockNodeRepository extends NodeRepository {
         NodePrioritizer.unitTesting = true;
         List<Node> nodes = new ArrayList<>();
 
+        MockNameResolver nameResolver = (MockNameResolver)nameResolver();
+        nameResolver.addRecord("docker-node-1", "::2");
+        nameResolver.addRecord("docker-node-2", "::3");
+        nameResolver.addRecord("docker-node-3", "::4");
         final List<String> ipAddressesForAllHost = Arrays.asList("127.0.0.1", "::1");
         Collections.sort(ipAddressesForAllHost);
         final HashSet<String> ipAddresses = new HashSet<>(ipAddressesForAllHost);