summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJon Marius Venstad <venstad@gmail.com>2021-04-26 16:41:58 +0200
committerJon Marius Venstad <venstad@gmail.com>2021-04-26 16:41:58 +0200
commitde80724b756e8f3e617755e838189948b8d835cc (patch)
tree619147321378a14274612c6dad3b13438ac94dd1
parent9458ad2d810ff988ccc19cd8554bdbc78dabe588 (diff)
Revert "Merge pull request #17603 from vespa-engine/jonmv/revert-apache-config-server-client"
This reverts commit 9458ad2d810ff988ccc19cd8554bdbc78dabe588, reversing changes made to 6e94e192a07a540bff9b3e13ad05c8d0af093928.
-rw-r--r--configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java83
-rw-r--r--configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java141
-rw-r--r--configserver-client/src/main/java/ai/vespa/hosted/client/HttpConfigServerClient.java1
-rw-r--r--configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java8
-rw-r--r--controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java53
-rw-r--r--controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java6
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java8
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java6
-rw-r--r--controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java3
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java6
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java6
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java3
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemRoutingPolicyMaintainerTest.java2
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java2
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java2
-rw-r--r--controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java4
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java12
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java14
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java2
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java33
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java42
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java21
-rw-r--r--node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java2
-rw-r--r--node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java14
-rw-r--r--node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java2
-rw-r--r--node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java62
26 files changed, 339 insertions, 199 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 d467402f79f..2e5e445d63a 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,11 +9,12 @@ 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;
@@ -21,7 +22,6 @@ 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 RequestBuilder send(HostStrategy hosts, Method method) {
+ public ConfigServerClient.RequestBuilder send(HostStrategy hosts, Method method) {
return new RequestBuilder(hosts, method);
}
@@ -114,10 +114,11 @@ 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 BiConsumer<ClassicHttpResponse, ClassicHttpRequest> handler = ConfigServerClient::throwOnError;
- private Consumer<IOException> catcher = ConfigServerClient::retryAll;
+ private ResponseVerifier verifier = ConfigServerClient.throwOnError;
+ private Consumer<IOException> catcher = ConfigServerClient.retryAll;
private RequestBuilder(HostStrategy hosts, Method method) {
if ( ! hosts.iterator().hasNext())
@@ -129,7 +130,7 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
@Override
public RequestBuilder at(List<String> pathSegments) {
- uriBuilder.setPathSegments(requireNonNull(pathSegments));
+ this.pathSegments.addAll(pathSegments);
return this;
}
@@ -145,12 +146,23 @@ 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(); )
- uriBuilder.setParameter(pairs.get(i++), pairs.get(i++));
+ for (int i = 0; i < pairs.size(); ) {
+ String key = pairs.get(i++), value = pairs.get(i++);
+ if (value != null)
+ uriBuilder.setParameter(key, value);
+ }
return this;
}
@@ -175,56 +187,54 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient {
}
@Override
- public RequestBuilder handling(BiConsumer<ClassicHttpResponse, ClassicHttpRequest> handler) {
- this.handler = requireNonNull(handler);
+ public RequestBuilder throwing(ResponseVerifier verifier) {
+ this.verifier = requireNonNull(verifier);
return this;
}
@Override
- public <T> T read(Function<byte[], T> mapper) {
- return mapIfSuccess(input -> {
- try (input) {
- return mapper.apply(input.readAllBytes());
+ public String read() {
+ return handle((response, __) -> {
+ try (response) {
+ return response.getEntity() == null ? "" : EntityUtils.toString(response.getEntity());
}
- catch (IOException e) {
- throw new RetryException(e);
+ 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()));
}
});
}
@Override
public void discard() throws UncheckedIOException, ResponseException {
- mapIfSuccess(input -> {
- try (input) {
+ handle((response, __) -> {
+ try (response) {
return null;
}
- catch (IOException e) {
- throw new RetryException(e);
- }
});
}
@Override
- public InputStream stream() throws UncheckedIOException, ResponseException {
- return mapIfSuccess(input -> input);
+ public HttpInputStream stream() throws UncheckedIOException, ResponseException {
+ return handle((response, __) -> new HttpInputStream(response));
}
- /** Returns the mapped body, if successful, retrying any IOException. The caller must close the body stream. */
- private <T> T mapIfSuccess(Function<InputStream, T> mapper) {
+ @Override
+ public <T> T handle(ResponseHandler<T> handler) {
+ uriBuilder.setPathSegments(pathSegments);
return execute(this,
(response, request) -> {
try {
- 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();
- }
- });
+ verifier.verify(response, request); // This throws on unacceptable responses.
+ return handler.handle(response, request);
}
catch (IOException | RuntimeException | Error e) {
try {
@@ -247,6 +257,7 @@ 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 c0f72378c1b..c92acd7cd0b 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 AutoCloseable {
+public interface ConfigServerClient extends Closeable {
RequestConfig defaultRequestConfig = RequestConfig.custom()
.setConnectionRequestTimeout(Timeout.ofSeconds(5))
@@ -40,20 +40,12 @@ public interface ConfigServerClient extends AutoCloseable {
.build();
/** Wraps with a {@link RetryException} and rethrows. */
- static void retryAll(IOException e) {
+ Consumer<IOException> retryAll = (e) -> {
throw new RetryException(e);
- }
+ };
/** Throws a a {@link RetryException} if {@code statusCode == 503}, or a {@link ResponseException} unless {@code 200 <= statusCode < 300}. */
- 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;
- }
- }
+ ResponseVerifier throwOnError = new DefaultResponseVerifier() { };
/** Reads the response body, throwing an {@link UncheckedIOException} if this fails, or {@code null} if there is none. */
static byte[] getBytes(ClassicHttpResponse response) {
@@ -71,10 +63,10 @@ public interface ConfigServerClient extends AutoCloseable {
/** Builder for a request against a given set of hosts, using this config server client. */
interface RequestBuilder {
- /** Sets the request path. */
+ /** Appends to the request path. */
default RequestBuilder at(String... pathSegments) { return at(List.of(pathSegments)); }
- /** Sets the request path. */
+ /** Appends to the request path. */
RequestBuilder at(List<String> pathSegments);
/** Sets the request body as UTF-8 application/json. */
@@ -83,12 +75,20 @@ public interface ConfigServerClient extends AutoCloseable {
/** Sets the request body. */
RequestBuilder body(HttpEntity entity);
- /** Sets the parameter key/values for the request. Number of arguments must be even. */
+ /** 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. */
default RequestBuilder parameters(String... pairs) {
return parameters(Arrays.asList(pairs));
}
- /** Sets the parameter key/values for the request. Number of arguments must be even. */
+ /** Sets the parameter key/values for the request. Number of arguments must be even. Null values are omitted. */
RequestBuilder parameters(List<String> pairs);
/** Overrides the default socket read timeout of the request. {@code Duration.ZERO} gives infinite timeout. */
@@ -109,7 +109,10 @@ public interface ConfigServerClient extends AutoCloseable {
* 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 handling(BiConsumer<ClassicHttpResponse, ClassicHttpRequest> handler);
+ RequestBuilder throwing(ResponseVerifier handler);
+
+ /** Reads the response as a {@link String}, or throws if unsuccessful. */
+ String read();
/** Reads and maps the response, or throws if unsuccessful. */
<T> T read(Function<byte[], T> mapper);
@@ -118,10 +121,88 @@ public interface ConfigServerClient extends AutoCloseable {
void discard();
/** Returns the raw response input stream, or throws if unsuccessful. The caller must close the returned stream. */
- InputStream 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;
}
+
+ /** 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> {
@@ -147,6 +228,7 @@ public interface ConfigServerClient extends AutoCloseable {
}
+
/** Exception wrapper that signals retries should be attempted. */
final class RetryException extends RuntimeException {
@@ -160,25 +242,12 @@ public interface ConfigServerClient extends AutoCloseable {
}
+
/** 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, 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);
+ public ResponseException(String message) {
+ super(message);
}
}
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 c5b07eceaf5..e00489f0c64 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,7 +54,6 @@ 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 3b46501a4bf..b2f17f43f5c 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,14 +70,15 @@ class HttpConfigServerClientTest {
server.resetRequests();
// Successful attempt returns.
- server.stubFor(get("/root/boot"))
+ server.stubFor(get("/root/boot/toot"))
.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")));
+ server.verify(1, getRequestedFor(urlEqualTo("/root/boot/toot")));
server.verify(1, anyRequestedFor(anyUrl()));
server.resetRequests();
@@ -88,10 +89,9 @@ 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 no body", thrown.getMessage());
+ assertEquals("GET / failed with status 409 and body 'hi'", 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 4240b0d9fa6..5e4d3345b7a 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,38 +1,32 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. 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 java.net.URI;
-import java.util.Objects;
+import com.yahoo.slime.Inspector;
+import com.yahoo.slime.SlimeUtils;
+import org.apache.hc.core5.http.ClassicHttpRequest;
+
+import java.util.stream.Stream;
/**
- * @author Tony Vaagenes
+ * An exception due to server error, a bad request, or similar.
+ *
+ * @author jonmv
*/
public class ConfigServerException extends RuntimeException {
- private final URI serverUri;
- private final ErrorCode errorCode;
- private final String serverMessage;
+ private final ErrorCode code;
+ private final String 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 ConfigServerException(ErrorCode code, String message, String context) {
+ super(context + ": " + message);
+ this.code = code;
+ this.message = message;
}
- public ErrorCode getErrorCode() {
- return errorCode;
- }
+ public ErrorCode code() { return code; }
- public URI getServerUri() {
- return serverUri;
- }
+ public String message() { return message; }
- 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,
@@ -46,7 +40,18 @@ public class ConfigServerException extends RuntimeException {
UNKNOWN_VESPA_VERSION,
PARENT_HOST_NOT_READY,
CERTIFICATE_NOT_READY,
- LOAD_BALANCER_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);
}
}
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 05bdf3c3412..d2f19f4df9f 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 HostName hostname;
+ private final Optional<HostName> hostname;
private final State state;
private final Optional<String> dnsZone;
- public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, HostName hostname, State state,
+ public LoadBalancer(String id, ApplicationId application, ClusterSpec.Id cluster, Optional<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 HostName hostname() {
+ public Optional<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 a907cbe2406..4ab51ab2e36 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.getErrorCode()) {
+ switch (e.code()) {
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.getErrorCode() +
+ logger.log("Deployment failed with possibly transient error " + e.code() +
", will retry: " + e.getMessage());
return result;
case LOAD_BALANCER_NOT_READY:
case PARENT_HOST_NOT_READY:
- logger.log(e.getServerMessage());
+ logger.log(e.message());
return result;
case OUT_OF_CAPACITY:
- logger.log(e.getServerMessage());
+ logger.log(e.message());
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 9a358f63293..37907d6071c 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.getErrorCode()) {
+ switch (e.code()) {
case NOT_FOUND:
return ErrorResponse.notFoundError(Exceptions.toMessageString(e));
case ACTIVATION_CONFLICT:
- return new ErrorResponse(CONFLICT, e.getErrorCode().name(), Exceptions.toMessageString(e));
+ return new ErrorResponse(CONFLICT, e.code().name(), Exceptions.toMessageString(e));
case INTERNAL_SERVER_ERROR:
return ErrorResponse.internalServerError(Exceptions.toMessageString(e));
default:
- return new ErrorResponse(BAD_REQUEST, e.getErrorCode().name(), Exceptions.toMessageString(e));
+ return new ErrorResponse(BAD_REQUEST, e.code().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 0356e11ae36..898b2531460 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,9 +188,10 @@ 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(), loadBalancer.dnsZone(),
+ var newPolicy = new RoutingPolicy(policyId, loadBalancer.hostname().get(), 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 4a4159180b5..976cdb5c674 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,11 +275,9 @@ public class DeploymentContext {
/** Fail current deployment in given job */
public DeploymentContext outOfCapacity(JobType type) {
return failDeployment(type,
- new ConfigServerException(URI.create("https://config.server"),
- "Failed to deploy application",
+ new ConfigServerException(ConfigServerException.ErrorCode.OUT_OF_CAPACITY,
"Out of capacity",
- ConfigServerException.ErrorCode.OUT_OF_CAPACITY,
- new RuntimeException("Out of capacity from test code")));
+ "Failed to deploy application"));
}
/** 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 6bd7feb8d96..14244d7bdda 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,11 +99,9 @@ public class InternalStepRunnerTest {
@Test
public void retriesDeploymentForOneHour() {
- RuntimeException exception = new ConfigServerException(URI.create("https://server"),
- "test failure",
+ RuntimeException exception = new ConfigServerException(ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE,
"Exception to retry",
- ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE,
- new RuntimeException("Retry me"));
+ "test failure");
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 ca52e31d04c..f377e957284 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,7 +30,6 @@ 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;
@@ -390,7 +389,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
putLoadBalancers(id.zoneId(), List.of(new LoadBalancer(UUID.randomUUID().toString(),
id.applicationId(),
cluster,
- HostName.from("lb-0--" + id.applicationId().serializedForm() + "--" + id.zoneId().toString()),
+ Optional.of(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 d7440a706ea..1eadae18668 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"),
- HostName.from("lb1.example.com"),
+ Optional.of(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 abea055dde8..0137ea7eeba 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(new URI("server-url"), "Failed to prepare application", "Invalid application package", ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, null));
+ configServer.throwOnNextPrepare(new ConfigServerException(ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, "Failed to prepare application", "Invalid application package"));
// 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 f574d6bc3f1..1be7f16e85f 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(URI.create("url"), "Failed to deploy application", "ERROR!", INVALID_APPLICATION_PACKAGE, null));
+ tester.configServer().throwOnNextPrepare(new ConfigServerException(INVALID_APPLICATION_PACKAGE, "ERROR!", "Failed to deploy application"));
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 e96af475216..d03dec06753 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"),
- newHostname,
+ Optional.of(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),
- lbHostname,
+ Optional.of(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 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());
}