Skip to content

Commit 111738e

Browse files
github-actions[bot]Nishtha Mehrotra
and
Nishtha Mehrotra
committed
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>
1 parent ed9c4d9 commit 111738e

File tree

2 files changed

+37
-4
lines changed

2 files changed

+37
-4
lines changed

alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteWorkflowAction.kt

+6-4
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,12 @@ class TransportDeleteWorkflowAction @Inject constructor(
137137
if (canDelete) {
138138
val delegateMonitorIds = (workflow.inputs[0] as CompositeInput).getMonitorIds()
139139
var deletableMonitors = listOf<Monitor>()
140+
val delegateMonitors = getDeletableDelegates(workflowId, delegateMonitorIds, user)
140141
// User can only delete the delegate monitors only in the case if all monitors can be deleted
141142
// if there are monitors in this workflow that are referenced in other workflows, we cannot delete the monitors.
142143
// We will not partially delete monitors. we delete them all or fail the request.
143144
if (deleteDelegateMonitors == true) {
144-
deletableMonitors = getDeletableDelegates(workflowId, delegateMonitorIds, user)
145+
deletableMonitors = delegateMonitors
145146
val monitorsDiff = delegateMonitorIds.toMutableList()
146147
monitorsDiff.removeAll(deletableMonitors.map { it.id })
147148

@@ -168,10 +169,11 @@ class TransportDeleteWorkflowAction @Inject constructor(
168169
val failedMonitorIds = tryDeletingMonitors(deletableMonitors, RefreshPolicy.IMMEDIATE)
169170
// Update delete workflow response
170171
deleteWorkflowResponse.nonDeletedMonitors = failedMonitorIds
171-
// Delete monitors workflow metadata
172-
// Monitor metadata will be in workflowId-monitorId-metadata format
173-
metadataIdsToDelete.addAll(deletableMonitors.map { MonitorMetadata.getId(it, workflowMetadataId) })
174172
}
173+
174+
// Delete monitors workflow metadata
175+
// Monitor metadata will be in workflowId-metadata-monitorId-metadata format
176+
metadataIdsToDelete.addAll(delegateMonitors.map { MonitorMetadata.getId(it, workflowMetadataId) })
175177
try {
176178
// Delete the monitors workflow metadata
177179
val deleteMonitorWorkflowMetadataResponse: BulkByScrollResponse = client.suspendUntil {

alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt

+31
Original file line numberDiff line numberDiff line change
@@ -5055,6 +5055,9 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
50555055
assertNotNull(getWorkflowResponse)
50565056
assertEquals(workflowId, getWorkflowResponse.id)
50575057

5058+
// Verify that monitor workflow metadata exists
5059+
assertNotNull(searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata"))
5060+
50585061
deleteWorkflow(workflowId, false)
50595062
// Verify that the workflow is deleted
50605063
try {
@@ -5070,6 +5073,19 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
50705073
// Verify that the monitor is not deleted
50715074
val existingDelegate = getMonitorResponse(monitorResponse.id)
50725075
assertNotNull(existingDelegate)
5076+
5077+
// Verify that the monitor workflow metadata is deleted
5078+
try {
5079+
searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata")
5080+
fail("expected searchMonitorMetadata method to throw exception")
5081+
} catch (e: Exception) {
5082+
e.message?.let {
5083+
assertTrue(
5084+
"Expected 0 hits for searchMonitorMetadata, got non-0 results.",
5085+
it.contains("List is empty")
5086+
)
5087+
}
5088+
}
50735089
}
50745090

50755091
fun `test delete workflow delegate monitor deleted`() {
@@ -5095,6 +5111,9 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
50955111
assertNotNull(getWorkflowResponse)
50965112
assertEquals(workflowId, getWorkflowResponse.id)
50975113

5114+
// Verify that monitor workflow metadata exists
5115+
assertNotNull(searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata"))
5116+
50985117
deleteWorkflow(workflowId, true)
50995118
// Verify that the workflow is deleted
51005119
try {
@@ -5118,6 +5137,18 @@ class MonitorDataSourcesIT : AlertingSingleNodeTestCase() {
51185137
)
51195138
}
51205139
}
5140+
// Verify that the monitor workflow metadata is deleted
5141+
try {
5142+
searchMonitorMetadata("${workflowResponse.id}-metadata-${monitorResponse.id}-metadata")
5143+
fail("expected searchMonitorMetadata method to throw exception")
5144+
} catch (e: Exception) {
5145+
e.message?.let {
5146+
assertTrue(
5147+
"Expected 0 hits for searchMonitorMetadata, got non-0 results.",
5148+
it.contains("List is empty")
5149+
)
5150+
}
5151+
}
51215152
}
51225153

51235154
fun `test delete executed workflow with metadata deleted`() {

0 commit comments

Comments
 (0)