Merge pull request #9887 from vespa-engine/mpolden/never-provision-new-lb-on-activate

Never provision a new LB in activate MERGEOK

3 files changed, 32 insertions, 7 deletions

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 d0dc090bc74..52e7a28acc8 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
@@ -76,7 +76,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         metricsReporter = new MetricsReporter(nodeRepository, metric, orchestrator, serviceMonitor, periodicApplicationMaintainer::pendingDeployments, durationFromEnv("metrics_interval").orElse(defaults.metricsInterval));
         infrastructureProvisioner = new InfrastructureProvisioner(nodeRepository, infraDeployer, durationFromEnv("infrastructure_provision_interval").orElse(defaults.infrastructureProvisionInterval));
         loadBalancerExpirer = provisionServiceProvider.getLoadBalancerService().map(lbService ->
-                new LoadBalancerExpirer(nodeRepository, durationFromEnv("load_balancer_expiry").orElse(defaults.loadBalancerExpiry), lbService));
+                new LoadBalancerExpirer(nodeRepository, durationFromEnv("load_balancer_expirer_interval").orElse(defaults.loadBalancerExpirerInterval), lbService));
         hostProvisionMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner ->
                 new HostProvisionMaintainer(nodeRepository, durationFromEnv("host_provisioner_interval").orElse(defaults.hostProvisionerInterval), hostProvisioner, flagSource));
         hostDeprovisionMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner ->
@@ -143,7 +143,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         private final Duration metricsInterval;
         private final Duration retiredInterval;
         private final Duration infrastructureProvisionInterval;
-        private final Duration loadBalancerExpiry;
+        private final Duration loadBalancerExpirerInterval;
         private final Duration hostProvisionerInterval;
         private final Duration hostDeprovisionerInterval;
 
@@ -161,7 +161,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
             metricsInterval = Duration.ofMinutes(1);
             infrastructureProvisionInterval = Duration.ofMinutes(1);
             throttlePolicy = NodeFailer.ThrottlePolicy.hosted;
-            loadBalancerExpiry = Duration.ofMinutes(10);
+            loadBalancerExpirerInterval = 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/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 ca7ee1b13a1..4e5d483c171 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
@@ -103,10 +103,11 @@ public class LoadBalancerProvisioner {
             try (var loadBalancersLock = db.lockLoadBalancers()) {
                 var id = new LoadBalancerId(application, clusterId);
                 var now = nodeRepository.clock().instant();
-                var instance = create(application, clusterId, allocatedContainers(application, clusterId));
                 var loadBalancer = db.readLoadBalancers().get(id);
+                if (loadBalancer == null && activate) return; // Nothing to activate as this load balancer was never prepared
+
+                var instance = create(application, clusterId, allocatedContainers(application, clusterId));
                 if (loadBalancer == null) {
-                    if (activate) return; // Nothing to activate as this load balancer was never prepared
                     loadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now);
                 } else {
                     var newState = activate ? LoadBalancer.State.active : loadBalancer.state();
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 0b3c3d209be..3cee00f6a84 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
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.NodeType;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
@@ -37,6 +38,8 @@ public class LoadBalancerProvisionerTest {
     private final ApplicationId app1 = ApplicationId.from("tenant1", "application1", "default");
     private final ApplicationId app2 = ApplicationId.from("tenant2", "application2", "default");
 
+    private final ApplicationId infraApp1 = ApplicationId.from("vespa", "tenant-host", "default");
+
     private ProvisioningTester tester = new ProvisioningTester.Builder().build();
 
     @Test
@@ -131,15 +134,36 @@ public class LoadBalancerProvisionerTest {
                                  .orElseThrow());
     }
 
+    @Test
+    public void does_not_provision_load_balancers_for_non_tenant_node_type() {
+        tester.activate(infraApp1, prepare(infraApp1, Capacity.fromRequiredNodeType(NodeType.host),
+                                           clusterRequest(ClusterSpec.Type.container,
+                                                          ClusterSpec.Id.from("tenant-host"))));
+        assertTrue("No load balancer provisioned", tester.loadBalancerService().instances().isEmpty());
+        assertEquals(List.of(), tester.nodeRepository().loadBalancers().owner(infraApp1).asList());
+    }
+
+    @Test
+    public void does_not_provision_load_balancers_for_non_container_cluster() {
+        tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.content,
+                                                           ClusterSpec.Id.from("tenant-host"))));
+        assertTrue("No load balancer provisioned", tester.loadBalancerService().instances().isEmpty());
+        assertEquals(List.of(), tester.nodeRepository().loadBalancers().owner(app1).asList());
+    }
+
     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");
+        return prepare(application, Capacity.fromCount(2, new NodeResources(1, 1, 1), false, true), specs);
+    }
+
+    private Set<HostSpec> prepare(ApplicationId application, Capacity capacity, ClusterSpec... specs) {
+        tester.makeReadyNodes(specs.length * 2, "d-1-1-1", capacity.type());
         Set<HostSpec> allNodes = new LinkedHashSet<>();
         for (ClusterSpec spec : specs) {
-            allNodes.addAll(tester.prepare(application, spec, Capacity.fromCount(2, new NodeResources(1, 1, 1), false, true), 1, false));
+            allNodes.addAll(tester.prepare(application, spec, capacity, 1, false));
         }
         return allNodes;
     }