From cc29eea74f47245d7f9325d7218873115feceab0 Mon Sep 17 00:00:00 2001 From: Elena Stoeva <59341489+ElenaStoeva@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:55:06 +0100 Subject: [PATCH] [Advanced Settings] Add retry for 409 conflicts in API integration tests (#189813) Fixes https://github.com/elastic/kibana/issues/176445 ## Summary 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 https://github.com/elastic/kibana/pull/174185 which resolves a similar failure. --- .../advanced_settings/feature_controls.ts | 35 ++++++++------ .../advanced_settings/utils/index.ts | 8 ++++ .../utils/retry_if_conflicts.ts | 46 +++++++++++++++++++ 3 files changed, 75 insertions(+), 14 deletions(-) create mode 100644 x-pack/test/api_integration/apis/management/advanced_settings/utils/index.ts create mode 100644 x-pack/test/api_integration/apis/management/advanced_settings/utils/retry_if_conflicts.ts diff --git a/x-pack/test/api_integration/apis/management/advanced_settings/feature_controls.ts b/x-pack/test/api_integration/apis/management/advanced_settings/feature_controls.ts index 45630dac148d8..caf98ec1ece02 100644 --- a/x-pack/test/api_integration/apis/management/advanced_settings/feature_controls.ts +++ b/x-pack/test/api_integration/apis/management/advanced_settings/feature_controls.ts @@ -12,9 +12,11 @@ import { 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 supertestWithoutAuth = getService('supertestWithoutAuth'); + const log = getService('log'); const security = getService('security'); const spaces = getService('spaces'); const deployment = getService('deployment'); @@ -54,11 +56,14 @@ export default function featureControlsTests({ getService }: FtrProviderContext) async function saveAdvancedSetting(username: string, password: string, spaceId?: string) { const basePath = spaceId ? `/s/${spaceId}` : ''; - return await supertestWithoutAuth - .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 })); } @@ -66,19 +71,21 @@ export default function featureControlsTests({ getService }: FtrProviderContext) async function saveTelemetrySetting(username: string, password: string, spaceId?: string) { const basePath = spaceId ? `/s/${spaceId}` : ''; - return 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 }) + 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'; diff --git a/x-pack/test/api_integration/apis/management/advanced_settings/utils/index.ts b/x-pack/test/api_integration/apis/management/advanced_settings/utils/index.ts new file mode 100644 index 0000000000000..9d293f9058a7d --- /dev/null +++ b/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'; diff --git a/x-pack/test/api_integration/apis/management/advanced_settings/utils/retry_if_conflicts.ts b/x-pack/test/api_integration/apis/management/advanced_settings/utils/retry_if_conflicts.ts new file mode 100644 index 0000000000000..09a07b9b7a1f1 --- /dev/null +++ b/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, + retries: number = RETRY_ATTEMPTS, + retryDelay: number = RETRY_DELAY +): Promise => { + 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 { + await new Promise((resolve) => setTimeout(resolve, retryDelay)); +}