Skip to content

Commit

Permalink
Fix race condition in flaky alerting test (#60438)
Browse files Browse the repository at this point in the history 
* Fix race condition in flaky test

* Fix flakiness in test

* Fix more flakiness
  • Loading branch information
@mikecote
mikecote committed Mar 19, 2020
1 parent 3bd3364 commit bafd45f
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 22 deletions.
40 changes: 39 additions & 1 deletion x-pack/test/alerting_api_integration/common/lib/task_manager_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class TaskManagerUtils {
this.retry = retry;
}

async waitForIdle(taskRunAtFilter: Date) {
async waitForEmpty(taskRunAtFilter: Date) {
return await this.retry.try(async () => {
const searchResult = await this.es.search({
index: '.kibana_task_manager',
Expand Down Expand Up @@ -44,6 +44,44 @@ export class TaskManagerUtils {
});
}

async waitForAllTasksIdle(taskRunAtFilter: Date) {
return await this.retry.try(async () => {
const searchResult = await this.es.search({
index: '.kibana_task_manager',
body: {
query: {
bool: {
must: [
{
terms: {
'task.scope': ['actions', 'alerting'],
},
},
{
range: {
'task.scheduledAt': {
gte: taskRunAtFilter,
},
},
},
],
must_not: [
{
term: {
'task.status': 'idle',
},
},
],
},
},
},
});
if (searchResult.hits.total.value) {
throw new Error(`Expected 0 non-idle tasks but received ${searchResult.hits.total.value}`);
}
});
}

async waitForActionTaskParamsToBeCleanedUp(createdAtFilter: Date): Promise<void> {
return await this.retry.try(async () => {
const searchResult = await this.es.search({
Expand Down
51 changes: 30 additions & 21 deletions x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/alerts.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ export default function alertTests({ getService }: FtrProviderContext) {
const esTestIndexTool = new ESTestIndexTool(es, retry);
const taskManagerUtils = new TaskManagerUtils(es, retry);

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

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

await taskManagerUtils.waitForAllTasksIdle(testStart);

const alertId = response.body.id;
await alertUtils.disable(alertId);
await taskManagerUtils.waitForIdle(testStart);
await taskManagerUtils.waitForEmpty(testStart);

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

it('should pass updated alert params to executor', async () => {
const testStart = new Date();
// create an alert
const reference = alertUtils.generateReference();
const overwrites = {
throttle: '1s',
schedule: { interval: '1s' },
};
const response = await alertUtils.createAlwaysFiringAction({ reference, overwrites });
const response = await alertUtils.createAlwaysFiringAction({
reference,
overwrites: { throttle: null },
});

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

// Wait for the action to index a document before disabling the alert and waiting for tasks to finish
await esTestIndexTool.waitForDocs('action:test.index-record', reference);

// Avoid invalidating an API key while the alert is executing
await taskManagerUtils.waitForAllTasksIdle(testStart);

// update the alert with super user
const alertId = response.body.id;
const reference2 = alertUtils.generateReference();
Expand All @@ -188,15 +194,18 @@ instanceStateValue: true
overwrites: {
name: 'def',
tags: ['fee', 'fi', 'fo'],
throttle: '1s',
schedule: { interval: '1s' },
// This will cause the task to re-run on update
schedule: { interval: '59s' },
},
});

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

// make sure alert info passed to executor is correct
await esTestIndexTool.waitForDocs('alert:test.always-firing', reference2);

await taskManagerUtils.waitForAllTasksIdle(testStart);

await alertUtils.disable(alertId);
const alertSearchResult = await esTestIndexTool.search(
'alert:test.always-firing',
Expand Down Expand Up @@ -359,7 +368,7 @@ instanceStateValue: true
// Wait for test.authorization to index a document before disabling the alert and waiting for tasks to finish
await esTestIndexTool.waitForDocs('alert:test.authorization', reference);
await alertUtils.disable(response.body.id);
await taskManagerUtils.waitForIdle(testStart);
await taskManagerUtils.waitForEmpty(testStart);

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

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

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

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

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

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

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

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

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

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

0 comments on commit bafd45f

Please sign in to comment.