From 3887b6095275595326a9331ac1082b6155ccfc19 Mon Sep 17 00:00:00 2001 From: pranikum <109206473+pranikum@users.noreply.github.com> Date: Wed, 26 Oct 2022 21:46:56 +0530 Subject: [PATCH] Add delay timeout for decommission request (#4931) * Add delay timeout for decommission request Signed-off-by: pranikum <109206473+pranikum@users.noreply.github.com> --- CHANGELOG.md | 2 +- .../awareness/put/DecommissionRequest.java | 10 +++++++++- .../awareness/put/DecommissionRequestBuilder.java | 11 +++++++++++ .../admin/cluster/RestDecommissionAction.java | 8 ++++++-- .../awareness/put/DecommissionRequestTests.java | 12 ++++++++++++ .../cluster/RestDecommissionActionTests.java | 15 +++++++++++++++ 6 files changed, 54 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1230ef2ba402d..42208bf11bcae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -170,7 +170,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [BUG]: flaky test index/80_geo_point/Single point test([#4860](https://github.com/opensearch-project/OpenSearch/pull/4860)) - Fix bug in SlicedInputStream with zero length ([#4863](https://github.com/opensearch-project/OpenSearch/pull/4863)) - Fix a bug on handling an invalid array value for point type field #4900([#4900](https://github.com/opensearch-project/OpenSearch/pull/4900)) - +- [BUG]: Allow decommission to support delay timeout [#4930](https://github.com/opensearch-project/OpenSearch/pull/4930)) ### Security - CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341)) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequest.java index e2fb353b6c749..ae96c8ddb2fde 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequest.java @@ -48,6 +48,7 @@ public DecommissionRequest(StreamInput in) throws IOException { super(in); decommissionAttribute = new DecommissionAttribute(in); this.delayTimeout = in.readTimeValue(); + this.noDelay = in.readBoolean(); } @Override @@ -55,6 +56,7 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); decommissionAttribute.writeTo(out); out.writeTimeValue(delayTimeout); + out.writeBoolean(noDelay); } /** @@ -75,12 +77,18 @@ public DecommissionAttribute getDecommissionAttribute() { return this.decommissionAttribute; } + public void setDelayTimeout(TimeValue delayTimeout) { + this.delayTimeout = delayTimeout; + } + public TimeValue getDelayTimeout() { return this.delayTimeout; } public void setNoDelay(boolean noDelay) { - this.delayTimeout = TimeValue.ZERO; + if (noDelay) { + this.delayTimeout = TimeValue.ZERO; + } this.noDelay = noDelay; } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestBuilder.java index 47af3b952c895..1c7a03fa10e76 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestBuilder.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestBuilder.java @@ -12,6 +12,7 @@ import org.opensearch.action.support.clustermanager.ClusterManagerNodeOperationRequestBuilder; import org.opensearch.client.OpenSearchClient; import org.opensearch.cluster.decommission.DecommissionAttribute; +import org.opensearch.common.unit.TimeValue; /** * Register decommission request builder @@ -35,4 +36,14 @@ public DecommissionRequestBuilder setDecommissionedAttribute(DecommissionAttribu request.setDecommissionAttribute(decommissionAttribute); return this; } + + public DecommissionRequestBuilder setDelayTimeOut(TimeValue delayTimeOut) { + request.setDelayTimeout(delayTimeOut); + return this; + } + + public DecommissionRequestBuilder setNoDelay(boolean noDelay) { + request.setNoDelay(noDelay); + return this; + } } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDecommissionAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDecommissionAction.java index 5f1d1ba48c88b..c041974165eb6 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDecommissionAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDecommissionAction.java @@ -12,6 +12,7 @@ import org.opensearch.client.Requests; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.decommission.DecommissionAttribute; +import org.opensearch.common.unit.TimeValue; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; @@ -51,8 +52,11 @@ DecommissionRequest createRequest(RestRequest request) throws IOException { String attributeValue = request.param("awareness_attribute_value"); // Check if we have no delay set. boolean noDelay = request.paramAsBoolean("no_delay", false); - if (noDelay) { - decommissionRequest.setNoDelay(noDelay); + decommissionRequest.setNoDelay(noDelay); + + if (request.hasParam("delay_timeout")) { + TimeValue delayTimeout = request.paramAsTime("delay_timeout", DecommissionRequest.DEFAULT_NODE_DRAINING_TIMEOUT); + decommissionRequest.setDelayTimeout(delayTimeout); } return decommissionRequest.setDecommissionAttribute(new DecommissionAttribute(attributeName, attributeValue)); } diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestTests.java index 112609b0cf8ec..8cd407b3aecf2 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestTests.java @@ -71,5 +71,17 @@ public void testValidation() { assertNull(e); assertEquals(DecommissionRequest.DEFAULT_NODE_DRAINING_TIMEOUT, request.getDelayTimeout()); } + { + String attributeName = "zone"; + String attributeValue = "test"; + DecommissionAttribute decommissionAttribute = new DecommissionAttribute(attributeName, attributeValue); + + final DecommissionRequest request = new DecommissionRequest(decommissionAttribute); + request.setNoDelay(true); + request.setDelayTimeout(TimeValue.timeValueSeconds(30)); + ActionRequestValidationException e = request.validate(); + assertNotNull(e); + assertTrue(e.getMessage().contains("Invalid decommission request")); + } } } diff --git a/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestDecommissionActionTests.java b/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestDecommissionActionTests.java index bbb21ff8f816c..b5f61f751b19f 100644 --- a/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestDecommissionActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/admin/cluster/RestDecommissionActionTests.java @@ -72,6 +72,21 @@ public void testCreateRequestWithNoDelay() throws IOException { assertEquals(deprecatedRequest.getHttpRequest().method(), RestRequest.Method.PUT); } + public void testCreateRequestWithDelayTimeout() throws IOException { + Map params = new HashMap<>(); + params.put("awareness_attribute_name", "zone"); + params.put("awareness_attribute_value", "zone-1"); + params.put("delay_timeout", "300s"); + + RestRequest deprecatedRequest = buildRestRequest(params); + + DecommissionRequest request = action.createRequest(deprecatedRequest); + assertEquals(request.getDecommissionAttribute().attributeName(), "zone"); + assertEquals(request.getDecommissionAttribute().attributeValue(), "zone-1"); + assertEquals(request.getDelayTimeout().getSeconds(), 300); + assertEquals(deprecatedRequest.getHttpRequest().method(), RestRequest.Method.PUT); + } + private FakeRestRequest buildRestRequest(Map params) { return new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.PUT) .withPath("/_cluster/decommission/awareness/{awareness_attribute_name}/{awareness_attribute_value}")