Merge pull request #17603 from vespa-engine/jonmv/revert-apache-config-server-client

Jonmv/revert apache config server client

26 files changed, 199 insertions, 339 deletions

diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java
index 2e5e445d63a..d467402f79f 100644
--- a/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java
+++ b/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java
@@ -9,12 +9,11 @@ import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.Method;
-import org.apache.hc.core5.http.ParseException;
-import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.http.io.entity.HttpEntities;
 import org.apache.hc.core5.net.URIBuilder;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -22,6 +21,7 @@ import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -104,7 +104,7 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
     }
 
     @Override
-    public ConfigServerClient.RequestBuilder send(HostStrategy hosts, Method method) {
+    public RequestBuilder send(HostStrategy hosts, Method method) {
         return new RequestBuilder(hosts, method);
     }
 
@@ -114,11 +114,10 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
         private final Method method;
         private final HostStrategy hosts;
         private final URIBuilder uriBuilder = new URIBuilder();
-        private final List<String> pathSegments = new ArrayList<>();
         private HttpEntity entity;
         private RequestConfig config = ConfigServerClient.defaultRequestConfig;
-        private ResponseVerifier verifier = ConfigServerClient.throwOnError;
-        private Consumer<IOException> catcher = ConfigServerClient.retryAll;
+        private BiConsumer<ClassicHttpResponse, ClassicHttpRequest> handler = ConfigServerClient::throwOnError;
+        private Consumer<IOException> catcher = ConfigServerClient::retryAll;
 
         private RequestBuilder(HostStrategy hosts, Method method) {
             if ( ! hosts.iterator().hasNext())
@@ -130,7 +129,7 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
 
         @Override
         public RequestBuilder at(List<String> pathSegments) {
-            this.pathSegments.addAll(pathSegments);
+            uriBuilder.setPathSegments(requireNonNull(pathSegments));
             return this;
         }
 
@@ -146,23 +145,12 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
         }
 
         @Override
-        public ConfigServerClient.RequestBuilder emptyParameters(List<String> keys) {
-            for (String key : keys)
-                uriBuilder.setParameter(key, null);
-
-            return this;
-        }
-
-        @Override
         public RequestBuilder parameters(List<String> pairs) {
             if (pairs.size() % 2 != 0)
                 throw new IllegalArgumentException("Must supply parameter key/values in pairs");
 
-            for (int i = 0; i < pairs.size(); ) {
-                String key = pairs.get(i++), value = pairs.get(i++);
-                if (value != null)
-                    uriBuilder.setParameter(key, value);
-            }
+            for (int i = 0; i < pairs.size(); )
+                uriBuilder.setParameter(pairs.get(i++), pairs.get(i++));
 
             return this;
         }
@@ -187,54 +175,56 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
         }
 
         @Override
