Code review fixes

6 files changed, 13 insertions, 8 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java
index faddcb9f5ac..1e4016f2112 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainer.java
@@ -9,7 +9,6 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 
 import java.time.Duration;
-import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
@@ -37,7 +36,7 @@ public class HostDeprovisionMaintainer extends Maintainer {
         try (Mutex lock = nodeRepository().lockUnallocated()) {
             NodeList nodes = nodeRepository().list();
 
-            for (Node node : actionableNodes(nodes)) {
+            for (Node node : candidates(nodes)) {
                 try {
                     hostProvisioner.deprovision(node);
                     nodeRepository().removeRecursively(node.hostname());
@@ -49,7 +48,7 @@ public class HostDeprovisionMaintainer extends Maintainer {
     }
 
     /** @return Nodes of type host, in any state, that have no children with allocation */
-    static Set<Node> actionableNodes(NodeList nodes) {
+    static Set<Node> candidates(NodeList nodes) {
         Map<String, Node> hostsByHostname = nodes.nodeType(NodeType.host)
                 .asList().stream()
                 .collect(Collectors.toMap(Node::hostname, Function.identity()));
@@ -60,6 +59,6 @@ public class HostDeprovisionMaintainer extends Maintainer {
                 .distinct()
                 .forEach(hostsByHostname::remove);
 
-        return new HashSet<>(hostsByHostname.values());
+        return Set.copyOf(hostsByHostname.values());
     }
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java
index edc18ab3ecd..0479f20ba37 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainer.java
@@ -39,7 +39,7 @@ public class HostProvisionMaintainer extends Maintainer {
         try (Mutex lock = nodeRepository().lockUnallocated()) {
             NodeList nodes = nodeRepository().list();
 
-            actionableNodes(nodes).forEach((host, children) -> {
+            candidates(nodes).forEach((host, children) -> {
                 try {
                     List<Node> updatedNodes = hostProvisioner.provision(host, children);
                     nodeRepository().write(updatedNodes);
@@ -56,7 +56,7 @@ public class HostProvisionMaintainer extends Maintainer {
     }
 
     /** @return map of set of children by parent Node, where parent is of type host and in state provisioned */
-    static Map<Node, Set<Node>> actionableNodes(NodeList nodes) {
+    static Map<Node, Set<Node>> candidates(NodeList nodes) {
         Map<String, Node> provisionedHostsByHostname = nodes.state(Node.State.provisioned).nodeType(NodeType.host)
                 .asList().stream()
                 .collect(Collectors.toMap(Node::hostname, Function.identity()));
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java
index e5f92ea7fbf..1f38ef9b3e2 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FatalProvisioningException.java
@@ -2,6 +2,10 @@
 package com.yahoo.vespa.hosted.provision.provisioning;
 
 /**
+ * Thrown by {@link HostProvisioner} to indicate that the provisioning of a host has
+ * irrecoverably failed. The host should be deprovisioned and (together with its children)
+ * removed from the node-repository.
+ *
  * @author freva
  */
 public class FatalProvisioningException extends RuntimeException {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
index e78b1e32162..d82e3900e76 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
@@ -8,6 +8,8 @@ import java.util.List;
 import java.util.Set;
 
 /**
+ * Service for provisioning physical docker tenant hosts inside the zone.
+ *
  * @author freva
  */
 public interface HostProvisioner {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java
index c0a93b034a7..95c857527a1 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostDeprovisionMaintainerTest.java
@@ -92,7 +92,7 @@ public class HostDeprovisionMaintainerTest {
                 createNode("proxy2", Optional.of("proxyhost2"), NodeType.proxy, Node.State.active, Optional.of(proxyApp)));
 
         Set<Node> expected = Set.of(host2, host3);
-        Set<Node> actual = HostDeprovisionMaintainer.actionableNodes(new NodeList(nodes));
+        Set<Node> actual = HostDeprovisionMaintainer.candidates(new NodeList(nodes));
         assertEquals(expected, actual);
     }
 }
\ No newline at end of file
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java
index 32861aa96ab..01c83bbeff4 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostProvisionMaintainerTest.java
@@ -113,7 +113,7 @@ public class HostProvisionMaintainerTest {
                 createNode("proxy2", Optional.of("proxyhost2"), NodeType.proxy, Node.State.active, Optional.of(proxyApp)));
 
         Map<Node, Set<Node>> expected = Map.of(host4, Set.of(host41));
-        Map<Node, Set<Node>> actual = HostProvisionMaintainer.actionableNodes(new NodeList(nodes));
+        Map<Node, Set<Node>> actual = HostProvisionMaintainer.candidates(new NodeList(nodes));
         assertEquals(expected, actual);
     }