Set openstackId in real node repo client

7 files changed, 55 insertions, 32 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java
index 7d52b9d72b0..917bd64690f 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.configserver;
 
 import com.yahoo.vespa.flags.FlagRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.flags.RealFlagRepository;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.RealNodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorImpl;
@@ -18,7 +17,7 @@ import com.yahoo.vespa.hosted.node.admin.configserver.state.StateImpl;
  */
 public class RealConfigServerClients implements ConfigServerClients {
     private final ConfigServerApi configServerApi;
-    private final NodeRepository nodeRepository;
+    private final RealNodeRepository nodeRepository;
     private final Orchestrator orchestrator;
     private final State state;
     private final RealFlagRepository flagRepository;
@@ -35,7 +34,7 @@ public class RealConfigServerClients implements ConfigServerClients {
     }
 
     @Override
-    public NodeRepository nodeRepository() {
+    public RealNodeRepository nodeRepository() {
         return nodeRepository;
     }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java
index d1b70450226..1cf9c2bfb76 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java
@@ -15,6 +15,7 @@ import java.util.Set;
 public class AddNode {
 
     public final String hostname;
+    public final Optional<String> id;
     public final Optional<String> parentHostname;
     public final Optional<String> nodeFlavor;
     public final Optional<FlavorOverrides> flavorOverrides;
@@ -23,19 +24,20 @@ public class AddNode {
     public final Set<String> ipAddresses;
     public final Set<String> additionalIpAddresses;
 
-    public static AddNode forHost(String hostname, String nodeFlavor, Optional<FlavorOverrides> flavorOverrides, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) {
-        return new AddNode(hostname, Optional.empty(), Optional.of(nodeFlavor), flavorOverrides, Optional.empty(), nodeType, ipAddresses, additionalIpAddresses);
+    public static AddNode forHost(String hostname, Optional<String> id, String nodeFlavor, Optional<FlavorOverrides> flavorOverrides, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) {
+        return new AddNode(hostname, id, Optional.empty(), Optional.of(nodeFlavor), flavorOverrides, Optional.empty(), nodeType, ipAddresses, additionalIpAddresses);
     }
 
     public static AddNode forNode(String hostname, String parentHostname, NodeResources nodeResources, NodeType nodeType, Set<String> ipAddresses) {
-        return new AddNode(hostname, Optional.of(parentHostname), Optional.empty(), Optional.empty(), Optional.of(nodeResources), nodeType, ipAddresses, Set.of());
+        return new AddNode(hostname, Optional.empty(), Optional.of(parentHostname), Optional.empty(), Optional.empty(), Optional.of(nodeResources), nodeType, ipAddresses, Set.of());
     }
 
-    private AddNode(String hostname, Optional<String> parentHostname,
+    private AddNode(String hostname, Optional<String> id, Optional<String> parentHostname,
                     Optional<String> nodeFlavor, Optional<FlavorOverrides> flavorOverrides,
                     Optional<NodeResources> nodeResources,
                     NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) {
         this.hostname = hostname;
+        this.id = id;
         this.parentHostname = parentHostname;
         this.nodeFlavor = nodeFlavor;
         this.flavorOverrides = flavorOverrides;
@@ -51,8 +53,11 @@ public class AddNode {
         if (o == null || getClass() != o.getClass()) return false;
         AddNode addNode = (AddNode) o;
         return Objects.equals(hostname, addNode.hostname) &&
+                Objects.equals(id, addNode.id) &&
                 Objects.equals(parentHostname, addNode.parentHostname) &&
                 Objects.equals(nodeFlavor, addNode.nodeFlavor) &&
+                Objects.equals(flavorOverrides, addNode.flavorOverrides) &&
+                Objects.equals(nodeResources, addNode.nodeResources) &&
                 nodeType == addNode.nodeType &&
                 Objects.equals(ipAddresses, addNode.ipAddresses) &&
                 Objects.equals(additionalIpAddresses, addNode.additionalIpAddresses);
@@ -60,15 +65,18 @@ public class AddNode {
 
     @Override
     public int hashCode() {
-        return Objects.hash(hostname, parentHostname, nodeFlavor, nodeType, ipAddresses, additionalIpAddresses);
+        return Objects.hash(hostname, id, parentHostname, nodeFlavor, flavorOverrides, nodeResources, nodeType, ipAddresses, additionalIpAddresses);
     }
 
     @Override
     public String toString() {
         return "AddNode{" +
                 "hostname='" + hostname + '\'' +
+                ", id=" + id +
                 ", parentHostname=" + parentHostname +
                 ", nodeFlavor='" + nodeFlavor + '\'' +
+                ", flavorOverrides='" + flavorOverrides + '\'' +
+                ", nodeResources='" + nodeResources + '\'' +
                 ", nodeType=" + nodeType +
                 ", ipAddresses=" + ipAddresses +
                 ", additionalIpAddresses=" + additionalIpAddresses +
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepository.java
index 4cce6ce08d6..1a7193b3096 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepository.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepository.java
@@ -10,8 +10,6 @@ import java.util.Optional;
  */
 public interface NodeRepository {
 
-    void addNodes(List<AddNode> nodes);
-
     List<NodeSpec> getNodes(String baseHostName);
 
     default NodeSpec getNode(String hostName) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
index f7d68fe87ea..41f0932419b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
@@ -25,6 +25,7 @@ import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow;
 public class NodeSpec {
 
     private final String hostname;
+    private final Optional<String> id;
     private final NodeState state;
     private final NodeType type;
     private final String flavor;
@@ -66,6 +67,7 @@ public class NodeSpec {
 
     public NodeSpec(
             String hostname,
+            Optional<String> id,
             Optional<DockerImage> wantedDockerImage,
             Optional<DockerImage> currentDockerImage,
             NodeState state,
@@ -102,6 +104,7 @@ public class NodeSpec {
         }
 
         this.hostname = Objects.requireNonNull(hostname);
+        this.id = Objects.requireNonNull(id);
         this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage);
         this.currentDockerImage = Objects.requireNonNull(currentDockerImage);
         this.state = Objects.requireNonNull(state);
@@ -134,6 +137,11 @@ public class NodeSpec {
         return hostname;
     }
 
+    /** Returns the cloud-specific ID of the host. */
+    public Optional<String> id() {
+        return id;
+    }
+
     public NodeState state() {
         return state;
     }
@@ -268,11 +276,13 @@ public class NodeSpec {
         NodeSpec that = (NodeSpec) o;
 
         return Objects.equals(hostname, that.hostname) &&
+                Objects.equals(id, that.id) &&
                 Objects.equals(wantedDockerImage, that.wantedDockerImage) &&
                 Objects.equals(currentDockerImage, that.currentDockerImage) &&
                 Objects.equals(state, that.state) &&
                 Objects.equals(type, that.type) &&
                 Objects.equals(flavor, that.flavor) &&
+                Objects.equals(modelName, that.modelName) &&
                 Objects.equals(wantedVespaVersion, that.wantedVespaVersion) &&
                 Objects.equals(currentVespaVersion, that.currentVespaVersion) &&
                 Objects.equals(wantedOsVersion, that.wantedOsVersion) &&
@@ -299,11 +309,13 @@ public class NodeSpec {
     public int hashCode() {
         return Objects.hash(
                 hostname,
+                id,
                 wantedDockerImage,
                 currentDockerImage,
                 state,
                 type,
                 flavor,
+                modelName,
                 wantedVespaVersion,
                 currentVespaVersion,
                 wantedOsVersion,
@@ -330,11 +342,13 @@ public class NodeSpec {
     public String toString() {
         return getClass().getSimpleName() + " {"
                 + " hostname=" + hostname
+                + " id=" + id
                 + " wantedDockerImage=" + wantedDockerImage
                 + " currentDockerImage=" + currentDockerImage
                 + " state=" + state
                 + " type=" + type
                 + " flavor=" + flavor
+                + " modelName=" + modelName
                 + " wantedVespaVersion=" + wantedVespaVersion
                 + " currentVespaVersion=" + currentVespaVersion
                 + " wantedOsVersion=" + wantedOsVersion
@@ -360,6 +374,7 @@ public class NodeSpec {
 
     public static class Builder {
         private String hostname;
+        private Optional<String> id = Optional.empty();
         private NodeState state;
         private NodeType type;
         private String flavor;
@@ -423,6 +438,11 @@ public class NodeSpec {
             return this;
         }
 
+        public Builder id(String id) {
+            this.id = Optional.of(id);
+            return this;
+        }
+
         public Builder wantedDockerImage(DockerImage wantedDockerImage) {
             this.wantedDockerImage = Optional.of(wantedDockerImage);
             return this;
@@ -681,7 +701,7 @@ public class NodeSpec {
         }
 
         public NodeSpec build() {
-            return new NodeSpec(hostname, wantedDockerImage, currentDockerImage, state, type, flavor,
+            return new NodeSpec(hostname, id, wantedDockerImage, currentDockerImage, state, type, flavor,
                     wantedVespaVersion, currentVespaVersion, wantedOsVersion, currentOsVersion, orchestratorStatus,
                     owner, membership,
                     wantedRestartGeneration, currentRestartGeneration,
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
index 0747912bba2..8ddafea4260 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
@@ -42,7 +42,6 @@ public class RealNodeRepository implements NodeRepository {
         this.configServerApi = configServerApi;
     }
 
-    @Override
     public void addNodes(List<AddNode> nodes) {
         List<NodeRepositoryNode> nodesToPost = nodes.stream()
                 .map(RealNodeRepository::nodeRepositoryNodeFromAddNode)
@@ -153,6 +152,7 @@ public class RealNodeRepository implements NodeRepository {
         NodeReports reports = NodeReports.fromMap(Optional.ofNullable(node.reports).orElseGet(Map::of));
         return new NodeSpec(
                 node.hostname,
+                Optional.ofNullable(node.openStackId),
                 Optional.ofNullable(node.wantedDockerImage).map(DockerImage::fromString),
                 Optional.ofNullable(node.currentDockerImage).map(DockerImage::fromString),
                 nodeState,
@@ -227,7 +227,7 @@ public class RealNodeRepository implements NodeRepository {
 
     private static NodeRepositoryNode nodeRepositoryNodeFromAddNode(AddNode addNode) {
         NodeRepositoryNode node = new NodeRepositoryNode();
-        node.openStackId = "fake-" + addNode.hostname;
+        node.openStackId = addNode.id.orElse("fake-" + addNode.hostname);
         node.hostname = addNode.hostname;
         node.parentHostname = addNode.parentHostname.orElse(null);
         addNode.nodeFlavor.ifPresent(f -> node.flavor = f);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
index 6d26e16f314..75e8590154b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
@@ -40,7 +40,7 @@ public class RealNodeRepositoryTest {
 
     private static final double delta = 0.00000001;
     private JDisc container;
-    private NodeRepository nodeRepositoryApi;
+    private RealNodeRepository nodeRepository;
 
     private int findRandomOpenPort() throws IOException {
         try (ServerSocket socket = new ServerSocket(0)) {
@@ -79,10 +79,10 @@ public class RealNodeRepositoryTest {
 
     private void waitForJdiscContainerToServe(ConfigServerApi configServerApi) throws InterruptedException {
         Instant start = Instant.now();
-        nodeRepositoryApi = new RealNodeRepository(configServerApi);
+        nodeRepository = new RealNodeRepository(configServerApi);
         while (Instant.now().minusSeconds(120).isBefore(start)) {
             try {
-                nodeRepositoryApi.getNodes("foobar");
+                nodeRepository.getNodes("foobar");
                 return;
             } catch (Exception e) {
                 Thread.sleep(100);
@@ -102,7 +102,7 @@ public class RealNodeRepositoryTest {
     public void testGetContainersToRunApi() {
         String dockerHostHostname = "dockerhost1.yahoo.com";
 
-        List<NodeSpec> containersToRun = nodeRepositoryApi.getNodes(dockerHostHostname);
+        List<NodeSpec> containersToRun = nodeRepository.getNodes(dockerHostHostname);
         assertThat(containersToRun.size(), is(1));
         NodeSpec node = containersToRun.get(0);
         assertThat(node.hostname(), is("host4.yahoo.com"));
@@ -118,7 +118,7 @@ public class RealNodeRepositoryTest {
     @Test
     public void testGetContainer() {
         String hostname = "host4.yahoo.com";
-        Optional<NodeSpec> node = nodeRepositoryApi.getOptionalNode(hostname);
+        Optional<NodeSpec> node = nodeRepository.getOptionalNode(hostname);
         assertTrue(node.isPresent());
         assertEquals(hostname, node.get().hostname());
     }
@@ -126,14 +126,14 @@ public class RealNodeRepositoryTest {
     @Test
     public void testGetContainerForNonExistingNode() {
         String hostname = "host-that-does-not-exist";
-        Optional<NodeSpec> node = nodeRepositoryApi.getOptionalNode(hostname);
+        Optional<NodeSpec> node = nodeRepository.getOptionalNode(hostname);
         assertFalse(node.isPresent());
     }
 
     @Test
     public void testUpdateNodeAttributes() {
         String hostname = "host4.yahoo.com";
-        nodeRepositoryApi.updateNodeAttributes(
+        nodeRepository.updateNodeAttributes(
                 hostname,
                 new NodeAttributes()
                         .withRestartGeneration(1)
@@ -142,18 +142,18 @@ public class RealNodeRepositoryTest {
 
     @Test
     public void testMarkAsReady() {
-        nodeRepositoryApi.setNodeState("host5.yahoo.com", NodeState.dirty);
-        nodeRepositoryApi.setNodeState("host5.yahoo.com", NodeState.ready);
+        nodeRepository.setNodeState("host5.yahoo.com", NodeState.dirty);
+        nodeRepository.setNodeState("host5.yahoo.com", NodeState.ready);
 
         try {
-            nodeRepositoryApi.setNodeState("host4.yahoo.com", NodeState.ready);
+            nodeRepository.setNodeState("host4.yahoo.com", NodeState.ready);
             fail("Should not be allowed to be marked ready as it is not registered as provisioned, dirty, failed or parked");
         } catch (RuntimeException ignored) {
             // expected
         }
 
         try {
-            nodeRepositoryApi.setNodeState("host101.yahoo.com", NodeState.ready);
+            nodeRepository.setNodeState("host101.yahoo.com", NodeState.ready);
             fail("Expected failure because host101 does not exist");
         } catch (RuntimeException ignored) {
             // expected
@@ -163,6 +163,7 @@ public class RealNodeRepositoryTest {
     @Test
     public void testAddNodes() {
         AddNode host = AddNode.forHost("host123.domain.tld",
+                                       Optional.of("id1"),
                                        "default",
                                        Optional.of(FlavorOverrides.ofDisk(123)),
                                        NodeType.confighost,
@@ -171,15 +172,16 @@ public class RealNodeRepositoryTest {
         NodeResources nodeResources = new NodeResources(1, 2, 3, 4, NodeResources.DiskSpeed.slow, NodeResources.StorageType.local);
         AddNode node = AddNode.forNode("host123-1.domain.tld", "host123.domain.tld", nodeResources, NodeType.config, Set.of("::2", "::3"));
 
-        assertFalse(nodeRepositoryApi.getOptionalNode("host123.domain.tld").isPresent());
-        nodeRepositoryApi.addNodes(List.of(host, node));
+        assertFalse(nodeRepository.getOptionalNode("host123.domain.tld").isPresent());
+        nodeRepository.addNodes(List.of(host, node));
 
-        NodeSpec hostSpec = nodeRepositoryApi.getOptionalNode("host123.domain.tld").orElseThrow();
+        NodeSpec hostSpec = nodeRepository.getOptionalNode("host123.domain.tld").orElseThrow();
+        assertEquals("id1", hostSpec.id().orElseThrow());
         assertEquals("default", hostSpec.flavor());
         assertEquals(123, hostSpec.diskGb(), 0);
         assertEquals(NodeType.confighost, hostSpec.type());
 
-        NodeSpec nodeSpec = nodeRepositoryApi.getOptionalNode("host123-1.domain.tld").orElseThrow();
+        NodeSpec nodeSpec = nodeRepository.getOptionalNode("host123-1.domain.tld").orElseThrow();
         assertEquals(nodeResources, nodeSpec.resources());
         assertEquals(NodeType.config, nodeSpec.type());
     }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java
index 80069b38748..aa7b0a8af1b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.node.admin.integrationTests;
 
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.AddNode;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -25,9 +24,6 @@ public class NodeRepoMock implements NodeRepository {
     private final Map<String, NodeSpec> nodeRepositoryNodesByHostname = new HashMap<>();
 
     @Override
-    public void addNodes(List<AddNode> nodes) { }
-
-    @Override
     public List<NodeSpec> getNodes(String baseHostName) {
         synchronized (monitor) {
             return nodeRepositoryNodesByHostname.values().stream()