Skip to content
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.

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

Improve task manager functional tests in preperation for mget task claimer being the default #196399

Merged
merged 13 commits into from
Oct 21, 2024
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/lib/retry_if_conflicts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const RetryForConflictsAttempts = 2;
// note: we considered making this random, to help avoid a stampede, but
// with 1 retry it probably doesn't matter, and adding randomness could
// make it harder to diagnose issues
const RetryForConflictsDelay = 250;
const RetryForConflictsDelay = 100;

// retry an operation if it runs into 409 Conflict's, up to a limit
export async function retryIfConflicts<T>(
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/alerting/server/lib/retry_if_conflicts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const RetryForConflictsAttempts = 2;
// note: we considered making this random, to help avoid a stampede, but
// with 1 retry it probably doesn't matter, and adding randomness could
// make it harder to diagnose issues
const RetryForConflictsDelay = 250;
const RetryForConflictsDelay = 100;

// retry an operation if it runs into 409 Conflict's, up to a limit
export async function retryIfConflicts<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,10 @@ async function searchAvailableTasks({
// Task must be enabled
EnabledTask,
// a task type that's not excluded (may be removed or not)
OneOfTaskTypes('task.taskType', claimPartitions.unlimitedTypes),
OneOfTaskTypes(
'task.taskType',
claimPartitions.unlimitedTypes.concat(Array.from(removedTypes))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test to ensure removedTypes get marked as unrecognized, this should fix it.

),
// Either a task with idle status and runAt <= now or
// status running or claiming with a retryAt <= now.
shouldBeOneOf(IdleTaskWithExpiredRunAt, RunningOrClaimingTaskWithExpiredRetryAt),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function getIndexRecordActionType() {
secrets,
reference: params.reference,
source: 'action:test.index-record',
'@timestamp': new Date(),
},
});
return { status: 'ok', actionId };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { omit } from 'lodash';
import type { Client } from '@elastic/elasticsearch';
import { DeleteByQueryRequest } from '@elastic/elasticsearch/lib/api/types';

Expand Down Expand Up @@ -61,6 +62,9 @@ export class ESTestIndexTool {
group: {
type: 'keyword',
},
'@timestamp': {
type: 'date',
},
host: {
properties: {
hostname: {
Expand Down Expand Up @@ -109,6 +113,7 @@ export class ESTestIndexTool {
async search(source: string, reference?: string) {
const body = reference
? {
sort: [{ '@timestamp': 'asc' }],
query: {
bool: {
must: [
Expand All @@ -127,6 +132,7 @@ export class ESTestIndexTool {
},
}
: {
sort: [{ '@timestamp': 'asc' }],
query: {
term: {
source,
Expand All @@ -138,7 +144,16 @@ export class ESTestIndexTool {
size: 1000,
body,
};
return await this.es.search(params, { meta: true });
const result = await this.es.search(params, { meta: true });
result.body.hits.hits = result.body.hits.hits.map((hit) => {
return {
...hit,
// Easier to remove @timestamp than to have all the downstream code ignore it
// in their assertions
_source: omit(hit._source as Record<string, unknown>, '@timestamp'),
};
});
return result;
Comment on lines +147 to +156
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a rare case, we receive the document we expect 2nd in the list instead of first. Having a sort on timestamp will make sure it's consistent.

}

async getAll(size: number = 10) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default function apiKeyBackfillTests({ getService }: FtrProviderContext)
}

it('should wait to invalidate API key until backfill for rule is complete', async () => {
const start = moment().utc().startOf('day').subtract(7, 'days').toISOString();
const start = moment().utc().startOf('day').subtract(13, 'days').toISOString();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backfill jobs sometimes ran too fast, adding more back days will compensate for this so we still have the backfill running while doing assertions on the API keys to remove.

const end = moment().utc().startOf('day').subtract(4, 'day').toISOString();
const spaceId = SuperuserAtSpace1.space.id;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ instanceStateValue: true
reference,
overwrites: {
enabled: false,
schedule: { interval: '1s' },
schedule: { interval: '1m' },
},
});

Expand Down Expand Up @@ -1288,7 +1288,7 @@ instanceStateValue: true
);

// @ts-expect-error doesnt handle total: number
expect(searchResult.body.hits.total.value).to.eql(1);
expect(searchResult.body.hits.total.value).to.be.greaterThan(0);
// @ts-expect-error _source: unknown
expect(searchResult.body.hits.hits[0]._source.params.message).to.eql(
'Alerts, all:2, new:2 IDs:[1,2,], ongoing:0 IDs:[], recovered:0 IDs:[]'
Expand All @@ -1304,7 +1304,7 @@ instanceStateValue: true
const response = await alertUtils.createAlwaysFiringRuleWithSummaryAction({
reference,
overwrites: {
schedule: { interval: '1s' },
schedule: { interval: '1h' },
},
notifyWhen: 'onActiveAlert',
throttle: null,
Expand Down Expand Up @@ -1435,7 +1435,7 @@ instanceStateValue: true
const response = await alertUtils.createAlwaysFiringRuleWithSummaryAction({
reference,
overwrites: {
schedule: { interval: '1s' },
schedule: { interval: '3s' },
},
notifyWhen: 'onThrottleInterval',
throttle: '10s',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
.send(
getTestRuleData({
rule_type_id: 'test.patternFiring',
schedule: { interval: '1s' },
throttle: null,
schedule: { interval: '2s' },
throttle: '1s',
params: {
pattern,
},
Expand Down Expand Up @@ -665,6 +665,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand Down Expand Up @@ -763,6 +767,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand Down Expand Up @@ -871,6 +879,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand Down Expand Up @@ -964,6 +976,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand Down Expand Up @@ -1067,6 +1083,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 5,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand Down Expand Up @@ -1166,6 +1186,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand All @@ -1192,7 +1216,8 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
.send(
getTestRuleData({
rule_type_id: 'test.patternFiring',
schedule: { interval: '1s' },
schedule: { interval: '2s' },
notify_when: RuleNotifyWhen.THROTTLE,
throttle: '1s',
params: {
pattern,
Expand Down Expand Up @@ -1263,6 +1288,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand All @@ -1289,7 +1318,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
.send(
getTestRuleData({
rule_type_id: 'test.patternFiring',
schedule: { interval: '1s' },
schedule: { interval: '2s' },
throttle: null,
notify_when: null,
params: {
Expand All @@ -1302,8 +1331,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
params: {},
frequency: {
summary: false,
throttle: '1s',
notify_when: RuleNotifyWhen.THROTTLE,
notify_when: RuleNotifyWhen.ACTIVE,
},
},
{
Expand All @@ -1312,8 +1340,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
params: {},
frequency: {
summary: false,
throttle: '1s',
notify_when: RuleNotifyWhen.THROTTLE,
notify_when: RuleNotifyWhen.ACTIVE,
},
},
],
Expand Down Expand Up @@ -1371,6 +1398,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand All @@ -1396,7 +1427,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
.send(
getTestRuleData({
rule_type_id: 'test.patternFiring',
schedule: { interval: '1s' },
schedule: { interval: '2s' },
throttle: '1s',
params: {
pattern,
Expand Down Expand Up @@ -1463,6 +1494,10 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
status_change_threshold: 4,
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

const { body: createdAction } = await supertest
.post(`${getUrlPrefix(space.id)}/api/actions/connector`)
.set('kbn-xsrf', 'foo')
Expand All @@ -1488,7 +1523,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
.send(
getTestRuleData({
rule_type_id: 'test.patternFiring',
schedule: { interval: '1s' },
schedule: { interval: '2s' },
throttle: null,
notify_when: null,
params: {
Expand All @@ -1501,8 +1536,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
params: {},
frequency: {
summary: false,
throttle: '1s',
notify_when: RuleNotifyWhen.THROTTLE,
notify_when: RuleNotifyWhen.ACTIVE,
},
},
{
Expand All @@ -1511,8 +1545,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
params: {},
frequency: {
summary: false,
throttle: '1s',
notify_when: RuleNotifyWhen.THROTTLE,
notify_when: RuleNotifyWhen.ACTIVE,
},
},
],
Expand Down Expand Up @@ -1567,6 +1600,9 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

// flap and then recover, then active again
const instance = [true, false, true, false, true].concat(
...new Array(6).fill(false),
Expand Down Expand Up @@ -1709,8 +1745,8 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
.send(
getTestRuleData({
rule_type_id: 'test.patternFiring',
schedule: { interval: '1s' },
throttle: null,
schedule: { interval: '2s' },
throttle: '1s',
params: {
pattern,
},
Expand Down Expand Up @@ -1942,8 +1978,8 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
provider: 'alerting',
actions: new Map([
// make sure the counts of the # of events per type are as expected
['execute-start', { equal: 6 }],
['execute', { equal: 6 }],
['execute-start', { gte: 6 }],
['execute', { gte: 6 }],
['new-instance', { equal: 1 }],
['active-instance', { equal: 2 }],
['recovered-instance', { equal: 1 }],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,9 @@ export default function createAlertsAsDataFlappingTest({ getService }: FtrProvid
})
.expect(200);

// wait so cache expires
await setTimeoutAsync(TEST_CACHE_EXPIRATION_TIME);

// Wait for the rule to run once
let run = 1;
let runWhichItFlapped = 0;
Expand Down Expand Up @@ -752,6 +755,11 @@ export default function createAlertsAsDataFlappingTest({ getService }: FtrProvid
const searchResult = await es.search({
index: alertsAsDataIndex,
body: {
sort: [
{
'@timestamp': 'desc',
},
],
query: {
bool: {
must: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,13 @@ export default function ruleTests({ getService }: FtrProviderContext) {
});
});

const { status, body: rule } = await supertest.get(
`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule/${ruleId}`
);
expect(status).to.eql(200);
expect(rule.execution_status.status).to.eql('active');
await retry.try(async () => {
const { status, body: rule } = await supertest.get(
`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule/${ruleId}`
);
expect(status).to.eql(200);
expect(rule.execution_status.status).to.eql('active');
});
Comment on lines +113 to +119
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes the event log documents are persisted before the rule finishes running and update its own status, causing this code to expect active while the rule is not updated yet.

});

it('still logs alert docs when rule exceeds timeout when cancelAlertsOnRuleTimeout is false on rule type', async () => {
Expand Down
Loading