Extract filterable load balancer list

6 files changed, 69 insertions, 40 deletions

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 c870d7d830c..a3542166e72 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
@@ -16,6 +16,7 @@ import com.yahoo.transaction.Mutex;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.hosted.provision.flag.Flags;
+import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList;
 import com.yahoo.vespa.hosted.provision.maintenance.PeriodicApplicationMaintainer;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.NodeAcl;
@@ -173,6 +174,11 @@ public class NodeRepository extends AbstractComponent {
         return new NodeList(getNodes());
     }
 
+    /** Returns a filterable list of all load balancers in this repository */
+    public LoadBalancerList loadBalancers() {
+        return new LoadBalancerList(database().readLoadBalancers().values());
+    }
+
     public List<Node> getNodes(ApplicationId id, Node.State ... inState) { return db.getNodes(id, inState); }
     public List<Node> getInactive() { return db.getNodes(Node.State.inactive); }
     public List<Node> getFailed() { return db.getNodes(Node.State.failed); }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java
new file mode 100644
index 00000000000..dae677d1eaf
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java
@@ -0,0 +1,38 @@
+// 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.lb;
+
+import com.google.common.collect.ImmutableList;
+import com.yahoo.config.provision.ApplicationId;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+import static java.util.stream.Collectors.collectingAndThen;
+
+/**
+ * A filterable load balancer list
+ *
+ * @author mpolden
+ */
+public class LoadBalancerList {
+
+    private final List<LoadBalancer> loadBalancers;
+
+    public LoadBalancerList(Collection<LoadBalancer> loadBalancers) {
+        this.loadBalancers = ImmutableList.copyOf(Objects.requireNonNull(loadBalancers, "loadBalancers must be non-null"));
+    }
+
+    /** Returns the subset of load balancers owned by given application */
+    public LoadBalancerList owner(ApplicationId application) {
+        return loadBalancers.stream()
+                            .filter(lb -> lb.id().application().equals(application))
+                            .collect(collectingAndThen(Collectors.toList(), LoadBalancerList::new));
+    }
+
+    public List<LoadBalancer> asList() {
+        return loadBalancers;
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index 139f42b0f37..cddf668fd34 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -25,7 +25,6 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.Status;
 
-import java.nio.charset.StandardCharsets;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
@@ -456,12 +455,6 @@ public class CuratorDatabaseClient {
                                                          Collections::unmodifiableMap));
     }
 
