Allow single-container hosts to share IP with container

2 files changed, 70 insertions, 15 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
index 424d12740cb..bbac3241988 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
@@ -20,6 +20,10 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static com.yahoo.config.provision.NodeType.confighost;
+import static com.yahoo.config.provision.NodeType.controllerhost;
+import static com.yahoo.config.provision.NodeType.proxyhost;
+
 /**
  * This handles IP address configuration and allocation.
  *
@@ -121,11 +125,13 @@ public class IP {
          * @throws IllegalArgumentException if there are IP conflicts with existing nodes
          */
         public static List<Node> verify(List<Node> nodes, LockedNodeList allNodes) {
-            for (Node node : nodes) {
-                for (Node other : allNodes) {
+            for (var node : nodes) {
+                for (var other : allNodes) {
                     if (node.equals(other)) continue;
-                    Set<String> addresses = new HashSet<>(node.ipConfig().primary());
-                    Set<String> otherAddresses = new HashSet<>(other.ipConfig().primary());
+                    if (canAssignIpOf(other, node)) continue;
+
+                    var addresses = new HashSet<>(node.ipConfig().primary());
+                    var otherAddresses = new HashSet<>(other.ipConfig().primary());
                     if (node.type().isDockerHost()) { // Addresses of a host can never overlap with any other nodes
                         addresses.addAll(node.ipConfig().pool().asSet());
                         otherAddresses.addAll(other.ipConfig().pool().asSet());
@@ -140,6 +146,18 @@ public class IP {
             return nodes;
         }
 
+        /** Returns whether IP address of existing node can be assigned to node */
+        private static boolean canAssignIpOf(Node existingNode, Node node) {
+            if (node.parentHostname().isEmpty()) return false; // Not a child node
+            if (!node.parentHostname().get().equals(existingNode.hostname())) return false; // Wrong host
+            switch (node.type()) {
+                case proxy: return existingNode.type() == proxyhost;
+                case config: return existingNode.type() == confighost;
+                case controller: return existingNode.type() == controllerhost;
+            }
+            return false;
+        }
+
         public static Node verify(Node node, LockedNodeList allNodes) {
             return verify(List.of(node), allNodes).get(0);
         }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index def21970388..fa2ddc841cb 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -5,6 +5,7 @@ import com.yahoo.application.Networking;
 import com.yahoo.application.container.JDisc;
 import com.yahoo.application.container.handler.Request;
 import com.yahoo.application.container.handler.Response;
+import com.yahoo.config.provision.NodeType;
 import com.yahoo.io.IOUtils;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Slime;
@@ -273,7 +274,7 @@ public class RestApiTest {
     }
 
     @Test
-    public void post_node_with_invalid_ip_address() throws Exception {
+    public void post_node_with_duplicate_ip_address() throws Exception {
         Request req = new Request("http://localhost:8080/nodes/v2/node",
                 ("[" + asNodeJson("host-with-ip.yahoo.com", "default", "foo") + "]").
                         getBytes(StandardCharsets.UTF_8),
@@ -303,6 +304,32 @@ public class RestApiTest {
                                    "{\"ipAddresses\": [\"::104:3\"]}",
                                    Request.Method.PATCH), 400,
                        "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not set field 'ipAddresses': Cannot assign [::100:4, ::100:3, ::100:2, ::104:3] to dockerhost1.yahoo.com: [::104:3] already assigned to dockerhost5.yahoo.com\"}");
+
+        // Node types running a single container can share their IP address with child node
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                                   "[" + asNodeJson("cfghost42.yahoo.com", NodeType.confighost, "default", "127.0.42.1") + "]",
+                                   Request.Method.POST), 200,
+                       "{\"message\":\"Added 1 nodes to the provisioned state\"}");
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                                   "[" + asDockerNodeJson("cfg42.yahoo.com", NodeType.config, "cfghost42.yahoo.com", "127.0.42.1") + "]",
+                                   Request.Method.POST), 200,
+                       "{\"message\":\"Added 1 nodes to the provisioned state\"}");
+
+        // ... but cannot share with child node of wrong type
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                                   "[" + asDockerNodeJson("proxy42.yahoo.com", NodeType.proxy, "cfghost42.yahoo.com", "127.0.42.1") + "]",
+                                   Request.Method.POST), 400,
+                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot assign [127.0.42.1] to proxy42.yahoo.com: [127.0.42.1] already assigned to cfg42.yahoo.com\"}");
+
+        // ... nor with child node on different host
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node",
+                                   "[" + asNodeJson("cfghost43.yahoo.com", NodeType.confighost, "default", "127.0.43.1") + "]",
+                                   Request.Method.POST), 200,
+                       "{\"message\":\"Added 1 nodes to the provisioned state\"}");
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node/cfg42.yahoo.com",
+                                   "{\"ipAddresses\": [\"127.0.43.1\"]}",
+                                   Request.Method.PATCH), 400,
+                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not set field 'ipAddresses': Cannot assign [127.0.43.1] to cfg42.yahoo.com: [127.0.43.1] already assigned to cfghost43.yahoo.com\"}");
     }
 
     @Test
