Improve task manager functional tests in preperation for mget task cl…

…aimer being the default (elastic#196399) Resolves elastic#184942 Resolves elastic#192023 Resolves elastic#195573 In this PR, I'm improving the flakiness found in our functional tests in preperation for mget being the default task claimer that all these tests run with (elastic#194625). Because the mget task claimer works differently and also polls more frequently, we end-up in situations where tasks run faster than they were with update_by_query, creating more race conditions that are now fixed in this PR. Issues were surfaced via elastic#190148 where I set `mget` as the default task claiming strategy. Flaky test runs (some of these failed on other tests that are flaky): - https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7151 - https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7169 - https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7172 - https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7175 - https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7176 - https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7185 (for elastic@0fcf1ae) (cherry picked from commit 3b8cf12) # Conflicts: # x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/alerts_as_data/alerts_as_data_flapping.ts
mikecote · Oct 21, 2024 · 9df52dd · 9df52dd
1 parent 904d201
commit 9df52dd
Show file tree

Hide file tree

Showing 18 changed files with 174 additions and 59 deletions.
diff --git a/x-pack/plugins/actions/server/lib/retry_if_conflicts.ts b/x-pack/plugins/actions/server/lib/retry_if_conflicts.ts
@@ -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>(

diff --git a/x-pack/plugins/alerting/server/lib/retry_if_conflicts.ts b/x-pack/plugins/alerting/server/lib/retry_if_conflicts.ts
@@ -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>(

diff --git a/x-pack/plugins/task_manager/server/task_claimers/strategy_mget.ts b/x-pack/plugins/task_manager/server/task_claimers/strategy_mget.ts
@@ -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))
+      ),
       // Either a task with idle status and runAt <= now or
       // status running or claiming with a retryAt <= now.
       shouldBeOneOf(IdleTaskWithExpiredRunAt, RunningOrClaimingTaskWithExpiredRetryAt),

diff --git a/x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts b/x-pack/test/alerting_api_integration/common/plugins/alerts/server/action_types.ts
@@ -132,6 +132,7 @@ function getIndexRecordActionType() {
           secrets,
           reference: params.reference,
           source: 'action:test.index-record',
+          '@timestamp': new Date(),
         },
       });
       return { status: 'ok', actionId };

diff --git a/x-pack/test/alerting_api_integration/packages/helpers/es_test_index_tool.ts b/x-pack/test/alerting_api_integration/packages/helpers/es_test_index_tool.ts
@@ -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';
 
@@ -61,6 +62,9 @@ export class ESTestIndexTool {
               group: {
                 type: 'keyword',
               },
+              '@timestamp': {
+                type: 'date',
+              },
               host: {
                 properties: {
                   hostname: {
@@ -109,6 +113,7 @@ export class ESTestIndexTool {
   async search(source: string, reference?: string) {
     const body = reference
       ? {
+          sort: [{ '@timestamp': 'asc' }],
           query: {
             bool: {
               must: [
@@ -127,6 +132,7 @@ export class ESTestIndexTool {
           },
         }
       : {
+          sort: [{ '@timestamp': 'asc' }],
           query: {
             term: {
               source,
@@ -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) {

diff --git a/...st/alerting_api_integration/security_and_spaces/group1/tests/alerting/backfill/api_key.ts b/...st/alerting_api_integration/security_and_spaces/group1/tests/alerting/backfill/api_key.ts
@@ -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;
 

diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group4/tests/alerting/alerts.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group4/tests/alerting/alerts.ts
@@ -1184,7 +1184,7 @@ instanceStateValue: true
             reference,
             overwrites: {
               enabled: false,
-              schedule: { interval: '1s' },
+              schedule: { interval: '1m' },
             },
           });
 
@@ -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:[]'
@@ -1304,7 +1304,7 @@ instanceStateValue: true
           const response = await alertUtils.createAlwaysFiringRuleWithSummaryAction({
             reference,
             overwrites: {
-              schedule: { interval: '1s' },
+              schedule: { interval: '1h' },
             },
             notifyWhen: 'onActiveAlert',
             throttle: null,
@@ -1435,7 +1435,7 @@ instanceStateValue: true
           const response = await alertUtils.createAlwaysFiringRuleWithSummaryAction({
             reference,
             overwrites: {
-              schedule: { interval: '1s' },
+              schedule: { interval: '3s' },
             },
             notifyWhen: 'onThrottleInterval',
             throttle: '10s',

diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group1/event_log.ts
@@ -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,
                 },
@@ -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')
@@ -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')
@@ -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')
@@ -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')
@@ -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')
@@ -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')
@@ -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,
@@ -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')
@@ -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: {
@@ -1302,8 +1331,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
                     params: {},
                     frequency: {
                       summary: false,
-                      throttle: '1s',
-                      notify_when: RuleNotifyWhen.THROTTLE,
+                      notify_when: RuleNotifyWhen.ACTIVE,
                     },
                   },
                   {
@@ -1312,8 +1340,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
                     params: {},
                     frequency: {
                       summary: false,
-                      throttle: '1s',
-                      notify_when: RuleNotifyWhen.THROTTLE,
+                      notify_when: RuleNotifyWhen.ACTIVE,
                     },
                   },
                 ],
@@ -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')
@@ -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,
@@ -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')
@@ -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: {
@@ -1501,8 +1536,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
                     params: {},
                     frequency: {
                       summary: false,
-                      throttle: '1s',
-                      notify_when: RuleNotifyWhen.THROTTLE,
+                      notify_when: RuleNotifyWhen.ACTIVE,
                     },
                   },
                   {
@@ -1511,8 +1545,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) {
                     params: {},
                     frequency: {
                       summary: false,
-                      throttle: '1s',
-                      notify_when: RuleNotifyWhen.THROTTLE,
+                      notify_when: RuleNotifyWhen.ACTIVE,
                     },
                   },
                 ],
@@ -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),
@@ -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,
                 },
@@ -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 }],

diff --git a/...i_integration/spaces_only/tests/alerting/group4/alerts_as_data/alerts_as_data_flapping.ts b/...i_integration/spaces_only/tests/alerting/group4/alerts_as_data/alerts_as_data_flapping.ts
@@ -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({
@@ -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;
@@ -753,6 +755,11 @@ export default function createAlertsAsDataFlappingTest({ getService }: FtrProvid
     const searchResult = await es.search({
       index: alertsAsDataIndex,
       body: {
+        sort: [
+          {
+            '@timestamp': 'desc',
+          },
+        ],
         query: {
           bool: {
             must: {