Expire reserved load balancers

2 files changed, 64 insertions, 4 deletions

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 dd59cc72cca..7d7f8d479fe 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,6 +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;
@@ -17,15 +18,19 @@ import java.util.Objects;
 import java.util.stream.Collectors;
 
 /**
- * Periodically remove inactive load balancers permanently.
+ * Periodically expire load balancers.
  *
- * When an application is removed, any associated load balancers are only deactivated. This maintainer ensures that
- * underlying load balancer instances are eventually freed.
+ * Load balancers expire from the following states:
+ *
+ * {@link LoadBalancer.State#inactive}: An application is removed and load balancers are deactivated.
+ * {@link LoadBalancer.State#reserved}: An prepared application is never successfully activated, thus never activating
+ *                                      any prepared load balancers.
  *
  * @author mpolden
  */
 public class LoadBalancerExpirer extends Maintainer {
 
+    private static final Duration reservedExpiry = Duration.ofHours(1);
     private static final Duration inactiveExpiry = Duration.ofHours(1);
 
     private final LoadBalancerService service;
@@ -39,9 +44,21 @@ public class LoadBalancerExpirer extends Maintainer {
 
     @Override
     protected void maintain() {
+        expireReserved();
         removeInactive();
     }
 
+    private void expireReserved() {
+        try (Lock lock = db.lockLoadBalancers()) {
+            var now = nodeRepository().clock().instant();
+            var expirationTime = now.minus(reservedExpiry);
+            var expired = nodeRepository().loadBalancers()
+                                          .in(State.reserved)
+                                          .changedBefore(expirationTime);
+            expired.forEach(lb -> db.writeLoadBalancer(lb.with(State.inactive, now)));
+        }
+    }
+
     private void removeInactive() {
         List<LoadBalancerId> failed = new ArrayList<>();
         Exception lastException = null;
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 b18afc9284b..33faf1eaf76 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
@@ -73,6 +73,43 @@ public class LoadBalancerExpirerTest {
         assertTrue("Active load balancer is not removed", tester.loadBalancerService().instances().containsKey(lb2));
     }
 
+    @Test
+    public void test_expire_reserved() {
+        LoadBalancerExpirer expirer = new LoadBalancerExpirer(tester.nodeRepository(),
+                                                              Duration.ofDays(1),
+                                                              tester.loadBalancerService());
+        Supplier<Map<LoadBalancerId, LoadBalancer>> loadBalancers = () -> tester.nodeRepository().database().readLoadBalancers();
+
+
+        // Prepare application
+        ClusterSpec.Id cluster = ClusterSpec.Id.from("qrs");
+        ApplicationId app = tester.makeApplicationId();
+        LoadBalancerId lb = new LoadBalancerId(app, cluster);
+        deployApplication(app, cluster, false);
+
+        // Provisions load balancer in reserved
+        assertSame(LoadBalancer.State.reserved, loadBalancers.get().get(lb).state());
+
+        // Expirer does nothing
+        expirer.maintain();
+        assertSame(LoadBalancer.State.reserved, loadBalancers.get().get(lb).state());
+
+        // Application never activates and nodes are dirtied. Expirer moves load balancer to inactive after timeout
+        dirtyNodesOf(app);
+        tester.clock().advance(Duration.ofHours(1));
+        expirer.maintain();
+        assertSame(LoadBalancer.State.inactive, loadBalancers.get().get(lb).state());
+
+        // Expirer does nothing as inactive expiration time has not yet passed
+        expirer.maintain();
+        assertSame(LoadBalancer.State.inactive, loadBalancers.get().get(lb).state());
+
+        // Expirer removes inactive load balancer
+        tester.clock().advance(Duration.ofHours(1));
+        expirer.maintain();
+        assertFalse("Inactive load balancer removed", loadBalancers.get().containsKey(lb));
+    }
+
     private void dirtyNodesOf(ApplicationId application) {
         tester.nodeRepository().setDirty(tester.nodeRepository().getNodes(application), Agent.system, this.getClass().getSimpleName());
     }
@@ -84,12 +121,18 @@ public class LoadBalancerExpirerTest {
     }
 
     private void deployApplication(ApplicationId application, ClusterSpec.Id cluster) {
+        deployApplication(application, cluster, true);
+    }
+
+    private void deployApplication(ApplicationId application, ClusterSpec.Id cluster, boolean activate) {
         tester.makeReadyNodes(10, "d-1-1-1");
         List<HostSpec> hosts = tester.prepare(application, ClusterSpec.request(ClusterSpec.Type.container, cluster,
                                                                                Vtag.currentVersion, false, Collections.emptySet()),
                                               2, 1,
                                               new NodeResources(1, 1, 1));
-        tester.activate(application, hosts);
+        if (activate) {
+            tester.activate(application, hosts);
+        }
     }
 
 }