@@ -919,26 +946,36 @@ public class RestApiTest {
                 "\"minDiskAvailableGb\":12.0,\"minMainMemoryAvailableGb\":34.0,\"minCpuCores\":56.0,\"fastDisk\":true,\"bandwidthGbps\":78.0");
     }
 
-    private String asDockerNodeJson(String hostname, String parentHostname, String... ipAddress) {
+    private static String asDockerNodeJson(String hostname, String parentHostname, String... ipAddress) {
+        return asDockerNodeJson(hostname, NodeType.tenant, parentHostname, ipAddress);
+    }
+
+    private static String asDockerNodeJson(String hostname, NodeType nodeType, String parentHostname, String... ipAddress) {
         return "{\"hostname\":\"" + hostname + "\", \"parentHostname\":\"" + parentHostname + "\"," +
-                createIpAddresses(ipAddress) +
-                "\"openStackId\":\"" + hostname + "\",\"flavor\":\"d-1-1-100\"}";
+               createIpAddresses(ipAddress) +
+               "\"openStackId\":\"" + hostname + "\",\"flavor\":\"d-1-1-100\"" +
+               (nodeType != NodeType.tenant ? ",\"type\":\"" + nodeType +  "\"" : "") +
+               "}";
     }
 
-    private String asNodeJson(String hostname, String flavor, String... ipAddress) {
+    private static String asNodeJson(String hostname, String flavor, String... ipAddress) {
         return "{\"hostname\":\"" + hostname + "\", \"openStackId\":\"" + hostname + "\"," +
                 createIpAddresses(ipAddress) +
                 "\"flavor\":\"" + flavor + "\"}";
     }
 
-    private String asHostJson(String hostname, String flavor, String... ipAddress) {
+    private static String asHostJson(String hostname, String flavor, String... ipAddress) {
+        return asNodeJson(hostname,  NodeType.host, flavor, ipAddress);
+    }
+
+    private static String asNodeJson(String hostname, NodeType nodeType, String flavor, String... ipAddress) {
         return "{\"hostname\":\"" + hostname + "\", \"openStackId\":\"" + hostname + "\"," +
-                createIpAddresses(ipAddress) +
-                "\"flavor\":\"" + flavor + "\"" +
-                ", \"type\":\"host\"}";
+               createIpAddresses(ipAddress) +
+               "\"flavor\":\"" + flavor + "\"" +
+               ", \"type\":\"" + nodeType + "\"}";
     }
 
-    private String createIpAddresses(String... ipAddress) {
+    private static String createIpAddresses(String... ipAddress) {
         return "\"ipAddresses\":[" +
                 Arrays.stream(ipAddress)
                       .map(ip -> "\"" + ip + "\"")
@@ -946,7 +983,7 @@ public class RestApiTest {
                 "],";
     }
 
-    private boolean getHardwareFailure(String json) {
+    private static boolean getHardwareFailure(String json) {
         Slime slime = SlimeUtils.jsonToSlime(json.getBytes());
         Cursor hardwareFailure = slime.get().field("hardwareFailure");
         if (!hardwareFailure.valid())