From 9b2202aeb3a881c0c2ad72522fa1e2656d5d4d1e Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Tue, 27 Sep 2022 21:32:29 +0530 Subject: [PATCH 1/2] Add PUT api to update shard routing weights (#4272) * Add PUT api to update shard routing weights Signed-off-by: Anshu Agarwal --- CHANGELOG.md | 2 +- .../client/RestHighLevelClientTests.java | 3 +- .../api/cluster.put_weighted_routing.json | 25 ++ .../org/opensearch/action/ActionModule.java | 5 + .../put/ClusterAddWeightedRoutingAction.java | 26 ++ .../put/ClusterPutWeightedRoutingRequest.java | 173 +++++++++++++ ...usterPutWeightedRoutingRequestBuilder.java | 33 +++ .../ClusterPutWeightedRoutingResponse.java | 29 +++ .../TransportAddWeightedRoutingAction.java | 128 ++++++++++ .../routing/weighted/put/package-info.java | 10 + .../opensearch/client/ClusterAdminClient.java | 19 ++ .../java/org/opensearch/client/Requests.java | 10 + .../client/support/AbstractClient.java | 22 ++ .../routing/WeightedRoutingService.java | 88 +++++++ .../RestClusterPutWeightedRoutingAction.java | 58 +++++ ...ClusterPutWeightedRoutingRequestTests.java | 65 +++++ .../routing/WeightedRoutingServiceTests.java | 234 ++++++++++++++++++ ...tClusterAddWeightedRoutingActionTests.java | 76 ++++++ 18 files changed, 1004 insertions(+), 2 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/api/cluster.put_weighted_routing.json create mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterAddWeightedRoutingAction.java create mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java create mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestBuilder.java create mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingResponse.java create mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/TransportAddWeightedRoutingAction.java create mode 100644 server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/package-info.java create mode 100644 server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java create mode 100644 server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterPutWeightedRoutingAction.java create mode 100644 server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java create mode 100644 server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java create mode 100644 server/src/test/java/org/opensearch/rest/action/admin/cluster/RestClusterAddWeightedRoutingActionTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index b6af9b7041db3..3b15a28c55c43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,10 +45,10 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Segment Replication] Update replicas to commit SegmentInfos instead of relying on SIS files from primary shards. ([#4402](https://github.com/opensearch-project/OpenSearch/pull/4402)) - [CCR] Add getHistoryOperationsFromTranslog method to fetch the history snapshot from translogs ([#3948](https://github.com/opensearch-project/OpenSearch/pull/3948)) - [Remote Store] Change behaviour in replica recovery for remote translog enabled indices ([#4318](https://github.com/opensearch-project/OpenSearch/pull/4318)) +- PUT api for weighted shard routing ([#4272](https://github.com/opensearch-project/OpenSearch/pull/4272)) - Unmute test RelocationIT.testRelocationWhileIndexingRandom ([#4580](https://github.com/opensearch-project/OpenSearch/pull/4580)) - Add DecommissionService and helper to execute awareness attribute decommissioning ([#4084](https://github.com/opensearch-project/OpenSearch/pull/4084)) - ### Deprecated ### Removed diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/RestHighLevelClientTests.java index ad8da7244eae0..c0eb344a64dba 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/RestHighLevelClientTests.java @@ -888,7 +888,8 @@ public void testApiNamingConventions() throws Exception { "nodes.usage", "nodes.reload_secure_settings", "search_shards", - "remote_store.restore", }; + "remote_store.restore", + "cluster.put_weighted_routing", }; List booleanReturnMethods = Arrays.asList("security.enable_user", "security.disable_user", "security.change_password"); Set deprecatedMethods = new HashSet<>(); deprecatedMethods.add("indices.force_merge"); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.put_weighted_routing.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.put_weighted_routing.json new file mode 100644 index 0000000000000..88498517ba336 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.put_weighted_routing.json @@ -0,0 +1,25 @@ +{ + "cluster.put_weighted_routing": { + "documentation": { + "url": "https://opensearch.org/docs/latest/opensearch/rest-api/weighted-routing/put", + "description": "Updates weighted shard routing weights" + }, + "stability": "stable", + "url": { + "paths": [ + { + "path": "/_cluster/routing/awareness/{attribute}/weights", + "methods": [ + "PUT" + ], + "parts": { + "attribute": { + "type": "string", + "description": "Awareness attribute name" + } + } + } + ] + } + } +} diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index c5fcfdd047a09..e745e505d9fe5 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -79,6 +79,8 @@ import org.opensearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction; import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsAction; import org.opensearch.action.admin.cluster.shards.TransportClusterSearchShardsAction; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterAddWeightedRoutingAction; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.TransportAddWeightedRoutingAction; import org.opensearch.action.admin.cluster.snapshots.clone.CloneSnapshotAction; import org.opensearch.action.admin.cluster.snapshots.clone.TransportCloneSnapshotAction; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotAction; @@ -294,6 +296,7 @@ import org.opensearch.rest.action.admin.cluster.RestClusterAllocationExplainAction; import org.opensearch.rest.action.admin.cluster.RestClusterGetSettingsAction; import org.opensearch.rest.action.admin.cluster.RestClusterHealthAction; +import org.opensearch.rest.action.admin.cluster.RestClusterPutWeightedRoutingAction; import org.opensearch.rest.action.admin.cluster.RestClusterRerouteAction; import org.opensearch.rest.action.admin.cluster.RestClusterSearchShardsAction; import org.opensearch.rest.action.admin.cluster.RestClusterStateAction; @@ -563,6 +566,7 @@ public void reg actions.register(RestoreSnapshotAction.INSTANCE, TransportRestoreSnapshotAction.class); actions.register(SnapshotsStatusAction.INSTANCE, TransportSnapshotsStatusAction.class); + actions.register(ClusterAddWeightedRoutingAction.INSTANCE, TransportAddWeightedRoutingAction.class); actions.register(IndicesStatsAction.INSTANCE, TransportIndicesStatsAction.class); actions.register(IndicesSegmentsAction.INSTANCE, TransportIndicesSegmentsAction.class); actions.register(IndicesShardStoresAction.INSTANCE, TransportIndicesShardStoresAction.class); @@ -744,6 +748,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestCloseIndexAction()); registerHandler.accept(new RestOpenIndexAction()); registerHandler.accept(new RestAddIndexBlockAction()); + registerHandler.accept(new RestClusterPutWeightedRoutingAction()); registerHandler.accept(new RestUpdateSettingsAction()); registerHandler.accept(new RestGetSettingsAction()); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterAddWeightedRoutingAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterAddWeightedRoutingAction.java new file mode 100644 index 0000000000000..65c5ccca71461 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterAddWeightedRoutingAction.java @@ -0,0 +1,26 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; + +import org.opensearch.action.ActionType; + +/** + * Action to update weights for weighted round-robin shard routing policy. + * + * @opensearch.internal + */ +public final class ClusterAddWeightedRoutingAction extends ActionType { + + public static final ClusterAddWeightedRoutingAction INSTANCE = new ClusterAddWeightedRoutingAction(); + public static final String NAME = "cluster:admin/routing/awareness/weights/put"; + + private ClusterAddWeightedRoutingAction() { + super(NAME, ClusterPutWeightedRoutingResponse::new); + } +} diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java new file mode 100644 index 0000000000000..af229fb12b4f0 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java @@ -0,0 +1,173 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchGenerationException; +import org.opensearch.OpenSearchParseException; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.support.clustermanager.ClusterManagerNodeRequest; +import org.opensearch.cluster.routing.WeightedRouting; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.xcontent.DeprecationHandler; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentHelper; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.common.xcontent.XContentType; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +import static org.opensearch.action.ValidateActions.addValidationError; + +/** + * Request to update weights for weighted round-robin shard routing policy. + * + * @opensearch.internal + */ +public class ClusterPutWeightedRoutingRequest extends ClusterManagerNodeRequest { + private static final Logger logger = LogManager.getLogger(ClusterPutWeightedRoutingRequest.class); + + private WeightedRouting weightedRouting; + private String attributeName; + + public ClusterPutWeightedRoutingRequest() {} + + public WeightedRouting getWeightedRouting() { + return weightedRouting; + } + + public ClusterPutWeightedRoutingRequest setWeightedRouting(WeightedRouting weightedRouting) { + this.weightedRouting = weightedRouting; + return this; + } + + public void attributeName(String attributeName) { + this.attributeName = attributeName; + } + + public ClusterPutWeightedRoutingRequest(StreamInput in) throws IOException { + super(in); + weightedRouting = new WeightedRouting(in); + } + + public ClusterPutWeightedRoutingRequest(String attributeName) { + this.attributeName = attributeName; + } + + public void setWeightedRouting(Map source) { + try { + if (source.isEmpty()) { + throw new OpenSearchParseException(("Empty request body")); + } + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.map(source); + setWeightedRouting(BytesReference.bytes(builder), builder.contentType()); + } catch (IOException e) { + throw new OpenSearchGenerationException("Failed to generate [" + source + "]", e); + } + } + + public void setWeightedRouting(BytesReference source, XContentType contentType) { + try ( + XContentParser parser = XContentHelper.createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + source, + contentType + ) + ) { + String attrValue = null; + Map weights = new HashMap<>(); + Double attrWeight = null; + XContentParser.Token token; + // move to the first alias + parser.nextToken(); + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + attrValue = parser.currentName(); + } else if (token == XContentParser.Token.VALUE_STRING) { + attrWeight = Double.parseDouble(parser.text()); + weights.put(attrValue, attrWeight); + } else { + throw new OpenSearchParseException( + "failed to parse weighted routing request attribute [{}], " + "unknown type", + attrWeight + ); + } + } + this.weightedRouting = new WeightedRouting(this.attributeName, weights); + } catch (IOException e) { + logger.error("error while parsing put for weighted routing request object", e); + } + } + + @Override + public ActionRequestValidationException validate() { + ActionRequestValidationException validationException = null; + if (weightedRouting == null) { + validationException = addValidationError("Weighted routing request object is null", validationException); + } + if (weightedRouting.attributeName() == null || weightedRouting.attributeName().isEmpty()) { + validationException = addValidationError("Attribute name is missing", validationException); + } + if (weightedRouting.weights() == null || weightedRouting.weights().isEmpty()) { + validationException = addValidationError("Weights are missing", validationException); + } + int countValueWithZeroWeights = 0; + double weight; + try { + for (Object value : weightedRouting.weights().values()) { + if (value == null) { + validationException = addValidationError(("Weight is null"), validationException); + } else { + weight = Double.parseDouble(value.toString()); + countValueWithZeroWeights = (weight == 0) ? countValueWithZeroWeights + 1 : countValueWithZeroWeights; + } + } + } catch (NumberFormatException e) { + validationException = addValidationError(("Weight is not a number"), validationException); + } + if (countValueWithZeroWeights > 1) { + validationException = addValidationError( + (String.format(Locale.ROOT, "More than one [%d] value has weight set as 0", countValueWithZeroWeights)), + validationException + ); + } + return validationException; + } + + /** + * @param source weights definition from request body + * @return this request + */ + public ClusterPutWeightedRoutingRequest source(Map source) { + setWeightedRouting(source); + return this; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + weightedRouting.writeTo(out); + } + + @Override + public String toString() { + return "ClusterPutWeightedRoutingRequest{" + "weightedRouting= " + weightedRouting.toString() + "}"; + } + +} diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestBuilder.java new file mode 100644 index 0000000000000..b437f4c54d8d6 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestBuilder.java @@ -0,0 +1,33 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; + +import org.opensearch.action.support.clustermanager.ClusterManagerNodeOperationRequestBuilder; +import org.opensearch.client.OpenSearchClient; +import org.opensearch.cluster.routing.WeightedRouting; + +/** + * Request builder to update weights for weighted round-robin shard routing policy. + * + * @opensearch.internal + */ +public class ClusterPutWeightedRoutingRequestBuilder extends ClusterManagerNodeOperationRequestBuilder< + ClusterPutWeightedRoutingRequest, + ClusterPutWeightedRoutingResponse, + ClusterPutWeightedRoutingRequestBuilder> { + public ClusterPutWeightedRoutingRequestBuilder(OpenSearchClient client, ClusterAddWeightedRoutingAction action) { + super(client, action, new ClusterPutWeightedRoutingRequest()); + } + + public ClusterPutWeightedRoutingRequestBuilder setWeightedRouting(WeightedRouting weightedRouting) { + request.setWeightedRouting(weightedRouting); + return this; + } + +} diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingResponse.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingResponse.java new file mode 100644 index 0000000000000..b0154aceef0c2 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingResponse.java @@ -0,0 +1,29 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; + +import org.opensearch.action.support.master.AcknowledgedResponse; +import org.opensearch.common.io.stream.StreamInput; + +import java.io.IOException; + +/** + * Response from updating weights for weighted round-robin search routing policy. + * + * @opensearch.internal + */ +public class ClusterPutWeightedRoutingResponse extends AcknowledgedResponse { + public ClusterPutWeightedRoutingResponse(boolean acknowledged) { + super(acknowledged); + } + + public ClusterPutWeightedRoutingResponse(StreamInput in) throws IOException { + super(in); + } +} diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/TransportAddWeightedRoutingAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/TransportAddWeightedRoutingAction.java new file mode 100644 index 0000000000000..8c29ab2199848 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/TransportAddWeightedRoutingAction.java @@ -0,0 +1,128 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; + +import org.opensearch.action.ActionListener; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.block.ClusterBlockException; +import org.opensearch.cluster.block.ClusterBlockLevel; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.routing.WeightedRoutingService; +import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; + +import java.io.IOException; +import java.util.List; +import java.util.Locale; + +import static org.opensearch.action.ValidateActions.addValidationError; + +/** + * Transport action for updating weights for weighted round-robin search routing policy + * + * @opensearch.internal + */ +public class TransportAddWeightedRoutingAction extends TransportClusterManagerNodeAction< + ClusterPutWeightedRoutingRequest, + ClusterPutWeightedRoutingResponse> { + + private final WeightedRoutingService weightedRoutingService; + private volatile List awarenessAttributes; + + @Inject + public TransportAddWeightedRoutingAction( + Settings settings, + ClusterSettings clusterSettings, + TransportService transportService, + ClusterService clusterService, + WeightedRoutingService weightedRoutingService, + ThreadPool threadPool, + ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver + ) { + super( + ClusterAddWeightedRoutingAction.NAME, + transportService, + clusterService, + threadPool, + actionFilters, + ClusterPutWeightedRoutingRequest::new, + indexNameExpressionResolver + ); + this.weightedRoutingService = weightedRoutingService; + this.awarenessAttributes = AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.get(settings); + clusterSettings.addSettingsUpdateConsumer( + AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING, + this::setAwarenessAttributes + ); + } + + List getAwarenessAttributes() { + return awarenessAttributes; + } + + private void setAwarenessAttributes(List awarenessAttributes) { + this.awarenessAttributes = awarenessAttributes; + } + + @Override + protected String executor() { + return ThreadPool.Names.SAME; + } + + @Override + protected ClusterPutWeightedRoutingResponse read(StreamInput in) throws IOException { + return new ClusterPutWeightedRoutingResponse(in); + } + + @Override + protected void clusterManagerOperation( + ClusterPutWeightedRoutingRequest request, + ClusterState state, + ActionListener listener + ) throws Exception { + verifyAwarenessAttribute(request.getWeightedRouting().attributeName()); + weightedRoutingService.registerWeightedRoutingMetadata( + request, + ActionListener.delegateFailure( + listener, + (delegatedListener, response) -> { + delegatedListener.onResponse(new ClusterPutWeightedRoutingResponse(response.isAcknowledged())); + } + ) + ); + } + + private void verifyAwarenessAttribute(String attributeName) { + if (getAwarenessAttributes().contains(attributeName) == false) { + ActionRequestValidationException validationException = null; + + validationException = addValidationError( + String.format(Locale.ROOT, "invalid awareness attribute %s requested for updating weighted routing", attributeName), + validationException + ); + throw validationException; + } + } + + @Override + protected ClusterBlockException checkBlock(ClusterPutWeightedRoutingRequest request, ClusterState state) { + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE); + } + +} diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/package-info.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/package-info.java new file mode 100644 index 0000000000000..4f18b220cd343 --- /dev/null +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/package-info.java @@ -0,0 +1,10 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** add/update weighted-round robin shard routing weights. */ +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; diff --git a/server/src/main/java/org/opensearch/client/ClusterAdminClient.java b/server/src/main/java/org/opensearch/client/ClusterAdminClient.java index 7a7b98bf724f6..2f62ab13b131c 100644 --- a/server/src/main/java/org/opensearch/client/ClusterAdminClient.java +++ b/server/src/main/java/org/opensearch/client/ClusterAdminClient.java @@ -86,6 +86,9 @@ import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsRequest; import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsRequestBuilder; import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsResponse; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequestBuilder; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingResponse; import org.opensearch.action.admin.cluster.snapshots.clone.CloneSnapshotRequest; import org.opensearch.action.admin.cluster.snapshots.clone.CloneSnapshotRequestBuilder; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; @@ -791,4 +794,20 @@ public interface ClusterAdminClient extends OpenSearchClient { * Delete specified dangling indices. */ ActionFuture deleteDanglingIndex(DeleteDanglingIndexRequest request); + + /** + * Updates weights for weighted round-robin search routing policy. + */ + ActionFuture putWeightedRouting(ClusterPutWeightedRoutingRequest request); + + /** + * Updates weights for weighted round-robin search routing policy. + */ + void putWeightedRouting(ClusterPutWeightedRoutingRequest request, ActionListener listener); + + /** + * Updates weights for weighted round-robin search routing policy. + */ + ClusterPutWeightedRoutingRequestBuilder prepareWeightedRouting(); + } diff --git a/server/src/main/java/org/opensearch/client/Requests.java b/server/src/main/java/org/opensearch/client/Requests.java index b04de7830a780..7154742de04fb 100644 --- a/server/src/main/java/org/opensearch/client/Requests.java +++ b/server/src/main/java/org/opensearch/client/Requests.java @@ -47,6 +47,7 @@ import org.opensearch.action.admin.cluster.reroute.ClusterRerouteRequest; import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsRequest; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.opensearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest; import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest; @@ -548,4 +549,13 @@ public static DeleteSnapshotRequest deleteSnapshotRequest(String repository, Str public static SnapshotsStatusRequest snapshotsStatusRequest(String repository) { return new SnapshotsStatusRequest(repository); } + + /** + * Updates weights for weighted round-robin search routing policy + * + * @return update weight request + */ + public static ClusterPutWeightedRoutingRequest putWeightedRoutingRequest(String attributeName) { + return new ClusterPutWeightedRoutingRequest(attributeName); + } } diff --git a/server/src/main/java/org/opensearch/client/support/AbstractClient.java b/server/src/main/java/org/opensearch/client/support/AbstractClient.java index 21cd01bf65a45..efd18dd4947ad 100644 --- a/server/src/main/java/org/opensearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/opensearch/client/support/AbstractClient.java @@ -110,6 +110,10 @@ import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsRequest; import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsRequestBuilder; import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsResponse; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterAddWeightedRoutingAction; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequestBuilder; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingResponse; import org.opensearch.action.admin.cluster.snapshots.clone.CloneSnapshotAction; import org.opensearch.action.admin.cluster.snapshots.clone.CloneSnapshotRequest; import org.opensearch.action.admin.cluster.snapshots.clone.CloneSnapshotRequestBuilder; @@ -1272,6 +1276,24 @@ public ActionFuture deleteDanglingIndex(DeleteDanglingInde return execute(DeleteDanglingIndexAction.INSTANCE, request); } + @Override + public ActionFuture putWeightedRouting(ClusterPutWeightedRoutingRequest request) { + return execute(ClusterAddWeightedRoutingAction.INSTANCE, request); + } + + @Override + public void putWeightedRouting( + ClusterPutWeightedRoutingRequest request, + ActionListener listener + ) { + execute(ClusterAddWeightedRoutingAction.INSTANCE, request, listener); + } + + @Override + public ClusterPutWeightedRoutingRequestBuilder prepareWeightedRouting() { + return new ClusterPutWeightedRoutingRequestBuilder(this, ClusterAddWeightedRoutingAction.INSTANCE); + } + @Override public void deleteDanglingIndex(DeleteDanglingIndexRequest request, ActionListener listener) { execute(DeleteDanglingIndexAction.INSTANCE, request, listener); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java new file mode 100644 index 0000000000000..da454865ac866 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -0,0 +1,88 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.opensearch.action.ActionListener; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.ClusterStateUpdateTask; +import org.opensearch.cluster.ack.ClusterStateUpdateResponse; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.metadata.WeightedRoutingMetadata; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.Priority; +import org.opensearch.common.inject.Inject; + +import org.opensearch.threadpool.ThreadPool; + +/** + * * Service responsible for updating cluster state metadata with weighted routing weights + */ +public class WeightedRoutingService { + private static final Logger logger = LogManager.getLogger(WeightedRoutingService.class); + private final ClusterService clusterService; + private final ThreadPool threadPool; + + @Inject + public WeightedRoutingService(ClusterService clusterService, ThreadPool threadPool) { + this.clusterService = clusterService; + this.threadPool = threadPool; + } + + public void registerWeightedRoutingMetadata( + final ClusterPutWeightedRoutingRequest request, + final ActionListener listener + ) { + final WeightedRoutingMetadata newWeightedRoutingMetadata = new WeightedRoutingMetadata(request.getWeightedRouting()); + clusterService.submitStateUpdateTask("update_weighted_routing", new ClusterStateUpdateTask(Priority.URGENT) { + @Override + public ClusterState execute(ClusterState currentState) { + Metadata metadata = currentState.metadata(); + Metadata.Builder mdBuilder = Metadata.builder(currentState.metadata()); + WeightedRoutingMetadata weightedRoutingMetadata = metadata.custom(WeightedRoutingMetadata.TYPE); + if (weightedRoutingMetadata == null) { + logger.info("put weighted routing weights in metadata [{}]", request.getWeightedRouting()); + weightedRoutingMetadata = new WeightedRoutingMetadata(request.getWeightedRouting()); + } else { + if (!checkIfSameWeightsInMetadata(newWeightedRoutingMetadata, weightedRoutingMetadata)) { + logger.info("updated weighted routing weights [{}] in metadata", request.getWeightedRouting()); + weightedRoutingMetadata = new WeightedRoutingMetadata(newWeightedRoutingMetadata.getWeightedRouting()); + } else { + return currentState; + } + } + mdBuilder.putCustom(WeightedRoutingMetadata.TYPE, weightedRoutingMetadata); + logger.info("building cluster state with weighted routing weights [{}]", request.getWeightedRouting()); + return ClusterState.builder(currentState).metadata(mdBuilder).build(); + } + + @Override + public void onFailure(String source, Exception e) { + logger.warn(() -> new ParameterizedMessage("failed to update cluster state for weighted routing weights [{}]", e)); + listener.onFailure(e); + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + logger.debug("cluster weighted routing weights metadata change is processed by all the nodes"); + listener.onResponse(new ClusterStateUpdateResponse(true)); + } + }); + } + + private boolean checkIfSameWeightsInMetadata( + WeightedRoutingMetadata newWeightedRoutingMetadata, + WeightedRoutingMetadata oldWeightedRoutingMetadata + ) { + return newWeightedRoutingMetadata.getWeightedRouting().equals(oldWeightedRoutingMetadata.getWeightedRouting()); + } +} diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterPutWeightedRoutingAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterPutWeightedRoutingAction.java new file mode 100644 index 0000000000000..1cf44e665cf84 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterPutWeightedRoutingAction.java @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.admin.cluster; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest; +import org.opensearch.client.Requests; +import org.opensearch.client.node.NodeClient; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.action.RestToXContentListener; + +import java.io.IOException; +import java.util.List; + +import static java.util.Collections.singletonList; +import static org.opensearch.rest.RestRequest.Method.PUT; + +/** + * Update Weighted Round Robin based shard routing weights + * + * @opensearch.api + * + */ +public class RestClusterPutWeightedRoutingAction extends BaseRestHandler { + + private static final Logger logger = LogManager.getLogger(RestClusterPutWeightedRoutingAction.class); + + @Override + public List routes() { + return singletonList(new Route(PUT, "/_cluster/routing/awareness/{attribute}/weights")); + } + + @Override + public String getName() { + return "put_weighted_routing_action"; + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + ClusterPutWeightedRoutingRequest putWeightedRoutingRequest = createRequest(request); + return channel -> client.admin().cluster().putWeightedRouting(putWeightedRoutingRequest, new RestToXContentListener<>(channel)); + } + + public static ClusterPutWeightedRoutingRequest createRequest(RestRequest request) throws IOException { + ClusterPutWeightedRoutingRequest putWeightedRoutingRequest = Requests.putWeightedRoutingRequest(request.param("attribute")); + request.applyContentParser(p -> putWeightedRoutingRequest.source(p.mapStrings())); + return putWeightedRoutingRequest; + } + +} diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java new file mode 100644 index 0000000000000..186e7e8638f17 --- /dev/null +++ b/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java @@ -0,0 +1,65 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.admin.cluster.shards.routing.weighted.put; + +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.cluster.routing.WeightedRouting; +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Map; + +public class ClusterPutWeightedRoutingRequestTests extends OpenSearchTestCase { + + public void testSetWeightedRoutingWeight() { + String reqString = "{\"us-east-1c\" : \"0\", \"us-east-1b\":\"1\",\"us-east-1a\":\"1\"}"; + ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone"); + Map weights = Map.of("us-east-1a", 1.0, "us-east-1b", 1.0, "us-east-1c", 0.0); + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON); + assertEquals(request.getWeightedRouting(), weightedRouting); + } + + public void testValidate_ValuesAreProper() { + String reqString = "{\"us-east-1c\" : \"1\", \"us-east-1b\":\"0\",\"us-east-1a\":\"1\"}"; + ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone"); + request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON); + ActionRequestValidationException actionRequestValidationException = request.validate(); + assertNull(actionRequestValidationException); + } + + public void testValidate_TwoZonesWithZeroWeight() { + String reqString = "{\"us-east-1c\" : \"0\", \"us-east-1b\":\"0\",\"us-east-1a\":\"1\"}"; + ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone"); + request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON); + ActionRequestValidationException actionRequestValidationException = request.validate(); + assertNotNull(actionRequestValidationException); + assertTrue(actionRequestValidationException.getMessage().contains("More than one [2] value has weight set as " + "0")); + } + + public void testValidate_MissingWeights() { + String reqString = "{}"; + ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone"); + request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON); + ActionRequestValidationException actionRequestValidationException = request.validate(); + assertNotNull(actionRequestValidationException); + assertTrue(actionRequestValidationException.getMessage().contains("Weights are missing")); + } + + public void testValidate_AttributeMissing() { + String reqString = "{\"us-east-1c\" : \"0\", \"us-east-1b\":\"1\",\"us-east-1a\":\"1\"}"; + ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest(); + request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON); + ActionRequestValidationException actionRequestValidationException = request.validate(); + assertNotNull(actionRequestValidationException); + assertTrue(actionRequestValidationException.getMessage().contains("Attribute name is missing")); + } + +} diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java new file mode 100644 index 0000000000000..e5cca998d3f06 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -0,0 +1,234 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.junit.After; +import org.junit.Before; +import org.opensearch.Version; +import org.opensearch.action.ActionListener; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterAddWeightedRoutingAction; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequestBuilder; +import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.ack.ClusterStateUpdateResponse; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.metadata.WeightedRoutingMetadata; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.node.DiscoveryNodeRole; +import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.test.ClusterServiceUtils; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.transport.MockTransport; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +public class WeightedRoutingServiceTests extends OpenSearchTestCase { + private ThreadPool threadPool; + private ClusterService clusterService; + private TransportService transportService; + private WeightedRoutingService weightedRoutingService; + private ClusterSettings clusterSettings; + NodeClient client; + + final private static Set CLUSTER_MANAGER_ROLE = Collections.unmodifiableSet( + new HashSet<>(Collections.singletonList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + ); + + final private static Set DATA_ROLE = Collections.unmodifiableSet( + new HashSet<>(Collections.singletonList(DiscoveryNodeRole.DATA_ROLE)) + ); + + @Override + public void setUp() throws Exception { + super.setUp(); + threadPool = new TestThreadPool("test", Settings.EMPTY); + clusterService = ClusterServiceUtils.createClusterService(threadPool); + } + + @Before + public void setUpService() { + ClusterState clusterState = ClusterState.builder(new ClusterName("test")).build(); + clusterState = addClusterManagerNodes(clusterState); + clusterState = addDataNodes(clusterState); + clusterState = setLocalNode(clusterState, "nodeA1"); + + ClusterState.Builder builder = ClusterState.builder(clusterState); + ClusterServiceUtils.setState(clusterService, builder); + + final MockTransport transport = new MockTransport(); + transportService = transport.createTransportService( + Settings.EMPTY, + threadPool, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + boundTransportAddress -> clusterService.state().nodes().get("nodes1"), + null, + Collections.emptySet() + + ); + + Settings.Builder settingsBuilder = Settings.builder() + .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone"); + + clusterSettings = new ClusterSettings(settingsBuilder.build(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + transportService.start(); + transportService.acceptIncomingRequests(); + + this.weightedRoutingService = new WeightedRoutingService(clusterService, threadPool); + client = new NodeClient(Settings.EMPTY, threadPool); + } + + @After + public void shutdown() { + clusterService.stop(); + threadPool.shutdown(); + } + + private ClusterState addDataNodes(ClusterState clusterState) { + clusterState = addDataNodeForAZone(clusterState, "zone_A", "nodeA1", "nodeA2", "nodeA3"); + clusterState = addDataNodeForAZone(clusterState, "zone_B", "nodeB1", "nodeB2", "nodeB3"); + clusterState = addDataNodeForAZone(clusterState, "zone_C", "nodeC1", "nodeC2", "nodeC3"); + return clusterState; + } + + private ClusterState addClusterManagerNodes(ClusterState clusterState) { + clusterState = addClusterManagerNodeForAZone(clusterState, "zone_A", "nodeMA"); + clusterState = addClusterManagerNodeForAZone(clusterState, "zone_B", "nodeMB"); + clusterState = addClusterManagerNodeForAZone(clusterState, "zone_C", "nodeMC"); + return clusterState; + } + + private ClusterState addDataNodeForAZone(ClusterState clusterState, String zone, String... nodeIds) { + DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.nodes()); + org.opensearch.common.collect.List.of(nodeIds) + .forEach( + nodeId -> nodeBuilder.add( + new DiscoveryNode( + nodeId, + buildNewFakeTransportAddress(), + Collections.singletonMap("zone", zone), + DATA_ROLE, + Version.CURRENT + ) + ) + ); + clusterState = ClusterState.builder(clusterState).nodes(nodeBuilder).build(); + return clusterState; + } + + private ClusterState addClusterManagerNodeForAZone(ClusterState clusterState, String zone, String... nodeIds) { + + DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.nodes()); + org.opensearch.common.collect.List.of(nodeIds) + .forEach( + nodeId -> nodeBuilder.add( + new DiscoveryNode( + nodeId, + buildNewFakeTransportAddress(), + Collections.singletonMap("zone", zone), + CLUSTER_MANAGER_ROLE, + Version.CURRENT + ) + ) + ); + clusterState = ClusterState.builder(clusterState).nodes(nodeBuilder).build(); + return clusterState; + } + + private ClusterState setLocalNode(ClusterState clusterState, String nodeId) { + DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.nodes()); + nodeBuilder.localNodeId(nodeId); + nodeBuilder.clusterManagerNodeId(nodeId); + clusterState = ClusterState.builder(clusterState).nodes(nodeBuilder).build(); + return clusterState; + } + + private ClusterState setWeightedRoutingWeights(ClusterState clusterState, Map weights) { + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + WeightedRoutingMetadata weightedRoutingMetadata = new WeightedRoutingMetadata(weightedRouting); + Metadata.Builder metadataBuilder = Metadata.builder(clusterState.metadata()); + metadataBuilder.putCustom(WeightedRoutingMetadata.TYPE, weightedRoutingMetadata); + clusterState = ClusterState.builder(clusterState).metadata(metadataBuilder).build(); + return clusterState; + } + + public void testRegisterWeightedRoutingMetadataWithChangedWeights() throws InterruptedException { + Map weights = Map.of("zone_A", 1.0, "zone_B", 1.0, "zone_C", 1.0); + ClusterState state = clusterService.state(); + state = setWeightedRoutingWeights(state, weights); + ClusterState.Builder builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); + + ClusterPutWeightedRoutingRequestBuilder request = new ClusterPutWeightedRoutingRequestBuilder( + client, + ClusterAddWeightedRoutingAction.INSTANCE + ); + WeightedRouting updatedWeightedRouting = new WeightedRouting("zone", Map.of("zone_A", 1.0, "zone_B", 0.0, "zone_C", 0.0)); + request.setWeightedRouting(updatedWeightedRouting); + final CountDownLatch countDownLatch = new CountDownLatch(1); + ActionListener listener = new ActionListener() { + @Override + public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) { + assertTrue(clusterStateUpdateResponse.isAcknowledged()); + assertEquals(updatedWeightedRouting, clusterService.state().metadata().weightedRoutingMetadata().getWeightedRouting()); + countDownLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail("request should not fail"); + } + }; + weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); + assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); + } + + public void testRegisterWeightedRoutingMetadataWithSameWeights() throws InterruptedException { + Map weights = Map.of("zone_A", 1.0, "zone_B", 1.0, "zone_C", 1.0); + ClusterState state = clusterService.state(); + state = setWeightedRoutingWeights(state, weights); + ClusterState.Builder builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); + + ClusterPutWeightedRoutingRequestBuilder request = new ClusterPutWeightedRoutingRequestBuilder( + client, + ClusterAddWeightedRoutingAction.INSTANCE + ); + WeightedRouting updatedWeightedRouting = new WeightedRouting("zone", weights); + request.setWeightedRouting(updatedWeightedRouting); + final CountDownLatch countDownLatch = new CountDownLatch(1); + ActionListener listener = new ActionListener() { + @Override + public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) { + assertTrue(clusterStateUpdateResponse.isAcknowledged()); + assertEquals(updatedWeightedRouting, clusterService.state().metadata().weightedRoutingMetadata().getWeightedRouting()); + countDownLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail("request should not fail"); + } + }; + weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); + assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); + } +} diff --git a/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestClusterAddWeightedRoutingActionTests.java b/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestClusterAddWeightedRoutingActionTests.java new file mode 100644 index 0000000000000..a4cd6224217b7 --- /dev/null +++ b/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestClusterAddWeightedRoutingActionTests.java @@ -0,0 +1,76 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.rest.action.admin.cluster; + +import com.fasterxml.jackson.core.JsonParseException; +import org.junit.Before; +import org.opensearch.OpenSearchParseException; +import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest; +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.rest.RestRequest; +import org.opensearch.test.rest.FakeRestRequest; +import org.opensearch.test.rest.RestActionTestCase; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.singletonMap; + +public class RestClusterAddWeightedRoutingActionTests extends RestActionTestCase { + private RestClusterPutWeightedRoutingAction action; + + @Before + public void setupAction() { + action = new RestClusterPutWeightedRoutingAction(); + controller().registerHandler(action); + } + + public void testCreateRequest_SupportedRequestBody() throws IOException { + String req = "{\"us-east-1c\" : \"1\", \"us-east-1d\":\"1.0\", \"us-east-1a\":\"0.0\"}"; + RestRequest restRequest = buildRestRequest(req); + ClusterPutWeightedRoutingRequest clusterPutWeightedRoutingRequest = RestClusterPutWeightedRoutingAction.createRequest(restRequest); + assertEquals("zone", clusterPutWeightedRoutingRequest.getWeightedRouting().attributeName()); + assertNotNull(clusterPutWeightedRoutingRequest.getWeightedRouting().weights()); + assertEquals("1.0", clusterPutWeightedRoutingRequest.getWeightedRouting().weights().get("us-east-1c").toString()); + assertEquals("1.0", clusterPutWeightedRoutingRequest.getWeightedRouting().weights().get("us-east-1d").toString()); + assertEquals("0.0", clusterPutWeightedRoutingRequest.getWeightedRouting().weights().get("us-east-1a").toString()); + } + + public void testCreateRequest_UnsupportedRequestBody() throws IOException { + Map params = new HashMap<>(); + String req = "[\"us-east-1c\" : \"1\", \"us-east-1d\":\"1\", \"us-east-1a\":\"0\"]"; + RestRequest restRequest = buildRestRequest(req); + assertThrows(OpenSearchParseException.class, () -> RestClusterPutWeightedRoutingAction.createRequest(restRequest)); + } + + public void testCreateRequest_MalformedRequestBody() throws IOException { + Map params = new HashMap<>(); + + String req = "{\"us-east-1c\" : \1\", \"us-east-1d\":\"1\", \"us-east-1a\":\"0\"}"; + RestRequest restRequest = buildRestRequest(req); + assertThrows(JsonParseException.class, () -> RestClusterPutWeightedRoutingAction.createRequest(restRequest)); + } + + public void testCreateRequest_EmptyRequestBody() throws IOException { + String req = "{}"; + RestRequest restRequest = buildRestRequest(req); + assertThrows(OpenSearchParseException.class, () -> RestClusterPutWeightedRoutingAction.createRequest(restRequest)); + } + + private RestRequest buildRestRequest(String content) { + return new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.PUT) + .withPath("/_cluster/routing/awareness/zone/weights") + .withParams(singletonMap("attribute", "zone")) + .withContent(new BytesArray(content), XContentType.JSON) + .build(); + } + +} From ab6849f317d343bbbaba9f076b5a50fa9a95eb96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Vl=C4=8Dek?= Date: Tue, 27 Sep 2022 22:28:55 +0200 Subject: [PATCH 2/2] Further simplification of the ZIP publication implementation (#4360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A follow-up of PR #4156 that brings in further simplification of the ZIP publication implementation and new tests. It is possible to remove some of the initialization code because the work is already handled by other library under the hood (most likely by NebulaPublishPlugin). For instance the condition to test the `groupId` presence: `if (groupId == null)` was pointless. It was never `true`. If the `project.group` is not defined the publication task fails with an exception, if there is a custom `groupId` value setup in publication config then it overrides the `project.group` as well. Tests are provided to cover these use cases. As for the new tests they cover the following cases: - verify that the task "publishToMavenLocal" gets expected results. The inspiration for this comes from https://github.com/opensearch-project/opensearch-plugin-template-java/pull/35 - verify that applying only the 'opensearch.pluginzip' is enough, because both the NebulaPublish and MavenPublishPlugin plugins are added explicitly and tasks are chained correctly - verify that if the plugin is applied but no publication is defined then a message is printed for the user Signed-off-by: Lukáš Vlček Signed-off-by: Lukáš Vlček --- CHANGELOG.md | 1 + .../opensearch/gradle/pluginzip/Publish.java | 83 +++---- .../gradle/pluginzip/PublishTests.java | 235 +++++++++++++++++- .../pluginzip/customizedGroupValue.gradle | 1 - .../customizedInvalidGroupValue.gradle | 1 - .../pluginzip/missingGroupValue.gradle | 1 - .../pluginzip/missingPOMEntity.gradle | 1 - .../pluginzip/missingPublications.gradle | 21 ++ .../pluginzip/publishToMavenLocal.gradle | 47 ++++ ...onValue.gradle => useDefaultValues.gradle} | 5 +- 10 files changed, 330 insertions(+), 66 deletions(-) create mode 100644 buildSrc/src/test/resources/pluginzip/missingPublications.gradle create mode 100644 buildSrc/src/test/resources/pluginzip/publishToMavenLocal.gradle rename buildSrc/src/test/resources/pluginzip/{groupAndVersionValue.gradle => useDefaultValues.gradle} (90%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b15a28c55c43..42f86d6d89a5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - PUT api for weighted shard routing ([#4272](https://github.com/opensearch-project/OpenSearch/pull/4272)) - Unmute test RelocationIT.testRelocationWhileIndexingRandom ([#4580](https://github.com/opensearch-project/OpenSearch/pull/4580)) - Add DecommissionService and helper to execute awareness attribute decommissioning ([#4084](https://github.com/opensearch-project/OpenSearch/pull/4084)) +- Further simplification of the ZIP publication implementation ([#4360](https://github.com/opensearch-project/OpenSearch/pull/4360)) ### Deprecated diff --git a/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java b/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java index 70c3737ba3674..6dc7d660922b2 100644 --- a/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java +++ b/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java @@ -9,30 +9,31 @@ import org.gradle.api.Plugin; import org.gradle.api.Project; -import org.gradle.api.logging.Logger; -import org.gradle.api.logging.Logging; import org.gradle.api.publish.PublishingExtension; import org.gradle.api.publish.maven.MavenPublication; -import org.gradle.api.publish.maven.plugins.MavenPublishPlugin; import java.nio.file.Path; import org.gradle.api.Task; +import org.gradle.api.publish.maven.plugins.MavenPublishPlugin; public class Publish implements Plugin { - private static final Logger LOGGER = Logging.getLogger(Publish.class); - - public final static String EXTENSION_NAME = "zipmavensettings"; + // public final static String PLUGIN_ZIP_PUBLISH_POM_TASK = "generatePomFileForPluginZipPublication"; public final static String PUBLICATION_NAME = "pluginZip"; public final static String STAGING_REPO = "zipStaging"; - public final static String PLUGIN_ZIP_PUBLISH_POM_TASK = "generatePomFileForPluginZipPublication"; - public final static String LOCALMAVEN = "publishToMavenLocal"; public final static String LOCAL_STAGING_REPO_PATH = "/build/local-staging-repo"; - public String zipDistributionLocation = "/build/distributions/"; + // TODO: Does the path ^^ need to use platform dependant file separators ? + + private boolean isZipPublicationPresent(Project project) { + PublishingExtension pe = project.getExtensions().findByType(PublishingExtension.class); + if (pe == null) { + return false; + } + return pe.getPublications().findByName(PUBLICATION_NAME) != null; + } - public static void configMaven(Project project) { + private void addLocalMavenRepo(Project project) { final Path buildDirectory = project.getRootDir().toPath(); - project.getPluginManager().apply(MavenPublishPlugin.class); project.getExtensions().configure(PublishingExtension.class, publishing -> { publishing.repositories(repositories -> { repositories.maven(maven -> { @@ -40,52 +41,40 @@ public static void configMaven(Project project) { maven.setUrl(buildDirectory.toString() + LOCAL_STAGING_REPO_PATH); }); }); + }); + } + + private void addZipArtifact(Project project) { + project.getExtensions().configure(PublishingExtension.class, publishing -> { publishing.publications(publications -> { MavenPublication mavenZip = (MavenPublication) publications.findByName(PUBLICATION_NAME); - - if (mavenZip == null) { - mavenZip = publications.create(PUBLICATION_NAME, MavenPublication.class); + if (mavenZip != null) { + mavenZip.artifact(project.getTasks().named("bundlePlugin")); } - - String groupId = mavenZip.getGroupId(); - if (groupId == null) { - // The groupId is not customized thus we get the value from "project.group". - // See https://docs.gradle.org/current/userguide/publishing_maven.html#sec:identity_values_in_the_generated_pom - groupId = getProperty("group", project); - } - - String artifactId = project.getName(); - String pluginVersion = getProperty("version", project); - mavenZip.artifact(project.getTasks().named("bundlePlugin")); - mavenZip.setGroupId(groupId); - mavenZip.setArtifactId(artifactId); - mavenZip.setVersion(pluginVersion); }); }); } - static String getProperty(String name, Project project) { - if (project.hasProperty(name)) { - Object property = project.property(name); - if (property != null) { - return property.toString(); - } - } - return null; - } - @Override public void apply(Project project) { + project.getPluginManager().apply("nebula.maven-base-publish"); + project.getPluginManager().apply(MavenPublishPlugin.class); project.afterEvaluate(evaluatedProject -> { - configMaven(project); - Task validatePluginZipPom = project.getTasks().findByName("validatePluginZipPom"); - if (validatePluginZipPom != null) { - project.getTasks().getByName("validatePluginZipPom").dependsOn("generatePomFileForNebulaPublication"); - } - Task publishPluginZipPublicationToZipStagingRepository = project.getTasks() - .findByName("publishPluginZipPublicationToZipStagingRepository"); - if (publishPluginZipPublicationToZipStagingRepository != null) { - publishPluginZipPublicationToZipStagingRepository.dependsOn("generatePomFileForNebulaPublication"); + if (isZipPublicationPresent(project)) { + addLocalMavenRepo(project); + addZipArtifact(project); + Task validatePluginZipPom = project.getTasks().findByName("validatePluginZipPom"); + if (validatePluginZipPom != null) { + validatePluginZipPom.dependsOn("generatePomFileForNebulaPublication"); + } + Task publishPluginZipPublicationToZipStagingRepository = project.getTasks() + .findByName("publishPluginZipPublicationToZipStagingRepository"); + if (publishPluginZipPublicationToZipStagingRepository != null) { + publishPluginZipPublicationToZipStagingRepository.dependsOn("generatePomFileForNebulaPublication"); + } + } else { + project.getLogger() + .warn(String.format("Plugin 'opensearch.pluginzip' is applied but no '%s' publication is defined.", PUBLICATION_NAME)); } }); } diff --git a/buildSrc/src/test/java/org/opensearch/gradle/pluginzip/PublishTests.java b/buildSrc/src/test/java/org/opensearch/gradle/pluginzip/PublishTests.java index 06632e2dfa476..2ca0e507acb44 100644 --- a/buildSrc/src/test/java/org/opensearch/gradle/pluginzip/PublishTests.java +++ b/buildSrc/src/test/java/org/opensearch/gradle/pluginzip/PublishTests.java @@ -8,6 +8,8 @@ package org.opensearch.gradle.pluginzip; +import org.gradle.api.Project; +import org.gradle.testfixtures.ProjectBuilder; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.UnexpectedBuildFailure; @@ -54,20 +56,152 @@ public void tearDown() { projectDir.delete(); } + /** + * This test is used to verify that adding the 'opensearch.pluginzip' to the project + * adds some other transitive plugins and tasks under the hood. This is basically + * a behavioral test of the {@link Publish#apply(Project)} method. + * + * This is equivalent of having a build.gradle script with just the following section: + * 
+     *     plugins {
+     *       id 'opensearch.pluginzip'
+     *     }
+     *
+ */ + @Test + public void applyZipPublicationPluginNoConfig() { + // All we do here is creating an empty project and applying the Publish plugin. + Project project = ProjectBuilder.builder().build(); + project.getPluginManager().apply(Publish.class); + + // WARNING: ===================================================================== + // All the following tests will work only before the gradle project is evaluated. + // There are some methods that will cause the project to be evaluated, such as: + // project.getTasksByName() + // After the project is evaluated there are more tasks found in the project, like + // the [assemble, build, ...] and other standard tasks. + // This can potentially break in future gradle versions (?) + // =============================================================================== + + assertEquals( + "The Publish plugin is applied which adds total of five tasks from Nebula and MavenPublishing plugins.", + 5, + project.getTasks().size() + ); + + // Tasks applied from "nebula.maven-base-publish" + assertTrue( + project.getTasks() + .findByName("generateMetadataFileForNebulaPublication") instanceof org.gradle.api.publish.tasks.GenerateModuleMetadata + ); + assertTrue( + project.getTasks() + .findByName("generatePomFileForNebulaPublication") instanceof org.gradle.api.publish.maven.tasks.GenerateMavenPom + ); + assertTrue( + project.getTasks() + .findByName("publishNebulaPublicationToMavenLocal") instanceof org.gradle.api.publish.maven.tasks.PublishToMavenLocal + ); + + // Tasks applied from MavenPublishPlugin + assertTrue(project.getTasks().findByName("publishToMavenLocal") instanceof org.gradle.api.DefaultTask); + assertTrue(project.getTasks().findByName("publish") instanceof org.gradle.api.DefaultTask); + + // And we miss the pluginzip publication task (because no publishing was defined for it) + assertNull(project.getTasks().findByName(ZIP_PUBLISH_TASK)); + + // We have the following publishing plugins + assertEquals(4, project.getPlugins().size()); + // ... of the following types: + assertNotNull( + "Project is expected to have OpenSearch pluginzip Publish plugin", + project.getPlugins().findPlugin(org.opensearch.gradle.pluginzip.Publish.class) + ); + assertNotNull( + "Project is expected to have MavenPublishPlugin (applied from OpenSearch pluginzip plugin)", + project.getPlugins().findPlugin(org.gradle.api.publish.maven.plugins.MavenPublishPlugin.class) + ); + assertNotNull( + "Project is expected to have Publishing plugin (applied from MavenPublishPublish plugin)", + project.getPlugins().findPlugin(org.gradle.api.publish.plugins.PublishingPlugin.class) + ); + assertNotNull( + "Project is expected to have nebula MavenBasePublishPlugin plugin (applied from OpenSearch pluginzip plugin)", + project.getPlugins().findPlugin(nebula.plugin.publishing.maven.MavenBasePublishPlugin.class) + ); + } + + /** + * Verify that if the zip publication is configured then relevant tasks are chained correctly. + * This test that the dependsOn() is applied correctly. + */ + @Test + public void applyZipPublicationPluginWithConfig() throws IOException, URISyntaxException, InterruptedException { + + /* ------------------------------- + // The ideal approach would be to create a project (via ProjectBuilder) with publishzip plugin, + // have it evaluated (API call) and then check if there are tasks that the plugin uses to hookup into + // and how these tasks are chained. The problem is that there is a known gradle issue (#20301) that does + // not allow for it ATM. If, however, it is fixed in the future the following is the code that can + // be used... + + Project project = ProjectBuilder.builder().build(); + project.getPluginManager().apply(Publish.class); + // add publications via API + + // evaluate the project + ((DefaultProject)project).evaluate(); + + // - Check that "validatePluginZipPom" and/or "publishPluginZipPublicationToZipStagingRepository" + // tasks have dependencies on "generatePomFileForNebulaPublication". + // - Check that there is the staging repository added. + + // However, due to known issue(1): https://github.com/gradle/gradle/issues/20301 + // it is impossible to reach to individual tasks and work with them. + // (1): https://docs.gradle.org/7.4/release-notes.html#known-issues + + // I.e.: The following code throws exception, basically any access to individual tasks fails. + project.getTasks().getByName("validatePluginZipPom"); + ------------------------------- */ + + // Instead, we run the gradle project via GradleRunner (this way we get fully evaluated project) + // and using the minimal possible configuration (missingPOMEntity) we test that as soon as the zip publication + // configuration is specified then all the necessary tasks are hooked up and executed correctly. + // However, this does not test execution order of the tasks. + GradleRunner runner = prepareGradleRunnerFromTemplate("missingPOMEntity.gradle", ZIP_PUBLISH_TASK/*, "-m"*/); + BuildResult result = runner.build(); + + assertEquals(SUCCESS, result.task(":" + "bundlePlugin").getOutcome()); + assertEquals(SUCCESS, result.task(":" + "generatePomFileForNebulaPublication").getOutcome()); + assertEquals(SUCCESS, result.task(":" + "generatePomFileForPluginZipPublication").getOutcome()); + assertEquals(SUCCESS, result.task(":" + ZIP_PUBLISH_TASK).getOutcome()); + } + + /** + * If the plugin is used but relevant publication is not defined then a message is printed. + */ + @Test + public void missingPublications() throws IOException, URISyntaxException { + GradleRunner runner = prepareGradleRunnerFromTemplate("missingPublications.gradle", "build", "-m"); + BuildResult result = runner.build(); + + assertTrue(result.getOutput().contains("Plugin 'opensearch.pluginzip' is applied but no 'pluginZip' publication is defined.")); + } + @Test public void missingGroupValue() throws IOException, URISyntaxException, XmlPullParserException { - GradleRunner runner = prepareGradleRunnerFromTemplate("missingGroupValue.gradle"); + GradleRunner runner = prepareGradleRunnerFromTemplate("missingGroupValue.gradle", "build", ZIP_PUBLISH_TASK); Exception e = assertThrows(UnexpectedBuildFailure.class, runner::build); assertTrue(e.getMessage().contains("Invalid publication 'pluginZip': groupId cannot be empty.")); } /** - * This would be the most common use case where user declares Maven publication entity with basic info - * and the resulting POM file will use groupId and version values from the Gradle project object. + * This would be the most common use case where user declares Maven publication entity with minimal info + * and the resulting POM file will use artifactId, groupId and version values based on the Gradle project object. */ @Test - public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPullParserException { - GradleRunner runner = prepareGradleRunnerFromTemplate("groupAndVersionValue.gradle"); + public void useDefaultValues() throws IOException, URISyntaxException, XmlPullParserException { + GradleRunner runner = prepareGradleRunnerFromTemplate("useDefaultValues.gradle", "build", ZIP_PUBLISH_TASK); BuildResult result = runner.build(); /** Check if build and {@value ZIP_PUBLISH_TASK} tasks have run well */ @@ -108,7 +242,7 @@ public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPu ).exists() ); - // Parse the maven file and validate the groupID + // Parse the maven file and validate default values MavenXpp3Reader reader = new MavenXpp3Reader(); Model model = reader.read( new FileReader( @@ -130,6 +264,10 @@ public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPu ); assertEquals(model.getVersion(), "2.0.0.0"); assertEquals(model.getGroupId(), "org.custom.group"); + assertEquals(model.getArtifactId(), PROJECT_NAME); + assertNull(model.getName()); + assertNull(model.getDescription()); + assertEquals(model.getUrl(), "https://github.com/doe/sample-plugin"); } @@ -139,7 +277,7 @@ public void groupAndVersionValue() throws IOException, URISyntaxException, XmlPu */ @Test public void missingPOMEntity() throws IOException, URISyntaxException, XmlPullParserException { - GradleRunner runner = prepareGradleRunnerFromTemplate("missingPOMEntity.gradle"); + GradleRunner runner = prepareGradleRunnerFromTemplate("missingPOMEntity.gradle", "build", ZIP_PUBLISH_TASK); BuildResult result = runner.build(); /** Check if build and {@value ZIP_PUBLISH_TASK} tasks have run well */ @@ -186,7 +324,7 @@ public void missingPOMEntity() throws IOException, URISyntaxException, XmlPullPa */ @Test public void customizedGroupValue() throws IOException, URISyntaxException, XmlPullParserException { - GradleRunner runner = prepareGradleRunnerFromTemplate("customizedGroupValue.gradle"); + GradleRunner runner = prepareGradleRunnerFromTemplate("customizedGroupValue.gradle", "build", ZIP_PUBLISH_TASK); BuildResult result = runner.build(); /** Check if build and {@value ZIP_PUBLISH_TASK} tasks have run well */ @@ -223,21 +361,94 @@ public void customizedGroupValue() throws IOException, URISyntaxException, XmlPu */ @Test public void customizedInvalidGroupValue() throws IOException, URISyntaxException { - GradleRunner runner = prepareGradleRunnerFromTemplate("customizedInvalidGroupValue.gradle"); + GradleRunner runner = prepareGradleRunnerFromTemplate("customizedInvalidGroupValue.gradle", "build", ZIP_PUBLISH_TASK); Exception e = assertThrows(UnexpectedBuildFailure.class, runner::build); assertTrue( e.getMessage().contains("Invalid publication 'pluginZip': groupId ( ) is not a valid Maven identifier ([A-Za-z0-9_\\-.]+).") ); } - private GradleRunner prepareGradleRunnerFromTemplate(String templateName) throws IOException, URISyntaxException { + /** + * This test verifies that use of the pluginZip does not clash with other maven publication plugins. + * It covers the case when user calls the "publishToMavenLocal" task. + */ + @Test + public void publishToMavenLocal() throws IOException, URISyntaxException, XmlPullParserException { + // By default, the "publishToMavenLocal" publishes artifacts to a local m2 repo, typically + // found in `~/.m2/repository`. But this is not practical for this unit test at all. We need to point + // the 'maven-publish' plugin to a custom m2 repo located in temporary directory associated with this + // test case instead. + // + // According to Gradle documentation this should be possible by proper configuration of the publishing + // task (https://docs.gradle.org/current/userguide/publishing_maven.html#publishing_maven:install). + // But for some reason this never worked as expected and artifacts created during this test case + // were always pushed into the default local m2 repository (ie: `~/.m2/repository`). + // The only workaround that seems to work is to pass "-Dmaven.repo.local" property via runner argument. + // (Kudos to: https://stackoverflow.com/questions/72265294/gradle-publishtomavenlocal-specify-custom-directory) + // + // The temporary directory that is used as the local m2 repository is created via in task "prepareLocalMVNRepo". + GradleRunner runner = prepareGradleRunnerFromTemplate( + "publishToMavenLocal.gradle", + String.join(File.separator, "-Dmaven.repo.local=" + projectDir.getRoot(), "build", "local-staging-repo"), + "build", + "prepareLocalMVNRepo", + "publishToMavenLocal" + ); + BuildResult result = runner.build(); + + assertEquals(SUCCESS, result.task(":" + "build").getOutcome()); + assertEquals(SUCCESS, result.task(":" + "publishToMavenLocal").getOutcome()); + + // Parse the maven file and validate it + MavenXpp3Reader reader = new MavenXpp3Reader(); + Model model = reader.read( + new FileReader( + new File( + projectDir.getRoot(), + String.join( + File.separator, + "build", + "local-staging-repo", + "org", + "custom", + "group", + PROJECT_NAME, + "2.0.0.0", + PROJECT_NAME + "-2.0.0.0.pom" + ) + ) + ) + ); + + // The "publishToMavenLocal" task will run ALL maven publications, hence we can expect the ZIP publication + // present as well: https://docs.gradle.org/current/userguide/publishing_maven.html#publishing_maven:tasks + assertEquals(model.getArtifactId(), PROJECT_NAME); + assertEquals(model.getGroupId(), "org.custom.group"); + assertEquals(model.getVersion(), "2.0.0.0"); + assertEquals(model.getPackaging(), "zip"); + + // We have two publications in the build.gradle file, both are "MavenPublication" based. + // Both the mavenJava and pluginZip publications publish to the same location (coordinates) and + // artifacts (the POM file) overwrite each other. However, we can verify that the Zip plugin is + // the last one and "wins" over the mavenJava. + assertEquals(model.getDescription(), "pluginZip publication"); + } + + /** + * A helper method for use cases + * + * @param templateName The name of the file (from "pluginzip" folder) to use as a build.gradle for the test + * @param gradleArguments Optional CLI parameters to pass into Gradle runner + */ + private GradleRunner prepareGradleRunnerFromTemplate(String templateName, String... gradleArguments) throws IOException, + URISyntaxException { useTemplateFile(projectDir.newFile("build.gradle"), templateName); prepareGradleFilesAndSources(); GradleRunner runner = GradleRunner.create() .forwardOutput() .withPluginClasspath() - .withArguments("build", ZIP_PUBLISH_TASK) + .withArguments(gradleArguments) .withProjectDir(projectDir.getRoot()); return runner; @@ -246,7 +457,7 @@ private GradleRunner prepareGradleRunnerFromTemplate(String templateName) throws private void prepareGradleFilesAndSources() throws IOException { // A dummy "source" file that is processed with bundlePlugin and put into a ZIP artifact file File bundleFile = new File(projectDir.getRoot(), PROJECT_NAME + "-source.txt"); - Path zipFile = Files.createFile(bundleFile.toPath()); + Files.createFile(bundleFile.toPath()); // Setting a project name via settings.gradle file writeString(projectDir.newFile("settings.gradle"), "rootProject.name = '" + PROJECT_NAME + "'"); } diff --git a/buildSrc/src/test/resources/pluginzip/customizedGroupValue.gradle b/buildSrc/src/test/resources/pluginzip/customizedGroupValue.gradle index 1bde3edda2d91..94f03132faa80 100644 --- a/buildSrc/src/test/resources/pluginzip/customizedGroupValue.gradle +++ b/buildSrc/src/test/resources/pluginzip/customizedGroupValue.gradle @@ -1,6 +1,5 @@ plugins { id 'java-gradle-plugin' - id 'nebula.maven-base-publish' id 'opensearch.pluginzip' } diff --git a/buildSrc/src/test/resources/pluginzip/customizedInvalidGroupValue.gradle b/buildSrc/src/test/resources/pluginzip/customizedInvalidGroupValue.gradle index b6deeeb12ca6a..6f2abbdacd6d4 100644 --- a/buildSrc/src/test/resources/pluginzip/customizedInvalidGroupValue.gradle +++ b/buildSrc/src/test/resources/pluginzip/customizedInvalidGroupValue.gradle @@ -1,6 +1,5 @@ plugins { id 'java-gradle-plugin' - id 'nebula.maven-base-publish' id 'opensearch.pluginzip' } diff --git a/buildSrc/src/test/resources/pluginzip/missingGroupValue.gradle b/buildSrc/src/test/resources/pluginzip/missingGroupValue.gradle index 602c178ea1a5b..8fcd1d6600b5a 100644 --- a/buildSrc/src/test/resources/pluginzip/missingGroupValue.gradle +++ b/buildSrc/src/test/resources/pluginzip/missingGroupValue.gradle @@ -1,6 +1,5 @@ plugins { id 'java-gradle-plugin' - id 'nebula.maven-base-publish' id 'opensearch.pluginzip' } diff --git a/buildSrc/src/test/resources/pluginzip/missingPOMEntity.gradle b/buildSrc/src/test/resources/pluginzip/missingPOMEntity.gradle index 2cc67c2e98954..394bc53622769 100644 --- a/buildSrc/src/test/resources/pluginzip/missingPOMEntity.gradle +++ b/buildSrc/src/test/resources/pluginzip/missingPOMEntity.gradle @@ -1,6 +1,5 @@ plugins { id 'java-gradle-plugin' - id 'nebula.maven-base-publish' id 'opensearch.pluginzip' } diff --git a/buildSrc/src/test/resources/pluginzip/missingPublications.gradle b/buildSrc/src/test/resources/pluginzip/missingPublications.gradle new file mode 100644 index 0000000000000..ba6b33ad86463 --- /dev/null +++ b/buildSrc/src/test/resources/pluginzip/missingPublications.gradle @@ -0,0 +1,21 @@ +plugins { + id 'java-gradle-plugin' + id 'opensearch.pluginzip' +} + +group="org.custom.group" +version='2.0.0.0' + +// A bundlePlugin task mockup +tasks.register('bundlePlugin', Zip.class) { + archiveFileName = "sample-plugin-${version}.zip" + destinationDirectory = layout.buildDirectory.dir('distributions') + from layout.projectDirectory.file('sample-plugin-source.txt') +} + +//publishing { +// publications { +// pluginZip(MavenPublication) { +// } +// } +//} diff --git a/buildSrc/src/test/resources/pluginzip/publishToMavenLocal.gradle b/buildSrc/src/test/resources/pluginzip/publishToMavenLocal.gradle new file mode 100644 index 0000000000000..8d248dbe08a42 --- /dev/null +++ b/buildSrc/src/test/resources/pluginzip/publishToMavenLocal.gradle @@ -0,0 +1,47 @@ +plugins { + // The java-gradle-plugin adds a new publication called 'pluginMaven' that causes some warnings because it + // clashes a bit with other publications defined in this file. If you are running at the --info level then you can + // expect some warning like the following: + // "Multiple publications with coordinates 'org.custom.group:sample-plugin:2.0.0.0' are published to repository 'mavenLocal'." + id 'java-gradle-plugin' + id 'opensearch.pluginzip' +} + +group="org.custom.group" +version='2.0.0.0' + +// A bundlePlugin task mockup +tasks.register('bundlePlugin', Zip.class) { + archiveFileName = "sample-plugin-${version}.zip" + destinationDirectory = layout.buildDirectory.dir('distributions') + from layout.projectDirectory.file('sample-plugin-source.txt') +} + +// A task to prepare directory for a temporary maven local repository +tasks.register('prepareLocalMVNRepo') { + dependsOn ':bundlePlugin' + doFirst { + File localMVNRepo = new File (layout.buildDirectory.get().getAsFile().getPath(), 'local-staging-repo') + System.out.println('Creating temporary folder for mavenLocal repo: '+ localMVNRepo.toString()) + System.out.println("Success: " + localMVNRepo.mkdir()) + } +} + +publishing { + publications { + // Plugin zip publication + pluginZip(MavenPublication) { + pom { + url = 'http://www.example.com/library' + description = 'pluginZip publication' + } + } + // Standard maven publication + mavenJava(MavenPublication) { + pom { + url = 'http://www.example.com/library' + description = 'mavenJava publication' + } + } + } +} diff --git a/buildSrc/src/test/resources/pluginzip/groupAndVersionValue.gradle b/buildSrc/src/test/resources/pluginzip/useDefaultValues.gradle similarity index 90% rename from buildSrc/src/test/resources/pluginzip/groupAndVersionValue.gradle rename to buildSrc/src/test/resources/pluginzip/useDefaultValues.gradle index bdab385f6082c..52f1c042fd47c 100644 --- a/buildSrc/src/test/resources/pluginzip/groupAndVersionValue.gradle +++ b/buildSrc/src/test/resources/pluginzip/useDefaultValues.gradle @@ -1,6 +1,5 @@ plugins { id 'java-gradle-plugin' - id 'nebula.maven-base-publish' id 'opensearch.pluginzip' } @@ -18,8 +17,8 @@ publishing { publications { pluginZip(MavenPublication) { pom { - name = "sample-plugin" - description = "pluginDescription" +// name = "plugin name" +// description = "plugin description" licenses { license { name = "The Apache License, Version 2.0"