Check for IPv4 conflicts within each cloud account, not across

3 files changed, 43 insertions, 2 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 3ef1951c19c..f2dd14bbf6d 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.node;
 
 import com.google.common.net.InetAddresses;
 import com.google.common.primitives.UnsignedBytes;
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.hosted.provision.LockedNodeList;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -16,6 +17,7 @@ import java.util.Comparator;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -107,9 +109,9 @@ public record IP() {
          * @throws IllegalArgumentException if there are IP conflicts with existing nodes
          */
         public static List<Node> verify(List<Node> nodes, LockedNodeList allNodes) {
-            NodeList sortedNodes = allNodes.sortedBy(Comparator.comparing(Node::hostname));
+            Map<CloudAccount, NodeList> sortedNodes = allNodes.sortedBy(Comparator.comparing(Node::hostname)).groupingBy(Node::cloudAccount);
             for (var node : nodes) {
-                for (var other : sortedNodes) {
+                for (var other : sortedNodes.getOrDefault(node.cloudAccount(), NodeList.of())) {
                     if (node.equals(other)) continue;
                     if (canAssignIpOf(other, node)) continue;
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
index f45a5cd1c5f..867509e1ae3 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
@@ -1,12 +1,14 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision;
 
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.History;
 import com.yahoo.vespa.hosted.provision.node.IP;
 import com.yahoo.vespa.hosted.provision.node.Report;
 import com.yahoo.vespa.hosted.provision.node.Reports;
+import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
 import org.junit.Test;
 
 import java.time.Duration;
@@ -18,6 +20,7 @@ import java.util.function.Predicate;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -50,6 +53,41 @@ public class NodeRepositoryTest {
     }
 
     @Test
+    public void test_ip_conflicts() {
+        NodeRepositoryTester tester = new NodeRepositoryTester();
+        ((MockNameResolver) tester.nodeRepository().nameResolver()).addRecord("host1", "1.2.3.4")
+                                                                   .addRecord("host2", "1.2.3.4")
+                                                                   .addRecord("host3", "1.2.3.4");
+        tester.addHost("id1", "host1", "default", NodeType.host);
+
+        IP.Config ipConfig = IP.Config.of(Set.of("1.2.3.4"), Set.of("1.2.3.4"));
+
+        Node host2 = Node.create("id2", ipConfig, "host2", tester.nodeFlavors().getFlavorOrThrow("default"), NodeType.host)
+                         .build();
+
+        Node enclaveHost2 = Node.create("id2", ipConfig, "host2", tester.nodeFlavors().getFlavorOrThrow("default"), NodeType.host)
+                        .cloudAccount(CloudAccount.from("gcp:foo-bar-baz"))
+                        .build();
+
+        Node enclaveHost3 = Node.create("id3", ipConfig, "host3", tester.nodeFlavors().getFlavorOrThrow("default"), NodeType.host)
+                                .cloudAccount(CloudAccount.from("gcp:foo-bar-baz"))
+                                .build();
+
+        assertEquals("Cannot assign [1.2.3.4] to host2: [1.2.3.4] already assigned to host1",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> tester.nodeRepository().nodes().addNodes(List.of(host2), Agent.system))
+                             .getMessage());
+
+        tester.nodeRepository().nodes().addNodes(List.of(enclaveHost2), Agent.system);
+
+        assertEquals("Cannot assign [1.2.3.4] to host3: [1.2.3.4] already assigned to host2",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> tester.nodeRepository().nodes().addNodes(List.of(enclaveHost3), Agent.system))
+                             .getMessage());
+
+    }
+
+    @Test
     public void only_allow_docker_containers_remove_in_ready() {
         NodeRepositoryTester tester = new NodeRepositoryTester();
         tester.addHost("id1", "host1", "docker", NodeType.tenant);
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
index 4d0b3e75740..00c4d95b0da 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
@@ -55,6 +55,7 @@ public class NodeRepositoryTester {
     
     public NodeRepository nodeRepository() { return nodeRepository; }
     public MockCurator curator() { return curator; }
+    public NodeFlavors nodeFlavors() { return nodeFlavors; }
     
     public List<Node> getNodes(NodeType type, Node.State ... inState) {
         return nodeRepository.nodes().list(inState).nodeType(type).asList();