Add delay timeout for decommission request (opensearch-project#4931)

* Add delay timeout for decommission request Signed-off-by: pranikum <109206473+pranikum@users.noreply.github.com>
pranikum · Nov 3, 2022 · 1eb6139 · 1eb6139
1 parent 09cc5e7
commit 1eb6139
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -93,6 +93,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Fix decommission status update to non leader nodes ([4800](https://github.com/opensearch-project/OpenSearch/pull/4800))
 - Fix bug in AwarenessAttributeDecommissionIT([4822](https://github.com/opensearch-project/OpenSearch/pull/4822))
 - Fix for failing checkExtraction, checkLicense and checkNotice tasks for windows gradle check ([#4941](https://github.com/opensearch-project/OpenSearch/pull/4941))
+- [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/...a/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequest.java b/...a/org/opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequest.java
@@ -48,13 +48,15 @@ public DecommissionRequest(StreamInput in) throws IOException {
         super(in);
         decommissionAttribute = new DecommissionAttribute(in);
         this.delayTimeout = in.readTimeValue();
+        this.noDelay = in.readBoolean();
     }
 
     @Override
     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/...pensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestBuilder.java b/...pensearch/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
@@ -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/.../opensearch/action/admin/cluster/decommission/awareness/put/DecommissionRequestTests.java b/.../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/...r/src/test/java/org/opensearch/rest/action/admin/cluster/RestDecommissionActionTests.java b/...r/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<String, String> 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<String, String> params) {
         return new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.PUT)
             .withPath("/_cluster/decommission/awareness/{awareness_attribute_name}/{awareness_attribute_value}")