Merge pull request #17611 from vespa-engine/jonmv/reapply-apache-client

Jonmv/reapply apache client

10 files changed, 132 insertions, 72 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java
index 6f7b7c4d57d..a6bb57bef29 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java
@@ -5,6 +5,7 @@ import com.yahoo.vespa.hosted.provision.maintenance.LoadBalancerExpirer;
 
 import java.time.Instant;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Represents a load balancer for an application's cluster. This is immutable.
@@ -14,15 +15,18 @@ import java.util.Objects;
 public class LoadBalancer {
 
     private final LoadBalancerId id;
-    private final LoadBalancerInstance instance;
+    private final Optional<LoadBalancerInstance> instance;
     private final State state;
     private final Instant changedAt;
 
-    public LoadBalancer(LoadBalancerId id, LoadBalancerInstance instance, State state, Instant changedAt) {
+    public LoadBalancer(LoadBalancerId id, Optional<LoadBalancerInstance> instance, State state, Instant changedAt) {
         this.id = Objects.requireNonNull(id, "id must be non-null");
         this.instance = Objects.requireNonNull(instance, "instance must be non-null");
         this.state = Objects.requireNonNull(state, "state must be non-null");
         this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null");
+        if (state == State.active && instance.isEmpty()) {
+            throw new IllegalArgumentException("Load balancer instance is required in state " + state);
+        }
     }
 
     /** An identifier for this load balancer. The ID is unique inside the zone */
@@ -31,7 +35,7 @@ public class LoadBalancer {
     }
 
     /** The instance associated with this */
-    public LoadBalancerInstance instance() {
+    public Optional<LoadBalancerInstance> instance() {
         return instance;
     }
 
@@ -58,7 +62,7 @@ public class LoadBalancer {
     }
 
     /** Returns a copy of this with instance set to given instance */
-    public LoadBalancer with(LoadBalancerInstance instance) {
+    public LoadBalancer with(Optional<LoadBalancerInstance> instance) {
         return new LoadBalancer(id, instance, state, changedAt);
     }
 
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 2ef12177eaf..9665d8872de 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
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancerSpec;
 import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
 
 import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -56,10 +57,10 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
 
     /** Move reserved load balancer that have expired to inactive */
     private void expireReserved() {
-        var expiry = nodeRepository().clock().instant().minus(reservedExpiry);
-        patchLoadBalancers(lb -> lb.state() == State.reserved &&
-                                 lb.changedAt().isBefore(expiry),
-                           lb -> db.writeLoadBalancer(lb.with(State.inactive, nodeRepository().clock().instant())));
+        Instant now = nodeRepository().clock().instant();
+        Instant expiry = now.minus(reservedExpiry);
+        patchLoadBalancers(lb -> lb.state() == State.reserved && lb.changedAt().isBefore(expiry),
+                           lb -> db.writeLoadBalancer(lb.with(State.inactive, now)));
     }
 
     /** Deprovision inactive load balancers that have expired */
@@ -95,13 +96,14 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
         var failed = new ArrayList<LoadBalancerId>();
         var lastException = new AtomicReference<Exception>();
         patchLoadBalancers(lb -> lb.state() == State.inactive, lb -> {
+            if (lb.instance().isEmpty()) return;
             var allocatedNodes = allocatedNodes(lb.id()).stream().map(Node::hostname).collect(Collectors.toSet());
-            var reals = new LinkedHashSet<>(lb.instance().reals());
+            var reals = new LinkedHashSet<>(lb.instance().get().reals());
             // Remove any real no longer allocated to this application
             reals.removeIf(real -> !allocatedNodes.contains(real.hostname().value()));
             try {
                 service.create(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals), true);
-                db.writeLoadBalancer(lb.with(lb.instance().withReals(reals)));
+                db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals))));
             } catch (Exception e) {
                 failed.add(lb.id());
                 lastException.set(e);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java
index dddc535b36a..4cd01167293 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancers;
 import java.util.Comparator;
 import java.util.LinkedHashSet;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -70,6 +71,7 @@ public class NodeAcl {
             loadBalancers.list(allocation.owner()).asList()
                          .stream()
                          .map(LoadBalancer::instance)
+                         .flatMap(Optional::stream)
                          .map(LoadBalancerInstance::networks)
                          .forEach(trustedNetworks::addAll);
         });
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
index 66172521d4c..04c8f012b8b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java
@@ -18,6 +18,7 @@ import java.io.UncheckedIOException;
 import java.time.Instant;
 import java.util.LinkedHashSet;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Function;
 
 /**
@@ -50,21 +51,22 @@ public class LoadBalancerSerializer {
         Cursor root = slime.setObject();
 
         root.setString(idField, loadBalancer.id().serializedForm());
-        root.setString(hostnameField, loadBalancer.instance().hostname().toString());
+        // TODO(mpolden): Stop writing this field for empty instance after 2021-06-01
+        root.setString(hostnameField, loadBalancer.instance().map(instance -> instance.hostname().value()).orElse(""));
         root.setString(stateField, asString(loadBalancer.state()));
         root.setLong(changedAtField, loadBalancer.changedAt().toEpochMilli());
-        loadBalancer.instance().dnsZone().ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id()));
+        loadBalancer.instance().flatMap(LoadBalancerInstance::dnsZone).ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id()));
         Cursor portArray = root.setArray(portsField);
-        loadBalancer.instance().ports().forEach(portArray::addLong);
+        loadBalancer.instance().ifPresent(instance -> instance.ports().forEach(portArray::addLong));
         Cursor networkArray = root.setArray(networksField);
-        loadBalancer.instance().networks().forEach(networkArray::addString);
+        loadBalancer.instance().ifPresent(instance -> instance.networks().forEach(networkArray::addString));
         Cursor realArray = root.setArray(realsField);
-        loadBalancer.instance().reals().forEach(real -> {
+        loadBalancer.instance().ifPresent(instance -> instance.reals().forEach(real -> {
             Cursor realObject = realArray.addObject();
             realObject.setString(hostnameField, real.hostname().value());
             realObject.setString(ipAddressField, real.ipAddress());
             realObject.setLong(portField, real.port());
-        });
+        }));
         try {
             return SlimeUtils.toJsonBytes(slime);
         } catch (IOException e) {
@@ -75,7 +77,7 @@ public class LoadBalancerSerializer {
     public static LoadBalancer fromJson(byte[] data) {
         Cursor object = SlimeUtils.jsonToSlime(data).get();
 
-        var reals = new LinkedHashSet<Real>();
+        Set<Real> reals = new LinkedHashSet<>();
         object.field(realsField).traverse((ArrayTraverser) (i, realObject) -> {
             reals.add(new Real(HostName.from(realObject.field(hostnameField).asString()),
                                realObject.field(ipAddressField).asString(),
@@ -83,20 +85,19 @@ public class LoadBalancerSerializer {
 
         });
 
-        var ports = new LinkedHashSet<Integer>();
+        Set<Integer> ports = new LinkedHashSet<>();
         object.field(portsField).traverse((ArrayTraverser) (i, port) -> ports.add((int) port.asLong()));
 
-        var networks = new LinkedHashSet<String>();
+        Set<String> networks = new LinkedHashSet<>();
         object.field(networksField).traverse((ArrayTraverser) (i, network) -> networks.add(network.asString()));
 
+        Optional<HostName> hostname = optionalString(object.field(hostnameField), Function.identity()).filter(s -> !s.isEmpty()).map(HostName::from);
+        Optional<DnsZone> dnsZone = optionalString(object.field(dnsZoneField), DnsZone::new);
+        Optional<LoadBalancerInstance> instance = hostname.map(h -> new LoadBalancerInstance(h, dnsZone, ports,
+                                                                                             networks, reals));
+
         return new LoadBalancer(LoadBalancerId.fromSerializedForm(object.field(idField).asString()),
-                                new LoadBalancerInstance(
-                                        HostName.from(object.field(hostnameField).asString()),
-                                        optionalString(object.field(dnsZoneField), DnsZone::new),
-                                        ports,
-                                        networks,
-                                        reals
-                                ),
+                                instance,
                                 stateFromString(object.field(stateField).asString()),
                                 Instant.ofEpochMilli(object.field(changedAtField).asLong()));
     }
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 09d19300c59..f53eb189ec1 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
@@ -170,18 +170,35 @@ public class LoadBalancerProvisioner {
         Instant now = nodeRepository.clock().instant();
         Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id);
         if (loadBalancer.isEmpty() && activate) return; // Nothing to activate as this load balancer was never prepared
-        LoadBalancerInstance instance = provisionInstance(id, realsOf(nodes), loadBalancer);
+
+        Set<Real> reals = realsOf(nodes);
+        Optional<LoadBalancerInstance> instance = provisionInstance(id, reals, loadBalancer);
         LoadBalancer newLoadBalancer;
         if (loadBalancer.isEmpty()) {
             newLoadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now);
         } else {
-            var newState = activate ? LoadBalancer.State.active : loadBalancer.get().state();
-            newLoadBalancer = loadBalancer.get().with(instance).with(newState, now);
+            LoadBalancer.State state = activate && instance.isPresent()
+                    ? LoadBalancer.State.active
+                    : loadBalancer.get().state();
+            newLoadBalancer = loadBalancer.get().with(instance).with(state, now);
             if (loadBalancer.get().state() != newLoadBalancer.state()) {
                 log.log(Level.FINE, "Moving " + newLoadBalancer.id() + " to state " + newLoadBalancer.state());
             }
         }
-        db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested());
+
+        if (activate) {
+            db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested());
+        } else {
+            // Always store load balancer so that LoadBalancerExpirer can expire partially provisioned load balancers
+            db.writeLoadBalancer(newLoadBalancer);
+        }
+
+        // Signal that load balancer is not ready yet
+        if (instance.isEmpty()) {
+            throw new LoadBalancerServiceException("Could not (re)configure " + id + ", targeting: " +
+                                                   reals + ". The operation will be retried on next deployment",
+                                                   null);
+        }
     }
 
     private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, NodeList nodes) {
@@ -189,17 +206,18 @@ public class LoadBalancerProvisioner {
     }
 
     /** Provision or reconfigure a load balancer instance, if necessary */
-    private LoadBalancerInstance provisionInstance(LoadBalancerId id, Set<Real> reals,
-                                                   Optional<LoadBalancer> currentLoadBalancer) {
+    private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, Set<Real> reals,
+                                                             Optional<LoadBalancer> currentLoadBalancer) {
         if (hasReals(currentLoadBalancer, reals)) return currentLoadBalancer.get().instance();
         log.log(Level.FINE, "Creating " + id + ", targeting: " + reals);
         try {
-            return service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals),
-                                  allowEmptyReals(currentLoadBalancer));
+            return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals),
+                                              allowEmptyReals(currentLoadBalancer)));
         } catch (Exception e) {
-            throw new LoadBalancerServiceException("Failed to (re)configure " + id + ", targeting: " +
-                                                   reals + ". The operation will be retried on next deployment", e);
+            log.log(Level.WARNING, "Could not (re)configure " + id + ", targeting: " +
+                                   reals + ". The operation will be retried on next deployment", e);
         }
