Expire inactive load balancers after a fixed timeout

4 files changed, 39 insertions, 16 deletions

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 c0bb53ddfe4..7fd50bf0930 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
@@ -3,18 +3,20 @@ package com.yahoo.vespa.hosted.provision.lb;
 
 import com.yahoo.config.provision.ApplicationId;
 
+import java.time.Instant;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 /**
- * A filterable load balancer list.
+ * A filterable load balancer list. This is immutable.
  *
  * @author mpolden
  */
-public class LoadBalancerList {
+public class LoadBalancerList implements Iterable<LoadBalancer> {
 
     private final List<LoadBalancer> loadBalancers;
 
@@ -27,9 +29,14 @@ public class LoadBalancerList {
         return of(loadBalancers.stream().filter(lb -> lb.id().application().equals(application)));
     }
 
-    /** Returns the subset of load balancers that are inactive */
-    public LoadBalancerList inactive() {
-        return of(loadBalancers.stream().filter(lb -> lb.state() == LoadBalancer.State.inactive));
+    /** Returns the subset of load balancers that are in given state */
+    public LoadBalancerList in(LoadBalancer.State state) {
+        return of(loadBalancers.stream().filter(lb -> lb.state() == state));
+    }
+
+    /** Returns the subset of load balancers that last changed before given instant */
+    public LoadBalancerList changedBefore(Instant instant) {
+        return of(loadBalancers.stream().filter(lb -> lb.changedAt().isBefore(instant)));
     }
 
     public List<LoadBalancer> asList() {
@@ -40,4 +47,9 @@ public class LoadBalancerList {
         return new LoadBalancerList(stream.collect(Collectors.toUnmodifiableList()));
     }
 
+    @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 d6b392c4d64..dd59cc72cca 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
@@ -5,7 +5,7 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
-import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
+import com.yahoo.vespa.hosted.provision.lb.LoadBalancer.State;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancerService;
 import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
@@ -26,6 +26,8 @@ import java.util.stream.Collectors;
  */
 public class LoadBalancerExpirer extends Maintainer {
 
+    private static final Duration inactiveExpiry = Duration.ofHours(1);
+
     private final LoadBalancerService service;
     private final CuratorDatabaseClient db;
 
@@ -44,15 +46,20 @@ public class LoadBalancerExpirer extends Maintainer {
         List<LoadBalancerId> failed = new ArrayList<>();
         Exception lastException = null;
         try (Lock lock = db.lockLoadBalancers()) {
-            for (LoadBalancer loadBalancer : nodeRepository().loadBalancers().inactive().asList()) {
-                if (hasNodes(loadBalancer.id().application())) { // Defer removal if there are still nodes allocated to application
+            var now = nodeRepository().clock().instant();
+            var expirationTime = now.minus(inactiveExpiry);
+            var expired = nodeRepository().loadBalancers()
+                                          .in(State.inactive)
+                                          .changedBefore(expirationTime);
+            for (var lb : expired) {
+                if (hasNodes(lb.id().application())) { // Defer removal if there are still nodes allocated to application
                     continue;
                 }
                 try {
-                    service.remove(loadBalancer.id().application(), loadBalancer.id().cluster());
-                    db.removeLoadBalancer(loadBalancer.id());
+                    service.remove(lb.id().application(), lb.id().cluster());
+                    db.removeLoadBalancer(lb.id());
                 } catch (Exception e) {
-                    failed.add(loadBalancer.id());
+                    failed.add(lb.id());
                     lastException = e;
                 }
             }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
index 25549abe9ed..b71c7c7ec81 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
@@ -7,7 +7,6 @@ import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostLivenessTracker;
 import com.yahoo.config.provision.InfraDeployer;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.flags.FlagSource;
@@ -176,7 +175,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
             metricsInterval = Duration.ofMinutes(1);
             infrastructureProvisionInterval = Duration.ofMinutes(1);
             throttlePolicy = NodeFailer.ThrottlePolicy.hosted;
-            loadBalancerExpiry = Duration.ofHours(1);
+            loadBalancerExpiry = Duration.ofMinutes(10);
             reservationExpiry = Duration.ofMinutes(20); // Need to be long enough for deployment to be finished for all config model versions
             hostProvisionerInterval = Duration.ofMinutes(5);
             hostDeprovisionerInterval = Duration.ofMinutes(5);
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 c0ea64d4c68..b18afc9284b 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
@@ -33,7 +33,7 @@ public class LoadBalancerExpirerTest {
     private ProvisioningTester tester = new ProvisioningTester.Builder().build();
 
     @Test
-    public void test_maintain() {
+    public void test_remove_inactive() {
         LoadBalancerExpirer expirer = new LoadBalancerExpirer(tester.nodeRepository(),
                                                               Duration.ofDays(1),
                                                               tester.loadBalancerService());
@@ -57,9 +57,14 @@ public class LoadBalancerExpirerTest {
         // Expirer defers removal while nodes are still allocated to application
         expirer.maintain();
         assertEquals(2, tester.loadBalancerService().instances().size());
-
-        // Expirer removes load balancers once nodes are deallocated
         dirtyNodesOf(app1);
+
+        // Expirer defers removal until expiration time passes
+        expirer.maintain();
+        assertTrue("Inactive load balancer not removed", tester.loadBalancerService().instances().containsKey(lb1));
+
+        // Expirer removes load balancers once expiration time passes
+        tester.clock().advance(Duration.ofHours(1));
         expirer.maintain();
         assertFalse("Inactive load balancer removed", tester.loadBalancerService().instances().containsKey(lb1));