Delete workflow metadata on deleting a detector (#1766)

* Delete workflow metadata on deleting workflow Signed-off-by: Nishtha Mehrotra <[email protected]> * Added UTs for ensuring monitor workflow metadata is deleted on deleting monitor workflow Signed-off-by: Nishtha Mehrotra <[email protected]> * Updated documentation notes Signed-off-by: Nishtha Mehrotra <[email protected]> * Updated error logs Signed-off-by: Nishtha Mehrotra <[email protected]> * Updated assert Signed-off-by: Nishtha Mehrotra <[email protected]> --------- Signed-off-by: Nishtha Mehrotra <[email protected]> Co-authored-by: Nishtha Mehrotra <[email protected]> (cherry picked from commit 03595f8) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Jan 16, 2025 · 6456b33 · 6456b33
1 parent 1dfe19e
commit 6456b33
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteWorkflowAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteWorkflowAction.kt
@@ -137,11 +137,12 @@ class TransportDeleteWorkflowAction @Inject constructor(
                 if (canDelete) {
                     val delegateMonitorIds = (workflow.inputs[0] as CompositeInput).getMonitorIds()
                     var deletableMonitors = listOf<Monitor>()
+                    val delegateMonitors = getDeletableDelegates(workflowId, delegateMonitorIds, user)
                     // User can only delete the delegate monitors only in the case if all monitors can be deleted
                     // if there are monitors in this workflow that are referenced in other workflows, we cannot delete the monitors.
                     // We will not partially delete monitors. we delete them all or fail the request.
                     if (deleteDelegateMonitors == true) {
-                        deletableMonitors = getDeletableDelegates(workflowId, delegateMonitorIds, user)
+                        deletableMonitors = delegateMonitors
                         val monitorsDiff = delegateMonitorIds.toMutableList()
                         monitorsDiff.removeAll(deletableMonitors.map { it.id })
 
@@ -168,10 +169,11 @@ class TransportDeleteWorkflowAction @Inject constructor(
                         val failedMonitorIds = tryDeletingMonitors(deletableMonitors, RefreshPolicy.IMMEDIATE)
                         // Update delete workflow response
                         deleteWorkflowResponse.nonDeletedMonitors = failedMonitorIds
-                        // Delete monitors workflow metadata
-                        // Monitor metadata will be in workflowId-monitorId-metadata format
-                        metadataIdsToDelete.addAll(deletableMonitors.map { MonitorMetadata.getId(it, workflowMetadataId) })
                     }
+
+                    // Delete monitors workflow metadata
+                    // Monitor metadata will be in workflowId-metadata-monitorId-metadata format
+                    metadataIdsToDelete.addAll(delegateMonitors.map { MonitorMetadata.getId(it, workflowMetadataId) })
                     try {
                         // Delete the monitors workflow metadata
                         val deleteMonitorWorkflowMetadataResponse: BulkByScrollResponse = client.suspendUntil {

diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt
@@ -5055,6 +5055,9 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
         assertNotNull(getWorkflowResponse)
         assertEquals(workflowId, getWorkflowResponse.id)
 
+        // Verify that monitor workflow metadata exists
+        assertNotNull(searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata"))
+
         deleteWorkflow(workflowId, false)
         // Verify that the workflow is deleted
         try {
@@ -5070,6 +5073,19 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
         // Verify that the monitor is not deleted
         val existingDelegate = getMonitorResponse(monitorResponse.id)
         assertNotNull(existingDelegate)
+
+        // Verify that the monitor workflow metadata is deleted
+        try {
+            searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata")
+            fail("expected searchMonitorMetadata method to throw exception")
+        } catch (e: Exception) {
+            e.message?.let {
+                assertTrue(
+                    "Expected 0 hits for searchMonitorMetadata, got non-0 results.",
+                    it.contains("List is empty")
+                )
+            }
+        }
     }
 
     fun `test delete workflow delegate monitor deleted`() {
@@ -5095,6 +5111,9 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
         assertNotNull(getWorkflowResponse)
         assertEquals(workflowId, getWorkflowResponse.id)
 
+        // Verify that monitor workflow metadata exists
+        assertNotNull(searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata"))
+
         deleteWorkflow(workflowId, true)
         // Verify that the workflow is deleted
         try {
@@ -5118,6 +5137,18 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
                 )
             }
         }
+        // Verify that the monitor workflow metadata is deleted
+        try {
+            searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata")
+            fail("expected searchMonitorMetadata method to throw exception")
+        } catch (e: Exception) {
+            e.message?.let {
+                assertTrue(
+                    "Expected 0 hits for searchMonitorMetadata, got non-0 results.",
+                    it.contains("List is empty")
+                )
+            }
+        }
     }
 
     fun `test delete executed workflow with metadata deleted`() {