Remove duplicated child node filtering

9 files changed, 35 insertions, 40 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
index 5f0bac7f194..5ef5a46dc45 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
@@ -86,14 +86,18 @@ public class NodeList {
     }
 
     /** Returns the child nodes of the given parent node */
-    public NodeList childrenOf(Node parent) {
+    public NodeList childrenOf(String hostname) {
         return nodes.stream()
                     .filter(n -> n.parentHostname()
-                                  .map(hostName -> hostName.equals(parent.hostname()))
+                                  .map(hostName -> hostName.equals(hostname))
                                   .orElse(false))
                     .collect(collectingAndThen(Collectors.toList(), NodeList::new));
     }
 
+    public NodeList childrenOf(Node parent) {
+        return childrenOf(parent.hostname());
+    }
+
     public int size() { return nodes.size(); }
 
     /** Returns the immutable list of nodes in this */
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 3b133e9f6c8..dca4ea331fd 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
@@ -162,18 +162,9 @@ public class NodeRepository extends AbstractComponent {
         return db.getNodes(inState).stream().filter(node -> node.type().equals(type)).collect(Collectors.toList());
     }
 
-    /**
-     * Finds and returns all nodes that are children of the given parent node
-     *
-     * @param hostname Parent hostname
-     * @return List of child nodes
-     */
-    public List<Node> getChildNodes(String hostname) {
-        return db.getNodes().stream()
-                .filter(node -> node.parentHostname()
-                        .map(parentHostname -> parentHostname.equals(hostname))
-                        .orElse(false))
-                .collect(Collectors.toList());
+    /** Returns a filterable list of all nodes in this repository */
+    public NodeList list() {
+        return new NodeList(getNodes());
     }
 
     public List<Node> getNodes(ApplicationId id, Node.State ... inState) { return db.getNodes(id, inState); }
@@ -255,7 +246,7 @@ public class NodeRepository extends AbstractComponent {
      * @return List of node ACLs
      */
     public List<NodeAcl> getNodeAcls(Node node, boolean children) {
-        NodeList candidates = new NodeList(getNodes());
+        NodeList candidates = list();
         if (children) {
             return candidates.childrenOf(node).asList().stream()
                              .map(childNode -> getNodeAcl(childNode, candidates))
@@ -413,7 +404,7 @@ public class NodeRepository extends AbstractComponent {
 
         List<Node> nodesToDirty =
                 (nodeToDirty.type().isDockerHost() ?
-                        Stream.concat(getChildNodes(hostname).stream(), Stream.of(nodeToDirty)) :
+                        Stream.concat(list().childrenOf(hostname).asList().stream(), Stream.of(nodeToDirty)) :
                         Stream.of(nodeToDirty))
                 .filter(node -> node.state() != Node.State.dirty)
                 .collect(Collectors.toList());
@@ -483,9 +474,9 @@ public class NodeRepository extends AbstractComponent {
     }
 
     private List<Node> moveRecursively(String hostname, Node.State toState, Agent agent, Optional<String> reason) {
-        List<Node> moved = getChildNodes(hostname).stream()
-                .map(child -> move(child, toState, agent, reason))
-                .collect(Collectors.toList());
+        List<Node> moved = list().childrenOf(hostname).asList().stream()
+                                         .map(child -> move(child, toState, agent, reason))
+                                         .collect(Collectors.toList());
 
         moved.add(move(hostname, toState, agent, reason));
         return moved;
@@ -547,9 +538,9 @@ public class NodeRepository extends AbstractComponent {
             List<Node> removed = new ArrayList<>();
 
              if (node.type().isDockerHost()) {
-                 getChildNodes(node.hostname()).stream()
-                                               .filter(child -> force || canRemove(child, true))
-                                               .forEach(removed::add);
+                 list().childrenOf(node).asList().stream()
+                       .filter(child -> force || canRemove(child, true))
+                       .forEach(removed::add);
              }
 
             if (force || canRemove(node, false)) removed.add(node);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
index 01eef3c2aa1..280f723f268 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
@@ -101,10 +101,10 @@ public class FailedExpirer extends Maintainer {
         for (Node candidate : nodes) {
             if (hasHardwareIssue(candidate)) {
                 List<String> unparkedChildren = !candidate.type().isDockerHost() ? Collections.emptyList() :
-                        nodeRepository.getChildNodes(candidate.hostname()).stream()
-                                .filter(node -> node.state() != Node.State.parked)
-                                .map(Node::hostname)
-                                .collect(Collectors.toList());
+                        nodeRepository.list().childrenOf(candidate).asList().stream()
+                                      .filter(node -> node.state() != Node.State.parked)
+                                      .map(Node::hostname)
+                                      .collect(Collectors.toList());
 
                 if (unparkedChildren.isEmpty()) {
                     nodeRepository.park(candidate.hostname(), Agent.system,
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
index 0dabe252406..17351ab3c10 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java
@@ -298,7 +298,7 @@ public class NodeFailer extends Maintainer {
             // the children nodes running on it before we fail the host
             boolean allTenantNodesFailedOutSuccessfully = true;
             String reasonForChildFailure = "Failing due to parent host " + node.hostname() + " failure: " + reason;
-            for (Node failingTenantNode : nodeRepository().getChildNodes(node.hostname())) {
+            for (Node failingTenantNode : nodeRepository().list().childrenOf(node).asList()) {
                 if (failingTenantNode.state() == Node.State.active) {
                     allTenantNodesFailedOutSuccessfully &= failActive(failingTenantNode, reasonForChildFailure);
                 } else {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
index 6036399d83e..dcd623a3212 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
@@ -93,9 +93,9 @@ public class RetiredExpirer extends Maintainer {
     private boolean canRemove(Node node) {
         if (node.type().isDockerHost()) {
             if (nodeRepository()
-                        .getChildNodes(node.hostname()).stream()
-                        .allMatch(child -> child.state() == Node.State.parked ||
-                                child.state() == Node.State.failed)) {
+                    .list().childrenOf(node).asList().stream()
+                    .allMatch(child -> child.state() == Node.State.parked ||
+                                       child.state() == Node.State.failed)) {
                 log.info("Docker host " + node + " has no non-parked/failed children");
                 return true;
             }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index c2e2ea36c13..5e98466fd7c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -161,7 +161,7 @@ class NodeAllocation {
      */
     private boolean exclusiveTo(TenantName tenant, Optional<String> parentHostname) {
         if ( ! parentHostname.isPresent()) return true;
-        for (Node nodeOnHost : nodeRepository.getChildNodes(parentHostname.get())) {
+        for (Node nodeOnHost : nodeRepository.list().childrenOf(parentHostname.get()).asList()) {
             if ( ! nodeOnHost.allocation().isPresent()) continue;
 
             if ( nodeOnHost.allocation().get().membership().cluster().isExclusive() &&
@@ -174,7 +174,7 @@ class NodeAllocation {
     private boolean hostsOnly(TenantName tenant, Optional<String> parentHostname) {
         if ( ! parentHostname.isPresent()) return true; // yes, as host is exclusive
 
-        for (Node nodeOnHost : nodeRepository.getChildNodes(parentHostname.get())) {
+        for (Node nodeOnHost : nodeRepository.list().childrenOf(parentHostname.get()).asList()) {
             if ( ! nodeOnHost.allocation().isPresent()) continue;
             if ( ! nodeOnHost.allocation().get().owner().tenant().equals(tenant))
                 return false;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
index f013620f88d..3c54c480c44 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
@@ -49,7 +49,7 @@ public class NodePatcher {
             this.node = node;
             this.nodeRepository = nodeRepository;
             this.children = node.type().isDockerHost() ?
-                    nodeRepository.getChildNodes(node.hostname()) :
+                    nodeRepository.list().childrenOf(node).asList() :
                     Collections.emptyList();
         }
         catch (IOException e) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java
index b13e5c418f9..2374eda8bca 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java
@@ -52,8 +52,8 @@ public class NodeFailerTest {
         });
 
         // The host should have 2 nodes in active and 1 ready
-        Map<Node.State, List<String>> hostnamesByState = tester.nodeRepository.getChildNodes(hostWithHwFailure).stream()
-                .collect(Collectors.groupingBy(Node::state, Collectors.mapping(Node::hostname, Collectors.toList())));
+        Map<Node.State, List<String>> hostnamesByState = tester.nodeRepository.list().childrenOf(hostWithHwFailure).asList().stream()
+                                                                              .collect(Collectors.groupingBy(Node::state, Collectors.mapping(Node::hostname, Collectors.toList())));
         assertEquals(2, hostnamesByState.get(Node.State.active).size());
         assertEquals(1, hostnamesByState.get(Node.State.ready).size());
 
@@ -70,8 +70,8 @@ public class NodeFailerTest {
         expectedHostnamesByState1Iter.put(Node.State.failed,
                 Arrays.asList(hostnamesByState.get(Node.State.active).get(0), hostnamesByState.get(Node.State.ready).get(0)));
         expectedHostnamesByState1Iter.put(Node.State.active, hostnamesByState.get(Node.State.active).subList(1, 2));
-        Map<Node.State, List<String>> hostnamesByState1Iter = tester.nodeRepository.getChildNodes(hostWithHwFailure).stream()
-                .collect(Collectors.groupingBy(Node::state, Collectors.mapping(Node::hostname, Collectors.toList())));
+        Map<Node.State, List<String>> hostnamesByState1Iter = tester.nodeRepository.list().childrenOf(hostWithHwFailure).asList().stream()
+                                                                                   .collect(Collectors.groupingBy(Node::state, Collectors.mapping(Node::hostname, Collectors.toList())));
         assertEquals(expectedHostnamesByState1Iter, hostnamesByState1Iter);
 
         // Suspend the second of the active nodes
@@ -82,8 +82,8 @@ public class NodeFailerTest {
         tester.failer.run();
 
         // All of the children should be failed now
-        Set<Node.State> childStates2Iter = tester.nodeRepository.getChildNodes(hostWithHwFailure).stream()
-                .map(Node::state).collect(Collectors.toSet());
+        Set<Node.State> childStates2Iter = tester.nodeRepository.list().childrenOf(hostWithHwFailure).asList().stream()
+                                                                .map(Node::state).collect(Collectors.toSet());
         assertEquals(Collections.singleton(Node.State.failed), childStates2Iter);
         // The host itself is still active as it too must be allowed to suspend
         assertEquals(Node.State.active, tester.nodeRepository.getNode(hostWithHwFailure).get().state());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java
index ff51869273d..a0198f9c3f7 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisioningTest.java
@@ -135,7 +135,7 @@ public class DynamicDockerProvisioningTest {
 
         Map<Integer, Integer> numberOfChildrenStat = new HashMap<>();
         for (Node node : dockerHosts) {
-            int nofChildren = tester.nodeRepository().getChildNodes(node.hostname()).size();
+            int nofChildren = tester.nodeRepository().list().childrenOf(node).size();
             if (!numberOfChildrenStat.containsKey(nofChildren)) {
                 numberOfChildrenStat.put(nofChildren, 0);
             }