-    public List<LoadBalancer> readLoadBalancers(ApplicationId application) {
-        return readLoadBalancers().values().stream()
-                                  .filter(lb -> lb.id().application().equals(application))
-                                  .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
-    }
-
     public Optional<LoadBalancer> readLoadBalancer(LoadBalancerId id) {
         return read(loadBalancerPath(id), LoadBalancerSerializer::fromJson);
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
index 27ab25be3d5..e486f76c727 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
@@ -42,11 +42,6 @@ public class LoadBalancerProvisioner {
         this.service = service;
     }
 
-    /** Get load balancers assigned to given application */
-    public List<LoadBalancer> get(ApplicationId application) {
-        return db.readLoadBalancers(application);
-    }
-
     /**
      * Provision load balancer(s) for given application.
      *
@@ -73,9 +68,9 @@ public class LoadBalancerProvisioner {
     public void deactivate(ApplicationId application, NestedTransaction transaction) {
         try (Mutex applicationLock = nodeRepository.lock(application)) {
             try (Mutex loadBalancersLock = db.lockLoadBalancers()) {
-                List<LoadBalancer> deactivatedLoadBalancers = db.readLoadBalancers(application).stream()
-                                                                .map(LoadBalancer::deactivate)
-                                                                .collect(Collectors.toList());
+                List<LoadBalancer> deactivatedLoadBalancers = nodeRepository.loadBalancers().owner(application).asList().stream()
+                                                                            .map(LoadBalancer::deactivate)
+                                                                            .collect(Collectors.toList());
                 db.writeLoadBalancers(deactivatedLoadBalancers, transaction);
             }
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java
index 04a1cdaeeda..e3d0eb10a2c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java
@@ -9,11 +9,11 @@ import com.yahoo.slime.JsonFormat;
 import com.yahoo.slime.Slime;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
+import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList;
 import com.yahoo.vespa.hosted.provision.node.filter.ApplicationFilter;
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 
@@ -32,12 +32,15 @@ public class LoadBalancersResponse extends HttpResponse {
     }
 
     private Optional<ApplicationId> application() {
-        return Optional.ofNullable(request.getProperty("application")).map(ApplicationFilter::toApplicationId);
+        return Optional.ofNullable(request.getProperty("application"))
+                       .map(ApplicationFilter::toApplicationId);
     }
 
     private List<LoadBalancer> loadBalancers() {
-        return application().map(nodeRepository.database()::readLoadBalancers)
-                            .orElseGet(() -> new ArrayList<>(nodeRepository.database().readLoadBalancers().values()));
+        LoadBalancerList loadBalancers = nodeRepository.loadBalancers();
+        return application().map(loadBalancers::owner)
+                            .map(LoadBalancerList::asList)
+                            .orElseGet(loadBalancers::asList);
     }
 
     @Override
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
index ed7b4292eb3..da99188b113 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java
@@ -12,7 +12,6 @@ import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.flag.FlagId;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
-import com.yahoo.vespa.hosted.provision.lb.LoadBalancerService;
 import com.yahoo.vespa.hosted.provision.lb.Real;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import org.junit.Before;
@@ -22,6 +21,7 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
@@ -36,14 +36,10 @@ public class LoadBalancerProvisionerTest {
     private final ApplicationId app2 = ApplicationId.from("tenant2", "application2", "default");
 
     private ProvisioningTester tester;
-    private LoadBalancerService service;
-    private LoadBalancerProvisioner loadBalancerProvisioner;
 
     @Before
     public void before() {
         tester = new ProvisioningTester(Zone.defaultZone());
-        service = tester.loadBalancerService();
-        loadBalancerProvisioner = new LoadBalancerProvisioner(tester.nodeRepository(), service);
     }
 
     @Test
@@ -58,16 +54,16 @@ public class LoadBalancerProvisionerTest {
                                       clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs"))));
 
         // Provision a load balancer for each application
-        List<LoadBalancer> loadBalancers = loadBalancerProvisioner.get(app1);
-        assertEquals(1, loadBalancers.size());
+        Supplier<List<LoadBalancer>> loadBalancers = () -> tester.nodeRepository().loadBalancers().owner(app1).asList();
+        assertEquals(1, loadBalancers.get().size());
 
-        assertEquals(app1, loadBalancers.get(0).id().application());
-        assertEquals(containerCluster1, loadBalancers.get(0).id().cluster());
-        assertEquals(Collections.singleton(4443), loadBalancers.get(0).ports());
-        assertEquals("127.0.0.1", get(loadBalancers.get(0).reals(), 0).ipAddress());
-        assertEquals(4080, get(loadBalancers.get(0).reals(), 0).port());
-        assertEquals("127.0.0.2", get(loadBalancers.get(0).reals(), 1).ipAddress());
-        assertEquals(4080, get(loadBalancers.get(0).reals(), 1).port());
+        assertEquals(app1, loadBalancers.get().get(0).id().application());
+        assertEquals(containerCluster1, loadBalancers.get().get(0).id().cluster());
+        assertEquals(Collections.singleton(4443), loadBalancers.get().get(0).ports());
+        assertEquals("127.0.0.1", get(loadBalancers.get().get(0).reals(), 0).ipAddress());
+        assertEquals(4080, get(loadBalancers.get().get(0).reals(), 0).port());
+        assertEquals("127.0.0.2", get(loadBalancers.get().get(0).reals(), 1).ipAddress());
+        assertEquals(4080, get(loadBalancers.get().get(0).reals(), 1).port());
 
         // A container is failed
         List<Node> containers = tester.getNodes(app1).type(ClusterSpec.Type.container).asList();
@@ -80,7 +76,7 @@ public class LoadBalancerProvisionerTest {
 
         // Redeploy removed replaced failed node in load balancer
         containers = tester.getNodes(app1).type(ClusterSpec.Type.container).asList();
-        LoadBalancer loadBalancer = loadBalancerProvisioner.get(app1).get(0);
+        LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers().owner(app1).asList().get(0);
         assertEquals(2, loadBalancer.reals().size());
         assertEquals(containers.get(0).hostname(), get(loadBalancer.reals(), 0).hostname().value());
         assertEquals(containers.get(1).hostname(), get(loadBalancer.reals(), 1).hostname().value());
@@ -93,8 +89,7 @@ public class LoadBalancerProvisionerTest {
                                       clusterRequest(ClusterSpec.Type.content, contentCluster)));
 
         // Load balancer is provisioned for second container cluster
-        loadBalancers = loadBalancerProvisioner.get(app1);
-        assertEquals(2, loadBalancers.size());
+        assertEquals(2, loadBalancers.get().size());
         List<HostName> activeContainers = tester.getNodes(app1, Node.State.active)
                                                 .type(ClusterSpec.Type.container).asList()
                                                 .stream()
@@ -102,7 +97,7 @@ public class LoadBalancerProvisionerTest {
                                                 .map(HostName::from)
                                                 .sorted()
                                                 .collect(Collectors.toList());
-        List<HostName> reals = loadBalancers.stream()
+        List<HostName> reals = loadBalancers.get().stream()
                                             .flatMap(lb -> lb.reals().stream())
                                             .map(Real::hostname)
                                             .sorted()
@@ -114,9 +109,8 @@ public class LoadBalancerProvisionerTest {
         tester.provisioner().remove(removeTransaction, app1);
         removeTransaction.commit();
 
-        List<LoadBalancer> assignedLoadBalancer = tester.nodeRepository().database().readLoadBalancers(app1);
-        assertEquals(2, loadBalancers.size());
-        assertTrue("Deactivated load balancers", assignedLoadBalancer.stream().allMatch(LoadBalancer::inactive));
+        assertEquals(2, loadBalancers.get().size());
+        assertTrue("Deactivated load balancers", loadBalancers.get().stream().allMatch(LoadBalancer::inactive));
     }
 
     private ClusterSpec clusterRequest(ClusterSpec.Type type, ClusterSpec.Id id) {