Fix alerting flaky test by adding retryIfConflict to fixture APIs (#96226)

* Add retryIfConflict to fixture APIs

* Fix

* Fix import errors?

* Revert part of the fix

* Attempt fix

* Attempt 2

* Try again

* Remove dependency on core code

* Comment

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/index.ts

@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { PluginInitializerContext } from 'kibana/server';
 import { FixturePlugin } from './plugin';
 
-export const plugin = () => new FixturePlugin();
+export const plugin = (initContext: PluginInitializerContext) => new FixturePlugin(initContext);

x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/lib/retry_if_conflicts.ts

@@ -0,0 +1,64 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+// This module provides a helper to perform retries on a function if the
+// function ends up throwing a SavedObject 409 conflict.  This can happen
+// when alert SO's are updated in the background, and will avoid having to
+// have the caller make explicit conflict checks, where the conflict was
+// caused by a background update.
+
+import { Logger } from 'kibana/server';
+
+type RetryableForConflicts<T> = () => Promise<T>;
+
+// number of times to retry when conflicts occur
+export const RetryForConflictsAttempts = 2;
+
+// milliseconds to wait before retrying when conflicts occur
+// 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;
+
+// retry an operation if it runs into 409 Conflict's, up to a limit
+export async function retryIfConflicts<T>(
+  logger: Logger,
+  name: string,
+  operation: RetryableForConflicts<T>,
+  retries: number = RetryForConflictsAttempts
+): Promise<T> {
+  // run the operation, return if no errors or throw if not a conflict error
+  try {
+    return await operation();
+  } catch (err) {
+    if (!isConflictError(err)) {
+      throw err;
+    }
+
+    // must be a conflict; if no retries left, throw it
+    if (retries <= 0) {
+      logger.warn(`${name} conflict, exceeded retries`);
+      throw err;
+    }
+
+    // delay a bit before retrying
+    logger.debug(`${name} conflict, retrying ...`);
+    await waitBeforeNextRetry();
+    return await retryIfConflicts(logger, name, operation, retries - 1);
+  }
+}
+
+async function waitBeforeNextRetry(): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, RetryForConflictsDelay));
+}
+
+// This is a workaround to avoid having to add more code to compile for tests via
+// packages/kbn-test/src/functional_tests/lib/babel_register_for_test_plugins.js
+// to use SavedObjectsErrorHelpers.isConflictError.
+function isConflictError(error: any): boolean {
+  return error.isBoom === true && error.output.statusCode === 409;
+}

x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { Plugin, CoreSetup } from 'kibana/server';
+import { Plugin, CoreSetup, Logger, PluginInitializerContext } from 'kibana/server';
 import { PluginSetupContract as ActionsPluginSetup } from '../../../../../../../plugins/actions/server/plugin';
 import { PluginSetupContract as AlertingPluginSetup } from '../../../../../../../plugins/alerting/server/plugin';
 import { EncryptedSavedObjectsPluginStart } from '../../../../../../../plugins/encrypted_saved_objects/server';
