From 9dca5f5d584c41d09806dd128a73d62ce85a9154 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 29 Mar 2021 17:01:59 -0600 Subject: [PATCH 1/9] Integrate Node Shutdown API with cluster metadata This commit hooks up the Node Shutdown API to the Node Shutdown cluster metadata, so using the API will result in the appropriate writes to the cluster state. --- .../metadata/NodeShutdownComponentStatus.java | 4 + .../metadata/NodesShutdownMetadata.java | 49 +++++- .../metadata/SingleNodeShutdownMetadata.java | 166 +++++++++++++++++- .../metadata/NodesShutdownMetadataTests.java | 18 +- .../shutdown/DeleteShutdownNodeAction.java | 4 +- .../shutdown/GetShutdownStatusAction.java | 19 +- .../xpack/shutdown/PutShutdownNodeAction.java | 61 ++++++- .../shutdown/RestPutShutdownNodeAction.java | 14 +- .../TransportDeleteShutdownNodeAction.java | 32 +++- .../TransportGetShutdownStatusAction.java | 26 ++- .../TransportPutShutdownNodeAction.java | 45 ++++- 11 files changed, 396 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/NodeShutdownComponentStatus.java b/server/src/main/java/org/elasticsearch/cluster/metadata/NodeShutdownComponentStatus.java index 67b6ec9048022..1d6efcd6597ea 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/NodeShutdownComponentStatus.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/NodeShutdownComponentStatus.java @@ -46,6 +46,10 @@ public static NodeShutdownComponentStatus parse(XContentParser parser) { return PARSER.apply(parser, null); } + public NodeShutdownComponentStatus() { + this(SingleNodeShutdownMetadata.Status.NOT_STARTED, null, null); + } + public NodeShutdownComponentStatus( SingleNodeShutdownMetadata.Status status, @Nullable Long startedAtMillis, diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java index ce67b98260231..df3763ca41547 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java @@ -13,6 +13,7 @@ import org.elasticsearch.cluster.Diff; import org.elasticsearch.cluster.DiffableUtils; import org.elasticsearch.cluster.NamedDiff; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -21,8 +22,9 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; -import java.util.Collections; +import java.util.ArrayList; import java.util.EnumSet; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -81,11 +83,42 @@ public void writeTo(StreamOutput out) throws IOException { } /** - * Retrieve the data about nodes which are currently in the process of shutting down. - * @return A map of node IDs to information about the node's shutdown status. + * Retrieve metadata for a single node. + * @param nodeId The node ID to get the shutdown metadata for. + * @return The shutdown metadata for this node, if it exists, or {@code null} if it does not. */ - public Map getPerNodeInfo() { - return Collections.unmodifiableMap(nodes); + @Nullable + public SingleNodeShutdownMetadata getNodeMetadata(String nodeId) { + return nodes.get(nodeId); + } + + /** + * @return All {@link SingleNodeShutdownMetadata}s that currently exist. + */ + public List getAllNodeMetadata() { + return new ArrayList<>(nodes.values()); + } + + /** + * Add or update the shutdown metadata for a single node. + * @param nodeShutdownMetadata The single node shutdown metadata to add or update. + * @return A new {@link NodesShutdownMetadata} that reflects the updated value. + */ + public NodesShutdownMetadata putSingleNodeMetadata(SingleNodeShutdownMetadata nodeShutdownMetadata) { + HashMap newNodes = new HashMap<>(nodes); + newNodes.put(nodeShutdownMetadata.getNodeId(), nodeShutdownMetadata); + return new NodesShutdownMetadata(newNodes); + } + + /** + * Removes all shutdown metadata for a particular node ID. + * @param nodeId The node ID to remove shutdown metadata for. + * @return A new {@link NodesShutdownMetadata} that does not contain shutdown metadata for the given node. + */ + public NodesShutdownMetadata removeSingleNodeMetadata(String nodeId) { + HashMap newNodes = new HashMap<>(nodes); + newNodes.remove(nodeId); + return new NodesShutdownMetadata(newNodes); } @Override @@ -113,12 +146,12 @@ public boolean equals(Object o) { if (this == o) return true; if ((o instanceof NodesShutdownMetadata) == false) return false; NodesShutdownMetadata that = (NodesShutdownMetadata) o; - return getPerNodeInfo().equals(that.getPerNodeInfo()); + return nodes.equals(that.nodes); } @Override public int hashCode() { - return Objects.hash(getPerNodeInfo()); + return Objects.hash(nodes); } @Override @@ -150,7 +183,7 @@ public NodeShutdownMetadataDiff(StreamInput in) throws IOException { @Override public Metadata.Custom apply(Metadata.Custom part) { TreeMap newNodes = new TreeMap<>( - nodesDiff.apply(((NodesShutdownMetadata) part).getPerNodeInfo()) + nodesDiff.apply(((NodesShutdownMetadata) part).nodes) ); return new NodesShutdownMetadata(newNodes); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/SingleNodeShutdownMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/SingleNodeShutdownMetadata.java index 8bbe23ae73f2e..d533dbe06e7dc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/SingleNodeShutdownMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/SingleNodeShutdownMetadata.java @@ -89,15 +89,26 @@ public static SingleNodeShutdownMetadata parse(XContentParser parser) { private final NodeShutdownComponentStatus persistentTasksStatus; private final NodeShutdownComponentStatus pluginsStatus; - - public SingleNodeShutdownMetadata( + /** + * @param nodeId The node ID that this shutdown metadata refers to. + * @param type The type of shutdown. See {@link Type}. + * @param reason The reason for the shutdown, per the original shutdown request. + * @param status The overall status of this shutdown. + * @param startedAtMillis The timestamp at which this shutdown was requested. + * @param shardMigrationStatus The status of shard migrations away from this node. + * @param persistentTasksStatus The status of persistent task migration away from this node. + * @param pluginsStatus The status of plugin shutdown on this node. + */ + private SingleNodeShutdownMetadata( String nodeId, Type type, String reason, Status status, long startedAtMillis, NodeShutdownComponentStatus shardMigrationStatus, - NodeShutdownComponentStatus persistentTasksStatus, NodeShutdownComponentStatus pluginsStatus) { + NodeShutdownComponentStatus persistentTasksStatus, + NodeShutdownComponentStatus pluginsStatus + ) { this.nodeId = Objects.requireNonNull(nodeId, "node ID must not be null"); this.type = Objects.requireNonNull(type, "shutdown type must not be null"); this.reason = Objects.requireNonNull(reason, "shutdown reason must not be null"); @@ -143,7 +154,7 @@ public String getReason() { /** * @return The status of this node's shutdown. */ - public Status isStatus() { + public Status getStatus() { return status; } @@ -154,6 +165,27 @@ public long getStartedAtMillis() { return startedAtMillis; } + /** + * @return The status of shard migrations off of this node. + */ + public NodeShutdownComponentStatus getShardMigrationStatus() { + return shardMigrationStatus; + } + + /** + * @return The status of persistent task shutdown on this node. + */ + public NodeShutdownComponentStatus getPersistentTasksStatus() { + return persistentTasksStatus; + } + + /** + * @return The status of plugin shutdown on this node. + */ + public NodeShutdownComponentStatus getPluginsStatus() { + return pluginsStatus; + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(nodeId); @@ -213,11 +245,137 @@ public int hashCode() { ); } + public static Builder builder() { + return new Builder(); + } + + public static Builder builder(SingleNodeShutdownMetadata original) { + if (original == null) { + return builder(); + } + return new Builder() + .setNodeId(original.getNodeId()) + .setType(original.getType()) + .setReason(original.getReason()) + .setStatus(original.getStatus()) + .setStartedAtMillis(original.getStartedAtMillis()) + .setShardMigrationStatus(original.getShardMigrationStatus()) + .setPersistentTasksStatus(original.getPersistentTasksStatus()) + .setPluginsStatus(original.getPluginsStatus()); + } + + public static class Builder { + private String nodeId; + private Type type; + private String reason; + private long startedAtMillis = -1; + private Status status = Status.IN_PROGRESS; + private NodeShutdownComponentStatus shardMigrationStatus = new NodeShutdownComponentStatus(); + private NodeShutdownComponentStatus persistentTasksStatus = new NodeShutdownComponentStatus(); + private NodeShutdownComponentStatus pluginsStatus = new NodeShutdownComponentStatus(); + + private Builder() {} + + /** + * @param nodeId The node ID this metadata refers to. + * @return This builder. + */ + public Builder setNodeId(String nodeId) { + this.nodeId = nodeId; + return this; + } + + /** + * @param type The type of shutdown. + * @return This builder. + */ + public Builder setType(Type type) { + this.type = type; + return this; + } + + /** + * @param reason The reason for the shutdown. An arbitrary string provided by the user. + * @return This builder. + */ + public Builder setReason(String reason) { + this.reason = reason; + return this; + } + + /** + * @param startedAtMillis The timestamp at which this shutdown was requested. + * @return This builder. + */ + public Builder setStartedAtMillis(long startedAtMillis) { + this.startedAtMillis = startedAtMillis; + return this; + } + + /** + * @param status The status of this shutdown. + * @return This builder. + */ + public Builder setStatus(Status status) { + this.status = status; + return this; + } + + /** + * @param shardMigrationStatus An object describing the status of shard migration away from this node. + * @return This builder. + */ + public Builder setShardMigrationStatus(NodeShutdownComponentStatus shardMigrationStatus) { + this.shardMigrationStatus = shardMigrationStatus; + return this; + } + + /** + * @param persistentTasksStatus An object describing the status of persistent task migration away from this node. + * @return This builder. + */ + public Builder setPersistentTasksStatus(NodeShutdownComponentStatus persistentTasksStatus) { + this.persistentTasksStatus = persistentTasksStatus; + return this; + } + + /** + * @param pluginsStatus An object describing the status of plugin shutdown on this node. + * @return + */ + public Builder setPluginsStatus(NodeShutdownComponentStatus pluginsStatus) { + this.pluginsStatus = pluginsStatus; + return this; + } + + public SingleNodeShutdownMetadata build() { + if (startedAtMillis == -1) { + throw new IllegalArgumentException("start timestamp must be set"); + } + return new SingleNodeShutdownMetadata( + nodeId, + type, + reason, + status, + startedAtMillis, + shardMigrationStatus, + persistentTasksStatus, + pluginsStatus + ); + } + } + + /** + * Describes the type of node shutdown - permanent (REMOVE) or temporary (RESTART). + */ public enum Type { REMOVE, RESTART } + /** + * Describes the status of a component of shutdown. + */ public enum Status { NOT_STARTED, IN_PROGRESS, diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java index e971eda0f672f..8326e14c2aa32 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java @@ -45,15 +45,15 @@ protected NodesShutdownMetadata createTestInstance() { } private SingleNodeShutdownMetadata randomNodeShutdownInfo() { - return new SingleNodeShutdownMetadata( - randomAlphaOfLength(5), - randomBoolean() ? SingleNodeShutdownMetadata.Type.REMOVE : SingleNodeShutdownMetadata.Type.RESTART, - randomAlphaOfLength(5), - randomStatus(), - randomNonNegativeLong(), - randomComponentStatus(), - randomComponentStatus(), - randomComponentStatus()); + return SingleNodeShutdownMetadata.builder().setNodeId(randomAlphaOfLength(5)) + .setType(randomBoolean() ? SingleNodeShutdownMetadata.Type.REMOVE : SingleNodeShutdownMetadata.Type.RESTART) + .setReason(randomAlphaOfLength(5)) + .setStatus(randomStatus()) + .setStartedAtMillis(randomNonNegativeLong()) + .setShardMigrationStatus(randomComponentStatus()) + .setPersistentTasksStatus(randomComponentStatus()) + .setPluginsStatus(randomComponentStatus()) + .build(); } private SingleNodeShutdownMetadata.Status randomStatus() { diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/DeleteShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/DeleteShutdownNodeAction.java index 1574e888c6dca..153945e933d77 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/DeleteShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/DeleteShutdownNodeAction.java @@ -9,8 +9,8 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; +import org.elasticsearch.action.support.master.AcknowledgedRequest; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -26,7 +26,7 @@ public DeleteShutdownNodeAction() { super(NAME, AcknowledgedResponse::readFrom); } - public static class Request extends MasterNodeRequest { + public static class Request extends AcknowledgedRequest { private final String nodeId; diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java index d892853a6308e..52fb2d4c9f289 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java @@ -11,12 +11,15 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.ActionType; import org.elasticsearch.action.support.master.MasterNodeRequest; +import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Collections; +import java.util.List; public class GetShutdownStatusAction extends ActionType { @@ -55,21 +58,33 @@ public ActionRequestValidationException validate() { } public static class Response extends ActionResponse implements ToXContentObject { + List shutdownStatuses = Collections.emptyList(); - public Response(StreamInput in) throws IOException { + public Response(List shutdownStatuses) { + this.shutdownStatuses = shutdownStatuses; + } + public Response(StreamInput in) throws IOException { + in.readList(SingleNodeShutdownMetadata::new); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); + { + builder.startArray("nodes"); + for (SingleNodeShutdownMetadata nodeMetadata : shutdownStatuses) { + nodeMetadata.toXContent(builder, params); + } + builder.endArray(); + } builder.endObject(); return builder; } @Override public void writeTo(StreamOutput out) throws IOException { - + out.writeList(shutdownStatuses); } } } diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/PutShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/PutShutdownNodeAction.java index 688d7dcd5ce33..94a10b12faabc 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/PutShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/PutShutdownNodeAction.java @@ -9,11 +9,15 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; +import org.elasticsearch.action.support.master.AcknowledgedRequest; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.action.support.master.MasterNodeRequest; +import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; @@ -26,35 +30,82 @@ public PutShutdownNodeAction() { super(NAME, AcknowledgedResponse::readFrom); } - public static class Request extends MasterNodeRequest { + public static class Request extends AcknowledgedRequest { private final String nodeId; + private final SingleNodeShutdownMetadata.Type type; + private final String reason; - public Request(String nodeId) { + private static final ParseField TYPE_FIELD = new ParseField("type"); + public static final ParseField REASON_FIELD = new ParseField("reason"); + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "put_node_shutdown_request", + false, + (a, nodeId) -> new Request(nodeId, SingleNodeShutdownMetadata.Type.valueOf((String) a[0]), (String) a[1]) + ); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), TYPE_FIELD); + PARSER.declareString(ConstructingObjectParser.constructorArg(), REASON_FIELD); + } + + public static Request parseRequest(String nodeId, XContentParser parser) { + return PARSER.apply(parser, nodeId); + } + + public Request(String nodeId, SingleNodeShutdownMetadata.Type type, String reason) { this.nodeId = nodeId; + this.type = type; + this.reason = reason; } public Request(StreamInput in) throws IOException { this.nodeId = in.readString(); + this.type = in.readEnum(SingleNodeShutdownMetadata.Type.class); + this.reason = in.readString(); } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(this.nodeId); + out.writeEnum(type); + out.writeString(reason); } public String getNodeId() { return nodeId; } + public SingleNodeShutdownMetadata.Type getType() { + return type; + } + + public String getReason() { + return reason; + } + @Override public ActionRequestValidationException validate() { + ActionRequestValidationException arve = new ActionRequestValidationException(); + if (Strings.hasText(nodeId) == false) { - ActionRequestValidationException arve = new ActionRequestValidationException(); arve.addValidationError("the node id to shutdown is required"); + } + + if (type == null) { + arve.addValidationError("the shutdown type is required"); + } + + if (Strings.hasText(nodeId) == false) { + arve.addValidationError("the reason for shutdown is required"); + } + + if (arve.validationErrors().isEmpty() == false) { return arve; + } else { + return null; } - return null; } } } diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/RestPutShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/RestPutShutdownNodeAction.java index b1e27da9ca1fe..a26678511a585 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/RestPutShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/RestPutShutdownNodeAction.java @@ -8,10 +8,12 @@ package org.elasticsearch.xpack.shutdown; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestToXContentListener; +import java.io.IOException; import java.util.List; public class RestPutShutdownNodeAction extends BaseRestHandler { @@ -27,12 +29,12 @@ public List routes() { } @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { String nodeId = request.param("nodeId"); - return channel -> client.execute( - PutShutdownNodeAction.INSTANCE, - new PutShutdownNodeAction.Request(nodeId), - new RestToXContentListener<>(channel) - ); + try (XContentParser parser = request.contentParser()) { + PutShutdownNodeAction.Request parsedRequest = PutShutdownNodeAction.Request.parseRequest(nodeId, parser); + + return channel -> client.execute(PutShutdownNodeAction.INSTANCE, parsedRequest, new RestToXContentListener<>(channel)); + } } } diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java index 8446e87b144d5..b8a1c4326becd 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java @@ -11,10 +11,13 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction; +import org.elasticsearch.cluster.AckedClusterStateUpdateTask; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.NodesShutdownMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.tasks.Task; @@ -49,8 +52,33 @@ protected void masterOperation( ClusterState state, ActionListener listener ) throws Exception { - // TODO: implement me! - listener.onResponse(AcknowledgedResponse.of(true)); + { // This block solely to ensure this NodesShutdownMetadata isn't accidentally used in the cluster state update task below + NodesShutdownMetadata nodesShutdownMetadata = state.metadata().custom(NodesShutdownMetadata.TYPE); + if (nodesShutdownMetadata.getNodeMetadata(request.getNodeId()) == null) { + throw new IllegalArgumentException("node [" + request.getNodeId() + "] is not currently shutting down"); + } + } + + clusterService.submitStateUpdateTask( + "delete-node-shutdown-" + request.getNodeId(), + new AckedClusterStateUpdateTask(request, listener) { + @Override + public ClusterState execute(ClusterState currentState) throws Exception { + NodesShutdownMetadata currentShutdownMetadata = currentState.metadata().custom(NodesShutdownMetadata.TYPE); + + return ClusterState.builder(currentState) + .metadata( + Metadata.builder(currentState.metadata()) + .putCustom( + NodesShutdownMetadata.TYPE, + currentShutdownMetadata.removeSingleNodeMetadata(request.getNodeId()) + ) + ) + .build(); + + } + } + ); } @Override diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java index 29d2a18ffab8f..90095d225384b 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java @@ -14,12 +14,20 @@ import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.NodesShutdownMetadata; +import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; + public class TransportGetShutdownStatusAction extends TransportMasterNodeAction< GetShutdownStatusAction.Request, GetShutdownStatusAction.Response> { @@ -51,8 +59,22 @@ protected void masterOperation( ClusterState state, ActionListener listener ) throws Exception { - // TODO: implement me! - listener.onResponse(new GetShutdownStatusAction.Response(null)); + NodesShutdownMetadata nodesShutdownMetadata = state.metadata().custom(NodesShutdownMetadata.TYPE); + + GetShutdownStatusAction.Response response; + if (nodesShutdownMetadata == null) { + response = new GetShutdownStatusAction.Response(new ArrayList<>()); + } else if (request.getNodeIds().length == 0) { + response = new GetShutdownStatusAction.Response(nodesShutdownMetadata.getAllNodeMetadata()); + } else { + final List shutdownStatuses = Arrays.stream(request.getNodeIds()) + .map(nodesShutdownMetadata::getNodeMetadata) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + response = new GetShutdownStatusAction.Response(shutdownStatuses); + } + + listener.onResponse(response); } @Override diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java index 3b395565ce53f..28d3a49465477 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java @@ -11,16 +11,23 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction; +import org.elasticsearch.cluster.AckedClusterStateUpdateTask; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.NodesShutdownMetadata; +import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import java.util.HashMap; +import java.util.Objects; + public class TransportPutShutdownNodeAction extends AcknowledgedTransportMasterNodeAction { @Inject public TransportPutShutdownNodeAction( @@ -49,8 +56,42 @@ protected void masterOperation( ClusterState state, ActionListener listener ) throws Exception { - // TODO: implement me! - listener.onResponse(AcknowledgedResponse.of(true)); + // Verify that the requested node actually exists in the cluster + if (state.nodes().nodeExists(request.getNodeId()) == false) { + throw new IllegalArgumentException("there is no node with id [" + request.getNodeId() + "] in this cluster"); + } + + clusterService.submitStateUpdateTask( + "put-node-shutdown-" + request.getNodeId(), + new AckedClusterStateUpdateTask(request, listener) { + @Override + public ClusterState execute(ClusterState currentState) { + NodesShutdownMetadata currentShutdownMetadata = currentState.metadata().custom(NodesShutdownMetadata.TYPE); + if (currentShutdownMetadata == null) { + currentShutdownMetadata = new NodesShutdownMetadata(new HashMap<>()); + } + + // Verify that there's not already a shutdown metadata for this node + if (Objects.nonNull(currentShutdownMetadata.getNodeMetadata(request.getNodeId()))) { + throw new IllegalArgumentException("node [" + request.getNodeId() + "] is already shutting down"); + } + + SingleNodeShutdownMetadata newNodeMetadata = SingleNodeShutdownMetadata.builder() + .setNodeId(request.getNodeId()) + .setType(request.getType()) + .setReason(request.getReason()) + .setStartedAtMillis(System.currentTimeMillis()) + .build(); + + return ClusterState.builder(currentState) + .metadata( + Metadata.builder(currentState.metadata()) + .putCustom(NodesShutdownMetadata.TYPE, currentShutdownMetadata.putSingleNodeMetadata(newNodeMetadata)) + ) + .build(); + } + } + ); } @Override From cc3f92ecc88f78614e2bc6944fc3fd875375a054 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Apr 2021 15:30:02 -0600 Subject: [PATCH 2/9] Some unit tests + equals and hashcode where appropriate --- .../metadata/NodesShutdownMetadataTests.java | 35 +++++++++++ .../shutdown/GetShutdownStatusAction.java | 41 +++++++++++- .../GetShutdownStatusRequestTests.java | 38 +++++++++++ .../GetShutdownStatusResponseTests.java | 63 +++++++++++++++++++ 4 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java create mode 100644 x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java index 8326e14c2aa32..c0cead3742410 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java @@ -16,12 +16,47 @@ import java.io.IOException; import java.util.ArrayList; import java.util.EnumSet; +import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.function.Function; import java.util.stream.Collectors; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; + public class NodesShutdownMetadataTests extends AbstractDiffableSerializationTestCase { + public void testInsertNewNodeShutdownMetadata() { + NodesShutdownMetadata nodesShutdownMetadata = new NodesShutdownMetadata(new HashMap<>()); + SingleNodeShutdownMetadata newNodeMetadata = randomNodeShutdownInfo(); + + nodesShutdownMetadata = nodesShutdownMetadata.putSingleNodeMetadata(newNodeMetadata); + + assertThat(nodesShutdownMetadata.getNodeMetadata(newNodeMetadata.getNodeId()), equalTo(newNodeMetadata)); + assertThat(nodesShutdownMetadata.getAllNodeMetadata(), contains(newNodeMetadata)); + } + + public void testRemoveShutdownMetadata() { + NodesShutdownMetadata nodesShutdownMetadata = new NodesShutdownMetadata(new HashMap<>()); + List nodes = randomList(1, 20, this::randomNodeShutdownInfo); + + for (SingleNodeShutdownMetadata node : nodes) { + nodesShutdownMetadata = nodesShutdownMetadata.putSingleNodeMetadata(node); + } + + SingleNodeShutdownMetadata nodeToRemove = randomFrom(nodes); + nodesShutdownMetadata = nodesShutdownMetadata.removeSingleNodeMetadata(nodeToRemove.getNodeId()); + + assertThat(nodesShutdownMetadata.getNodeMetadata(nodeToRemove.getNodeId()), nullValue()); + assertThat(nodesShutdownMetadata.getAllNodeMetadata(), hasSize(nodes.size() - 1)); + assertThat(nodesShutdownMetadata.getAllNodeMetadata(), not(hasItem(nodeToRemove))); + } + @Override protected Writeable.Reader> diffReader() { return NodesShutdownMetadata.NodeShutdownMetadataDiff::new; diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java index 52fb2d4c9f289..daaf9b3ccc2f0 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java @@ -12,14 +12,17 @@ import org.elasticsearch.action.ActionType; import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; public class GetShutdownStatusAction extends ActionType { @@ -55,17 +58,34 @@ public String[] getNodeIds() { public ActionRequestValidationException validate() { return null; } + + @Override public boolean equals(Object o) { + if (this == o) + return true; + if ((o instanceof Request) == false) + return false; + Request request = (Request) o; + return Arrays.equals(nodeIds, request.nodeIds); + } + + @Override public int hashCode() { + return Arrays.hashCode(nodeIds); + } } public static class Response extends ActionResponse implements ToXContentObject { List shutdownStatuses = Collections.emptyList(); public Response(List shutdownStatuses) { - this.shutdownStatuses = shutdownStatuses; + this.shutdownStatuses = Objects.requireNonNull(shutdownStatuses, "shutdown statuses must not be null"); } public Response(StreamInput in) throws IOException { - in.readList(SingleNodeShutdownMetadata::new); + shutdownStatuses = in.readList(SingleNodeShutdownMetadata::new); + } + + public List getShutdownStatuses() { + return shutdownStatuses; } @Override @@ -86,5 +106,22 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public void writeTo(StreamOutput out) throws IOException { out.writeList(shutdownStatuses); } + + @Override public boolean equals(Object o) { + if (this == o) + return true; + if ((o instanceof Response) == false) + return false; + Response response = (Response) o; + return shutdownStatuses.equals(response.shutdownStatuses); + } + + @Override public int hashCode() { + return Objects.hash(shutdownStatuses); + } + + @Override public String toString() { + return Strings.toString(this); + } } } diff --git a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java new file mode 100644 index 0000000000000..9f3c1b90573e0 --- /dev/null +++ b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.shutdown; + +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import java.io.IOException; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class GetShutdownStatusRequestTests extends AbstractWireSerializingTestCase { + + @Override protected Writeable.Reader instanceReader() { + return GetShutdownStatusAction.Request::readFrom; + } + + @Override protected GetShutdownStatusAction.Request createTestInstance() { + return new GetShutdownStatusAction.Request(randomList(0, 20, () -> randomAlphaOfLengthBetween(15,25)).toArray(Strings.EMPTY_ARRAY)); + } + + @Override + protected GetShutdownStatusAction.Request mutateInstance(GetShutdownStatusAction.Request instance) throws IOException { + Set oldIds = new HashSet<>(Arrays.asList(instance.getNodeIds())); + String[] newNodeIds = randomList(1, 20, () -> randomValueOtherThanMany(oldIds::contains, () -> randomAlphaOfLengthBetween(15, 25))) + .toArray(Strings.EMPTY_ARRAY); + + return new GetShutdownStatusAction.Request(newNodeIds); + } +} diff --git a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java new file mode 100644 index 0000000000000..3b472657f16a1 --- /dev/null +++ b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java @@ -0,0 +1,63 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.shutdown; + +import org.elasticsearch.cluster.metadata.NodeShutdownComponentStatus; +import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class GetShutdownStatusResponseTests extends AbstractWireSerializingTestCase { + @Override protected Writeable.Reader instanceReader() { + return GetShutdownStatusAction.Response::new; + } + + @Override protected GetShutdownStatusAction.Response createTestInstance() { + List nodeMetadatas = randomList(0, 20, GetShutdownStatusResponseTests::randomNodeShutdownInfo); + return new GetShutdownStatusAction.Response(nodeMetadatas); + } + + @Override protected GetShutdownStatusAction.Response mutateInstance(GetShutdownStatusAction.Response instance) throws IOException { + Set oldNodes = new HashSet<>(instance.getShutdownStatuses()); + List newNodes = randomList(1, 20, () -> randomValueOtherThanMany(oldNodes::contains, + GetShutdownStatusResponseTests::randomNodeShutdownInfo)); + + return new GetShutdownStatusAction.Response(newNodes); + } + + public static SingleNodeShutdownMetadata randomNodeShutdownInfo() { + return SingleNodeShutdownMetadata.builder().setNodeId(randomAlphaOfLength(5)) + .setType(randomBoolean() ? SingleNodeShutdownMetadata.Type.REMOVE : SingleNodeShutdownMetadata.Type.RESTART) + .setReason(randomAlphaOfLength(5)) + .setStatus(randomStatus()) + .setStartedAtMillis(randomNonNegativeLong()) + .setShardMigrationStatus(randomComponentStatus()) + .setPersistentTasksStatus(randomComponentStatus()) + .setPluginsStatus(randomComponentStatus()) + .build(); + } + + public static SingleNodeShutdownMetadata.Status randomStatus() { + return randomFrom(new ArrayList<>(EnumSet.allOf(SingleNodeShutdownMetadata.Status.class))); + } + + public static NodeShutdownComponentStatus randomComponentStatus() { + return new NodeShutdownComponentStatus( + randomStatus(), + randomBoolean() ? null : randomNonNegativeLong(), + randomBoolean() ? null : randomAlphaOfLengthBetween(4, 10) + ); + } +} From 07ceb0a03a8429fa313d96cec93b975118890102 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Apr 2021 16:13:55 -0600 Subject: [PATCH 3/9] Unused import --- .../xpack/shutdown/GetShutdownStatusRequestTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java index 9f3c1b90573e0..0d52f8a0ee4df 100644 --- a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java +++ b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java @@ -14,7 +14,6 @@ import java.io.IOException; import java.util.Arrays; import java.util.HashSet; -import java.util.List; import java.util.Set; public class GetShutdownStatusRequestTests extends AbstractWireSerializingTestCase { From 8d9ec87d8680adacac9f04745e5a4efcf51ca6af Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Apr 2021 16:14:38 -0600 Subject: [PATCH 4/9] Spotless --- .../shutdown/GetShutdownStatusAction.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java index daaf9b3ccc2f0..6833932f6c9d1 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusAction.java @@ -59,16 +59,16 @@ public ActionRequestValidationException validate() { return null; } - @Override public boolean equals(Object o) { - if (this == o) - return true; - if ((o instanceof Request) == false) - return false; + @Override + public boolean equals(Object o) { + if (this == o) return true; + if ((o instanceof Request) == false) return false; Request request = (Request) o; return Arrays.equals(nodeIds, request.nodeIds); } - @Override public int hashCode() { + @Override + public int hashCode() { return Arrays.hashCode(nodeIds); } } @@ -107,20 +107,21 @@ public void writeTo(StreamOutput out) throws IOException { out.writeList(shutdownStatuses); } - @Override public boolean equals(Object o) { - if (this == o) - return true; - if ((o instanceof Response) == false) - return false; + @Override + public boolean equals(Object o) { + if (this == o) return true; + if ((o instanceof Response) == false) return false; Response response = (Response) o; return shutdownStatuses.equals(response.shutdownStatuses); } - @Override public int hashCode() { + @Override + public int hashCode() { return Objects.hash(shutdownStatuses); } - @Override public String toString() { + @Override + public String toString() { return Strings.toString(this); } } From c64fe3482a30d26df37d1f7d0243d76d14f8ba8d Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Apr 2021 16:15:03 -0600 Subject: [PATCH 5/9] More Spotless --- .../xpack/shutdown/GetShutdownStatusRequestTests.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java index 0d52f8a0ee4df..b8da9341f9700 100644 --- a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java +++ b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusRequestTests.java @@ -18,12 +18,16 @@ public class GetShutdownStatusRequestTests extends AbstractWireSerializingTestCase { - @Override protected Writeable.Reader instanceReader() { + @Override + protected Writeable.Reader instanceReader() { return GetShutdownStatusAction.Request::readFrom; } - @Override protected GetShutdownStatusAction.Request createTestInstance() { - return new GetShutdownStatusAction.Request(randomList(0, 20, () -> randomAlphaOfLengthBetween(15,25)).toArray(Strings.EMPTY_ARRAY)); + @Override + protected GetShutdownStatusAction.Request createTestInstance() { + return new GetShutdownStatusAction.Request( + randomList(0, 20, () -> randomAlphaOfLengthBetween(15, 25)).toArray(Strings.EMPTY_ARRAY) + ); } @Override From e6d5190d5fd9858eaa7a3ad5e2ba1dbdcfd8ac68 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Apr 2021 16:15:29 -0600 Subject: [PATCH 6/9] An unending amount of Spotless --- .../GetShutdownStatusResponseTests.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java index 3b472657f16a1..a84c08c8f9014 100644 --- a/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java +++ b/x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/GetShutdownStatusResponseTests.java @@ -20,25 +20,32 @@ import java.util.Set; public class GetShutdownStatusResponseTests extends AbstractWireSerializingTestCase { - @Override protected Writeable.Reader instanceReader() { + @Override + protected Writeable.Reader instanceReader() { return GetShutdownStatusAction.Response::new; } - @Override protected GetShutdownStatusAction.Response createTestInstance() { + @Override + protected GetShutdownStatusAction.Response createTestInstance() { List nodeMetadatas = randomList(0, 20, GetShutdownStatusResponseTests::randomNodeShutdownInfo); return new GetShutdownStatusAction.Response(nodeMetadatas); } - @Override protected GetShutdownStatusAction.Response mutateInstance(GetShutdownStatusAction.Response instance) throws IOException { + @Override + protected GetShutdownStatusAction.Response mutateInstance(GetShutdownStatusAction.Response instance) throws IOException { Set oldNodes = new HashSet<>(instance.getShutdownStatuses()); - List newNodes = randomList(1, 20, () -> randomValueOtherThanMany(oldNodes::contains, - GetShutdownStatusResponseTests::randomNodeShutdownInfo)); + List newNodes = randomList( + 1, + 20, + () -> randomValueOtherThanMany(oldNodes::contains, GetShutdownStatusResponseTests::randomNodeShutdownInfo) + ); return new GetShutdownStatusAction.Response(newNodes); } public static SingleNodeShutdownMetadata randomNodeShutdownInfo() { - return SingleNodeShutdownMetadata.builder().setNodeId(randomAlphaOfLength(5)) + return SingleNodeShutdownMetadata.builder() + .setNodeId(randomAlphaOfLength(5)) .setType(randomBoolean() ? SingleNodeShutdownMetadata.Type.REMOVE : SingleNodeShutdownMetadata.Type.RESTART) .setReason(randomAlphaOfLength(5)) .setStatus(randomStatus()) From b6e2fdac93443239ae02ae6bdc530676dc1e03d5 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Apr 2021 16:18:08 -0600 Subject: [PATCH 7/9] Integration test --- x-pack/plugin/shutdown/qa/build.gradle | 0 .../shutdown/qa/multi-node/build.gradle | 20 ++++++ .../xpack/shutdown/NodeShutdownIT.java | 65 +++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 x-pack/plugin/shutdown/qa/build.gradle create mode 100644 x-pack/plugin/shutdown/qa/multi-node/build.gradle create mode 100644 x-pack/plugin/shutdown/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownIT.java diff --git a/x-pack/plugin/shutdown/qa/build.gradle b/x-pack/plugin/shutdown/qa/build.gradle new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/x-pack/plugin/shutdown/qa/multi-node/build.gradle b/x-pack/plugin/shutdown/qa/multi-node/build.gradle new file mode 100644 index 0000000000000..40205e8aa6a01 --- /dev/null +++ b/x-pack/plugin/shutdown/qa/multi-node/build.gradle @@ -0,0 +1,20 @@ +apply plugin: 'elasticsearch.java-rest-test' + +dependencies { + javaRestTestImplementation(testArtifact(project(xpackModule('core')))) +} + +def clusterCredentials = [username: System.getProperty('tests.rest.cluster.username', 'test_admin'), + password: System.getProperty('tests.rest.cluster.password', 'x-pack-test-password')] + +tasks.named("javaRestTest").configure { + systemProperty 'tests.rest.cluster.username', clusterCredentials.username + systemProperty 'tests.rest.cluster.password', clusterCredentials.password +} + +testClusters.all { + testDistribution = 'DEFAULT' + numberOfNodes = 4 + + systemProperty 'es.shutdown_feature_flag_enabled', 'true' +} diff --git a/x-pack/plugin/shutdown/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownIT.java b/x-pack/plugin/shutdown/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownIT.java new file mode 100644 index 0000000000000..ad88b24ae2844 --- /dev/null +++ b/x-pack/plugin/shutdown/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownIT.java @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.shutdown; + +import org.elasticsearch.client.Request; +import org.elasticsearch.test.rest.ESRestTestCase; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; + +public class NodeShutdownIT extends ESRestTestCase { + + @SuppressWarnings("unchecked") + public void testCRUD() throws Exception { + Request nodesRequest = new Request("GET", "_nodes"); + Map nodesResponse = responseAsMap(client().performRequest(nodesRequest)); + Map nodesObject = (Map) nodesResponse.get("nodes"); + + String nodeIdToShutdown = randomFrom(nodesObject.keySet()); + String reason = "testing node shutdown crud: " + randomAlphaOfLength(5); + String type = randomFrom("RESTART", "REMOVE"); + + // Ensure if we do a GET before the cluster metadata is set up, we don't get an error + assertNoShuttingDownNodes(nodeIdToShutdown); + + // Put a shutdown request + Request putShutdown = new Request("PUT", "_nodes/" + nodeIdToShutdown + "/shutdown"); + putShutdown.setJsonEntity("{\"type\": \"" + type + "\", \"reason\": \"" + reason + "\"}"); + assertOK(client().performRequest(putShutdown)); + + // Ensure we can read it back + { + Request getShutdownStatus = new Request("GET", "_nodes/" + nodeIdToShutdown + "/shutdown"); + Map statusResponse = responseAsMap(client().performRequest(getShutdownStatus)); + List> nodesArray = (List>) statusResponse.get("nodes"); + assertThat(nodesArray, hasSize(1)); + assertThat(nodesArray.get(0).get("node_id"), equalTo(nodeIdToShutdown)); + assertThat(nodesArray.get(0).get("type"), equalTo(type)); + assertThat(nodesArray.get(0).get("reason"), equalTo(reason)); + } + + // Delete it and make sure it's deleted + Request deleteRequest = new Request("DELETE", "_nodes/" + nodeIdToShutdown + "/shutdown"); + assertOK(client().performRequest(deleteRequest)); + assertNoShuttingDownNodes(nodeIdToShutdown); + } + + @SuppressWarnings("unchecked") + private void assertNoShuttingDownNodes(String nodeIdToShutdown) throws IOException { + Request getShutdownStatus = new Request("GET", "_nodes/" + nodeIdToShutdown + "/shutdown"); + Map statusResponse = responseAsMap(client().performRequest(getShutdownStatus)); + List> nodesArray = (List>) statusResponse.get("nodes"); + assertThat(nodesArray, empty()); + } +} From 2e4cab6a8c51c349fc38438b8349243685caf652 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 5 Apr 2021 16:59:32 -0600 Subject: [PATCH 8/9] Per review, just expose the map from NodesShutdownMetadata --- .../metadata/NodesShutdownMetadata.java | 19 ++++--------------- .../metadata/NodesShutdownMetadataTests.java | 10 +++++----- .../TransportDeleteShutdownNodeAction.java | 2 +- .../TransportGetShutdownStatusAction.java | 6 ++++-- .../TransportPutShutdownNodeAction.java | 2 +- 5 files changed, 15 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java index df3763ca41547..441417641e03c 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadata.java @@ -13,7 +13,6 @@ import org.elasticsearch.cluster.Diff; import org.elasticsearch.cluster.DiffableUtils; import org.elasticsearch.cluster.NamedDiff; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -22,7 +21,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; -import java.util.ArrayList; +import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -83,20 +82,10 @@ public void writeTo(StreamOutput out) throws IOException { } /** - * Retrieve metadata for a single node. - * @param nodeId The node ID to get the shutdown metadata for. - * @return The shutdown metadata for this node, if it exists, or {@code null} if it does not. + * @return A map of NodeID to shutdown metadata. */ - @Nullable - public SingleNodeShutdownMetadata getNodeMetadata(String nodeId) { - return nodes.get(nodeId); - } - - /** - * @return All {@link SingleNodeShutdownMetadata}s that currently exist. - */ - public List getAllNodeMetadata() { - return new ArrayList<>(nodes.values()); + public Map getAllNodeMetdataMap() { + return Collections.unmodifiableMap(nodes); } /** diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java index c0cead3742410..db8fb03fd1da9 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/NodesShutdownMetadataTests.java @@ -37,8 +37,8 @@ public void testInsertNewNodeShutdownMetadata() { nodesShutdownMetadata = nodesShutdownMetadata.putSingleNodeMetadata(newNodeMetadata); - assertThat(nodesShutdownMetadata.getNodeMetadata(newNodeMetadata.getNodeId()), equalTo(newNodeMetadata)); - assertThat(nodesShutdownMetadata.getAllNodeMetadata(), contains(newNodeMetadata)); + assertThat(nodesShutdownMetadata.getAllNodeMetdataMap().get(newNodeMetadata.getNodeId()), equalTo(newNodeMetadata)); + assertThat(nodesShutdownMetadata.getAllNodeMetdataMap().values(), contains(newNodeMetadata)); } public void testRemoveShutdownMetadata() { @@ -52,9 +52,9 @@ public void testRemoveShutdownMetadata() { SingleNodeShutdownMetadata nodeToRemove = randomFrom(nodes); nodesShutdownMetadata = nodesShutdownMetadata.removeSingleNodeMetadata(nodeToRemove.getNodeId()); - assertThat(nodesShutdownMetadata.getNodeMetadata(nodeToRemove.getNodeId()), nullValue()); - assertThat(nodesShutdownMetadata.getAllNodeMetadata(), hasSize(nodes.size() - 1)); - assertThat(nodesShutdownMetadata.getAllNodeMetadata(), not(hasItem(nodeToRemove))); + assertThat(nodesShutdownMetadata.getAllNodeMetdataMap().get(nodeToRemove.getNodeId()), nullValue()); + assertThat(nodesShutdownMetadata.getAllNodeMetdataMap().values(), hasSize(nodes.size() - 1)); + assertThat(nodesShutdownMetadata.getAllNodeMetdataMap().values(), not(hasItem(nodeToRemove))); } @Override diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java index b8a1c4326becd..79941ef98d507 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportDeleteShutdownNodeAction.java @@ -54,7 +54,7 @@ protected void masterOperation( ) throws Exception { { // This block solely to ensure this NodesShutdownMetadata isn't accidentally used in the cluster state update task below NodesShutdownMetadata nodesShutdownMetadata = state.metadata().custom(NodesShutdownMetadata.TYPE); - if (nodesShutdownMetadata.getNodeMetadata(request.getNodeId()) == null) { + if (nodesShutdownMetadata.getAllNodeMetdataMap().get(request.getNodeId()) == null) { throw new IllegalArgumentException("node [" + request.getNodeId() + "] is not currently shutting down"); } } diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java index 90095d225384b..ed835b8250364 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -65,10 +66,11 @@ protected void masterOperation( if (nodesShutdownMetadata == null) { response = new GetShutdownStatusAction.Response(new ArrayList<>()); } else if (request.getNodeIds().length == 0) { - response = new GetShutdownStatusAction.Response(nodesShutdownMetadata.getAllNodeMetadata()); + response = new GetShutdownStatusAction.Response(new ArrayList<>(nodesShutdownMetadata.getAllNodeMetdataMap().values())); } else { + Map nodeShutdownMetadataMap = nodesShutdownMetadata.getAllNodeMetdataMap(); final List shutdownStatuses = Arrays.stream(request.getNodeIds()) - .map(nodesShutdownMetadata::getNodeMetadata) + .map(nodeShutdownMetadataMap::get) .filter(Objects::nonNull) .collect(Collectors.toList()); response = new GetShutdownStatusAction.Response(shutdownStatuses); diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java index 28d3a49465477..bac46259e5265 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java @@ -72,7 +72,7 @@ public ClusterState execute(ClusterState currentState) { } // Verify that there's not already a shutdown metadata for this node - if (Objects.nonNull(currentShutdownMetadata.getNodeMetadata(request.getNodeId()))) { + if (Objects.nonNull(currentShutdownMetadata.getAllNodeMetdataMap().get(request.getNodeId()))) { throw new IllegalArgumentException("node [" + request.getNodeId() + "] is already shutting down"); } From 7781a00d319a2a34ad87147f0956de7fcb186e42 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 5 Apr 2021 17:39:49 -0600 Subject: [PATCH 9/9] Remove node-in-cluster check per review --- .../xpack/shutdown/TransportPutShutdownNodeAction.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java index bac46259e5265..6bcfbb5e2d581 100644 --- a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java +++ b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportPutShutdownNodeAction.java @@ -56,11 +56,6 @@ protected void masterOperation( ClusterState state, ActionListener listener ) throws Exception { - // Verify that the requested node actually exists in the cluster - if (state.nodes().nodeExists(request.getNodeId()) == false) { - throw new IllegalArgumentException("there is no node with id [" + request.getNodeId() + "] in this cluster"); - } - clusterService.submitStateUpdateTask( "put-node-shutdown-" + request.getNodeId(), new AckedClusterStateUpdateTask(request, listener) {