Handle Not Found deprovision exceptions as successful deletions (#805)

Signed-off-by: Daniel Widdis <[email protected]> (cherry picked from commit 034dff7) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Jul 25, 2024 · 359319d · 359319d
1 parent 2996e7d
commit 359319d
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 - Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors ([#750](https://github.com/opensearch-project/flow-framework/pull/750))
 
 ### Bug Fixes
+- Handle Not Found exceptions as successful deletions for agents and models ([#805](https://github.com/opensearch-project/flow-framework/pull/805))
 
 ### Infrastructure
 ### Documentation

diff --git a/release-notes/opensearch-flow-framework.release-notes-2.16.0.0.md b/release-notes/opensearch-flow-framework.release-notes-2.16.0.0.md
@@ -8,6 +8,9 @@ Compatible with OpenSearch 2.16.0
 - Add allow_delete parameter to Deprovision API ([#763](https://github.com/opensearch-project/flow-framework/pull/763))
 - Improve Template and WorkflowState builders ([#778](https://github.com/opensearch-project/flow-framework/pull/778))
 
+### Bug Fixes
+- Handle Not Found deprovision exceptions as successful deletions ([#805](https://github.com/opensearch-project/flow-framework/pull/805))
+
 ### Infrastructure
 - Update dependency com.fasterxml.jackson.core:jackson-core to v2.17.2 ([#760](https://github.com/opensearch-project/flow-framework/pull/760))
 - Update dependency gradle to v8.8 ([#725](https://github.com/opensearch-project/flow-framework/pull/725))

diff --git a/src/main/java/org/opensearch/flowframework/workflow/DeleteAgentStep.java b/src/main/java/org/opensearch/flowframework/workflow/DeleteAgentStep.java
@@ -11,9 +11,11 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.ExceptionsHelper;
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.delete.DeleteResponse;
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.flowframework.exception.FlowFrameworkException;
 import org.opensearch.flowframework.exception.WorkflowStepException;
 import org.opensearch.flowframework.util.ParseUtils;
@@ -85,9 +87,21 @@ public void onResponse(DeleteResponse deleteResponse) {
                 @Override
                 public void onFailure(Exception ex) {
                     Exception e = getSafeException(ex);
-                    String errorMessage = (e == null ? "Failed to delete agent " + agentId : e.getMessage());
-                    logger.error(errorMessage, e);
-                    deleteAgentFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
+                    // NOT_FOUND exception should be treated as successful deletion
+                    // https://github.com/opensearch-project/ml-commons/issues/2751
+                    if (e instanceof OpenSearchStatusException && ((OpenSearchStatusException) e).status().equals(RestStatus.NOT_FOUND)) {
+                        deleteAgentFuture.onResponse(
+                            new WorkflowData(
+                                Map.ofEntries(Map.entry(AGENT_ID, agentId)),
+                                currentNodeInputs.getWorkflowId(),
+                                currentNodeInputs.getNodeId()
+                            )
+                        );
+                    } else {
+                        String errorMessage = (e == null ? "Failed to delete agent " + agentId : e.getMessage());
+                        logger.error(errorMessage, e);
+                        deleteAgentFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
+                    }
                 }
             });
         } catch (FlowFrameworkException e) {

diff --git a/src/main/java/org/opensearch/flowframework/workflow/DeleteModelStep.java b/src/main/java/org/opensearch/flowframework/workflow/DeleteModelStep.java
@@ -11,9 +11,11 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.ExceptionsHelper;
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.delete.DeleteResponse;
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.flowframework.exception.FlowFrameworkException;
 import org.opensearch.flowframework.exception.WorkflowStepException;
 import org.opensearch.flowframework.util.ParseUtils;
@@ -86,9 +88,21 @@ public void onResponse(DeleteResponse deleteResponse) {
                 @Override
                 public void onFailure(Exception ex) {
                     Exception e = getSafeException(ex);
-                    String errorMessage = (e == null ? "Failed to delete model " + modelId : e.getMessage());
-                    logger.error(errorMessage, e);
-                    deleteModelFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
+                    // NOT_FOUND exception should be treated as successful deletion
+                    // https://github.com/opensearch-project/ml-commons/issues/2751
+                    if (e instanceof OpenSearchStatusException && ((OpenSearchStatusException) e).status().equals(RestStatus.NOT_FOUND)) {
+                        deleteModelFuture.onResponse(
+                            new WorkflowData(
+                                Map.ofEntries(Map.entry(MODEL_ID, modelId)),
+                                currentNodeInputs.getWorkflowId(),
+                                currentNodeInputs.getNodeId()
+                            )
+                        );
+                    } else {
+                        String errorMessage = (e == null ? "Failed to delete model " + modelId : e.getMessage());
+                        logger.error(errorMessage, e);
+                        deleteModelFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
+                    }
                 }
             });
         } catch (FlowFrameworkException e) {

diff --git a/src/test/java/org/opensearch/flowframework/workflow/DeleteAgentStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/DeleteAgentStepTests.java
@@ -8,6 +8,7 @@
  */
 package org.opensearch.flowframework.workflow;
 
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.delete.DeleteResponse;
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.core.action.ActionListener;
@@ -73,6 +74,30 @@ public void testDeleteAgent() throws IOException, ExecutionException, Interrupte
         assertEquals(agentId, future.get().getContent().get(AGENT_ID));
     }
 
+    public void testDeleteAgentNotFound() throws IOException, ExecutionException, InterruptedException {
+
+        String agentId = randomAlphaOfLength(5);
+        DeleteAgentStep deleteAgentStep = new DeleteAgentStep(machineLearningNodeClient);
+
+        doAnswer(invocation -> {
+            ActionListener<DeleteResponse> actionListener = invocation.getArgument(1);
+            actionListener.onFailure(new OpenSearchStatusException("No agent found with that id", RestStatus.NOT_FOUND));
+            return null;
+        }).when(machineLearningNodeClient).deleteAgent(any(String.class), any());
+
+        PlainActionFuture<WorkflowData> future = deleteAgentStep.execute(
+            inputData.getNodeId(),
+            inputData,
+            Map.of("step_1", new WorkflowData(Map.of(AGENT_ID, agentId), "workflowId", "nodeId")),
+            Map.of("step_1", AGENT_ID),
+            Collections.emptyMap()
+        );
+        verify(machineLearningNodeClient).deleteAgent(any(String.class), any());
+
+        assertTrue(future.isDone());
+        assertEquals(agentId, future.get().getContent().get(AGENT_ID));
+    }
+
     public void testNoAgentIdInOutput() throws IOException {
         DeleteAgentStep deleteAgentStep = new DeleteAgentStep(machineLearningNodeClient);
 

diff --git a/src/test/java/org/opensearch/flowframework/workflow/DeleteModelStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/DeleteModelStepTests.java
@@ -8,6 +8,7 @@
  */
 package org.opensearch.flowframework.workflow;
 
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.delete.DeleteResponse;
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.core.action.ActionListener;
@@ -73,6 +74,30 @@ public void testDeleteModel() throws IOException, ExecutionException, Interrupte
         assertEquals(modelId, future.get().getContent().get(MODEL_ID));
     }
 
+    public void testDeleteModelNotFound() throws IOException, ExecutionException, InterruptedException {
+
+        String modelId = randomAlphaOfLength(5);
+        DeleteModelStep deleteModelStep = new DeleteModelStep(machineLearningNodeClient);
+
+        doAnswer(invocation -> {
+            ActionListener<DeleteResponse> actionListener = invocation.getArgument(1);
+            actionListener.onFailure(new OpenSearchStatusException("No model found with that id", RestStatus.NOT_FOUND));
+            return null;
+        }).when(machineLearningNodeClient).deleteModel(any(String.class), any());
+
+        PlainActionFuture<WorkflowData> future = deleteModelStep.execute(
+            inputData.getNodeId(),
+            inputData,
+            Map.of("step_1", new WorkflowData(Map.of(MODEL_ID, modelId), "workflowId", "nodeId")),
+            Map.of("step_1", MODEL_ID),
+            Collections.emptyMap()
+        );
+        verify(machineLearningNodeClient).deleteModel(any(String.class), any());
+
+        assertTrue(future.isDone());
+        assertEquals(modelId, future.get().getContent().get(MODEL_ID));
+    }
+
     public void testNoModelIdInOutput() throws IOException {
         DeleteModelStep deleteModelStep = new DeleteModelStep(machineLearningNodeClient);