[Advanced Settings] Add retry for 409 conflicts in API integration te…

…sts (elastic#189813)

Fixes elastic#176445

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6679

This PR fixes the advanced settings API integration tests that seem to
be flaky. The reason for the occasional failures is most likely a
`version_conflict_engine_exception` which is thrown when another node
indexes the same documents. This can happen when we save an advanced
setting since the settings API uses saved objects under the hood, and in
CI, multiple nodes can try to save an advanced setting simultaneously.

The solution in this PR is to retry the request if we encounter a 409
error. This is adapted from the solution in
elastic#174185 which resolves a similar
failure.

(cherry picked from commit cc29eea)

x-pack/test/api_integration/apis/management/advanced_settings/feature_controls.ts

@@ -6,16 +6,17 @@
  */
 
 import expect from '@kbn/expect';
-import { SuperTest } from 'supertest';
 import { CSV_QUOTE_VALUES_SETTING } from '@kbn/share-plugin/common/constants';
 import {
   ELASTIC_HTTP_VERSION_HEADER,
   X_ELASTIC_INTERNAL_ORIGIN_REQUEST,
 } from '@kbn/core-http-common';
 import { FtrProviderContext } from '../../../ftr_provider_context';
+import { retryRequestIfConflicts } from './utils';
 
 export default function featureControlsTests({ getService }: FtrProviderContext) {
-  const supertest: SuperTest<any> = getService('supertestWithoutAuth');
+  const supertestWithoutAuth = getService('supertestWithoutAuth');
+  const log = getService('log');
   const security = getService('security');
   const spaces = getService('spaces');
   const deployment = getService('deployment');
@@ -55,31 +56,36 @@ export default function featureControlsTests({ getService }: FtrProviderContext)
   async function saveAdvancedSetting(username: string, password: string, spaceId?: string) {
     const basePath = spaceId ? `/s/${spaceId}` : '';
 
-    return await supertest
-      .post(`${basePath}/internal/kibana/settings`)
-      .auth(username, password)
-      .set('kbn-xsrf', 'foo')
-      .send({ changes: { [CSV_QUOTE_VALUES_SETTING]: null } })
+    const sendRequest = async () =>
+      await supertestWithoutAuth
+        .post(`${basePath}/internal/kibana/settings`)
+        .auth(username, password)
+        .set('kbn-xsrf', 'foo')
+        .send({ changes: { [CSV_QUOTE_VALUES_SETTING]: null } });
+
+    return await retryRequestIfConflicts(log, 'saveAdvancedSetting', sendRequest)
       .then((response: any) => ({ error: undefined, response }))
       .catch((error: any) => ({ error, response: undefined }));
   }
 
   async function saveTelemetrySetting(username: string, password: string, spaceId?: string) {
     const basePath = spaceId ? `/s/${spaceId}` : '';
 
-    return await supertest
-      .post(`${basePath}/internal/telemetry/optIn`)
-      .auth(username, password)
-      .set('kbn-xsrf', 'foo')
-      .set(ELASTIC_HTTP_VERSION_HEADER, '2')
-      .set(X_ELASTIC_INTERNAL_ORIGIN_REQUEST, 'kibana')
-      .send({ enabled: true })
+    const sendRequest = async () =>
+      await supertestWithoutAuth
+        .post(`${basePath}/internal/telemetry/optIn`)
+        .auth(username, password)
+        .set('kbn-xsrf', 'foo')
+        .set(ELASTIC_HTTP_VERSION_HEADER, '2')
+        .set(X_ELASTIC_INTERNAL_ORIGIN_REQUEST, 'kibana')
+        .send({ enabled: true });
+
+    return await retryRequestIfConflicts(log, 'saveTelemetrySetting', sendRequest)
       .then((response: any) => ({ error: undefined, response }))
       .catch((error: any) => ({ error, response: undefined }));
   }
 
-  // Failing: See https://github.com/elastic/kibana/issues/176445
-  describe.skip('feature controls', () => {
+  describe('feature controls', () => {
     it(`settings can be saved with the advancedSettings: ["all"] feature privilege`, async () => {
       const username = 'settings_all';
       const roleName = 'settings_all';

x-pack/test/api_integration/apis/management/advanced_settings/utils/index.ts

@@ -0,0 +1,8 @@
+/*
+ * 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.
+ */
+
+export { retryRequestIfConflicts } from './retry_if_conflicts';

x-pack/test/api_integration/apis/management/advanced_settings/utils/retry_if_conflicts.ts

@@ -0,0 +1,46 @@
+/*
+ * 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.
+ */
+
+import { ToolingLog } from '@kbn/tooling-log';
+
+// Number of times to retry when conflicts occur
+const RETRY_ATTEMPTS = 2;
+
+// Delay between retries when conflicts occur
+const RETRY_DELAY = 200;
+
+/*
+ * Retry a request if it runs into 409 Conflicts,
+ * up to a maximum number of attempts.
+ */
+export const retryRequestIfConflicts = async (
+  logger: ToolingLog,
+  name: string,
+  sendRequest: () => Promise<any>,
+  retries: number = RETRY_ATTEMPTS,
+  retryDelay: number = RETRY_DELAY
+): Promise<any> => {
+  const response = await sendRequest();
+  if (response.statusCode !== 409) {
+    return response;
+  }
+
+  // If no retries left, throw it
+  if (retries <= 0) {
+    logger.error(`${name} conflict, exceeded retries`);
+    throw new Error(`${name} conflict, exceeded retries`);
+  }
+
+  // Otherwise, delay a bit before retrying
+  logger.debug(`${name} conflict, retrying ...`);
+  await waitBeforeNextRetry(retryDelay);
+  return await retryRequestIfConflicts(logger, name, sendRequest, retries - 1);
+};
+
+async function waitBeforeNextRetry(retryDelay: number): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, retryDelay));
+}