@@ -29,6 +29,12 @@ export interface FixtureStartDeps {
 }
 
 export class FixturePlugin implements Plugin<void, void, FixtureSetupDeps, FixtureStartDeps> {
+  private readonly logger: Logger;
+
+  constructor(initializerContext: PluginInitializerContext) {
+    this.logger = initializerContext.logger.get('fixtures', 'plugins', 'alerts');
+  }
+
   public setup(
     core: CoreSetup<FixtureStartDeps>,
     { features, actions, alerting }: FixtureSetupDeps
@@ -109,7 +115,7 @@ export class FixturePlugin implements Plugin<void, void, FixtureSetupDeps, Fixtu
 
     defineActionTypes(core, { actions });
     defineAlertTypes(core, { alerting });
-    defineRoutes(core);
+    defineRoutes(core, { logger: this.logger });
   }
 
   public start() {}

x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts

@@ -11,6 +11,7 @@ import {
   KibanaRequest,
   KibanaResponseFactory,
   IKibanaResponse,
+  Logger,
 } from 'kibana/server';
 import { schema } from '@kbn/config-schema';
 import { InvalidatePendingApiKey } from '../../../../../../../plugins/alerting/server/types';
@@ -20,8 +21,9 @@ import {
   TaskInstance,
 } from '../../../../../../../plugins/task_manager/server';
 import { FixtureStartDeps } from './plugin';
+import { retryIfConflicts } from './lib/retry_if_conflicts';
 
-export function defineRoutes(core: CoreSetup<FixtureStartDeps>) {
+export function defineRoutes(core: CoreSetup<FixtureStartDeps>, { logger }: { logger: Logger }) {
   const router = core.http.createRouter();
   router.put(
     {
@@ -84,28 +86,35 @@ export function defineRoutes(core: CoreSetup<FixtureStartDeps>) {
         throw new Error('Failed to grant an API Key');
       }
 
-      const result = await savedObjectsWithAlerts.update<RawAlert>(
-        'alert',
-        id,
-        {
-          ...(
-            await encryptedSavedObjectsWithAlerts.getDecryptedAsInternalUser<RawAlert>(
-              'alert',
-              id,
-              {
-                namespace,
-              }
-            )
-          ).attributes,
-          apiKey: Buffer.from(`${createAPIKeyResult.id}:${createAPIKeyResult.api_key}`).toString(
-            'base64'
-          ),
-          apiKeyOwner: user.username,
-        },
-        {
-          namespace,
+      const result = await retryIfConflicts(
+        logger,
+        `/api/alerts_fixture/${id}/replace_api_key`,
+        async () => {
+          return await savedObjectsWithAlerts.update<RawAlert>(
+            'alert',
+            id,
+            {
+              ...(
+                await encryptedSavedObjectsWithAlerts.getDecryptedAsInternalUser<RawAlert>(
+                  'alert',
+                  id,
+                  {
+                    namespace,
+                  }
+                )
+              ).attributes,
+              apiKey: Buffer.from(
+                `${createAPIKeyResult.id}:${createAPIKeyResult.api_key}`
+              ).toString('base64'),
+              apiKeyOwner: user.username,
+            },
+            {
+              namespace,
+            }
+          );
         }
       );
+
       return res.ok({ body: result });
     }
   );
@@ -147,11 +156,17 @@ export function defineRoutes(core: CoreSetup<FixtureStartDeps>) {
         includedHiddenTypes: ['alert'],
       });
       const savedAlert = await savedObjectsWithAlerts.get<RawAlert>(type, id);
-      const result = await savedObjectsWithAlerts.update(
-        type,
-        id,
-        { ...savedAlert.attributes, ...attributes },
-        options
+      const result = await retryIfConflicts(
+        logger,
+        `/api/alerts_fixture/saved_object/${type}/${id}`,
+        async () => {
+          return await savedObjectsWithAlerts.update(
+            type,
+            id,
+            { ...savedAlert.attributes, ...attributes },
+            options
+          );
+        }
       );
       return res.ok({ body: result });
     }
@@ -182,10 +197,16 @@ export function defineRoutes(core: CoreSetup<FixtureStartDeps>) {
         includedHiddenTypes: ['task', 'alert'],
       });
       const alert = await savedObjectsWithTasksAndAlerts.get<RawAlert>('alert', id);
-      const result = await savedObjectsWithTasksAndAlerts.update<TaskInstance>(
-        'task',
-        alert.attributes.scheduledTaskId!,
-        { runAt }
+      const result = await retryIfConflicts(
+        logger,
+        `/api/alerts_fixture/${id}/reschedule_task`,
+        async () => {
+          return await savedObjectsWithTasksAndAlerts.update<TaskInstance>(
+            'task',
+            alert.attributes.scheduledTaskId!,
+            { runAt }
+          );
+        }
       );
       return res.ok({ body: result });
     }
@@ -216,10 +237,16 @@ export function defineRoutes(core: CoreSetup<FixtureStartDeps>) {
         includedHiddenTypes: ['task', 'alert'],
       });
       const alert = await savedObjectsWithTasksAndAlerts.get<RawAlert>('alert', id);
-      const result = await savedObjectsWithTasksAndAlerts.update<ConcreteTaskInstance>(
-        'task',
-        alert.attributes.scheduledTaskId!,
-        { status }
+      const result = await retryIfConflicts(
+        logger,
+        `/api/alerts_fixture/{id}/reset_task_status`,
+        async () => {
+          return await savedObjectsWithTasksAndAlerts.update<ConcreteTaskInstance>(
+            'task',
+            alert.attributes.scheduledTaskId!,
+            { status }
+          );
+        }
       );
       return res.ok({ body: result });
     }