From 6456b333175c4f4b41162599a00d2bef52e5e2cc Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Thu, 16 Jan 2025 23:54:29 +0000
Subject: [PATCH] Delete workflow metadata on deleting a detector (#1766)
* Delete workflow metadata on deleting workflow
Signed-off-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
* Added UTs for ensuring monitor workflow metadata is deleted on deleting monitor workflow
Signed-off-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
* Updated documentation notes
Signed-off-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
* Updated error logs
Signed-off-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
* Updated assert
Signed-off-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
---------
Signed-off-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
Co-authored-by: Nishtha Mehrotra <nishtham.nishtham@amazon.com>
(cherry picked from commit 03595f838e925e378b59ad27fc1916e275acb913)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
---
.../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<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
index 7a26bc9e2..b0ee5abd0 100644
--- 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`() {