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.

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

[8.x] Improve task manager functional tests in preperation for mget task claimer being the default (#196399) #197062

Merged
merged 1 commit into from
Oct 21, 2024
+174 −59
Merged

[8.x] Improve task manager functional tests in preperation for mget task claimer being the default (#196399) #197062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/lib/retry_if_conflicts.ts
View file
Open in desktop
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
View file
Open in desktop
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
5 changes: 4 additions & 1 deletion x-pack/plugins/task_manager/server/task_claimers/strategy_mget.ts
View file
Open in desktop
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
Member

Choose a reason for hiding this comment

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

Is this going to cause a starvation problem? I don't think so, as it looks like we are removing them (marking them to unrecognized) at the ed of the claim, but not clear to me why this wasn't in here before. Perhaps there was some confusion since we also have an issue to handle these via a separate stand-alone task?

This code does seem right, so I'm guessing some test was expecting these to be marked as unrecognized, and perhaps that wasn't happening before this PR ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing some test was expecting these to be marked as unrecognized, and perhaps that wasn't happening before this PR ...

Yeah there was a test in the update_by_query suite that ensured removed task types get marked as unrecognized:

kibana/x-pack/test/plugin_api_integration/test_suites/task_manager/task_management_removed_types.ts

Line 79 in fb33997

it('should successfully schedule registered tasks, not claim unregistered tasks and mark removed task types as unrecognized', async () => {

I was able to make the test pass by adding this bit to the mget filter but I agree we should move this to its own task next via #192686

),
// Either a task with idle status and runAt <= now or
// status running or claiming with a retryAt <= now.
shouldBeOneOf(IdleTaskWithExpiredRunAt, RunningOrClaimingTaskWithExpiredRetryAt),
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts
View file
Open in desktop
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
17 changes: 16 additions & 1 deletion x-pack/test/alerting_api_integration/packages/helpers/es_test_index_tool.ts
View file
Open in desktop
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;
}

async getAll(size: number = 10) {
Expand Down
2 changes: 1 addition & 1 deletion ...st/alerting_api_integration/security_and_spaces/group1/tests/alerting/backfill/api_key.ts
View file
Open in desktop
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();
const end = moment().utc().startOf('day').subtract(4, 'day').toISOString();
const spaceId = SuperuserAtSpace1.space.id;

Expand Down
8 changes: 4 additions & 4 deletions x-pack/test/alerting_api_integration/security_and_spaces/group4/tests/alerting/alerts.ts
View file
Open in desktop
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
72 changes: 54 additions & 18 deletions x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts
View file
Open in desktop
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
11 changes: 9 additions & 2 deletions ...i_integration/spaces_only/tests/alerting/group4/alerts_as_data/alerts_as_data_flapping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ export default function createAlertsAsDataFlappingTest({ getService }: FtrProvid

const alertsAsDataIndex = '.alerts-test.patternfiring.alerts-default';

// Failing: See https://github.com/elastic/kibana/issues/195573
describe.skip('alerts as data flapping', function () {
describe('alerts as data flapping', function () {
this.tags('skipFIPS');
beforeEach(async () => {
await es.deleteByQuery({
Expand Down Expand Up @@ -711,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 @@ -753,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
Loading