Add retry logic to license fetcher (elastic#201843)

## Summary Added some retry logic to the license fetcher function and updated tests to match this new behavior. Resolves: elastic#197074 ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
jesuswr · Nov 27, 2024 · 791a3f0 · 791a3f0
1 parent 4fbec3f
commit 791a3f0
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 13 deletions.
diff --git a/x-pack/plugins/licensing/server/license_fetcher.test.ts b/x-pack/plugins/licensing/server/license_fetcher.test.ts
@@ -12,7 +12,8 @@ import { elasticsearchServiceMock } from '@kbn/core/server/mocks';
 
 type EsLicense = estypes.XpackInfoMinimalLicenseInformation;
 
-const delay = (ms: number) => new Promise((res) => setTimeout(res, ms));
+const maxRetryDelay = 30 * 1000;
+const sumOfRetryTimes = (1 + 2 + 4 + 8 + 16) * 1000;
 
 function buildRawLicense(options: Partial<EsLicense> = {}): EsLicense {
   return {
@@ -33,6 +34,9 @@ describe('LicenseFetcher', () => {
     logger = loggerMock.create();
     clusterClient = elasticsearchServiceMock.createClusterClient();
   });
+  afterEach(() => {
+    jest.useRealTimers();
+  });
 
   it('returns the license for successful calls', async () => {
     clusterClient.asInternalUser.xpack.info.mockResponse({
@@ -46,6 +50,7 @@ describe('LicenseFetcher', () => {
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
     const license = await fetcher();
@@ -71,6 +76,7 @@ describe('LicenseFetcher', () => {
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
     let license = await fetcher();
@@ -81,6 +87,7 @@ describe('LicenseFetcher', () => {
   });
 
   it('returns an error license in case of error', async () => {
+    jest.useFakeTimers();
     clusterClient.asInternalUser.xpack.info.mockResponseImplementation(() => {
       throw new Error('woups');
     });
@@ -89,13 +96,20 @@ describe('LicenseFetcher', () => {
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
-    const license = await fetcher();
+    const licensePromise = fetcher();
+    await jest.advanceTimersByTimeAsync(sumOfRetryTimes);
+    const license = await licensePromise;
+
     expect(license.error).toEqual('woups');
+    // should be called once to start and then in the retries after 1s, 2s, 4s, 8s and 16s
+    expect(clusterClient.asInternalUser.xpack.info).toHaveBeenCalledTimes(6);
   });
 
   it('returns a license successfully fetched after an error', async () => {
+    jest.useFakeTimers();
     clusterClient.asInternalUser.xpack.info
       .mockResponseImplementationOnce(() => {
         throw new Error('woups');
@@ -111,62 +125,77 @@ describe('LicenseFetcher', () => {
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
-    let license = await fetcher();
-    expect(license.error).toEqual('woups');
-    license = await fetcher();
+    const licensePromise = fetcher();
+    // wait one minute since we mocked only one error
+    await jest.advanceTimersByTimeAsync(1000);
+    const license = await licensePromise;
+
     expect(license.uid).toEqual('license-1');
+    expect(clusterClient.asInternalUser.xpack.info).toBeCalledTimes(2);
   });
 
   it('returns the latest fetched license after an error within the cache duration period', async () => {
+    jest.useFakeTimers();
     clusterClient.asInternalUser.xpack.info
       .mockResponseOnce({
         license: buildRawLicense({
           uid: 'license-1',
         }),
         features: {},
       } as any)
-      .mockResponseImplementationOnce(() => {
+      .mockResponseImplementation(() => {
         throw new Error('woups');
       });
 
     const fetcher = getLicenseFetcher({
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
     let license = await fetcher();
     expect(license.uid).toEqual('license-1');
-    license = await fetcher();
+    expect(clusterClient.asInternalUser.xpack.info).toBeCalledTimes(1);
+
+    const licensePromise = fetcher();
+    await jest.advanceTimersByTimeAsync(sumOfRetryTimes);
+    license = await licensePromise;
     expect(license.uid).toEqual('license-1');
+    // should be called once in the successful mock, once in the error mock
+    // and then in the retries after 1s, 2s, 4s, 8s and 16s
+    expect(clusterClient.asInternalUser.xpack.info).toBeCalledTimes(7);
   });
 
   it('returns an error license after an error exceeding the cache duration period', async () => {
+    jest.useFakeTimers();
     clusterClient.asInternalUser.xpack.info
       .mockResponseOnce({
         license: buildRawLicense({
           uid: 'license-1',
         }),
         features: {},
       } as any)
-      .mockResponseImplementationOnce(() => {
+      .mockResponseImplementation(() => {
         throw new Error('woups');
       });
 
     const fetcher = getLicenseFetcher({
       logger,
       clusterClient,
       cacheDurationMs: 1,
+      maxRetryDelay,
     });
 
     let license = await fetcher();
     expect(license.uid).toEqual('license-1');
 
-    await delay(50);
-
-    license = await fetcher();
+    const licensePromise = fetcher();
+    await jest.advanceTimersByTimeAsync(sumOfRetryTimes);
+    license = await licensePromise;
     expect(license.error).toEqual('woups');
   });
 
@@ -180,6 +209,7 @@ describe('LicenseFetcher', () => {
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
     const license = await fetcher();
@@ -203,6 +233,7 @@ describe('LicenseFetcher', () => {
       logger,
       clusterClient,
       cacheDurationMs: 50_000,
+      maxRetryDelay,
     });
 
     const license = await fetcher();
@@ -213,4 +244,27 @@ describe('LicenseFetcher', () => {
       ]
     `);
   });
+
+  it('testing the fetcher retry with a different maxRetryDelay using only errors', async () => {
+    jest.useFakeTimers();
+    clusterClient.asInternalUser.xpack.info.mockResponseImplementation(() => {
+      throw new Error('woups');
+    });
+
+    const fetcher = getLicenseFetcher({
+      logger,
+      clusterClient,
+      cacheDurationMs: 50_000,
+      maxRetryDelay: 10 * 1000,
+    });
+    const sumOfRetryTimesUntilTen = (1 + 2 + 4 + 8) * 1000;
+
+    const licensePromise = fetcher();
+    await jest.advanceTimersByTimeAsync(sumOfRetryTimesUntilTen);
+    const license = await licensePromise;
+
+    expect(license.error).toEqual('woups');
+    // should be called once to start and then in the retries after 1s, 2s, 4s and 8s
+    expect(clusterClient.asInternalUser.xpack.info).toHaveBeenCalledTimes(5);
+  });
 });
diff --git a/x-pack/plugins/licensing/server/license_fetcher.ts b/x-pack/plugins/licensing/server/license_fetcher.ts
@@ -7,6 +7,7 @@
 
 import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
 import { createHash } from 'crypto';
+import pRetry from 'p-retry';
 import stringify from 'json-stable-stringify';
 import type { MaybePromise } from '@kbn/utility-types';
 import { isPromise } from '@kbn/std';
@@ -25,18 +26,23 @@ export const getLicenseFetcher = ({
   clusterClient,
   logger,
   cacheDurationMs,
+  maxRetryDelay,
 }: {
   clusterClient: MaybePromise<IClusterClient>;
   logger: Logger;
   cacheDurationMs: number;
+  maxRetryDelay: number;
 }): LicenseFetcher => {
   let currentLicense: ILicense | undefined;
   let lastSuccessfulFetchTime: number | undefined;
+  const maxRetries = Math.floor(Math.log2(maxRetryDelay / 1000)) + 1;
 
   return async () => {
     const client = isPromise(clusterClient) ? await clusterClient : clusterClient;
     try {
-      const response = await client.asInternalUser.xpack.info();
+      const response = await pRetry(() => client.asInternalUser.xpack.info(), {
+        retries: maxRetries,
+      });
       const normalizedLicense =
         response.license && response.license.type !== 'missing'
           ? normalizeServerLicense(response.license)
@@ -63,7 +69,9 @@ export const getLicenseFetcher = ({
       lastSuccessfulFetchTime = Date.now();
 
       return currentLicense;
-    } catch (error) {
+    } catch (err) {
+      const error = err.originalError ?? err;
+
       logger.warn(
         `License information could not be obtained from Elasticsearch due to ${error} error`
       );

diff --git a/x-pack/plugins/licensing/server/plugin.ts b/x-pack/plugins/licensing/server/plugin.ts
@@ -125,6 +125,7 @@ export class LicensingPlugin implements Plugin<LicensingPluginSetup, LicensingPl
       clusterClient,
       logger: this.logger,
       cacheDurationMs: this.config.license_cache_duration.asMilliseconds(),
+      maxRetryDelay: pollingFrequency,
     });
 
     const { license$, refreshManually } = createLicenseUpdate(