diff --git a/CHANGELOG.md b/CHANGELOG.md index ec8e3c89b..6a53c2f68 100644 --- 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 index ecc4fc730..79e0bd9e3 100644 --- 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 index b49be90b3..b14d2a1bc 100644 --- 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 index 66a3e4ec0..888dc7276 100644 --- 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 index bf01971af..240dbe966 100644 --- 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 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 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 index d51fc25e6..a21793fb1 100644 --- 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 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 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);