Skip to content

Commit bafd45f

Browse files
mikecotemikecote
authored
Fix race condition in flaky alerting test (#60438)
* Fix race condition in flaky test * Fix flakiness in test * Fix more flakiness
1 parent 3bd3364 commit bafd45f

File tree

2 files changed

+69
-22
lines changed

2 files changed

+69
-22
lines changed

‎x-pack/test/alerting_api_integration/common/lib/task_manager_utils.ts‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export class TaskManagerUtils {
1313
this.retry = retry;
1414
}
1515

16-
async waitForIdle(taskRunAtFilter: Date) {
16+
async waitForEmpty(taskRunAtFilter: Date) {
1717
return await this.retry.try(async () => {
1818
const searchResult = await this.es.search({
1919
index: '.kibana_task_manager',
@@ -44,6 +44,44 @@ export class TaskManagerUtils {
4444
});
4545
}
4646

47+
async waitForAllTasksIdle(taskRunAtFilter: Date) {
48+
return await this.retry.try(async () => {
49+
const searchResult = await this.es.search({
50+
index: '.kibana_task_manager',
51+
body: {
52+
query: {
53+
bool: {
54+
must: [
55+
{
56+
terms: {
57+
'task.scope': ['actions', 'alerting'],
58+
},
59+
},
60+
{
61+
range: {
62+
'task.scheduledAt': {
63+
gte: taskRunAtFilter,
64+
},
65+
},
66+
},
67+
],
68+
must_not: [
69+
{
70+
term: {
71+
'task.status': 'idle',
72+
},
73+
},
74+
],
75+
},
76+
},
77+
},
78+
});
79+
if (searchResult.hits.total.value) {
80+
throw new Error(`Expected 0 non-idle tasks but received ${searchResult.hits.total.value}`);
81+
}
82+
});
83+
}
84+
4785
async waitForActionTaskParamsToBeCleanedUp(createdAtFilter: Date): Promise<void> {
4886
return await this.retry.try(async () => {
4987
const searchResult = await this.es.search({

‎x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@ export default function alertTests({ getService }: FtrProviderContext) {
2626
const esTestIndexTool = new ESTestIndexTool(es, retry);
2727
const taskManagerUtils = new TaskManagerUtils(es, retry);
2828

29-
// FLAKY: https://github.com/elastic/kibana/issues/58643
30-
// FLAKY: https://github.com/elastic/kibana/issues/58991
31-
describe.skip('alerts', () => {
29+
describe('alerts', () => {
3230
const authorizationIndex = '.kibana-test-authorization';
3331
const objectRemover = new ObjectRemover(supertest);
3432

@@ -99,9 +97,11 @@ export default function alertTests({ getService }: FtrProviderContext) {
9997
// Wait for the action to index a document before disabling the alert and waiting for tasks to finish
10098
await esTestIndexTool.waitForDocs('action:test.index-record', reference);
10199

100+
await taskManagerUtils.waitForAllTasksIdle(testStart);
101+
102102
const alertId = response.body.id;
103103
await alertUtils.disable(alertId);
104-
await taskManagerUtils.waitForIdle(testStart);
104+
await taskManagerUtils.waitForEmpty(testStart);
105105

106106
// Ensure only 1 alert executed with proper params
107107
const alertSearchResult = await esTestIndexTool.search(
@@ -166,17 +166,23 @@ instanceStateValue: true
166166
});
167167

168168
it('should pass updated alert params to executor', async () => {
169+
const testStart = new Date();
169170
// create an alert
170171
const reference = alertUtils.generateReference();
171-
const overwrites = {
172-
throttle: '1s',
173-
schedule: { interval: '1s' },
174-
};
175-
const response = await alertUtils.createAlwaysFiringAction({ reference, overwrites });
172+
const response = await alertUtils.createAlwaysFiringAction({
173+
reference,
174+
overwrites: { throttle: null },
175+
});
176176

177177
// only need to test creation success paths
178178
if (response.statusCode !== 200) return;
179179

180+
// Wait for the action to index a document before disabling the alert and waiting for tasks to finish
181+
await esTestIndexTool.waitForDocs('action:test.index-record', reference);
182+
183+
// Avoid invalidating an API key while the alert is executing
184+
await taskManagerUtils.waitForAllTasksIdle(testStart);
185+
180186
// update the alert with super user
181187
const alertId = response.body.id;
182188
const reference2 = alertUtils.generateReference();
@@ -188,15 +194,18 @@ instanceStateValue: true
188194
overwrites: {
189195
name: 'def',
190196
tags: ['fee', 'fi', 'fo'],
191-
throttle: '1s',
192-
schedule: { interval: '1s' },
197+
// This will cause the task to re-run on update
198+
schedule: { interval: '59s' },
193199
},
194200
});
195201

196202
expect(response2.statusCode).to.eql(200);
197203

198204
// make sure alert info passed to executor is correct
199205
await esTestIndexTool.waitForDocs('alert:test.always-firing', reference2);
206+
207+
await taskManagerUtils.waitForAllTasksIdle(testStart);
208+
200209
await alertUtils.disable(alertId);
201210
const alertSearchResult = await esTestIndexTool.search(
202211
'alert:test.always-firing',
@@ -359,7 +368,7 @@ instanceStateValue: true
359368
// Wait for test.authorization to index a document before disabling the alert and waiting for tasks to finish
360369
await esTestIndexTool.waitForDocs('alert:test.authorization', reference);
361370
await alertUtils.disable(response.body.id);
362-
await taskManagerUtils.waitForIdle(testStart);
371+
await taskManagerUtils.waitForEmpty(testStart);
363372

364373
// Ensure only 1 document exists with proper params
365374
searchResult = await esTestIndexTool.search('alert:test.authorization', reference);
@@ -387,7 +396,7 @@ instanceStateValue: true
387396
// Wait for test.authorization to index a document before disabling the alert and waiting for tasks to finish
388397
await esTestIndexTool.waitForDocs('alert:test.authorization', reference);
389398
await alertUtils.disable(response.body.id);
390-
await taskManagerUtils.waitForIdle(testStart);
399+
await taskManagerUtils.waitForEmpty(testStart);
391400

392401
// Ensure only 1 document exists with proper params
393402
searchResult = await esTestIndexTool.search('alert:test.authorization', reference);
@@ -467,7 +476,7 @@ instanceStateValue: true
467476
// Ensure test.authorization indexed 1 document before disabling the alert and waiting for tasks to finish
468477
await esTestIndexTool.waitForDocs('action:test.authorization', reference);
469478
await alertUtils.disable(response.body.id);
470-
await taskManagerUtils.waitForIdle(testStart);
479+
await taskManagerUtils.waitForEmpty(testStart);
471480

472481
// Ensure only 1 document with proper params exists
473482
searchResult = await esTestIndexTool.search('action:test.authorization', reference);
@@ -495,7 +504,7 @@ instanceStateValue: true
495504
// Ensure test.authorization indexed 1 document before disabling the alert and waiting for tasks to finish
496505
await esTestIndexTool.waitForDocs('action:test.authorization', reference);
497506
await alertUtils.disable(response.body.id);
498-
await taskManagerUtils.waitForIdle(testStart);
507+
await taskManagerUtils.waitForEmpty(testStart);
499508

500509
// Ensure only 1 document with proper params exists
501510
searchResult = await esTestIndexTool.search('action:test.authorization', reference);
@@ -544,7 +553,7 @@ instanceStateValue: true
544553
// Wait until alerts scheduled actions 3 times before disabling the alert and waiting for tasks to finish
545554
await esTestIndexTool.waitForDocs('alert:test.always-firing', reference, 3);
546555
await alertUtils.disable(response.body.id);
547-
await taskManagerUtils.waitForIdle(testStart);
556+
await taskManagerUtils.waitForEmpty(testStart);
548557

549558
// Ensure actions only executed once
550559
const searchResult = await esTestIndexTool.search(
@@ -610,7 +619,7 @@ instanceStateValue: true
610619
// Wait for actions to execute twice before disabling the alert and waiting for tasks to finish
611620
await esTestIndexTool.waitForDocs('action:test.index-record', reference, 2);
612621
await alertUtils.disable(response.body.id);
613-
await taskManagerUtils.waitForIdle(testStart);
622+
await taskManagerUtils.waitForEmpty(testStart);
614623

615624
// Ensure only 2 actions with proper params exists
616625
const searchResult = await esTestIndexTool.search(
@@ -660,7 +669,7 @@ instanceStateValue: true
660669
// Actions should execute twice before widning things down
661670
await esTestIndexTool.waitForDocs('action:test.index-record', reference, 2);
662671
await alertUtils.disable(response.body.id);
663-
await taskManagerUtils.waitForIdle(testStart);
672+
await taskManagerUtils.waitForEmpty(testStart);
664673

665674
// Ensure only 2 actions are executed
666675
const searchResult = await esTestIndexTool.search(
@@ -705,7 +714,7 @@ instanceStateValue: true
705714
// execution once before disabling the alert and waiting for tasks to finish
706715
await esTestIndexTool.waitForDocs('alert:test.always-firing', reference, 2);
707716
await alertUtils.disable(response.body.id);
708-
await taskManagerUtils.waitForIdle(testStart);
717+
await taskManagerUtils.waitForEmpty(testStart);
709718

710719
// Should not have executed any action
711720
const executedActionsResult = await esTestIndexTool.search(
@@ -750,7 +759,7 @@ instanceStateValue: true
750759
// once before disabling the alert and waiting for tasks to finish
751760
await esTestIndexTool.waitForDocs('alert:test.always-firing', reference, 2);
752761
await alertUtils.disable(response.body.id);
753-
await taskManagerUtils.waitForIdle(testStart);
762+
await taskManagerUtils.waitForEmpty(testStart);
754763

755764
// Should not have executed any action
756765
const executedActionsResult = await esTestIndexTool.search(
@@ -796,7 +805,7 @@ instanceStateValue: true
796805
// Ensure actions are executed once before disabling the alert and waiting for tasks to finish
797806
await esTestIndexTool.waitForDocs('action:test.index-record', reference, 1);
798807
await alertUtils.disable(response.body.id);
799-
await taskManagerUtils.waitForIdle(testStart);
808+
await taskManagerUtils.waitForEmpty(testStart);
800809

801810
// Should have one document indexed by the action
802811
const searchResult = await esTestIndexTool.search(

0 commit comments

Comments
 (0)