From e8e9cd5d722af2efa5489b9eb4a17aa4b58303a4 Mon Sep 17 00:00:00 2001 From: jonmv Date: Thu, 28 Apr 2022 22:12:00 +0200 Subject: Replace Jersey in orchestrator with apache, remove jaxrx_client_utils --- .../ApplicationStateChangeDeniedException.java | 5 +- .../yahoo/vespa/orchestrator/OrchestratorImpl.java | 28 +-- .../controller/ClusterControllerClient.java | 20 +- .../controller/ClusterControllerClientImpl.java | 204 +++++++++++++++------ .../ClusterControllerClientTimeouts.java | 42 ++--- .../controller/ClusterControllerJaxRsApi.java | 36 ---- .../ClusterControllerStateErrorResponse.java | 20 -- .../controller/ClusterControllerStateRequest.java | 103 ----------- .../controller/ClusterControllerStateResponse.java | 27 --- .../RetryingClusterControllerClientFactory.java | 59 +++--- .../vespa/orchestrator/model/StorageNodeImpl.java | 43 +---- .../vespa/orchestrator/OrchestratorImplTest.java | 15 +- .../ClusterControllerClientFactoryMock.java | 33 ++-- .../ClusterControllerClientImplTest.java | 158 ++++++++++++++++ .../controller/ClusterControllerClientTest.java | 62 ------- .../ClusterControllerClientTimeoutsTest.java | 15 +- ...RetryingClusterControllerClientFactoryTest.java | 56 ------ 17 files changed, 392 insertions(+), 534 deletions(-) delete mode 100644 orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java delete mode 100644 orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java delete mode 100644 orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java delete mode 100644 orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java create mode 100644 orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java delete mode 100644 orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java delete mode 100644 orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java (limited to 'orchestrator/src') diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java index dfe2b2ca344..cf2e923d018 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java @@ -9,11 +9,8 @@ package com.yahoo.vespa.orchestrator; */ public class ApplicationStateChangeDeniedException extends Exception { - final String reason; - public ApplicationStateChangeDeniedException(String reason) { - super(); - this.reason = reason; + super(reason); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java index ddaec86b340..db18a71c805 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java @@ -17,7 +17,6 @@ import com.yahoo.vespa.orchestrator.config.OrchestratorConfig; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse; import com.yahoo.vespa.orchestrator.model.ApplicationApi; import com.yahoo.vespa.orchestrator.model.ApplicationApiFactory; import com.yahoo.vespa.orchestrator.model.NodeGroup; @@ -37,7 +36,6 @@ import com.yahoo.vespa.orchestrator.status.StatusService; import com.yahoo.vespa.service.monitor.ServiceMonitor; import com.yahoo.yolean.Exceptions; -import java.io.IOException; import java.time.Clock; import java.util.HashMap; import java.util.List; @@ -427,10 +425,7 @@ public class OrchestratorImpl implements Orchestrator { ClusterControllerClient client = clusterControllerClientFactory.createClient(clusterControllers, cluster.clusterId().s()); for (ServiceInstance service : cluster.serviceInstances()) { try { - ClusterControllerStateResponse response = client.setNodeState(context, - VespaModelUtil.getStorageNodeIndex(service.configId()), - MAINTENANCE); - if ( ! response.wasModified) + if ( ! client.setNodeState(context, service.hostName(), VespaModelUtil.getStorageNodeIndex(service.configId()), MAINTENANCE)) return false; } catch (Exception e) { @@ -449,7 +444,7 @@ public class OrchestratorImpl implements Orchestrator { private void setClusterStateInController(OrchestratorContext context, ApplicationInstance application, ClusterControllerNodeState state) - throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException { + throws ApplicationStateChangeDeniedException { // Get all content clusters for this application Set contentClusterIds = application.serviceClusters().stream() .filter(VespaModelUtil::isContent) @@ -459,23 +454,8 @@ public class OrchestratorImpl implements Orchestrator { // For all content clusters set in maintenance for (ClusterId clusterId : contentClusterIds) { List clusterControllers = VespaModelUtil.getClusterControllerInstancesInOrder(application, clusterId); - ClusterControllerClient client = clusterControllerClientFactory.createClient( - clusterControllers, - clusterId.s()); - try { - ClusterControllerStateResponse response = client.setApplicationState(context, state); - if (!response.wasModified) { - String msg = String.format("Fail to set application %s, cluster name %s to cluster state %s due to: %s", - application.applicationInstanceId(), clusterId, state, response.reason); - throw new ApplicationStateChangeDeniedException(msg); - } - } catch (IOException e) { - throw new ApplicationStateChangeDeniedException(e.getMessage()); - } catch (UncheckedTimeoutException e) { - throw new ApplicationStateChangeDeniedException( - "Timed out while waiting for cluster controllers " + clusterControllers + - " with cluster ID " + clusterId.s() + ": " + e.getMessage()); - } + ClusterControllerClient client = clusterControllerClientFactory.createClient(clusterControllers, clusterId.s()); + client.setApplicationState(context, application.applicationInstanceId(), state); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java index fd62f2b4b70..2c31b475b21 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java @@ -1,10 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.controller; -import com.yahoo.concurrent.UncheckedTimeoutException; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; import com.yahoo.vespa.orchestrator.OrchestratorContext; - -import java.io.IOException; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; /** * @author bakksjo @@ -14,17 +15,18 @@ public interface ClusterControllerClient { /** * Requests that a cluster controller sets the requested node to the requested state. * - * @throws IOException if there was a problem communicating with the cluster controller - * @throws UncheckedTimeoutException if operation times out + * @return false is this was a probe operation, and permission would be denied. + * @throws HostStateChangeDeniedException if operation fails, or is otherwise disallowed. */ - ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException; + boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; /** * Requests that a cluster controller sets all nodes in the cluster to the requested state. * - * @throws IOException if there was a problem communicating with the cluster controller - * @throws UncheckedTimeoutException if operation times out + * @throws ApplicationStateChangeDeniedException if operation fails, or is disallowed. */ - ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException; + void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, + ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException; } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java index f4929a4f09e..211b9f9ff0a 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java @@ -1,92 +1,176 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.controller; +import ai.vespa.hosted.client.HttpClient; +import ai.vespa.hosted.client.HttpClient.HostStrategy; +import ai.vespa.hosted.client.HttpClient.ResponseException; +import ai.vespa.hosted.client.HttpClient.ResponseVerifier; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; +import ai.vespa.http.HttpURL.Scheme; import com.yahoo.concurrent.UncheckedTimeoutException; -import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; import com.yahoo.vespa.orchestrator.OrchestratorContext; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; +import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; +import com.yahoo.yolean.Exceptions; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.Method; import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URI; +import java.util.Iterator; +import java.util.List; + +import static java.nio.charset.StandardCharsets.UTF_8; /** * Default implementation of the ClusterControllerClient. * * @author smorgrav */ -public class ClusterControllerClientImpl implements ClusterControllerClient{ +public class ClusterControllerClientImpl implements ClusterControllerClient { + + enum Condition { + FORCE, SAFE; + } public static final String REQUEST_REASON = "Orchestrator"; - private final JaxRsStrategy clusterControllerApi; + private final HttpClient client; + private final List hosts; private final String clusterName; - public ClusterControllerClientImpl(JaxRsStrategy clusterControllerApi, - String clusterName) { + ClusterControllerClientImpl(HttpClient client, List hosts, String clusterName) { this.clusterName = clusterName; - this.clusterControllerApi = clusterControllerApi; + this.hosts = hosts; + this.client = client; } - /** - * Requests that a cluster controller sets the requested node to the requested state. - * - * @throws IOException if there was a problem communicating with the cluster controller - */ @Override - public ClusterControllerStateResponse setNodeState(OrchestratorContext context, - int storageNodeIndex, - ClusterControllerNodeState wantedState) throws IOException { - ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON); - ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest( - state, - ClusterControllerStateRequest.Condition.SAFE, - context.isProbe() ? true : null); - ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(); - + public boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) { try { - return clusterControllerApi.apply(api -> api.setNodeState( - clusterName, - storageNodeIndex, - timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f, - stateRequest), - timeouts); - } catch (IOException | UncheckedTimeoutException e) { - String message = String.format( - "Giving up setting %s for storage node with index %d in cluster %s: %s", - stateRequest, - storageNodeIndex, - clusterName, - e.getMessage()); + Inspector response = client.send(strategyWithTimeout(hosts, context.getClusterControllerTimeouts()), Method.POST) + .at("cluster", "v2", clusterName, "storage", Integer.toString(storageNodeIndex)) + .deadline(context.getClusterControllerTimeouts().readBudget()) + .body(stateChangeRequestBytes(wantedState, Condition.SAFE, context.isProbe())) + .throwing(retryOnRedirect) + .read(SlimeUtils::jsonToSlime).get(); + if ( ! response.field("wasModified").asBool()) { + if (context.isProbe()) + return false; - throw new IOException(message, e); + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, + "Failed to set state to " + wantedState + + " in cluster controller: " + response.field("reason").asString()); + } + return true; + } + catch (ResponseException e) { + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, + "Failed setting node " + storageNodeIndex + " in cluster " + + clusterName + " to state " + wantedState + ": " + e.getMessage()); + } + catch (UncheckedIOException e) { + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT, + String.format("Giving up setting %s for storage node with index %d in cluster %s: %s", + wantedState, + storageNodeIndex, + clusterName, + e.getMessage()), + e.getCause()); + } + catch (UncheckedTimeoutException e) { + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.DEADLINE_CONSTRAINT, + "Timeout while waiting for setNodeState(" + storageNodeIndex + ", " + wantedState + + ") against " + hosts + ": " + e.getMessage(), + e); } } - /** - * Requests that a cluster controller sets all nodes in the cluster to the requested state. - * - * @throws IOException if there was a problem communicating with the cluster controller - */ @Override - public ClusterControllerStateResponse setApplicationState( - OrchestratorContext context, - ClusterControllerNodeState wantedState) throws IOException { - ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON); - ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest( - state, ClusterControllerStateRequest.Condition.FORCE, null); - ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(); - + public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, + ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException { try { - return clusterControllerApi.apply(api -> api.setClusterState( - clusterName, - timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f, - stateRequest), - timeouts); - } catch (IOException | UncheckedTimeoutException e) { - final String message = String.format( - "Giving up setting %s for cluster %s", - stateRequest, - clusterName); - - throw new IOException(message, e); + Inspector response = client.send(strategyWithTimeout(hosts, context.getClusterControllerTimeouts()), Method.POST) + .at("cluster", "v2", clusterName) + .deadline(context.getClusterControllerTimeouts().readBudget()) + .body(stateChangeRequestBytes(wantedState, Condition.FORCE, false)) + .throwing(retryOnRedirect) + .read(SlimeUtils::jsonToSlime).get(); + if ( ! response.field("wasModified").asBool()) { + throw new ApplicationStateChangeDeniedException("Failed to set application " + applicationId + ", cluster name " + + clusterName + " to cluster state " + wantedState + " due to: " + + response.field("reason").asString()); + } + } + catch (ResponseException e) { + throw new ApplicationStateChangeDeniedException("Failed to set application " + applicationId + " cluster name " + + clusterName + " to cluster state " + wantedState + " due to: " + e.getMessage()); } + catch (UncheckedIOException e) { + throw new ApplicationStateChangeDeniedException("Failed communicating with cluster controllers " + hosts + + " with cluster ID " + clusterName + ": " + e.getCause().getMessage()); + } + catch (UncheckedTimeoutException e) { + throw new ApplicationStateChangeDeniedException("Timed out while waiting for cluster controllers " + hosts + + " with cluster ID " + clusterName + ": " + e.getMessage()); + } + } + + static byte[] stateChangeRequestBytes(ClusterControllerNodeState wantedState, Condition condition, boolean isProbe) { + Cursor root = new Slime().setObject(); + Cursor stateObject = root.setObject("user"); + stateObject.setString("reason", REQUEST_REASON); + stateObject.setString("state", wantedState.getWireName()); + root.setString("condition", condition.name()); + if (isProbe) root.setBool("probe", true); + return Exceptions.uncheck(() -> SlimeUtils.toJsonBytes(root)); + } + + /** ᕙ༼◕_◕༽ᕤ hack to vary query parameters with retries ᕙ༼◕_◕༽ᕤ */ + static HostStrategy strategyWithTimeout(List hosts, ClusterControllerClientTimeouts timeouts) { + return () -> new Iterator<>() { + final Iterator wrapped = hosts.iterator(); + @Override public boolean hasNext() { + return wrapped.hasNext(); + } + @Override public URI next() { + return HttpURL.create(Scheme.http, + DomainName.of(wrapped.next().s()), + 19050, + Path.empty(), + Query.empty().set("timeout", Double.toString(timeouts.getServerTimeoutOrThrow().toMillis() * 1e-3))) + .asURI(); + } + }; } + + static final ResponseVerifier retryOnRedirect = new ResponseVerifier() { + @Override + public boolean shouldRetry(int statusCode) { // Need to try the other servers when we get a redirect. + return statusCode < 400 || statusCode == 503; + } + @Override + public RuntimeException toException(int statusCode, byte[] body, ClassicHttpRequest request) { + Inspector root = SlimeUtils.jsonToSlime(body).get(); + String detail = root.field("message").valid() ? root.field("message").asString() + : new String(body, UTF_8); + return new ResponseException("got status code " + statusCode + " for " + request + (detail.isBlank() ? "" : ": " + detail)); + } + }; + } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java index cdae68a0d06..c54b6aae4fe 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java @@ -3,23 +3,23 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.time.TimeBudget; -import com.yahoo.vespa.jaxrs.client.JaxRsTimeouts; import java.time.Duration; +import java.util.Optional; /** * Calculates various timeouts associated with a REST call from the Orchestrator to the Cluster Controller. * *

Timeout handling of HTTP messaging is fundamentally flawed in various Java implementations. * We would like to specify a max time for the whole operation (connect, send request, and receive response). - * Jersey JAX-RS implementation and the Apache HTTP client library provides a way to set the connect timeout C + * The Apache HTTP client library provides a way to set the connect timeout C * and read timeout R. So if the operation takes NR reads, and the writes takes TW time, * the theoretical max time is: T = C + R * NR + TW. With both NR and TW unknown, there's no way to * set a proper C and R.

* * @author hakonhall */ -public class ClusterControllerClientTimeouts implements JaxRsTimeouts { +public class ClusterControllerClientTimeouts { static final Duration CONNECT_TIMEOUT = Duration.ofMillis(100); // Time reserved to guarantee that even though the server application honors a server timeout S, // some time will pass before the server sees the timeout, and after it has returned. @@ -36,40 +36,28 @@ public class ClusterControllerClientTimeouts implements JaxRsTimeouts { * A logical call to CC may in fact call the CC several times, if the first ones are down and/or not * the master. * - * @param timeBudget The time budget for a single logical call to the the Cluster Controller. + * @param timeBudget The time budget for a single logical call to the Cluster Controller. */ public ClusterControllerClientTimeouts(TimeBudget timeBudget) { this.timeBudget = timeBudget; } - @Override - public Duration getConnectTimeoutOrThrow() { - return CONNECT_TIMEOUT; - } - - @Override - public Duration getReadTimeoutOrThrow() { - Duration timeLeft = timeBudget.timeLeft().get(); - - // timeLeft = CONNECT_TIMEOUT + readTimeout - Duration readTimeout = timeLeft.minus(CONNECT_TIMEOUT); - - if (readTimeout.toMillis() <= 0) { + public Duration getServerTimeoutOrThrow() { + // readTimeout = DOWNSTREAM_OVERHEAD + serverTimeout + TimeBudget serverBudget = readBudget().withReserved(DOWNSTREAM_OVERHEAD); + if (serverBudget.timeLeft().get().compareTo(MIN_SERVER_TIMEOUT) < 0) throw new UncheckedTimeoutException("Timed out after " + timeBudget.originalTimeout().get()); - } - return readTimeout; + return serverBudget.timeLeft().get(); } - public Duration getServerTimeoutOrThrow() { - // readTimeout = DOWNSTREAM_OVERHEAD + serverTimeout - Duration serverTimeout = getReadTimeoutOrThrow().minus(DOWNSTREAM_OVERHEAD); - - if (serverTimeout.toMillis() < MIN_SERVER_TIMEOUT.toMillis()) { - throw new UncheckedTimeoutException("Timed out after " + timeBudget.originalTimeout().get()); - } + public Duration connectTimeout() { + return CONNECT_TIMEOUT; + } - return serverTimeout; + public TimeBudget readBudget() { + // timeLeft = CONNECT_TIMEOUT + readTimeout + return timeBudget.withReserved(CONNECT_TIMEOUT); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java deleted file mode 100644 index 01e3cc6718a..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import javax.ws.rs.Consumes; -import javax.ws.rs.POST; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.Produces; -import javax.ws.rs.QueryParam; -import javax.ws.rs.core.MediaType; - -/** - * @author hakonhall - */ -public interface ClusterControllerJaxRsApi { - - @POST - @Path("/cluster/v2/{clusterName}/storage/{storageNodeIndex}") - @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) - ClusterControllerStateResponse setNodeState( - @PathParam("clusterName") String clusterName, - @PathParam("storageNodeIndex") int storageNodeIndex, - @QueryParam("timeout") Float timeoutSeconds, - ClusterControllerStateRequest request); - - @POST - @Path("/cluster/v2/{clusterName}") - @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) - ClusterControllerStateResponse setClusterState( - @PathParam("clusterName") String clusterName, - @QueryParam("timeout") Float timeoutSeconds, - ClusterControllerStateRequest request); - -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java deleted file mode 100644 index b8613073228..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * Error response from cluster controller. - * - * @author hakonhall - */ -public class ClusterControllerStateErrorResponse { - @JsonProperty("message") - public final String message; - - @JsonCreator - public ClusterControllerStateErrorResponse(@JsonProperty("message") String message) { - this.message = message; - } -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java deleted file mode 100644 index e08788cb238..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; - -import javax.annotation.concurrent.Immutable; -import java.util.Collections; -import java.util.Map; -import java.util.Objects; - -/** - * @author hakonhall - */ -@Immutable -@JsonInclude(JsonInclude.Include.NON_NULL) -public class ClusterControllerStateRequest { - - @JsonProperty("state") - public final Map state; - - @JsonProperty("condition") - public final Condition condition; - - @JsonProperty("probe") - public final Boolean probe; - - public ClusterControllerStateRequest(State currentState, Condition condition, Boolean probe) { - Map state = Collections.singletonMap("user", currentState); - this.state = Collections.unmodifiableMap(state); - this.condition = condition; - this.probe = probe; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ClusterControllerStateRequest that = (ClusterControllerStateRequest) o; - return Objects.equals(state, that.state) && - condition == that.condition && - Objects.equals(probe, that.probe); - } - - @Override - public int hashCode() { - return Objects.hash(state, condition, probe); - } - - @Override - public String toString() { - return "NodeStateRequest {" - + " condition=" + condition - + " state=" + state - + " }"; - } - - public static class State { - @JsonProperty("state") - public final ClusterControllerNodeState state; - - /** - * The reason the client is making the request to set the node state. - * Useful for logging in the Cluster Controller. - */ - @JsonProperty("reason") - public final String reason; - - public State(ClusterControllerNodeState state, String reason) { - this.state = state; - this.reason = reason; - } - - @Override - public boolean equals(Object object) { - if (!(object instanceof State)) { - return false; - } - - State that = (State) object; - return this.state.equals(that.state) && - this.reason.equals(that.reason); - } - - @Override - public int hashCode() { - int hash = 1; - hash = 17 * hash + state.hashCode(); - hash = 13 * hash + reason.hashCode(); - return hash; - } - - @Override - public String toString() { - return "reason: " + reason + ", state: " + state; - } - } - - public enum Condition { - FORCE, SAFE; - } - -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java deleted file mode 100644 index 2d7ba3afa7d..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * The response returned by the cluster controller's set-node-state APIs. - * - * @author hakonhall - */ -public class ClusterControllerStateResponse { - - @JsonProperty("wasModified") - public final boolean wasModified; - - @JsonProperty("reason") - public final String reason; - - @JsonCreator - public ClusterControllerStateResponse(@JsonProperty("wasModified") boolean wasModified, - @JsonProperty("reason") String reason) { - this.wasModified = wasModified; - this.reason = reason; - } - -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java index c4132d490dc..c98cefc6c86 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java @@ -1,59 +1,60 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.controller; +import ai.vespa.hosted.client.AbstractHttpClient; +import ai.vespa.hosted.client.HttpClient; +import ai.vespa.util.http.hc5.VespaHttpClientBuilder; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; -import com.yahoo.vespa.jaxrs.client.JaxRsStrategyFactory; -import com.yahoo.vespa.jaxrs.client.VespaJerseyJaxRsClientFactory; -import java.util.HashSet; +import java.io.IOException; import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; /** * @author bakksjo */ -@SuppressWarnings("removal") // VespaJerseyJaxRsClientFactory public class RetryingClusterControllerClientFactory extends AbstractComponent implements ClusterControllerClientFactory { - // TODO: Figure this port out dynamically. - public static final int HARDCODED_CLUSTERCONTROLLER_PORT = 19050; - public static final String CLUSTERCONTROLLER_API_PATH = "/"; - public static final String CLUSTERCONTROLLER_SCHEME = "http"; + private static Logger log = Logger.getLogger(RetryingClusterControllerClientFactory.class.getName()); - private final VespaJerseyJaxRsClientFactory jaxRsClientFactory; + private final HttpClient client; @Inject public RetryingClusterControllerClientFactory() { - this(new VespaJerseyJaxRsClientFactory("orchestrator-cluster-controller-client")); + this(AbstractHttpClient.wrapping(VespaHttpClientBuilder.create() + .setUserAgent("orchestrator-cluster-controller-client") + .build())); } - RetryingClusterControllerClientFactory(VespaJerseyJaxRsClientFactory jaxRsClientFactory) { - this.jaxRsClientFactory = jaxRsClientFactory; + RetryingClusterControllerClientFactory(HttpClient client) { + this.client = client; } @Override public ClusterControllerClient createClient(List clusterControllers, String clusterName) { - JaxRsStrategy jaxRsApi = - new JaxRsStrategyFactory( - new HashSet<>(clusterControllers), - HARDCODED_CLUSTERCONTROLLER_PORT, - jaxRsClientFactory, - CLUSTERCONTROLLER_SCHEME) - .apiWithRetries(ClusterControllerJaxRsApi.class, CLUSTERCONTROLLER_API_PATH) - // Use max iteration 1: The JaxRsStrategyFactory will try host 1, 2, then 3: - // - If host 1 responds, it will redirect to master if necessary. Otherwise - // - If host 2 responds, it will redirect to master if necessary. Otherwise - // - If host 3 responds, it may redirect to master if necessary (if they're up - // after all), but more likely there's no quorum and this will fail too. - // If there's only 1 CC, we'll try that one twice. - .setMaxIterations(clusterControllers.size() > 1 ? 1 : 2); - return new ClusterControllerClientImpl(jaxRsApi, clusterName); + List hosts = clusterControllers.size() == 1 + // If there's only 1 CC, we'll try that one twice. + ? List.of(clusterControllers.get(0), clusterControllers.get(0)) + // Otherwise, try each host once: + // * if host 1 responds, it will redirect to master if necessary; otherwise + // * if host 2 responds, it will redirect to master if necessary; otherwise + // * if host 3 responds, it may redirect to master if necessary (if they're up + // after all), but more likely there's no quorum and this will fail too. + : List.copyOf(clusterControllers); + return new ClusterControllerClientImpl(client, hosts, clusterName); } @Override public void deconstruct() { - jaxRsClientFactory.close(); + try { + client.close(); + } + catch (IOException e) { + log.log(Level.WARNING, "failed shutting down", e); + } } + } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java index d393117d57c..f3e3fd0e674 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.model; -import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; @@ -11,14 +10,8 @@ import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateErrorResponse; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; -import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.core.Response; -import java.io.IOException; import java.util.List; import java.util.Objects; import java.util.logging.Level; @@ -69,41 +62,7 @@ public class StorageNodeImpl implements StorageNode { ", node index " + nodeIndex + ", node state " + wantedNodeState); - ClusterControllerStateResponse response; - try { - response = client.setNodeState(context, nodeIndex, wantedNodeState); - } catch (IOException e) { - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT, - "Failed to communicate with cluster controllers " + clusterControllers + ": " + e, - e); - } catch (WebApplicationException e) { - Response webResponse = e.getResponse(); - // Response may contain detail message - ClusterControllerStateErrorResponse errorResponse = webResponse.readEntity(ClusterControllerStateErrorResponse.class); - String detail = errorResponse.message == null ? "" : ": " + errorResponse.message; - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, - "Failure from cluster controllers " + clusterControllers + " when setting node " + nodeIndex + - " in cluster " + clusterId + " to state " + wantedNodeState + detail, - e); - } catch (UncheckedTimeoutException e) { - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.DEADLINE_CONSTRAINT, - "Timeout while waiting for setNodeState(" + nodeIndex + ", " + wantedNodeState + - ") against " + clusterControllers + ": " + e.getMessage(), - e); - } - - if ( ! response.wasModified) { - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, - "Failed to set state to " + wantedNodeState + " in cluster controller: " + response.reason); - } + client.setNodeState(context, storageService.hostName(), nodeIndex, wantedNodeState); } @Override diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java index eba8d3d4d66..6e8e1eb0ccb 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java @@ -23,7 +23,6 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse; import com.yahoo.vespa.orchestrator.model.ApplicationApiFactory; import com.yahoo.vespa.orchestrator.model.NodeGroup; import com.yahoo.vespa.orchestrator.policy.BatchHostStateChangeDeniedException; @@ -460,18 +459,16 @@ public class OrchestratorImplTest { applicationApiFactory, flagSource); - ClusterControllerStateResponse accepted = new ClusterControllerStateResponse(true, "OK"); - ClusterControllerStateResponse denied = new ClusterControllerStateResponse(false, "NO"); - when(fooClient.setNodeState(any(), eq(1), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); - when(fooClient.setNodeState(any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); - when(barClient.setNodeState(any(), eq(0), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); - when(barClient.setNodeState(any(), eq(3), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); + when(fooClient.setNodeState(any(), any(), eq(1), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(barClient.setNodeState(any(), any(), eq(0), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(barClient.setNodeState(any(), any(), eq(3), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); assertTrue(orchestrator.isQuiescent(id)); - when(fooClient.setNodeState(any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(denied); + when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(false); assertFalse(orchestrator.isQuiescent(id)); - when(fooClient.setNodeState(any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenThrow(new RuntimeException()); + when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenThrow(new RuntimeException()); assertFalse(orchestrator.isQuiescent(id)); } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java index fcd4c8404a3..27928618eeb 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java @@ -2,15 +2,16 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.vespa.applicationmodel.ApplicationInstance; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; import com.yahoo.vespa.orchestrator.DummyServiceMonitor; import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.model.VespaModelUtil; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; -import java.io.IOException; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -22,13 +23,14 @@ import java.util.Set; * @author smorgrav */ public class ClusterControllerClientFactoryMock implements ClusterControllerClientFactory { + Map nodes = new HashMap<>(); public boolean isInMaintenance(ApplicationInstance appInstance, HostName hostName) { try { ClusterId clusterName = VespaModelUtil.getContentClusterName(appInstance, hostName); int storageNodeIndex = VespaModelUtil.getStorageNodeIndex(appInstance, hostName); - String globalMapKey = clusterName.s() + storageNodeIndex; + String globalMapKey = clusterName + "/" + storageNodeIndex; return nodes.getOrDefault(globalMapKey, ClusterControllerNodeState.UP) == ClusterControllerNodeState.MAINTENANCE; } catch (Exception e) { //Catch all - meant to catch cases where the node is not part of a storage cluster @@ -42,7 +44,7 @@ public class ClusterControllerClientFactoryMock implements ClusterControllerClie for (HostName host : hosts) { ClusterId clusterName = VespaModelUtil.getContentClusterName(app, host); int storageNodeIndex = VespaModelUtil.getStorageNodeIndex(app, host); - String globalMapKey = clusterName.s() + storageNodeIndex; + String globalMapKey = clusterName + "/" + storageNodeIndex; nodes.put(globalMapKey, ClusterControllerNodeState.UP); } } @@ -51,23 +53,16 @@ public class ClusterControllerClientFactoryMock implements ClusterControllerClie @Override public ClusterControllerClient createClient(List clusterControllers, String clusterName) { return new ClusterControllerClient() { - - @Override - public ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException { - nodes.put(clusterName + storageNodeIndex, wantedState); - return new ClusterControllerStateResponse(true, "Yes"); + @Override public boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException { + nodes.put(clusterName + "/" + storageNodeIndex, wantedState); + return true; } - - @Override - public ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException { - Set keyCopy = new HashSet<>(nodes.keySet()); - for (String s : keyCopy) { - if (s.startsWith(clusterName)) { - nodes.put(s, wantedState); - } - } - return new ClusterControllerStateResponse(true, "It works"); + @Override public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, + ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException { + nodes.replaceAll((key, state) -> key.startsWith(clusterName + "/") ? wantedState : state); } }; } + } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java new file mode 100644 index 00000000000..49bd0ebbd97 --- /dev/null +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java @@ -0,0 +1,158 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.orchestrator.controller; + +import ai.vespa.hosted.client.MockHttpClient; +import com.yahoo.test.ManualClock; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; +import com.yahoo.vespa.orchestrator.OrchestratorContext; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.time.Clock; +import java.time.Duration; +import java.util.List; + +import static com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState.DOWN; +import static com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState.MAINTENANCE; +import static com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState.UP; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +public class ClusterControllerClientImplTest { + + final HostName host = new HostName("node"); + final ManualClock clock = new ManualClock(); + final List clusterControllers = List.of(new HostName("host1"), new HostName("host2"), new HostName("host3")); + + MockHttpClient wire; + RetryingClusterControllerClientFactory factory; + ClusterControllerClient client; + + @Before + public void setup() { + wire = new MockHttpClient(); + factory = new RetryingClusterControllerClientFactory(wire); + client = factory.createClient(clusterControllers, "cc"); + } + + @After + public void teardown() { + factory.deconstruct(); + } + + @Test + public void verifySetNodeState() { + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc/storage/2?timeout=9.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"down\"},\"condition\":\"SAFE\"}", + body); + return "{ \"wasModified\": true }"; + }, + 200); + assertTrue(client.setNodeState(context, host, 2, DOWN)); + + clock.advance(Duration.ofSeconds(9)); + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc/storage/1?timeout=0.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"down\"},\"condition\":\"SAFE\"}", + body); + return "{ \"wasModified\": false, \"reason\": \"because\" }"; + }, + 200); + assertEquals("Changing the state of node would violate controller-set-node-state: Failed to set state to DOWN in cluster controller: because", + assertThrows(HostStateChangeDeniedException.class, + () -> client.setNodeState(context, host, 1, DOWN)) + .getMessage()); + } + + @Test + public void verifyProbeNodeState() { + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc/storage/2?timeout=59.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"maintenance\"},\"condition\":\"SAFE\",\"probe\":true}", + body); + return "{ \"wasModified\": false, \"reason\": \"no reason\" }"; + }, + 200); + assertFalse(client.setNodeState(OrchestratorContext.createContextForBatchProbe(clock), host, 2, MAINTENANCE)); + } + + @Test + public void verifySetApplicationState() { + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc?timeout=299.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"up\"},\"condition\":\"FORCE\"}", + body); + return "{ \"message\": \":<\" }"; + }, + 500); + assertEquals("Failed to set application app cluster name cc to cluster state UP due to: " + + "got status code 500 for POST http://host1:19050/cluster/v2/cc?timeout=299.6: :<", + assertThrows(ApplicationStateChangeDeniedException.class, + () -> client.setApplicationState(OrchestratorContext.createContextForAdminOp(clock), + new ApplicationInstanceId("app"), + UP)) + .getMessage()); + } + + @Test + public void verifyRetriesUntilTimeout() { + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + // IOException + wire.expect(request -> { + clock.advance(Duration.ofSeconds(6)); + throw new IOException("Oh no!"); + }); + // Redirect + wire.expect((url, body) -> { + assertEquals("http://host2:19050/cluster/v2/cc/storage/2?timeout=3.6", + url.asURI().toString()); + clock.advance(Duration.ofSeconds(4)); + return ""; + }, 302); + // Timeout + assertEquals("Changing the state of node would violate deadline: Timeout while waiting for setNodeState(2, UP) " + + "against [host1, host2, host3]: Timed out after PT10S", + assertThrows(HostStateChangeDeniedException.class, + () -> client.setNodeState(context, host, 2, UP)) + .getMessage()); + } + + @Test + public void testRetriesUntilExhaustion() { + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + for (int i = 0; i < clusterControllers.size(); i++) { + int j = i + 1; + wire.expect((url, body) -> { + assertEquals("http://host" + j + ":19050/cluster/v2/cc/storage/2?timeout=9.6", + url.asURI().toString()); + return ""; + }, + 503); + } + // All retries exhausted + assertEquals("Changing the state of node would violate controller-set-node-state: Failed setting node 2 in cluster cc to state UP: " + + "got status code 503 for POST http://host1:19050/cluster/v2/cc/storage/2?timeout=9.6", + assertThrows(HostStateChangeDeniedException.class, + () -> client.setNodeState(context, host, 2, UP)) + .getMessage()); + } + +} diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java deleted file mode 100644 index 655b7921612..00000000000 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; -import com.yahoo.vespa.jaxrs.client.LocalPassThroughJaxRsStrategy; -import com.yahoo.vespa.orchestrator.OrchestratorContext; -import org.junit.Before; -import org.junit.Test; - -import java.io.IOException; -import java.time.Duration; - -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class ClusterControllerClientTest { - private static final String CLUSTER_NAME = "clusterName"; - private static final int STORAGE_NODE_INDEX = 0; - - private final ClusterControllerJaxRsApi clusterControllerApi = mock(ClusterControllerJaxRsApi.class); - private final JaxRsStrategy strategyMock = new LocalPassThroughJaxRsStrategy<>(clusterControllerApi); - private final ClusterControllerClient clusterControllerClient = new ClusterControllerClientImpl(strategyMock, CLUSTER_NAME); - private final ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE; - private final OrchestratorContext context = mock(OrchestratorContext.class); - private final ClusterControllerClientTimeouts timeouts = mock(ClusterControllerClientTimeouts.class); - private final ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, ClusterControllerClientImpl.REQUEST_REASON); - - @Before - public void setUp() { - when(context.getClusterControllerTimeouts()).thenReturn(timeouts); - when(context.isProbe()).thenReturn(false); - when(timeouts.getServerTimeoutOrThrow()).thenReturn(Duration.ofSeconds(1)); - } - - @Test - public void correctParametersArePassedThrough() throws IOException { - setNodeStateAndVerify(null); - } - - @Test - public void probingIsCorrectlyPassedThrough() throws IOException { - when(context.isProbe()).thenReturn(true); - setNodeStateAndVerify(true); - } - - private void setNodeStateAndVerify(Boolean expectedProbe) throws IOException { - clusterControllerClient.setNodeState(context, STORAGE_NODE_INDEX, wantedState); - - final ClusterControllerStateRequest expectedNodeStateRequest = new ClusterControllerStateRequest( - state, ClusterControllerStateRequest.Condition.SAFE, expectedProbe); - - verify(clusterControllerApi, times(1)) - .setNodeState( - eq(CLUSTER_NAME), - eq(STORAGE_NODE_INDEX), - eq(1.0f), - eq(expectedNodeStateRequest)); - } -} diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java index 6f22ff74ad8..9a34ddc7d64 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java @@ -17,6 +17,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; public class ClusterControllerClientTimeoutsTest { + // The minimum time that allows for a single RPC to CC. private static final Duration MINIMUM_TIME_LEFT = CONNECT_TIMEOUT .plus(DOWNSTREAM_OVERHEAD) @@ -44,27 +45,26 @@ public class ClusterControllerClientTimeoutsTest { @Test public void makesManyRequestsWithShortProcessingTime() { - assertEquals(Duration.ofMillis(100), timeouts.getConnectTimeoutOrThrow()); - assertEquals(Duration.ofMillis(2900), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(100), timeouts.connectTimeout()); + assertEquals(Duration.ofMillis(2900), timeouts.readBudget().timeLeftOrThrow().get()); assertEquals(Duration.ofMillis(2600), timeouts.getServerTimeoutOrThrow()); clock.advance(Duration.ofMillis(100)); - assertEquals(Duration.ofMillis(100), timeouts.getConnectTimeoutOrThrow()); - assertEquals(Duration.ofMillis(2800), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(100), timeouts.connectTimeout()); + assertEquals(Duration.ofMillis(2800), timeouts.readBudget().timeLeftOrThrow().get()); assertEquals(Duration.ofMillis(2500), timeouts.getServerTimeoutOrThrow()); clock.advance(Duration.ofMillis(100)); - assertEquals(Duration.ofMillis(100), timeouts.getConnectTimeoutOrThrow()); - assertEquals(Duration.ofMillis(2700), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(100), timeouts.connectTimeout()); + assertEquals(Duration.ofMillis(2700), timeouts.readBudget().timeLeftOrThrow().get()); assertEquals(Duration.ofMillis(2400), timeouts.getServerTimeoutOrThrow()); } @Test public void alreadyTimedOut() { clock.advance(Duration.ofSeconds(4)); - try { timeouts.getServerTimeoutOrThrow(); fail(); @@ -89,4 +89,5 @@ public class ClusterControllerClientTimeoutsTest { clock.advance(originalTimeout.minus(MINIMUM_TIME_LEFT)); timeouts.getServerTimeoutOrThrow(); } + } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java deleted file mode 100644 index ab54eb48a1a..00000000000 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.yahoo.test.ManualClock; -import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.jaxrs.client.VespaJerseyJaxRsClientFactory; -import com.yahoo.vespa.orchestrator.OrchestratorContext; -import org.junit.Test; -import org.mockito.ArgumentCaptor; - -import java.io.IOException; -import java.time.Clock; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.junit.Assert.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class RetryingClusterControllerClientFactoryTest { - private final Clock clock = new ManualClock(); - - @Test - @SuppressWarnings("removal") // VespaJerseyJaxRsClientFactory - public void verifyJerseyCallForSetNodeState() throws IOException { - VespaJerseyJaxRsClientFactory clientFactory = mock(VespaJerseyJaxRsClientFactory.class); - ClusterControllerJaxRsApi api = mock(ClusterControllerJaxRsApi.class); - when(clientFactory.createClient(any())).thenReturn(api); - RetryingClusterControllerClientFactory factory = new RetryingClusterControllerClientFactory(clientFactory); - String clusterName = "clustername"; - HostName host1 = new HostName("host1"); - HostName host2 = new HostName("host2"); - HostName host3 = new HostName("host3"); - List clusterControllers = Arrays.asList(host1, host2, host3); - ClusterControllerClient client = factory.createClient(clusterControllers, clusterName); - OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); - int storageNode = 2; - ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE; - client.setNodeState(context, storageNode, wantedState); - - ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ClusterControllerStateRequest.class); - - verify(api, times(1)).setNodeState(eq(clusterName), eq(storageNode), eq(9.6f), requestCaptor.capture()); - ClusterControllerStateRequest request = requestCaptor.getValue(); - assertEquals(ClusterControllerStateRequest.Condition.SAFE, request.condition); - Map expectedState = new HashMap<>(); - expectedState.put("user", new ClusterControllerStateRequest.State(wantedState, "Orchestrator")); - assertEquals(expectedState, request.state); - } -} -- cgit v1.2.3