-        public RequestBuilder throwing(ResponseVerifier verifier) {
-            this.verifier = requireNonNull(verifier);
+        public RequestBuilder handling(BiConsumer<ClassicHttpResponse, ClassicHttpRequest> handler) {
+            this.handler = requireNonNull(handler);
             return this;
         }
 
         @Override
-        public String read() {
-            return handle((response, __) -> {
-                try (response) {
-                    return response.getEntity() == null ? "" : EntityUtils.toString(response.getEntity());
-                }
-                catch (ParseException e) {
-                    throw new IllegalStateException(e); // This isn't actually thrown by apache >_<
-                }
-            });
-        }
-
-        @Override
         public <T> T read(Function<byte[], T> mapper) {
-            return handle((response, __) -> {
-                try (response) {
-                    return mapper.apply(response.getEntity() == null ? new byte[0] : EntityUtils.toByteArray(response.getEntity()));
+            return mapIfSuccess(input -> {
+                try (input) {
+                    return mapper.apply(input.readAllBytes());
+                }
+                catch (IOException e) {
+                    throw new RetryException(e);
                 }
             });
         }
 
         @Override
         public void discard() throws UncheckedIOException, ResponseException {
-            handle((response, __) -> {
-                try (response) {
+            mapIfSuccess(input -> {
+                try (input) {
                     return null;
                 }
+                catch (IOException e) {
+                    throw new RetryException(e);
+                }
             });
         }
 
         @Override
-        public HttpInputStream stream() throws UncheckedIOException, ResponseException {
-            return handle((response, __) -> new HttpInputStream(response));
+        public InputStream stream() throws UncheckedIOException, ResponseException {
+            return mapIfSuccess(input -> input);
         }
 
-        @Override
-        public <T> T handle(ResponseHandler<T> handler) {
-            uriBuilder.setPathSegments(pathSegments);
+        /** Returns the mapped body, if successful, retrying any IOException. The caller must close the body stream. */
+        private <T> T mapIfSuccess(Function<InputStream, T> mapper) {
             return execute(this,
                            (response, request) -> {
                                try {
-                                   verifier.verify(response, request); // This throws on unacceptable responses.
-                                   return handler.handle(response, request);
+                                   handler.accept(response, request); // This throws on unacceptable responses.
+
+                                   InputStream body = response.getEntity() != null ? response.getEntity().getContent()
+                                                                                   : InputStream.nullInputStream();
+                                   return mapper.apply(new ForwardingInputStream(body) {
+                                       @Override
+                                       public void close() throws IOException {
+                                           super.close();
+                                           response.close();
+                                       }
+                                   });
                                }
                                catch (IOException | RuntimeException | Error e) {
                                    try {
@@ -257,7 +247,6 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
 
     }
 
-
     @SuppressWarnings("unchecked")
     private static <T extends Throwable> void sneakyThrow(Throwable t) throws T {
         throw (T) t;
diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java
index c92acd7cd0b..c0f72378c1b 100644
--- a/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java
+++ b/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java
@@ -7,10 +7,10 @@ import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.Method;
+import org.apache.hc.core5.http.ParseException;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.util.Timeout;
 
-import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
@@ -20,18 +20,18 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.IntStream;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toUnmodifiableList;
 
 /**
  * @author jonmv
  */
-public interface ConfigServerClient extends Closeable {
+public interface ConfigServerClient extends AutoCloseable {
 
     RequestConfig defaultRequestConfig = RequestConfig.custom()
                                                       .setConnectionRequestTimeout(Timeout.ofSeconds(5))
@@ -40,12 +40,20 @@ public interface ConfigServerClient extends Closeable {
                                                       .build();
 
     /** Wraps with a {@link RetryException} and rethrows. */
-    Consumer<IOException> retryAll = (e) -> {
+    static void retryAll(IOException e) {
         throw new RetryException(e);
-    };
+    }
 
     /** Throws a a {@link RetryException} if {@code statusCode == 503}, or a {@link ResponseException} unless {@code 200 <= statusCode < 300}. */
-    ResponseVerifier throwOnError = new DefaultResponseVerifier() { };
+    static void throwOnError(ClassicHttpResponse response, ClassicHttpRequest request) {
+        if (response.getCode() < HttpStatus.SC_OK || response.getCode() >= HttpStatus.SC_REDIRECTION) {
+            ResponseException e = ResponseException.of(response, request);
+            if (response.getCode() == HttpStatus.SC_SERVICE_UNAVAILABLE)
+                throw new RetryException(e);
+
+            throw e;
+        }
+    }
 
     /** Reads the response body, throwing an {@link UncheckedIOException} if this fails, or {@code null} if there is none. */
     static byte[] getBytes(ClassicHttpResponse response) {
@@ -63,10 +71,10 @@ public interface ConfigServerClient extends Closeable {
     /** Builder for a request against a given set of hosts, using this config server client. */
     interface RequestBuilder {
 
-        /** Appends to the request path. */
+        /** Sets the request path. */
         default RequestBuilder at(String... pathSegments) { return at(List.of(pathSegments)); }
 
-        /** Appends to the request path. */
+        /** Sets the request path. */
         RequestBuilder at(List<String> pathSegments);
 
         /** Sets the request body as UTF-8 application/json. */
@@ -75,20 +83,12 @@ public interface ConfigServerClient extends Closeable {
         /** Sets the request body. */
         RequestBuilder body(HttpEntity entity);
 
-        /** Sets query parameters without a value, like {@code ?debug&recursive}. */
-        default RequestBuilder emptyParameters(String... keys) {
-            return emptyParameters(Arrays.asList(keys));
-        }
-
-        /** Sets query parameters without a value, like {@code ?debug&recursive}. */
-        RequestBuilder emptyParameters(List<String> keys);
-
-        /** Sets the parameter key/values for the request. Number of arguments must be even. Null values are omitted. */
+        /** Sets the parameter key/values for the request. Number of arguments must be even. */
         default RequestBuilder parameters(String... pairs) {
             return parameters(Arrays.asList(pairs));
         }
 
-        /** Sets the parameter key/values for the request. Number of arguments must be even. Null values are omitted. */
+        /** Sets the parameter key/values for the request. Number of arguments must be even. */
         RequestBuilder parameters(List<String> pairs);
 
         /** Overrides the default socket read timeout of the request. {@code Duration.ZERO} gives infinite timeout. */
@@ -109,10 +109,7 @@ public interface ConfigServerClient extends Closeable {
          * Sets the (error) response handler for this request. The default is {@link #throwOnError}.
          * When the handler returns normally, the response is treated as a success, and passed on to a response mapper.
          */
-         RequestBuilder throwing(ResponseVerifier handler);
-
-        /** Reads the response as a {@link String}, or throws if unsuccessful. */
-        String read();
+         RequestBuilder handling(BiConsumer<ClassicHttpResponse, ClassicHttpRequest> handler);
 
         /** Reads and maps the response, or throws if unsuccessful. */
         <T> T read(Function<byte[], T> mapper);
@@ -121,88 +118,10 @@ public interface ConfigServerClient extends Closeable {
         void discard();
 
         /** Returns the raw response input stream, or throws if unsuccessful. The caller must close the returned stream. */
-        HttpInputStream stream();
-
-        /** Uses the response and request, if successful, to generate a mapped response. */
-        <T> T handle(ResponseHandler<T> handler);
-
-    }
-
-
-    class HttpInputStream extends ForwardingInputStream {
-
-        private final ClassicHttpResponse response;
-
-        protected HttpInputStream(ClassicHttpResponse response) throws IOException {
-            super(response.getEntity() != null ? response.getEntity().getContent()
-                                               : InputStream.nullInputStream());
-            this.response = response;
-        }
-
-        public int statusCode() { return response.getCode(); }
-
-        public String contentType() { return response.getEntity().getContentType(); }
-
-        @Override
-        public void close() throws IOException {
-            super.close();
-            response.close();
-        }
-
-    }
-
-
-    /** Reads a successful response and request to compute a result. */
-    @FunctionalInterface
-    interface ResponseHandler<T> {
-
-        /** Called with successful responses, as per {@link ResponseVerifier}. The caller must close the response. */
-        T handle(ClassicHttpResponse response, ClassicHttpRequest request) throws IOException;
+        InputStream stream();
 
     }
 
-
-    /** Verifies a response, throwing on error responses, possibly indicating retries. */
-    @FunctionalInterface
-    interface ResponseVerifier {
-
-        /** Whether this status code means the response is an error response. */
-        default boolean isError(int statusCode) {
-            return statusCode < HttpStatus.SC_OK || HttpStatus.SC_REDIRECTION <= statusCode;
-        }
-
-        /** Whether this status code means we should retry. Has no effect if this is not also an error. */
-        default boolean shouldRetry(int statusCode) {
-            return statusCode == HttpStatus.SC_SERVICE_UNAVAILABLE;
-        }
-
-        /** Verifies the given response, consuming it and throwing if it is an error; or leaving it otherwise. */
-        default void verify(ClassicHttpResponse response, ClassicHttpRequest request) throws IOException {
-            if (isError(response.getCode())) {
-                try (response) {
-                    byte[] body = response.getEntity() == null ? new byte[0] : EntityUtils.toByteArray(response.getEntity());
-                    RuntimeException exception = toException(response.getCode(), body, request);
-                    throw shouldRetry(response.getCode()) ? new RetryException(exception) : exception;
-                }
-            }
-        }
-
-        /** Throws the appropriate exception, for the given status code and body. */
-        RuntimeException toException(int statusCode, byte[] body, ClassicHttpRequest request);
-
-    }
-
-
-    interface DefaultResponseVerifier extends ResponseVerifier {
-
-        @Override
-        default RuntimeException toException(int statusCode, byte[] body, ClassicHttpRequest request) {
-            return new ResponseException(request + " failed with status " + statusCode + " and body '" + new String(body, UTF_8) + "'");
-        }
-
-    }
-
-
     /** What host(s) to try for a request, in what order. A host may be specified multiple times, for retries.  */
     @FunctionalInterface
     interface HostStrategy extends Iterable<URI> {
@@ -228,7 +147,6 @@ public interface ConfigServerClient extends Closeable {
 
     }
 
-
     /** Exception wrapper that signals retries should be attempted. */
     final class RetryException extends RuntimeException {
 
@@ -242,12 +160,25 @@ public interface ConfigServerClient extends Closeable {
 
     }
 
-
     /** An exception due to server error, a bad request, or similar, which resulted in a non-OK HTTP response. */
     class ResponseException extends RuntimeException {
 
-        public ResponseException(String message) {
-            super(message);
+        public ResponseException(String message, Throwable cause) {
+            super(message, cause);
+        }
+
+        public static ResponseException of(ClassicHttpResponse response, ClassicHttpRequest request) {
+            String detail;
+            Throwable thrown = null;
+            try {
+                detail = request.getEntity() == null ? " and no body"
+                                                     : " and body '" + EntityUtils.toString(request.getEntity()) + "'";
+            }
+            catch (IOException | ParseException e) {
+                detail = ". Reading body failed";
+                thrown = e;
+            }
+            return new ResponseException(request + " failed with status " + response.getCode() + detail, thrown);
         }
 
     }
diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/HttpConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/HttpConfigServerClient.java
index e00489f0c64..c5b07eceaf5 100644
--- a/configserver-client/src/main/java/ai/vespa/hosted/client/HttpConfigServerClient.java
+++ b/configserver-client/src/main/java/ai/vespa/hosted/client/HttpConfigServerClient.java
@@ -54,6 +54,7 @@ public class HttpConfigServerClient extends AbstractConfigServerClient {
                                              false)
                                      .disableAutomaticRetries()
                                      .setUserAgent(userAgent)
+                                     .setDefaultRequestConfig(defaultRequestConfig)
                                      .build();
     }
 
diff --git a/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java b/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java
index b2f17f43f5c..3b46501a4bf 100644
--- a/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java
+++ b/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java
@@ -70,15 +70,14 @@ class HttpConfigServerClientTest {
         server.resetRequests();
 
         // Successful attempt returns.
-        server.stubFor(get("/root/boot/toot"))
+        server.stubFor(get("/root/boot"))
               .setResponse(okJson("{}").build());
         assertEquals("{}",
                      client.send(HostStrategy.repeating(URI.create("http://localhost:" + server.port()), 10),
                                  Method.GET)
                            .at("root", "boot")
-                           .at("toot")
                            .read(String::new));
-        server.verify(1, getRequestedFor(urlEqualTo("/root/boot/toot")));
+        server.verify(1, getRequestedFor(urlEqualTo("/root/boot")));
         server.verify(1, anyRequestedFor(anyUrl()));
         server.resetRequests();
 
@@ -89,9 +88,10 @@ class HttpConfigServerClientTest {
                                                 () -> client.send(HostStrategy.repeating(URI.create("http://localhost:" + server.port()), 10),
                                                                   Method.GET)
                                                             .read(String::new));
-        assertEquals("GET / failed with status 409 and body 'hi'", thrown.getMessage());
+        assertEquals("GET / failed with status 409 and no body", thrown.getMessage());
         server.verify(1, getRequestedFor(urlEqualTo("/")));
         server.verify(1, anyRequestedFor(anyUrl()));
+
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java
index 5e4d3345b7a..4240b0d9fa6 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java
@@ -1,32 +1,38 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.integration.configserver;
 
-import com.yahoo.slime.Inspector;
-import com.yahoo.slime.SlimeUtils;
-import org.apache.hc.core5.http.ClassicHttpRequest;
-
-import java.util.stream.Stream;
+import java.net.URI;
+import java.util.Objects;
 
 /**
- * An exception due to server error, a bad request, or similar.
- *
- * @author jonmv
+ * @author Tony Vaagenes
  */
 public class ConfigServerException extends RuntimeException {
 
-    private final ErrorCode code;
-    private final String message;
+    private final URI serverUri;
+    private final ErrorCode errorCode;
+    private final String serverMessage;
 
-    public ConfigServerException(ErrorCode code, String message, String context) {
-        super(context + ": " + message);
-        this.code = code;
-        this.message = message;
+    public ConfigServerException(URI serverUri, String context, String serverMessage, ErrorCode errorCode, Throwable cause) {
+        super(context + ": " + serverMessage, cause);
+        this.serverUri = Objects.requireNonNull(serverUri);
+        this.errorCode = Objects.requireNonNull(errorCode);
+        this.serverMessage = Objects.requireNonNull(serverMessage);
     }
 
-    public ErrorCode code() { return code; }
+    public ErrorCode getErrorCode() {
+        return errorCode;
+    }
 
-    public String message() { return message; }
+    public URI getServerUri() {
+        return serverUri;
+    }
 
+    public String getServerMessage() {
+        return serverMessage;
+    }
+
+    // TODO: Copied from Vespa. Expose these in Vespa and use them here
     public enum ErrorCode {
         APPLICATION_LOCK_FAILURE,
         BAD_REQUEST,
@@ -40,18 +46,7 @@ public class ConfigServerException extends RuntimeException {
         UNKNOWN_VESPA_VERSION,
         PARENT_HOST_NOT_READY,
         CERTIFICATE_NOT_READY,
-        LOAD_BALANCER_NOT_READY,
-        INCOMPLETE_RESPONSE
-    }
-
-    public static ConfigServerException readException(int statusCode, byte[] body, ClassicHttpRequest request) {
-        Inspector root = SlimeUtils.jsonToSlime(body).get();
-        String codeName = root.field("error-code").asString();
-        ErrorCode code = Stream.of(ErrorCode.values())
-                               .filter(value -> value.name().equals(codeName))
-                               .findAny().orElse(ErrorCode.INCOMPLETE_RESPONSE);
-        String message = root.field("message").valid() ? root.field("message").asString() : "(no message)";
-        return new ConfigServerException(code, message, request + " failed with status " + statusCode);
+        LOAD_BALANCER_NOT_READY
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java
index d2f19f4df9f..05bdf3c3412 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java
@@ -18,11 +18,11 @@ public class LoadBalancer {
     private final String id;
     private final ApplicationId application;
     private final ClusterSpec.Id cluster;
-    private final Optional<HostName> hostname;
+    private final HostName hostname;
     private final State state;
     private final Optional<String> dnsZone;
 
-    public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, Optional<HostName> hostname, State state,
+    public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, HostName hostname, State state,
                         Optional<String> dnsZone) {
         this.id = Objects.requireNonNull(id, "id must be non-null");
         this.application = Objects.requireNonNull(application, "application must be non-null");
@@ -44,7 +44,7 @@ public class LoadBalancer {
         return cluster;
     }
 
-    public Optional<HostName> hostname() {
+    public HostName hostname() {
         return hostname;
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index 4ab51ab2e36..a907cbe2406 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -228,7 +228,7 @@ public class InternalStepRunner implements StepRunner {
             // Retry certain failures for up to one hour.
             Optional<RunStatus> result = startTime.isBefore(controller.clock().instant().minus(Duration.ofHours(1)))
                                          ? Optional.of(deploymentFailed) : Optional.empty();
-            switch (e.code()) {
+            switch (e.getErrorCode()) {
                 case CERTIFICATE_NOT_READY:
                     logger.log("Waiting for certificate to become ready on config server: New application, or old one has expired");
                     if (startTime.plus(timeouts.endpointCertificate()).isBefore(controller.clock().instant())) {
@@ -238,15 +238,15 @@ public class InternalStepRunner implements StepRunner {
                     return result;
                 case ACTIVATION_CONFLICT:
                 case APPLICATION_LOCK_FAILURE:
-                    logger.log("Deployment failed with possibly transient error " + e.code() +
+                    logger.log("Deployment failed with possibly transient error " + e.getErrorCode() +
                             ", will retry: " + e.getMessage());
                     return result;
                 case LOAD_BALANCER_NOT_READY:
                 case PARENT_HOST_NOT_READY:
-                    logger.log(e.message());
+                    logger.log(e.getServerMessage());
                     return result;
                 case OUT_OF_CAPACITY:
-                    logger.log(e.message());
+                    logger.log(e.getServerMessage());
                     return controller.system().isCd() && startTime.plus(timeouts.capacity()).isAfter(controller.clock().instant())
                            ? Optional.empty()
                            : Optional.of(outOfCapacity);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 37907d6071c..9a358f63293 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -201,15 +201,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
             return ErrorResponse.badRequest(Exceptions.toMessageString(e));
         }
         catch (ConfigServerException e) {
-            switch (e.code()) {
+            switch (e.getErrorCode()) {
                 case NOT_FOUND:
                     return ErrorResponse.notFoundError(Exceptions.toMessageString(e));
                 case ACTIVATION_CONFLICT:
-                    return new ErrorResponse(CONFLICT, e.code().name(), Exceptions.toMessageString(e));
+                    return new ErrorResponse(CONFLICT, e.getErrorCode().name(), Exceptions.toMessageString(e));
                 case INTERNAL_SERVER_ERROR:
                     return ErrorResponse.internalServerError(Exceptions.toMessageString(e));
                 default:
-                    return new ErrorResponse(BAD_REQUEST, e.code().name(), Exceptions.toMessageString(e));
+                    return new ErrorResponse(BAD_REQUEST, e.getErrorCode().name(), Exceptions.toMessageString(e));
             }
         }
         catch (RuntimeException e) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
index 898b2531460..0356e11ae36 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
@@ -188,10 +188,9 @@ public class RoutingPolicies {
     private void storePoliciesOf(LoadBalancerAllocation allocation, @SuppressWarnings("unused") Lock lock) {
         var policies = new LinkedHashMap<>(get(allocation.deployment.applicationId()));
         for (LoadBalancer loadBalancer : allocation.loadBalancers) {
-            if (loadBalancer.hostname().isEmpty()) continue;
             var policyId = new RoutingPolicyId(loadBalancer.application(), loadBalancer.cluster(), allocation.deployment.zoneId());
             var existingPolicy = policies.get(policyId);
-            var newPolicy = new RoutingPolicy(policyId, loadBalancer.hostname().get(), loadBalancer.dnsZone(),
+            var newPolicy = new RoutingPolicy(policyId, loadBalancer.hostname(), loadBalancer.dnsZone(),
                                               allocation.endpointIdsOf(loadBalancer),
                                               new Status(isActive(loadBalancer), GlobalRouting.DEFAULT_STATUS));
             // Preserve global routing status for existing policy
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
index 976cdb5c674..4a4159180b5 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
@@ -275,9 +275,11 @@ public class DeploymentContext {
     /** Fail current deployment in given job */
     public DeploymentContext outOfCapacity(JobType type) {
         return failDeployment(type,
-                              new ConfigServerException(ConfigServerException.ErrorCode.OUT_OF_CAPACITY,
+                              new ConfigServerException(URI.create("https://config.server"),
+                                                        "Failed to deploy application",
                                                         "Out of capacity",
-                                                        "Failed to deploy application"));
+                                                        ConfigServerException.ErrorCode.OUT_OF_CAPACITY,
+                                                        new RuntimeException("Out of capacity from test code")));
     }
 
     /** Fail current deployment in given job */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index 14244d7bdda..6bd7feb8d96 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -99,9 +99,11 @@ public class InternalStepRunnerTest {
 
     @Test
     public void retriesDeploymentForOneHour() {
-        RuntimeException exception = new ConfigServerException(ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE,
+        RuntimeException exception = new ConfigServerException(URI.create("https://server"),
+                                                               "test failure",
                                                                "Exception to retry",
-                                                               "test failure");
+                                                               ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE,
+                                                               new RuntimeException("Retry me"));
         tester.configServer().throwOnNextPrepare(exception);
         tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage());
         assertEquals(unfinished, tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().stepStatuses().get(Step.deployReal));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index f377e957284..ca52e31d04c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -30,6 +30,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerE
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.NotFoundException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ProxyResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.QuotaUsage;
@@ -389,7 +390,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
             putLoadBalancers(id.zoneId(), List.of(new LoadBalancer(UUID.randomUUID().toString(),
                                                                    id.applicationId(),
                                                                    cluster,
-                                                                   Optional.of(HostName.from("lb-0--" + id.applicationId().serializedForm() + "--" + id.zoneId().toString())),
+                                                                   HostName.from("lb-0--" + id.applicationId().serializedForm() + "--" + id.zoneId().toString()),
                                                                    LoadBalancer.State.active,
                                                                    Optional.of("dns-zone-1"))));
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainerTest.java
index 1eadae18668..d7440a706ea 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainerTest.java
@@ -35,7 +35,7 @@ public class SystemRoutingPolicyMaintainerTest {
         tester.configServer().putLoadBalancers(zone, List.of(new LoadBalancer("lb1",
                                                                               SystemApplication.configServer.id(),
                                                                               ClusterSpec.Id.from("config"),
-                                                                              Optional.of(HostName.from("lb1.example.com")),
+                                                                              HostName.from("lb1.example.com"),
                                                                               LoadBalancer.State.active,
                                                                               Optional.of("dns-zone-1"))));
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 0137ea7eeba..abea055dde8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -1127,7 +1127,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               400);
 
         ConfigServerMock configServer = tester.serviceRegistry().configServerMock();
-        configServer.throwOnNextPrepare(new ConfigServerException(ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, "Failed to prepare application", "Invalid application package"));
+        configServer.throwOnNextPrepare(new ConfigServerException(new URI("server-url"), "Failed to prepare application", "Invalid application package", ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, null));
 
         // GET non-existent application package
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package", GET).userIdentity(HOSTED_VESPA_OPERATOR),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java
index 1be7f16e85f..f574d6bc3f1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java
@@ -81,7 +81,7 @@ public class JobControllerApiHandlerHelperTest {
         tester.triggerJobs();
 
         // us-east-3 eats the deployment failure and fails before deployment, while us-west-1 fails after.
-        tester.configServer().throwOnNextPrepare(new ConfigServerException(INVALID_APPLICATION_PACKAGE, "ERROR!", "Failed to deploy application"));
+        tester.configServer().throwOnNextPrepare(new ConfigServerException(URI.create("url"), "Failed to deploy application", "ERROR!", INVALID_APPLICATION_PACKAGE, null));
         tester.runner().run();
         assertEquals(deploymentFailed, tester.jobs().last(app.instanceId(), productionUsEast3).get().status());
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
index d03dec06753..e96af475216 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
@@ -416,7 +416,7 @@ public class RoutingPoliciesTest {
         var loadBalancer = new LoadBalancer("LB-0-Z-" + zone1.value(),
                                             context.instanceId(),
                                             ClusterSpec.Id.from("c0"),
-                                            Optional.of(newHostname),
+                                            newHostname,
                                             LoadBalancer.State.active,
                                             Optional.of("dns-zone-1"));
         tester.controllerTester().configServer().putLoadBalancers(zone1, List.of(loadBalancer));
@@ -705,7 +705,7 @@ public class RoutingPoliciesTest {
                     new LoadBalancer("LB-" + i + "-Z-" + zone.value(),
                                      application,
                                      ClusterSpec.Id.from("c" + i),
-                                     Optional.of(lbHostname),
+                                     lbHostname,
                                      LoadBalancer.State.active,
                                      Optional.of("dns-zone-1")));
         }
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 a6bb57bef29..6f7b7c4d57d 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,7 +5,6 @@ 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.
@@ -15,18 +14,15 @@ import java.util.Optional;
 public class LoadBalancer {
 
     private final LoadBalancerId id;
-    private final Optional<LoadBalancerInstance> instance;
+    private final LoadBalancerInstance instance;
     private final State state;
     private final Instant changedAt;
 
-    public LoadBalancer(LoadBalancerId id, Optional<LoadBalancerInstance> instance, State state, Instant changedAt) {
+    public LoadBalancer(LoadBalancerId id, 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 */
@@ -35,7 +31,7 @@ public class LoadBalancer {
     }
 
     /** The instance associated with this */
-    public Optional<LoadBalancerInstance> instance() {
+    public LoadBalancerInstance instance() {
         return instance;
     }
 
@@ -62,7 +58,7 @@ public class LoadBalancer {
     }
 
     /** Returns a copy of this with instance set to given instance */
-    public LoadBalancer with(Optional<LoadBalancerInstance> instance) {
+    public LoadBalancer with(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 9665d8872de..2ef12177eaf 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,7 +12,6 @@ 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;
@@ -57,10 +56,10 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer {
 
     /** Move reserved load balancer that have expired to inactive */
     private void expireReserved() {
-        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)));
+        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())));
     }
 
     /** Deprovision inactive load balancers that have expired */
@@ -96,14 +95,13 @@ 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().get().reals());
+            var reals = new LinkedHashSet<>(lb.instance().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().map(instance -> instance.withReals(reals))));
+                db.writeLoadBalancer(lb.with(lb.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 4cd01167293..dddc535b36a 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,7 +12,6 @@ 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;
 
@@ -71,7 +70,6 @@ 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 04c8f012b8b..66172521d4c 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,7 +18,6 @@ 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;
 
 /**
@@ -51,22 +50,21 @@ public class LoadBalancerSerializer {
         Cursor root = slime.setObject();
 
         root.setString(idField, loadBalancer.id().serializedForm());
-        // 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(hostnameField, loadBalancer.instance().hostname().toString());
         root.setString(stateField, asString(loadBalancer.state()));
         root.setLong(changedAtField, loadBalancer.changedAt().toEpochMilli());
-        loadBalancer.instance().flatMap(LoadBalancerInstance::dnsZone).ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id()));
+        loadBalancer.instance().dnsZone().ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id()));
         Cursor portArray = root.setArray(portsField);
-        loadBalancer.instance().ifPresent(instance -> instance.ports().forEach(portArray::addLong));
+        loadBalancer.instance().ports().forEach(portArray::addLong);
         Cursor networkArray = root.setArray(networksField);
-        loadBalancer.instance().ifPresent(instance -> instance.networks().forEach(networkArray::addString));
+        loadBalancer.instance().networks().forEach(networkArray::addString);
         Cursor realArray = root.setArray(realsField);
-        loadBalancer.instance().ifPresent(instance -> instance.reals().forEach(real -> {
+        loadBalancer.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) {
@@ -77,7 +75,7 @@ public class LoadBalancerSerializer {
     public static LoadBalancer fromJson(byte[] data) {
         Cursor object = SlimeUtils.jsonToSlime(data).get();
 
-        Set<Real> reals = new LinkedHashSet<>();
+        var reals = new LinkedHashSet<Real>();
         object.field(realsField).traverse((ArrayTraverser) (i, realObject) -> {
             reals.add(new Real(HostName.from(realObject.field(hostnameField).asString()),
                                realObject.field(ipAddressField).asString(),
@@ -85,19 +83,20 @@ public class LoadBalancerSerializer {
 
         });
 
-        Set<Integer> ports = new LinkedHashSet<>();
+        var ports = new LinkedHashSet<Integer>();
         object.field(portsField).traverse((ArrayTraverser) (i, port) -> ports.add((int) port.asLong()));
 
-        Set<String> networks = new LinkedHashSet<>();
+        var networks = new LinkedHashSet<String>();
         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()),
-                                instance,
+                                new LoadBalancerInstance(
+                                        HostName.from(object.field(hostnameField).asString()),
+                                        optionalString(object.field(dnsZoneField), DnsZone::new),
+                                        ports,
+                                        networks,
+                                        reals
+                                ),
                                 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 f53eb189ec1..09d19300c59 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,35 +170,18 @@ 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
-
-        Set<Real> reals = realsOf(nodes);
-        Optional<LoadBalancerInstance> instance = provisionInstance(id, reals, loadBalancer);
+        LoadBalancerInstance instance = provisionInstance(id, realsOf(nodes), loadBalancer);
         LoadBalancer newLoadBalancer;
         if (loadBalancer.isEmpty()) {
             newLoadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now);
         } else {
-            LoadBalancer.State state = activate && instance.isPresent()
-                    ? LoadBalancer.State.active
-                    : loadBalancer.get().state();
-            newLoadBalancer = loadBalancer.get().with(instance).with(state, now);
+            var newState = activate ? LoadBalancer.State.active : loadBalancer.get().state();
+            newLoadBalancer = loadBalancer.get().with(instance).with(newState, now);
             if (loadBalancer.get().state() != newLoadBalancer.state()) {
                 log.log(Level.FINE, "Moving " + newLoadBalancer.id() + " to state " + newLoadBalancer.state());
             }
         }
-
-        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);
-        }
+        db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested());
     }
 
     private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, NodeList nodes) {
@@ -206,18 +189,17 @@ public class LoadBalancerProvisioner {
     }
 
     /** Provision or reconfigure a load balancer instance, if necessary */
-    private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, Set<Real> reals,
-                                                             Optional<LoadBalancer> currentLoadBalancer) {
+    private 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 Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals),
-                                              allowEmptyReals(currentLoadBalancer)));
+            return service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals),
+                                  allowEmptyReals(currentLoadBalancer));
         } catch (Exception e) {
-            log.log(Level.WARNING, "Could not (re)configure " + id + ", targeting: " +
-                                   reals + ". The operation will be retried on next deployment", e);
+            throw new LoadBalancerServiceException("Failed to (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 */
@@ -264,8 +246,7 @@ 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;
-        if (loadBalancer.get().instance().isEmpty()) return false;
-        return loadBalancer.get().instance().get().reals().equals(reals);
+        return loadBalancer.get().instance().reals().equals(reals);
     }
 
     /** Returns whether to allow given load balancer to have no reals */
@@ -292,4 +273,5 @@ 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 bcd0af4e121..1ef449555d9 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,7 +9,6 @@ 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;
 
@@ -66,23 +65,21 @@ 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());
-            lb.instance().ifPresent(instance -> lbObject.setString("hostname", instance.hostname().value()));
-            lb.instance().flatMap(LoadBalancerInstance::dnsZone).ifPresent(dnsZone -> lbObject.setString("dnsZone", dnsZone.id()));
+            lbObject.setString("hostname", lb.instance().hostname().value());
+            lb.instance().dnsZone().ifPresent(dnsZone -> lbObject.setString("dnsZone", dnsZone.id()));
 
             Cursor networkArray = lbObject.setArray("networks");
-            lb.instance().ifPresent(instance -> instance.networks().forEach(networkArray::addString));
+            lb.instance().networks().forEach(networkArray::addString);
 
             Cursor portArray = lbObject.setArray("ports");
-            lb.instance().ifPresent(instance -> instance.ports().forEach(portArray::addLong));
+            lb.instance().ports().forEach(portArray::addLong);
 
             Cursor realArray = lbObject.setArray("reals");
-            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());
-                });
+            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());
             });
         });
 
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 4d19c2c1d41..fec6e40fd35 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().get().reals());
+        assertEquals(Set.of(), loadBalancers.get().get(lb1).instance().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 51d6f7fb2fb..bcb78844666 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")),
-                                            Optional.of(new LoadBalancerInstance(
+                                            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().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.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.state(), serialized.state());
         assertEquals(loadBalancer.changedAt().truncatedTo(MILLIS), serialized.changedAt());
-        assertEquals(loadBalancer.instance().get().reals(), serialized.instance().get().reals());
+        assertEquals(loadBalancer.instance().reals(), serialized.instance().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 744322c9edc..b7e671a7b76 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().get().networks();
+        var lbNetworks = loadBalancers.asList().get(0).instance().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 eb3bdff484d..a3780db4789 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,7 +10,6 @@ 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;
@@ -60,7 +59,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().get().reals().size());
+        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());
@@ -68,11 +67,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().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());
+        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());
 
         // A container is failed
         Supplier<List<Node>> containers = () -> tester.getNodes(app1).container().asList();
@@ -84,13 +83,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().get().reals().size());
-        assertTrue("Failed node is removed", loadBalancer.instance().get().reals().stream()
+        assertEquals(2, loadBalancer.instance().reals().size());
+        assertTrue("Failed node is removed", loadBalancer.instance().reals().stream()
                                                          .map(Real::hostname)
                                                          .map(HostName::value)
                                                          .noneMatch(hostname -> hostname.equals(toFail.hostname())));
-        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());
+        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());
         assertSame("State is unchanged", LoadBalancer.State.active, loadBalancer.state());
 
         // Add another container cluster to first app
@@ -111,7 +110,6 @@ 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)
@@ -154,7 +152,7 @@ public class LoadBalancerProvisionerTest {
                          .findFirst()
                          .orElseThrow());
 
-        // Next redeploy does not create a new load balancer instance because reals are unchanged
+        // Next redeploy does not create a new load balancer instance
         tester.loadBalancerService().throwOnCreate(true);
         tester.activate(app1, prepare(app1,
                                       clusterRequest(ClusterSpec.Type.container, containerCluster1),
@@ -210,7 +208,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().get().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size());
         tester.activate(app1, nodes);
         assertSame(LoadBalancer.State.active, lbs.get().get(0).state());
         assertEquals(combinedId, lbs.get().get(0).id().cluster());
@@ -224,7 +222,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().get().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size());
         tester.activate(configServerApp, nodes);
         assertSame(LoadBalancer.State.active, lbs.get().get(0).state());
         assertEquals(cluster, lbs.get().get(0).id().cluster());
@@ -238,7 +236,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().get().reals().size());
+        assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size());
         tester.activate(controllerApp, nodes);
         assertSame(LoadBalancer.State.active, lbs.get().get(0).state());
         assertEquals(cluster, lbs.get().get(0).id().cluster());
@@ -255,7 +253,7 @@ public class LoadBalancerProvisionerTest {
         // instance1 is deployed
         tester.activate(instance1, prepare(instance1, clusterRequest(ClusterSpec.Type.container, devCluster)));
 
-        // instance2 clashes because instance name matches instance1 cluster name
+        // instance2 clashes because cluster name matches instance1
         try {
             prepare(instance2, clusterRequest(ClusterSpec.Type.container, defaultCluster));
             fail("Expected exception");
@@ -265,38 +263,10 @@ 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 does not clash
+        // instance3 clashes because instance name matches instance2 cluster
         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());
     }