+        return Optional.empty();
     }
 
     /** Returns the nodes allocated to the given load balanced cluster */
@@ -246,7 +264,8 @@ public class LoadBalancerProvisioner {
     /** Returns whether load balancer has given reals */
     private static boolean hasReals(Optional<LoadBalancer> loadBalancer, Set<Real> reals) {
         if (loadBalancer.isEmpty()) return false;
-        return loadBalancer.get().instance().reals().equals(reals);
+        if (loadBalancer.get().instance().isEmpty()) return false;
+        return loadBalancer.get().instance().get().reals().equals(reals);
     }
 
     /** Returns whether to allow given load balancer to have no reals */
@@ -273,5 +292,4 @@ public class LoadBalancerProvisioner {
         return cluster.combinedId().orElse(cluster.id());
     }
 
-
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
index 1ef449555d9..bcd0af4e121 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java
@@ -9,6 +9,7 @@ import com.yahoo.slime.JsonFormat;
 import com.yahoo.slime.Slime;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
+import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList;
 import com.yahoo.vespa.hosted.provision.node.filter.ApplicationFilter;
 
@@ -65,21 +66,23 @@ public class LoadBalancersResponse extends HttpResponse {
             lbObject.setString("tenant", lb.id().application().tenant().value());
             lbObject.setString("instance", lb.id().application().instance().value());
             lbObject.setString("cluster", lb.id().cluster().value());
-            lbObject.setString("hostname", lb.instance().hostname().value());
-            lb.instance().dnsZone().ifPresent(dnsZone -> lbObject.setString("dnsZone", dnsZone.id()));
+            lb.instance().ifPresent(instance -> lbObject.setString("hostname", instance.hostname().value()));
+            lb.instance().flatMap(LoadBalancerInstance::dnsZone).ifPresent(dnsZone -> lbObject.setString("dnsZone", dnsZone.id()));
 
             Cursor networkArray = lbObject.setArray("networks");
-            lb.instance().networks().forEach(networkArray::addString);
+            lb.instance().ifPresent(instance -> instance.networks().forEach(networkArray::addString));
 
             Cursor portArray = lbObject.setArray("ports");
-            lb.instance().ports().forEach(portArray::addLong);
+            lb.instance().ifPresent(instance -> instance.ports().forEach(portArray::addLong));
 
             Cursor realArray = lbObject.setArray("reals");
-            lb.instance().reals().forEach(real -> {
-                Cursor realObject = realArray.addObject();
-                realObject.setString("hostname", real.hostname().value());
-                realObject.setString("ipAddress", real.ipAddress());
-                realObject.setLong("port", real.port());
+            lb.instance().ifPresent(instance -> {
+                instance.reals().forEach(real -> {
+                    Cursor realObject = realArray.addObject();
+                    realObject.setString("hostname", real.hostname().value());
+                    realObject.setString("ipAddress", real.ipAddress());
+                    realObject.setLong("port", real.port());
+                });
             });
         });
 
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 fec6e40fd35..4d19c2c1d41 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
@@ -63,7 +63,7 @@ public class LoadBalancerExpirerTest {
         tester.nodeRepository().nodes().setReady(tester.nodeRepository().nodes().list(Node.State.dirty).asList(), Agent.system, getClass().getSimpleName());
         expirer.maintain();
         assertEquals(Set.of(), tester.loadBalancerService().instances().get(lb1).reals());
-        assertEquals(Set.of(), loadBalancers.get().get(lb1).instance().reals());
+        assertEquals(Set.of(), loadBalancers.get().get(lb1).instance().get().reals());
 
         // Expirer defers removal of load balancer until expiration time passes
         expirer.maintain();
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java
index bcb78844666..51d6f7fb2fb 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java
@@ -30,7 +30,7 @@ public class LoadBalancerSerializerTest {
                                                                                   "application1",
                                                                                   "default"),
                                                                ClusterSpec.Id.from("qrs")),
-                                            new LoadBalancerInstance(
+                                            Optional.of(new LoadBalancerInstance(
                                                     HostName.from("lb-host"),
                                                     Optional.of(new DnsZone("zone-id-1")),
                                                     ImmutableSet.of(4080, 4443),
@@ -40,19 +40,19 @@ public class LoadBalancerSerializerTest {
                                                                              4080),
                                                                     new Real(HostName.from("real-2"),
                                                                              "127.0.0.2",
-                                                                             4080))),
+                                                                             4080)))),
                                             LoadBalancer.State.active,
                                             now);
 
         var serialized = LoadBalancerSerializer.fromJson(LoadBalancerSerializer.toJson(loadBalancer));
         assertEquals(loadBalancer.id(), serialized.id());
