Prepare nodes before load balancers

In case we're reactivating a inactive load balancer, we need a non-empty set of
nodes when re-configuring (a guard in the `LoadBalancerService` implementation
enforces this).

4 files changed, 28 insertions, 7 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java
index d6ecba7e551..e89a4dc8bf8 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java
@@ -30,8 +30,12 @@ public class LoadBalancerServiceMock implements LoadBalancerService {
 
     @Override
     public LoadBalancerInstance create(ApplicationId application, ClusterSpec.Id cluster, Set<Real> reals) {
-        LoadBalancerId id = new LoadBalancerId(application, cluster);
-        LoadBalancerInstance instance = new LoadBalancerInstance(
+        var id = new LoadBalancerId(application, cluster);
+        var oldInstance = instances.get(id);
+        if (oldInstance != null && !oldInstance.reals().isEmpty() && reals.isEmpty()) {
+            throw new IllegalArgumentException("Refusing to remove all reals from load balancer " + id);
+        }
+        var instance = new LoadBalancerInstance(
                 HostName.from("lb-" + application.toShortString() + "-" + cluster.value()),
                 Optional.of(new DnsZone("zone-id-1")),
                 Collections.singleton(4443),
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
index 31ec964dceb..44ba24174f2 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
@@ -38,8 +38,9 @@ class Preparer {
 
     /** Prepare all required resources for the given application and cluster */
     public List<Node> prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, int wantedGroups) {
+        var nodes = prepareNodes(application, cluster, requestedNodes, wantedGroups);
         prepareLoadBalancer(application, cluster, requestedNodes);
-        return prepareNodes(application, cluster, requestedNodes, wantedGroups);
+        return nodes;
     }
 
     /**
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 6d94e4ab992..0b3c3d209be 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
@@ -51,7 +51,7 @@ public class LoadBalancerProvisionerTest {
                 clusterRequest(ClusterSpec.Type.container, containerCluster1),
                 clusterRequest(ClusterSpec.Type.content, contentCluster));
         assertEquals(1, lbApp1.get().size());
-        assertEquals("Prepare provisions load balancer with 0 reals", Set.of(), lbApp1.get().get(0).instance().reals());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbApp1.get().get(0).instance().reals().size());
         tester.activate(app1, nodes);
         tester.activate(app2, prepare(app2, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs"))));
         assertEquals(1, lbApp2.get().size());
@@ -109,11 +109,12 @@ public class LoadBalancerProvisionerTest {
                                             .collect(Collectors.toList());
         assertEquals(activeContainers, reals);
 
-        // Application is removed and load balancer is deactivated
+        // Application is removed, nodes and load balancer are deactivated
         NestedTransaction removeTransaction = new NestedTransaction();
         tester.provisioner().remove(removeTransaction, app1);
         removeTransaction.commit();
-
+        dirtyNodesOf(app1);
+        assertTrue("No nodes are allocated to " + app1, tester.nodeRepository().getNodes(app1, Node.State.reserved, Node.State.active).isEmpty());
         assertEquals(2, lbApp1.get().size());
         assertTrue("Deactivated load balancers", lbApp1.get().stream().allMatch(lb -> lb.state() == LoadBalancer.State.inactive));
         assertTrue("Load balancers for " + app2 + " remain active", lbApp2.get().stream().allMatch(lb -> lb.state() == LoadBalancer.State.active));
@@ -130,6 +131,10 @@ public class LoadBalancerProvisionerTest {
                                  .orElseThrow());
     }
 
+    private void dirtyNodesOf(ApplicationId application) {
+        tester.nodeRepository().setDirty(tester.nodeRepository().getNodes(application), Agent.system, this.getClass().getSimpleName());
+    }
+
     private Set<HostSpec> prepare(ApplicationId application, ClusterSpec... specs) {
         tester.makeReadyNodes(specs.length * 2, "d-1-1-1");
         Set<HostSpec> allNodes = new LinkedHashSet<>();
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json
index 36d4de598e2..c9a45a9c3da 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json
@@ -17,7 +17,18 @@
       "ports": [
         4443
       ],
-      "reals": [],
+      "reals": [
+        {
+          "hostname": "host1.yahoo.com",
+          "ipAddress": "127.0.1.1",
+          "port": 4080
+        },
+        {
+          "hostname": "host10.yahoo.com",
+          "ipAddress": "127.0.10.1",
+          "port": 4080
+        }
+      ],
       "rotations": [],
       "inactive": false
     },