[7.5] [kbnClient] Retry uiSettings.replace() calls up to 5 tim… (#52780)

* [kbnClient] Retry uiSettings.replace() calls up to 5 times (#52601) * [kbn/dev-utils] target ES2019 to transpile ?? * Retry uiSettings.replace() calls up to 5 times * share logic for selecting junit report name to ensure they are unique * convert to junit report path helper # Conflicts: # packages/kbn-test/src/mocha/__tests__/junit_report_generation.js # packages/kbn-test/src/mocha/junit_report_generation.js # src/dev/jest/junit_reporter.js * remove ?? syntax * fix more null-ish syntax not supported in 7.5 * fix imports
elastic · Dec 12, 2019 · a4fca06 · a4fca06
1 parent d05585e
commit a4fca06
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 77 deletions.
diff --git a/packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts b/packages/kbn-dev-utils/src/kbn_client/kbn_client_requester.ts
@@ -62,8 +62,7 @@ export interface ReqOptions {
   query?: Record<string, any>;
   method: 'GET' | 'POST' | 'PUT' | 'DELETE';
   body?: any;
-  attempt?: number;
-  maxAttempts?: number;
+  retries?: number;
 }
 
 const delay = (ms: number) =>
@@ -87,44 +86,47 @@ export class KbnClientRequester {
   async request<T>(options: ReqOptions): Promise<T> {
     const url = Url.resolve(this.pickUrl(), options.path);
     const description = options.description || `${options.method} ${url}`;
-    const attempt = options.attempt === undefined ? 1 : options.attempt;
-    const maxAttempts =
-      options.maxAttempts === undefined ? DEFAULT_MAX_ATTEMPTS : options.maxAttempts;
-
-    try {
-      const response = await Axios.request<T>({
-        method: options.method,
-        url,
-        data: options.body,
-        params: options.query,
-        headers: {
-          'kbn-xsrf': 'kbn-client',
-        },
-      });
-
-      return response.data;
-    } catch (error) {
-      let retryErrorMsg: string | undefined;
-      if (isAxiosRequestError(error)) {
-        retryErrorMsg = `[${description}] request failed (attempt=${attempt})`;
-      } else if (isConcliftOnGetError(error)) {
-        retryErrorMsg = `Conflict on GET (path=${options.path}, attempt=${attempt})`;
-      }
+    let attempt = 0;
+    const maxAttempts = options.retries === undefined ? DEFAULT_MAX_ATTEMPTS : options.retries;
+
+    while (true) {
+      attempt += 1;
+
+      try {
+        const response = await Axios.request<T>({
+          method: options.method,
+          url,
+          data: options.body,
+          params: options.query,
+          headers: {
+            'kbn-xsrf': 'kbn-client',
+          },
+        });
+
+        return response.data;
+      } catch (error) {
+        const conflictOnGet = isConcliftOnGetError(error);
+        const requestedRetries = options.retries !== undefined;
+        const failedToGetResponse = isAxiosRequestError(error);
+
+        let errorMessage;
+        if (conflictOnGet) {
+          errorMessage = `Conflict on GET (path=${options.path}, attempt=${attempt}/${maxAttempts})`;
+          this.log.error(errorMessage);
+        } else if (requestedRetries || failedToGetResponse) {
+          errorMessage = `[${description}] request failed (attempt=${attempt}/${maxAttempts})`;
+          this.log.error(errorMessage);
+        } else {
+          throw error;
+        }
 
-      if (retryErrorMsg) {
         if (attempt < maxAttempts) {
-          this.log.error(retryErrorMsg);
           await delay(1000 * attempt);
-          return await this.request<T>({
-            ...options,
-            attempt: attempt + 1,
-          });
+          continue;
         }
 
-        throw new Error(retryErrorMsg + ' and ran out of retries');
+        throw new Error(`${errorMessage} -- and ran out of retries`);
       }
-
-      throw error;
     }
   }
 }
diff --git a/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts b/packages/kbn-dev-utils/src/kbn_client/kbn_client_ui_settings.ts
@@ -40,7 +40,7 @@ export class KbnClientUiSettings {
 
   async get(setting: string) {
     const all = await this.getAll();
-    const value = all.settings[setting] ? all.settings[setting].userValue : undefined;
+    const value = all[setting] === undefined ? undefined : all[setting].userValue;
 
     this.log.verbose('uiSettings.value: %j', value);
     return value;
@@ -68,24 +68,24 @@ export class KbnClientUiSettings {
    * with some defaults
    */
   async replace(doc: UiSettingValues) {
-    const all = await this.getAll();
-    for (const [name, { isOverridden }] of Object.entries(all.settings)) {
-      if (!isOverridden) {
-        await this.unset(name);
+    this.log.debug('replacing kibana config doc: %j', doc);
+
+    const changes: Record<string, any> = {
+      ...this.defaults,
+      ...doc,
+    };
+
+    for (const [name, { isOverridden }] of Object.entries(await this.getAll())) {
+      if (!isOverridden && !changes.hasOwnProperty(name)) {
+        changes[name] = null;
       }
     }
 
-    this.log.debug('replacing kibana config doc: %j', doc);
-
     await this.requester.request({
       method: 'POST',
       path: '/api/kibana/settings',
-      body: {
-        changes: {
-          ...this.defaults,
-          ...doc,
-        },
-      },
+      body: { changes },
+      retries: 5,
     });
   }
 
@@ -105,9 +105,11 @@ export class KbnClientUiSettings {
   }
 
   private async getAll() {
-    return await this.requester.request<UiSettingsApiResponse>({
+    const resp = await this.requester.request<UiSettingsApiResponse>({
       path: '/api/kibana/settings',
       method: 'GET',
     });
+
+    return resp.settings;
   }
 }
diff --git a/packages/kbn-dev-utils/tsconfig.json b/packages/kbn-dev-utils/tsconfig.json
@@ -2,6 +2,7 @@
   "extends": "../../tsconfig.json",
   "compilerOptions": {
     "outDir": "target",
+    "target": "ES2019",
     "declaration": true
   },
   "include": [

diff --git a/packages/kbn-test/src/index.ts b/packages/kbn-test/src/index.ts
@@ -42,3 +42,5 @@ export { readConfigFile } from './functional_test_runner/lib/config/read_config_
 export { runFtrCli } from './functional_test_runner/cli';
 
 export { runFailedTestsReporterCli } from './failed_tests_reporter';
+
+export { makeJunitReportPath } from './junit_report_path';
diff --git a/packages/kbn-test/src/junit_report_path.ts b/packages/kbn-test/src/junit_report_path.ts
@@ -0,0 +1,32 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { resolve } from 'path';
+
+const job = process.env.JOB ? `job-${process.env.JOB}-` : '';
+const num = process.env.CI_WORKER_NUMBER ? `worker-${process.env.CI_WORKER_NUMBER}-` : '';
+
+export function makeJunitReportPath(rootDirectory: string, reportName: string) {
+  return resolve(
+    rootDirectory,
+    'target/junit',
+    process.env.JOB || '.',
+    `TEST-${job}${num}${reportName}.xml`
+  );
+}
diff --git a/src/dev/jest/integration_tests/junit_reporter.test.js b/src/dev/jest/integration_tests/junit_reporter.test.js
@@ -24,12 +24,13 @@ import { readFileSync } from 'fs';
 import del from 'del';
 import execa from 'execa';
 import xml2js from 'xml2js';
+import { makeJunitReportPath } from '@kbn/test';
 
 const MINUTE = 1000 * 60;
 const ROOT_DIR = resolve(__dirname, '../../../../');
 const FIXTURE_DIR = resolve(__dirname, '__fixtures__');
 const TARGET_DIR = resolve(FIXTURE_DIR, 'target');
-const XML_PATH = resolve(TARGET_DIR, 'junit', process.env.JOB || '.', `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}Jest Tests.xml`);
+const XML_PATH = makeJunitReportPath(FIXTURE_DIR, 'Jest Tests');
 
 afterAll(async () => {
   await del(TARGET_DIR);

diff --git a/src/dev/jest/junit_reporter.js b/src/dev/jest/junit_reporter.js
@@ -23,6 +23,7 @@ import { writeFileSync, mkdirSync } from 'fs';
 import xmlBuilder from 'xmlbuilder';
 
 import { escapeCdata } from '../xml';
+import { makeJunitReportPath } from '@kbn/test';
 
 const ROOT_DIR = dirname(require.resolve('../../../package.json'));
 
@@ -102,13 +103,7 @@ export default class JestJUnitReporter {
       });
     });
 
-    const reportPath = resolve(
-      rootDirectory,
-      'target/junit',
-      process.env.JOB || '.',
-      `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml`
-    );
-
+    const reportPath = makeJunitReportPath(rootDirectory, reportName);
     const reportXML = root.end();
     mkdirSync(dirname(reportPath), { recursive: true });
     writeFileSync(reportPath, reportXML, 'utf8');

diff --git a/src/dev/mocha/__tests__/junit_report_generation.js b/src/dev/mocha/__tests__/junit_report_generation.js
@@ -25,6 +25,7 @@ import { parseString } from 'xml2js';
 import del from 'del';
 import Mocha from 'mocha';
 import expect from '@kbn/expect';
+import { makeJunitReportPath } from '@kbn/test';
 
 import { setupJUnitReportGeneration } from '../junit_report_generation';
 
@@ -50,17 +51,7 @@ describe('dev/mocha/junit report generation', () => {
     mocha.addFile(resolve(PROJECT_DIR, 'test.js'));
     await new Promise(resolve => mocha.run(resolve));
     const report = await fcb(cb =>
-      parseString(
-        readFileSync(
-          resolve(
-            PROJECT_DIR,
-            'target/junit',
-            process.env.JOB || '.',
-            `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}test.xml`
-          )
-        ),
-        cb
-      )
+      parseString(readFileSync(makeJunitReportPath(PROJECT_DIR, 'test')), cb)
     );
 
     // test case results are wrapped in <testsuites></testsuites>

diff --git a/src/dev/mocha/junit_report_generation.js b/src/dev/mocha/junit_report_generation.js
@@ -17,11 +17,12 @@
  * under the License.
  */
 
-import { resolve, dirname, relative } from 'path';
+import { dirname, relative } from 'path';
 import { writeFileSync, mkdirSync } from 'fs';
 import { inspect } from 'util';
 
 import xmlBuilder from 'xmlbuilder';
+import { makeJunitReportPath } from '@kbn/test';
 
 import { getSnapshotOfRunnableLogs } from './log_cache';
 import { escapeCdata } from '../xml';
@@ -137,13 +138,7 @@ export function setupJUnitReportGeneration(runner, options = {}) {
       }
     });
 
-    const reportPath = resolve(
-      rootDirectory,
-      'target/junit',
-      process.env.JOB || '.',
-      `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}${reportName}.xml`
-    );
-
+    const reportPath = makeJunitReportPath(rootDirectory, reportName);
     const reportXML = builder.end();
     mkdirSync(dirname(reportPath), { recursive: true });
     writeFileSync(reportPath, reportXML, 'utf8');

diff --git a/tasks/config/karma.js b/tasks/config/karma.js
@@ -17,8 +17,9 @@
  * under the License.
  */
 
-import { resolve, dirname } from 'path';
+import { dirname } from 'path';
 import { times } from 'lodash';
+import { makeJunitReportPath } from '@kbn/test';
 
 const TOTAL_CI_SHARDS = 4;
 const ROOT = dirname(require.resolve('../../package.json'));
@@ -67,7 +68,7 @@ module.exports = function (grunt) {
       reporters: process.env.CI ? ['dots', 'junit'] : ['progress'],
 
       junitReporter: {
-        outputFile: resolve(ROOT, 'target/junit', process.env.JOB || '.', `TEST-${process.env.JOB ? process.env.JOB + '-' : ''}karma.xml`),
+        outputFile: makeJunitReportPath(ROOT, 'karma'),
         useBrowserName: false,
         nameFormatter: (browser, result) => [...result.suite, result.description].join(' '),
         classNameFormatter: (browser, result) => {