-        assertEquals(loadBalancer.instance().hostname(), serialized.instance().hostname());
-        assertEquals(loadBalancer.instance().dnsZone(), serialized.instance().dnsZone());
-        assertEquals(loadBalancer.instance().ports(), serialized.instance().ports());
-        assertEquals(loadBalancer.instance().networks(), serialized.instance().networks());
+        assertEquals(loadBalancer.instance().get().hostname(), serialized.instance().get().hostname());
+        assertEquals(loadBalancer.instance().get().dnsZone(), serialized.instance().get().dnsZone());
+        assertEquals(loadBalancer.instance().get().ports(), serialized.instance().get().ports());
+        assertEquals(loadBalancer.instance().get().networks(), serialized.instance().get().networks());
         assertEquals(loadBalancer.state(), serialized.state());
         assertEquals(loadBalancer.changedAt().truncatedTo(MILLIS), serialized.changedAt());
-        assertEquals(loadBalancer.instance().reals(), serialized.instance().reals());
+        assertEquals(loadBalancer.instance().get().reals(), serialized.instance().get().reals());
     }
 
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java
index b7e671a7b76..744322c9edc 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java
@@ -180,7 +180,7 @@ public class AclProvisioningTest {
         // Load balancer is allocated to application
         var loadBalancers = tester.nodeRepository().loadBalancers().list(application);
         assertEquals(1, loadBalancers.asList().size());
-        var lbNetworks = loadBalancers.asList().get(0).instance().networks();
+        var lbNetworks = loadBalancers.asList().get(0).instance().get().networks();
         assertEquals(2, lbNetworks.size());
 
         // ACL for nodes with allocation trust their respective load balancer networks, if any
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 a3780db4789..eb3bdff484d 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
@@ -10,6 +10,7 @@ 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.config.provision.exception.LoadBalancerServiceException;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
@@ -59,7 +60,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 reserved nodes", 2, lbApp1.get().get(0).instance().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbApp1.get().get(0).instance().get().reals().size());
         tester.activate(app1, nodes);
         tester.activate(app2, prepare(app2, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs"))));
         assertEquals(1, lbApp2.get().size());
@@ -67,11 +68,11 @@ public class LoadBalancerProvisionerTest {
         // Reals are configured after activation
         assertEquals(app1, lbApp1.get().get(0).id().application());
         assertEquals(containerCluster1, lbApp1.get().get(0).id().cluster());
-        assertEquals(Collections.singleton(4443), lbApp1.get().get(0).instance().ports());
-        assertEquals("127.0.0.1", get(lbApp1.get().get(0).instance().reals(), 0).ipAddress());
-        assertEquals(4443, get(lbApp1.get().get(0).instance().reals(), 0).port());
-        assertEquals("127.0.0.2", get(lbApp1.get().get(0).instance().reals(), 1).ipAddress());
-        assertEquals(4443, get(lbApp1.get().get(0).instance().reals(), 1).port());
+        assertEquals(Collections.singleton(4443), lbApp1.get().get(0).instance().get().ports());
+        assertEquals("127.0.0.1", get(lbApp1.get().get(0).instance().get().reals(), 0).ipAddress());
+        assertEquals(4443, get(lbApp1.get().get(0).instance().get().reals(), 0).port());
+        assertEquals("127.0.0.2", get(lbApp1.get().get(0).instance().get().reals(), 1).ipAddress());
+        assertEquals(4443, get(lbApp1.get().get(0).instance().get().reals(), 1).port());
 
         // A container is failed
         Supplier<List<Node>> containers = () -> tester.getNodes(app1).container().asList();
@@ -83,13 +84,13 @@ public class LoadBalancerProvisionerTest {
                                       clusterRequest(ClusterSpec.Type.container, containerCluster1),
                                       clusterRequest(ClusterSpec.Type.content, contentCluster)));
         LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers().list(app1).asList().get(0);
-        assertEquals(2, loadBalancer.instance().reals().size());
-        assertTrue("Failed node is removed", loadBalancer.instance().reals().stream()
+        assertEquals(2, loadBalancer.instance().get().reals().size());
+        assertTrue("Failed node is removed", loadBalancer.instance().get().reals().stream()
                                                          .map(Real::hostname)
                                                          .map(HostName::value)
                                                          .noneMatch(hostname -> hostname.equals(toFail.hostname())));
-        assertEquals(containers.get().get(0).hostname(), get(loadBalancer.instance().reals(), 0).hostname().value());
-        assertEquals(containers.get().get(1).hostname(), get(loadBalancer.instance().reals(), 1).hostname().value());
+        assertEquals(containers.get().get(0).hostname(), get(loadBalancer.instance().get().reals(), 0).hostname().value());
+        assertEquals(containers.get().get(1).hostname(), get(loadBalancer.instance().get().reals(), 1).hostname().value());
         assertSame("State is unchanged", LoadBalancer.State.active, loadBalancer.state());
 
         // Add another container cluster to first app
@@ -110,6 +111,7 @@ public class LoadBalancerProvisionerTest {
                                                 .collect(Collectors.toList());
         List<HostName> reals = lbApp1.get().stream()
                                             .map(LoadBalancer::instance)
+                                            .flatMap(Optional::stream)
                                             .map(LoadBalancerInstance::reals)
                                             .flatMap(Collection::stream)
                                             .map(Real::hostname)
@@ -152,7 +154,7 @@ public class LoadBalancerProvisionerTest {
                          .findFirst()
                          .orElseThrow());
 
-        // Next redeploy does not create a new load balancer instance
+        // Next redeploy does not create a new load balancer instance because reals are unchanged
         tester.loadBalancerService().throwOnCreate(true);
         tester.activate(app1, prepare(app1,
                                       clusterRequest(ClusterSpec.Type.container, containerCluster1),
@@ -208,7 +210,7 @@ public class LoadBalancerProvisionerTest {
         var combinedId = ClusterSpec.Id.from("container1");
         var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, ClusterSpec.Id.from("content1"), Optional.of(combinedId)));
         assertEquals(1, lbs.get().size());
-        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size());
         tester.activate(app1, nodes);
         assertSame(LoadBalancer.State.active, lbs.get().get(0).state());
         assertEquals(combinedId, lbs.get().get(0).id().cluster());
@@ -222,7 +224,7 @@ public class LoadBalancerProvisionerTest {
         var nodes = prepare(configServerApp, Capacity.fromRequiredNodeType(NodeType.config),
                             clusterRequest(ClusterSpec.Type.admin, cluster));
         assertEquals(1, lbs.get().size());
-        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size());
         tester.activate(configServerApp, nodes);
         assertSame(LoadBalancer.State.active, lbs.get().get(0).state());
         assertEquals(cluster, lbs.get().get(0).id().cluster());
@@ -236,7 +238,7 @@ public class LoadBalancerProvisionerTest {
         var nodes = prepare(controllerApp, Capacity.fromRequiredNodeType(NodeType.controller),
                             clusterRequest(ClusterSpec.Type.container, cluster));
         assertEquals(1, lbs.get().size());
-        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size());
         tester.activate(controllerApp, nodes);
         assertSame(LoadBalancer.State.active, lbs.get().get(0).state());
         assertEquals(cluster, lbs.get().get(0).id().cluster());
@@ -253,7 +255,7 @@ public class LoadBalancerProvisionerTest {
         // instance1 is deployed
         tester.activate(instance1, prepare(instance1, clusterRequest(ClusterSpec.Type.container, devCluster)));
 
-        // instance2 clashes because cluster name matches instance1
+        // instance2 clashes because instance name matches instance1 cluster name
         try {
             prepare(instance2, clusterRequest(ClusterSpec.Type.container, defaultCluster));
             fail("Expected exception");
@@ -263,10 +265,38 @@ public class LoadBalancerProvisionerTest {
         // instance2 changes cluster name and does not clash
         tester.activate(instance2, prepare(instance2, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs"))));
 
-        // instance3 clashes because instance name matches instance2 cluster
+        // instance3 does not clash
         tester.activate(instance3, prepare(instance3, clusterRequest(ClusterSpec.Type.container, defaultCluster)));
     }
 
+    @Test
+    public void provisioning_load_balancer_fails_initially() {
+        Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers().list(app1).asList();
+        ClusterSpec.Id cluster = ClusterSpec.Id.from("qrs1");
+
+        // Provisioning load balancer fails on deployment
+        tester.loadBalancerService().throwOnCreate(true);
+        try {
+            prepare(app1, clusterRequest(ClusterSpec.Type.container, cluster));
+            fail("Expected exception");
+        } catch (LoadBalancerServiceException ignored) {}
+        List<LoadBalancer> loadBalancers = lbs.get();
+        assertEquals(1, loadBalancers.size());
+        assertSame(LoadBalancer.State.reserved, loadBalancers.get(0).state());
+        assertTrue("Load balancer has no instance", loadBalancers.get(0).instance().isEmpty());
+
+        // Next deployment succeeds
+        tester.loadBalancerService().throwOnCreate(false);
+        Set<HostSpec> nodes = prepare(app1, clusterRequest(ClusterSpec.Type.container, cluster));
+        loadBalancers = lbs.get();
+        assertSame(LoadBalancer.State.reserved, loadBalancers.get(0).state());
+        assertTrue("Load balancer has instance", loadBalancers.get(0).instance().isPresent());
+        tester.activate(app1, nodes);
+        loadBalancers = lbs.get();
+        assertSame(LoadBalancer.State.active, loadBalancers.get(0).state());
+        assertTrue("Load balancer has instance", loadBalancers.get(0).instance().isPresent());
+    }
+
     private void dirtyNodesOf(ApplicationId application) {
         tester.nodeRepository().nodes().deallocate(tester.nodeRepository().nodes().list().owner(application).asList(), Agent.system, this.getClass().getSimpleName());
     }