From 2bfdab8ceffe77358f043dd041d9f6fa8c252592 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 16 Jan 2025 23:54:25 +0000 Subject: [PATCH] Delete workflow metadata on deleting a detector (#1766) * Delete workflow metadata on deleting workflow Signed-off-by: Nishtha Mehrotra * Added UTs for ensuring monitor workflow metadata is deleted on deleting monitor workflow Signed-off-by: Nishtha Mehrotra * Updated documentation notes Signed-off-by: Nishtha Mehrotra * Updated error logs Signed-off-by: Nishtha Mehrotra * Updated assert Signed-off-by: Nishtha Mehrotra --------- Signed-off-by: Nishtha Mehrotra Co-authored-by: Nishtha Mehrotra (cherry picked from commit 03595f838e925e378b59ad27fc1916e275acb913) Signed-off-by: github-actions[bot] --- .../TransportDeleteWorkflowAction.kt | 10 +++--- .../alerting/MonitorDataSourcesIT.kt | 31 +++++++++++++++++++ 2 files changed, 37 insertions(+), 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 index 8fb2533fb..5a965689a 100644 --- 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() + 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 index 1bef411bd..711565eed 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt @@ -5050,6 +5050,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 { @@ -5065,6 +5068,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`() { @@ -5090,6 +5106,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 { @@ -5113,6 +5132,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`() {