Move methods only used by MetricsReporter out of DockerHostCapacity

4 files changed, 47 insertions, 144 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
index 8772e421394..a79ac908b9e 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
@@ -3,19 +3,17 @@ package com.yahoo.vespa.hosted.provision.maintenance;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceInstance;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
-import com.yahoo.vespa.hosted.provision.LockedNodeList;
 import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Allocation;
 import com.yahoo.vespa.hosted.provision.node.History;
-import com.yahoo.vespa.hosted.provision.provisioning.DockerHostCapacity;
 import com.yahoo.vespa.orchestrator.Orchestrator;
 import com.yahoo.vespa.orchestrator.status.HostStatus;
 import com.yahoo.vespa.service.monitor.ServiceMonitor;
@@ -29,6 +27,8 @@ import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
+import static com.yahoo.config.provision.NodeResources.DiskSpeed.any;
+
 /**
  * @author oyving
  */
@@ -55,8 +55,7 @@ public class MetricsReporter extends Maintainer {
 
     @Override
     public void maintain() {
-        LockedNodeList nodes = nodeRepository().list(() -> {}); // Ignore locking for the purposes of reporting metrics
-
+        NodeList nodes = nodeRepository().list();
         Map<HostName, List<ServiceInstance>> servicesByHost =
                 serviceMonitor.getServiceModelSnapshot().getServiceInstancesByHostName();
 
@@ -189,7 +188,7 @@ public class MetricsReporter extends Maintainer {
         return contextMap.computeIfAbsent(dimensions, metric::createContext);
     }
 
-    private void updateStateMetrics(LockedNodeList nodes) {
+    private void updateStateMetrics(NodeList nodes) {
         Map<Node.State, List<Node>> nodesByState = nodes.nodeType(NodeType.tenant).asList().stream()
                 .collect(Collectors.groupingBy(Node::state));
 
@@ -200,20 +199,35 @@ public class MetricsReporter extends Maintainer {
         }
     }
 
-    private void updateDockerMetrics(LockedNodeList nodes) {
-        // Capacity flavors for docker
-        DockerHostCapacity capacity = new DockerHostCapacity(nodes);
-        metric.set("hostedVespa.docker.totalCapacityCpu",
-                capacity.getCapacityTotal(NodeResources.DiskSpeed.any).vcpu(), null);
-        metric.set("hostedVespa.docker.totalCapacityMem",
-                capacity.getCapacityTotal(NodeResources.DiskSpeed.any).memoryGb(), null);
-        metric.set("hostedVespa.docker.totalCapacityDisk",
-                capacity.getCapacityTotal(NodeResources.DiskSpeed.any).diskGb(), null);
-        metric.set("hostedVespa.docker.freeCapacityCpu",
-                capacity.getFreeCapacityTotal(NodeResources.DiskSpeed.any).vcpu(), null);
-        metric.set("hostedVespa.docker.freeCapacityMem",
-                capacity.getFreeCapacityTotal(NodeResources.DiskSpeed.any).memoryGb(), null);
-        metric.set("hostedVespa.docker.freeCapacityDisk",
-                capacity.getFreeCapacityTotal(NodeResources.DiskSpeed.any).diskGb(), null);
+    private void updateDockerMetrics(NodeList nodes) {
+        NodeResources totalCapacity = getCapacityTotal(nodes);
+        metric.set("hostedVespa.docker.totalCapacityCpu", totalCapacity.vcpu(), null);
+        metric.set("hostedVespa.docker.totalCapacityMem", totalCapacity.memoryGb(), null);
+        metric.set("hostedVespa.docker.totalCapacityDisk", totalCapacity.diskGb(), null);
+
+        NodeResources totalFreeCapacity = getFreeCapacityTotal(nodes);
+        metric.set("hostedVespa.docker.freeCapacityCpu", totalFreeCapacity.vcpu(), null);
+        metric.set("hostedVespa.docker.freeCapacityMem", totalFreeCapacity.memoryGb(), null);
+        metric.set("hostedVespa.docker.freeCapacityDisk", totalFreeCapacity.diskGb(), null);
+    }
+
+    private static NodeResources getCapacityTotal(NodeList nodes) {
+        return nodes.nodeType(NodeType.host).asList().stream()
+                .map(host -> host.flavor().resources())
+                .map(resources -> resources.withDiskSpeed(any))
+                .reduce(new NodeResources(0, 0, 0, any), NodeResources::add);
+    }
+
+    private static NodeResources getFreeCapacityTotal(NodeList nodes) {
+        return nodes.nodeType(NodeType.host).asList().stream()
+                .map(n -> freeCapacityOf(nodes, n))
+                .map(resources -> resources.withDiskSpeed(any))
+                .reduce(new NodeResources(0, 0, 0, any), NodeResources::add);
+    }
+
+    private static NodeResources freeCapacityOf(NodeList nodes, Node dockerHost) {
+        return nodes.childrenOf(dockerHost).asList().stream()
+                .map(node -> node.flavor().resources().withDiskSpeed(any))
+                .reduce(dockerHost.flavor().resources().withDiskSpeed(any), NodeResources::subtract);
     }
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
index feccfb430e6..bc16eb5fb1a 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
@@ -1,7 +1,6 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.provisioning;
 
-import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.LockedNodeList;
@@ -21,22 +20,10 @@ public class DockerHostCapacity {
 
     private final LockedNodeList allNodes;
 
-    public DockerHostCapacity(LockedNodeList allNodes) {
+    DockerHostCapacity(LockedNodeList allNodes) {
         this.allNodes = Objects.requireNonNull(allNodes, "allNodes must be non-null");
     }
 
-    /**
-     * Compare hosts on free capacity.
-     * Used in prioritizing hosts for allocation in <b>descending</b> order.
-     */
-    int compare(Node hostA, Node hostB) {
-        int result = compare(freeCapacityOf(hostB, false), freeCapacityOf(hostA, false));
-        if (result != 0) return result;
-
-        // If resources are equal we want to assign to the one with the most IPaddresses free
-        return freeIPs(hostB) - freeIPs(hostA);
-    }
-
     int compareWithoutInactive(Node hostA, Node hostB) {
         int result = compare(freeCapacityOf(hostB,  true), freeCapacityOf(hostA, true));
         if (result != 0) return result;
@@ -64,57 +51,6 @@ public class DockerHostCapacity {
         return dockerHost.ipAddressPool().findUnused(allNodes).size();
     }
 
-    /** Return total free capacity for a given disk speed (or for any disk speed) */
-    public NodeResources getFreeCapacityTotal(NodeResources.DiskSpeed speed) {
-        return allNodes.asList().stream()
-                       .filter(n -> n.type().equals(NodeType.host))
-                       .filter(n -> speed == NodeResources.DiskSpeed.any || n.flavor().resources().diskSpeed() == speed)
-                       .map(n -> freeCapacityOf(n, false))
-                       .map(resources -> resources.withDiskSpeed(speed)) // Set speed to 'any' if necessary
-                       .reduce(new NodeResources(0, 0, 0, speed), NodeResources::add);
-    }
-
-    /** Return total capacity for a given disk speed (or for any disk speed) */
-    public NodeResources getCapacityTotal(NodeResources.DiskSpeed speed) {
-        return allNodes.asList().stream()
-                       .filter(n -> n.type().equals(NodeType.host))
-                       .filter(n -> speed == NodeResources.DiskSpeed.any || n.flavor().resources().diskSpeed() == speed)
-                       .map(host -> host.flavor().resources())
-                       .map(resources -> resources.withDiskSpeed(speed)) // Set speed to 'any' if necessary
-                       .reduce(new NodeResources(0, 0, 0, speed), NodeResources::add);
-    }
-
-    public int freeCapacityInFlavorEquivalence(Flavor flavor) {
-        return allNodes.asList().stream()
-                       .filter(n -> n.type().equals(NodeType.host))
-                       .map(n -> canFitNumberOf(n, flavor))
-                       .reduce(0, (a, b) -> a + b);
-    }
-
-    public long getNofHostsAvailableFor(Flavor flavor) {
-        return allNodes.asList().stream()
-                       .filter(n -> n.type().equals(NodeType.host))
-                       .filter(n -> hasCapacity(n, flavor.resources()))
-                       .count();
-    }
-
-    private int canFitNumberOf(Node node, Flavor flavor) {
-        NodeResources freeCapacity = freeCapacityOf(node, false);
-        int capacityFactor = freeCapacityInFlavorEquivalence(freeCapacity, flavor);
-        int ips = freeIPs(node);
-        return Math.min(capacityFactor, ips);
-    }
-
-    int freeCapacityInFlavorEquivalence(NodeResources freeCapacity, Flavor flavor) {
-        if ( ! freeCapacity.satisfies(flavor.resources())) return 0;
-
-        double cpuFactor = Math.floor(freeCapacity.vcpu() / flavor.getMinCpuCores());
-        double memoryFactor = Math.floor(freeCapacity.memoryGb() / flavor.getMinMainMemoryAvailableGb());
-        double diskFactor =  Math.floor(freeCapacity.diskGb() / flavor.getMinDiskAvailableGb());
-
-        return (int) Math.min(Math.min(memoryFactor, cpuFactor), diskFactor);
-    }
-
     /**
      * Calculate the remaining capacity for the dockerHost.
      *
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
index d5d4bcc954d..58d306adc75 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeFlavors;
-import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.LockedNodeList;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -11,7 +10,6 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Optional;
@@ -39,38 +37,27 @@ public class DockerHostCapacityTest {
         flavorDocker2 = nodeFlavors.getFlavorOrThrow("docker2");
 
         // Create three docker hosts
-        host1 = Node.create("host1", Collections.singleton("::1"), generateIPs(2, 4), "host1", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), NodeType.host);
-        host2 = Node.create("host2", Collections.singleton("::11"), generateIPs(12, 3), "host2", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), NodeType.host);
-        host3 = Node.create("host3", Collections.singleton("::21"), generateIPs(22, 1), "host3", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), NodeType.host);
+        host1 = Node.create("host1", Set.of("::1"), generateIPs(2, 4), "host1", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), NodeType.host);
+        host2 = Node.create("host2", Set.of("::11"), generateIPs(12, 3), "host2", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), NodeType.host);
+        host3 = Node.create("host3", Set.of("::21"), generateIPs(22, 1), "host3", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), NodeType.host);
 
         // Add two containers to host1
-        var nodeA = Node.create("nodeA", Collections.singleton("::2"), Collections.emptySet(), "nodeA", Optional.of("host1"), Optional.empty(), flavorDocker, NodeType.tenant);
-        var nodeB = Node.create("nodeB", Collections.singleton("::3"), Collections.emptySet(), "nodeB", Optional.of("host1"), Optional.empty(), flavorDocker, NodeType.tenant);
+        var nodeA = Node.create("nodeA", Set.of("::2"), Set.of(), "nodeA", Optional.of("host1"), Optional.empty(), flavorDocker, NodeType.tenant);
+        var nodeB = Node.create("nodeB", Set.of("::3"), Set.of(), "nodeB", Optional.of("host1"), Optional.empty(), flavorDocker, NodeType.tenant);
 
         // Add two containers to host 2 (same as host 1)
-        var nodeC = Node.create("nodeC", Collections.singleton("::12"), Collections.emptySet(), "nodeC", Optional.of("host2"), Optional.empty(), flavorDocker, NodeType.tenant);
-        var nodeD = Node.create("nodeD", Collections.singleton("::13"), Collections.emptySet(), "nodeD", Optional.of("host2"), Optional.empty(), flavorDocker, NodeType.tenant);
+        var nodeC = Node.create("nodeC", Set.of("::12"), Set.of(), "nodeC", Optional.of("host2"), Optional.empty(), flavorDocker, NodeType.tenant);
+        var nodeD = Node.create("nodeD", Set.of("::13"), Set.of(), "nodeD", Optional.of("host2"), Optional.empty(), flavorDocker, NodeType.tenant);
 
         // Add a larger container to host3
-        var nodeE = Node.create("nodeE", Collections.singleton("::22"), Collections.emptySet(), "nodeE", Optional.of("host3"), Optional.empty(), flavorDocker2, NodeType.tenant);
+        var nodeE = Node.create("nodeE", Set.of("::22"), Set.of(), "nodeE", Optional.of("host3"), Optional.empty(), flavorDocker2, NodeType.tenant);
 
         // init docker host capacity
-        nodes = new ArrayList<>();
-        Collections.addAll(nodes, host1, host2, host3, nodeA, nodeB, nodeC, nodeD, nodeE);
+        nodes = new ArrayList<>(List.of(host1, host2, host3, nodeA, nodeB, nodeC, nodeD, nodeE));
         capacity = new DockerHostCapacity(new LockedNodeList(nodes, () -> {}));
     }
 
     @Test
-    public void compare_used_to_sort_in_decending_order() {
-        assertEquals(host1, nodes.get(0)); // Make sure it is unsorted here
-
-        Collections.sort(nodes, capacity::compare);
-        assertEquals(host3, nodes.get(0));
-        assertEquals(host1, nodes.get(1));
-        assertEquals(host2, nodes.get(2));
-    }
-
-    @Test
     public void hasCapacity() {
         assertTrue(capacity.hasCapacity(host1, flavorDocker.resources()));
         assertTrue(capacity.hasCapacity(host1, flavorDocker2.resources()));
@@ -80,7 +67,7 @@ public class DockerHostCapacityTest {
         assertFalse(capacity.hasCapacity(host3, flavorDocker2.resources())); // No ip available
 
         // Add a new node to host1 to deplete the memory resource
-        Node nodeF = Node.create("nodeF", Collections.singleton("::6"), Collections.emptySet(),
+        Node nodeF = Node.create("nodeF", Set.of("::6"), Set.of(),
                 "nodeF", Optional.of("host1"), Optional.empty(), flavorDocker, NodeType.tenant);
         nodes.add(nodeF);
         capacity = new DockerHostCapacity(new LockedNodeList(nodes, () -> {}));
@@ -95,34 +82,6 @@ public class DockerHostCapacityTest {
         assertEquals(0, capacity.freeIPs(host3));
     }
 
-    @Test
-    public void getCapacityTotal() {
-        NodeResources total = capacity.getCapacityTotal(NodeResources.DiskSpeed.any);
-        assertEquals(21.0, total.vcpu(), 0.1);
-        assertEquals(30.0, total.memoryGb(), 0.1);
-        assertEquals(36.0, total.diskGb(), 0.1);
-    }
-
-    @Test
-    public void getFreeCapacityTotal() {
-        NodeResources totalFree = capacity.getFreeCapacityTotal(NodeResources.DiskSpeed.any);
-        assertEquals(15.0, totalFree.vcpu(), 0.1);
-        assertEquals(14.0, totalFree.memoryGb(), 0.1);
-        assertEquals(24.0, totalFree.diskGb(), 0.1);
-    }
-
-    @Test
-    public void freeCapacityInFlavorEquivalence() {
-        assertEquals(2, capacity.freeCapacityInFlavorEquivalence(flavorDocker));
-        assertEquals(2, capacity.freeCapacityInFlavorEquivalence(flavorDocker2));
-    }
-
-    @Test
-    public void getNofHostsAvailableFor() {
-        assertEquals(2, capacity.getNofHostsAvailableFor(flavorDocker));
-        assertEquals(2, capacity.getNofHostsAvailableFor(flavorDocker2));
-    }
-
     private Set<String> generateIPs(int start, int count) {
         // Allow 4 containers
         Set<String> ipAddressPool = new LinkedHashSet<>();
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java
index 664430a2de8..50e19e15da5 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java
@@ -19,7 +19,6 @@ import com.yahoo.config.provision.Zone;
 import com.yahoo.config.provisioning.FlavorsConfig;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.curator.transaction.CuratorTransaction;
-import com.yahoo.vespa.hosted.provision.LockedNodeList;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.node.Agent;
@@ -37,7 +36,6 @@ import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
@@ -221,17 +219,13 @@ public class DynamicDockerAllocationTest {
         List<HostSpec> hosts = tester.prepare(application1, clusterSpec, 3, 1, flavor);
         tester.activate(application1, ImmutableSet.copyOf(hosts));
 
-        DockerHostCapacity capacity = new DockerHostCapacity(new LockedNodeList(tester.nodeRepository().getNodes(Node.State.values()), () -> {}));
-        assertThat(capacity.freeCapacityInFlavorEquivalence(new Flavor(flavor)), greaterThan(0));
-
         List<Node> initialSpareCapacity = findSpareCapacity(tester);
         assertThat(initialSpareCapacity.size(), is(2));
 
         try {
             hosts = tester.prepare(application1, clusterSpec, 4, 1, flavor);
             fail("Was able to deploy with 4 nodes, should not be able to use spare capacity");
-        } catch (OutOfCapacityException e) {
-        }
+        } catch (OutOfCapacityException ignored) { }
 
         tester.fail(hosts.get(0));
         hosts = tester.prepare(application1, clusterSpec, 3, 1, flavor);