Skip to content

Fix race condition in flaky alerting test #60438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 19, 2020
Merged

Fix race condition in flaky alerting test #60438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f95aa73
Fix race condition in flaky test
mikecote Mar 17, 2020
413b6d2
Fix flakiness in test
mikecote Mar 18, 2020
4a3827b
Merge branch 'master' of github.com:elastic/kibana into alerting/fix-…
mikecote Mar 19, 2020
b3abda5
Fix more flakiness
mikecote Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion x-pack/test/alerting_api_integration/common/lib/task_manager_utils.ts
View file
Open in desktop
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
Open in desktop
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