Prune reals no longer allocated to application

5 files changed, 59 insertions, 9 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 f86c05da3c2..682d1419a5c 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
@@ -193,7 +193,7 @@ public class NodeRepository extends AbstractComponent {
 
     /** Returns a filterable list of all load balancers in this repository */
     public LoadBalancerList loadBalancers() {
-        return new LoadBalancerList(database().readLoadBalancers().values());
+        return LoadBalancerList.copyOf(database().readLoadBalancers().values());
     }
 
     public List<Node> getNodes(ApplicationId id, Node.State ... inState) { return db.getNodes(id, inState); }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java
index c1b4fe9b0f2..1e60b689489 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java
@@ -56,6 +56,11 @@ public class LoadBalancerInstance {
         return reals;
     }
 
+    /** Returns a copy of this with reals set to given reals */
+    public LoadBalancerInstance withReals(Set<Real> reals) {
+        return new LoadBalancerInstance(hostname, dnsZone, ports, networks, reals);
+    }
+
     private static Set<Integer> requirePorts(Set<Integer> ports) {
         Objects.requireNonNull(ports, "ports must be non-null");
         if (ports.isEmpty()) {
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
index 7fd50bf0930..014d3df8d9a 100644
--- 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
@@ -20,7 +20,7 @@ public class LoadBalancerList implements Iterable<LoadBalancer> {
 
     private final List<LoadBalancer> loadBalancers;
 
-    public LoadBalancerList(Collection<LoadBalancer> loadBalancers) {
+    private LoadBalancerList(Collection<LoadBalancer> loadBalancers) {
         this.loadBalancers = List.copyOf(Objects.requireNonNull(loadBalancers, "loadBalancers must be non-null"));
     }
 
@@ -47,6 +47,10 @@ public class LoadBalancerList implements Iterable<LoadBalancer> {
         return new LoadBalancerList(stream.collect(Collectors.toUnmodifiableList()));
     }
 
+    public static LoadBalancerList copyOf(Collection<LoadBalancer> loadBalancers) {
+        return new LoadBalancerList(loadBalancers);
+    }
+
     @Override
     public Iterator<LoadBalancer> iterator() {
         return loadBalancers.iterator();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
index a8268fe7237..bcb0c901f14 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.maintenance;
 
 import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer.State;
@@ -11,6 +12,7 @@ import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
 
 import java.time.Duration;
 import java.util.ArrayList;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
@@ -44,8 +46,10 @@ public class LoadBalancerExpirer extends Maintainer {
     protected void maintain() {
         expireReserved();
         removeInactive();
+        pruneReals();
     }
 
+    /** Move reserved load balancer that have expired to inactive */
     private void expireReserved() {
         try (var lock = db.lockLoadBalancers()) {
             var now = nodeRepository().clock().instant();
@@ -57,8 +61,9 @@ public class LoadBalancerExpirer extends Maintainer {
         }
     }
 
+    /** Deprovision inactive load balancers that have expired */
     private void removeInactive() {
-        List<LoadBalancerId> failed = new ArrayList<>();
+        var failed = new ArrayList<LoadBalancerId>();
         Exception lastException = null;
         try (var lock = db.lockLoadBalancers()) {
             var now = nodeRepository().clock().instant();
@@ -67,9 +72,7 @@ public class LoadBalancerExpirer extends Maintainer {
                                           .in(State.inactive)
                                           .changedBefore(expirationTime);
             for (var lb : expired) {
-                if (hasNodes(lb.id())) { // Defer removal if there are still nodes allocated
-                    continue;
-                }
+                if (!allocatedNodes(lb.id()).isEmpty()) continue; // Defer removal if there are still nodes allocated to application
                 try {
                     service.remove(lb.id().application(), lb.id().cluster());
                     db.removeLoadBalancer(lb.id());
@@ -90,8 +93,39 @@ public class LoadBalancerExpirer extends Maintainer {
         }
     }
 
-    private boolean hasNodes(LoadBalancerId loadBalancer) {
-        return nodeRepository().list().owner(loadBalancer.application()).cluster(loadBalancer.cluster()).size() > 0;
+    /** Remove reals from inactive load balancers */
+    private void pruneReals() {
+        var failed = new ArrayList<LoadBalancerId>();
+        Exception lastException = null;
+        try (var lock = db.lockLoadBalancers()) {
+            var deactivated = nodeRepository().loadBalancers().in(State.inactive);
+            for (var lb : deactivated) {
+                var allocatedNodes = allocatedNodes(lb.id()).stream().map(Node::hostname).collect(Collectors.toSet());
+                var reals = new LinkedHashSet<>(lb.instance().reals());
+                // Remove any real no longer allocated to this application
+                reals.removeIf(real -> !allocatedNodes.contains(real.hostname().value()));
+                try {
+                    service.create(lb.id().application(), lb.id().cluster(), reals, true);
+                    db.writeLoadBalancer(lb.with(lb.instance().withReals(reals)));
+                } catch (Exception e) {
+                    failed.add(lb.id());
+                    lastException = e;
+                }
+            }
+        }
+        if (!failed.isEmpty()) {
+            log.log(LogLevel.WARNING, String.format("Failed to remove reals from %d load balancers: %s, retrying in %s",
+                                                    failed.size(),
+                                                    failed.stream()
+                                                          .map(LoadBalancerId::serializedForm)
+                                                          .collect(Collectors.joining(", ")),
+                                                    interval()),
+                    lastException);
+        }
+    }
+
+    private List<Node> allocatedNodes(LoadBalancerId loadBalancer) {
+        return nodeRepository().list().owner(loadBalancer.application()).cluster(loadBalancer.cluster()).asList();
     }
 
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java
index 75224021464..12b48fd7a35 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java
@@ -17,6 +17,7 @@ import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -60,9 +61,15 @@ public class LoadBalancerExpirerTest {
         // Expirer defers removal while nodes are still allocated to application
         expirer.maintain();
         assertEquals(3, tester.loadBalancerService().instances().size());
+        assertEquals(2, tester.loadBalancerService().instances().get(lb1).reals().size());
         dirtyNodesOf(app1, cluster1);
 
-        // Expirer defers removal until expiration time passes
+        // Expirer prunes reals before expiration time of load balancer itself
+        expirer.maintain();
+        assertEquals(Set.of(), tester.loadBalancerService().instances().get(lb1).reals());
+        assertEquals(Set.of(), tester.nodeRepository().loadBalancers().owner(lb1.application()).asList().get(0).instance().reals());
+
+        // Expirer defers removal of load balancer until expiration time passes
         expirer.maintain();
         assertTrue("Inactive load balancer not removed", tester.loadBalancerService().instances().containsKey(lb1));