Skip to content

Commit

Permalink
[8.13] [kbn-ftr-common-functional-services] extend retry service (#17…
Browse files Browse the repository at this point in the history
…8660) (#178978)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[kbn-ftr-common-functional-services] extend retry service
(#178660)](#178660)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Dzmitry
Lemechko","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-03-19T15:32:59Z","message":"[kbn-ftr-common-functional-services]
extend retry service (#178660)\n\n## Summary\r\n\r\nI took the retry
wrapper created by\r\n[jpdjere](https://github.com/jpdjere) in #173998
and extended\r\n`retryForSuccess<T>` with required capabilities to
provide the same\r\nfunctionality.\r\n\r\nThis PR:\r\n1) extends retry
service with new function `tryWithRetries<T> => :\r\nPromise<T>` to
retry block `options.retryCount` times within\r\n`options.timeout`
period and return block result\r\n\r\n```ts\r\n const response = await
retry.tryWithRetries<SearchResponse>(\r\n 'search request',\r\n async ()
=> {\r\n const response = await supertest\r\n
.post(`/internal/search/es`)\r\n .set(ELASTIC_HTTP_VERSION_HEADER,
'1')\r\n .send({\r\n params: {\r\n body: {\r\n query: {\r\n match_all:
{},\r\n },\r\n },\r\n },\r\n })\r\n .expect(200);\r\n return
response.body as SearchResponse;\r\n },\r\n {\r\n retryCount: 4,\r\n
retryDelay: 100, // optional\r\n timeout: 30000, // optional\r\n
}\r\n```\r\n\r\n2) removes `utils/retry.ts` wrapper and migrate tests to
FTR Retry\r\nservice\r\n3) Adds descriptions to Retry service functions
explaining the default\r\nuse case\r\n\r\nHow the failures look
like:\r\n\r\n- when reached timeout before retry count limit \r\n```\r\n
Error: retry.tryWithRetries reached timeout 11000 ms waiting for 'run
console request'\r\n Error: expected '# Click the Variables button,
above, to create your own variables.\\nGET ${exampleVariable1} //
_search\\n{\\n \"query\": {\\n \"${exampleVariable2}\": {} //
match_all\\n }\\n}' to sort of equal '5'\r\n at Assertion.assert
(expect.js:100:11)\r\n at Assertion.eql (expect.js:244:8)\r\n at
_console.ts:47:41\r\n at processTicksAndRejections
(node:internal/process/task_queues:95:5)\r\n at runAttempt
(retry_with_retries.ts:29:15)\r\n at retryWithRetries
(retry_with_retries.ts:97:21)\r\n at RetryService.tryForTime
(retry.ts:38:12)\r\n at Context.<anonymous> (_console.ts:44:7)\r\n at
Object.apply (wrap_function.js:73:16)\r\n```\r\n- when reached retry
count limit before timeout\r\n```\r\n Error: retry.tryWithRetries
reached the limit of attempts waiting for 'run console request': 2 out
of 2\r\n Error: expected '# Click the Variables button, above, to create
your own variables.\\nGET ${exampleVariable1} // _search\\n{\\n
\"query\": {\\n \"${exampleVariable2}\": {} // match_all\\n }\\n}' to
sort of equal '5'\r\n at Assertion.assert (expect.js:100:11)\r\n at
Assertion.eql (expect.js:244:8)\r\n at _console.ts:47:41\r\n at
processTicksAndRejections (node:internal/process/task_queues:95:5)\r\n
at runAttempt (retry_for_success.ts:29:15)\r\n at retryForSuccess
(retry_for_success.ts:97:21)\r\n at RetryService.tryWithRetries
(retry.ts:115:12)\r\n at Context.<anonymous> (_console.ts:44:7)\r\n at
Object.apply
(wrap_function.js:73:16)\r\n```","sha":"277b3fbc24889bedd512f23674e768d2f4c43294","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","FTR","v8.14.0","v8.13.1"],"title":"[kbn-ftr-common-functional-services]
extend retry
service","number":178660,"url":"https://github.com/elastic/kibana/pull/178660","mergeCommit":{"message":"[kbn-ftr-common-functional-services]
extend retry service (#178660)\n\n## Summary\r\n\r\nI took the retry
wrapper created by\r\n[jpdjere](https://github.com/jpdjere) in #173998
and extended\r\n`retryForSuccess<T>` with required capabilities to
provide the same\r\nfunctionality.\r\n\r\nThis PR:\r\n1) extends retry
service with new function `tryWithRetries<T> => :\r\nPromise<T>` to
retry block `options.retryCount` times within\r\n`options.timeout`
period and return block result\r\n\r\n```ts\r\n const response = await
retry.tryWithRetries<SearchResponse>(\r\n 'search request',\r\n async ()
=> {\r\n const response = await supertest\r\n
.post(`/internal/search/es`)\r\n .set(ELASTIC_HTTP_VERSION_HEADER,
'1')\r\n .send({\r\n params: {\r\n body: {\r\n query: {\r\n match_all:
{},\r\n },\r\n },\r\n },\r\n })\r\n .expect(200);\r\n return
response.body as SearchResponse;\r\n },\r\n {\r\n retryCount: 4,\r\n
retryDelay: 100, // optional\r\n timeout: 30000, // optional\r\n
}\r\n```\r\n\r\n2) removes `utils/retry.ts` wrapper and migrate tests to
FTR Retry\r\nservice\r\n3) Adds descriptions to Retry service functions
explaining the default\r\nuse case\r\n\r\nHow the failures look
like:\r\n\r\n- when reached timeout before retry count limit \r\n```\r\n
Error: retry.tryWithRetries reached timeout 11000 ms waiting for 'run
console request'\r\n Error: expected '# Click the Variables button,
above, to create your own variables.\\nGET ${exampleVariable1} //
_search\\n{\\n \"query\": {\\n \"${exampleVariable2}\": {} //
match_all\\n }\\n}' to sort of equal '5'\r\n at Assertion.assert
(expect.js:100:11)\r\n at Assertion.eql (expect.js:244:8)\r\n at
_console.ts:47:41\r\n at processTicksAndRejections
(node:internal/process/task_queues:95:5)\r\n at runAttempt
(retry_with_retries.ts:29:15)\r\n at retryWithRetries
(retry_with_retries.ts:97:21)\r\n at RetryService.tryForTime
(retry.ts:38:12)\r\n at Context.<anonymous> (_console.ts:44:7)\r\n at
Object.apply (wrap_function.js:73:16)\r\n```\r\n- when reached retry
count limit before timeout\r\n```\r\n Error: retry.tryWithRetries
reached the limit of attempts waiting for 'run console request': 2 out
of 2\r\n Error: expected '# Click the Variables button, above, to create
your own variables.\\nGET ${exampleVariable1} // _search\\n{\\n
\"query\": {\\n \"${exampleVariable2}\": {} // match_all\\n }\\n}' to
sort of equal '5'\r\n at Assertion.assert (expect.js:100:11)\r\n at
Assertion.eql (expect.js:244:8)\r\n at _console.ts:47:41\r\n at
processTicksAndRejections (node:internal/process/task_queues:95:5)\r\n
at runAttempt (retry_for_success.ts:29:15)\r\n at retryForSuccess
(retry_for_success.ts:97:21)\r\n at RetryService.tryWithRetries
(retry.ts:115:12)\r\n at Context.<anonymous> (_console.ts:44:7)\r\n at
Object.apply
(wrap_function.js:73:16)\r\n```","sha":"277b3fbc24889bedd512f23674e768d2f4c43294"}},"sourceBranch":"main","suggestedTargetBranches":["8.13"],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/178660","number":178660,"mergeCommit":{"message":"[kbn-ftr-common-functional-services]
extend retry service (#178660)\n\n## Summary\r\n\r\nI took the retry
wrapper created by\r\n[jpdjere](https://github.com/jpdjere) in #173998
and extended\r\n`retryForSuccess<T>` with required capabilities to
provide the same\r\nfunctionality.\r\n\r\nThis PR:\r\n1) extends retry
service with new function `tryWithRetries<T> => :\r\nPromise<T>` to
retry block `options.retryCount` times within\r\n`options.timeout`
period and return block result\r\n\r\n```ts\r\n const response = await
retry.tryWithRetries<SearchResponse>(\r\n 'search request',\r\n async ()
=> {\r\n const response = await supertest\r\n
.post(`/internal/search/es`)\r\n .set(ELASTIC_HTTP_VERSION_HEADER,
'1')\r\n .send({\r\n params: {\r\n body: {\r\n query: {\r\n match_all:
{},\r\n },\r\n },\r\n },\r\n })\r\n .expect(200);\r\n return
response.body as SearchResponse;\r\n },\r\n {\r\n retryCount: 4,\r\n
retryDelay: 100, // optional\r\n timeout: 30000, // optional\r\n
}\r\n```\r\n\r\n2) removes `utils/retry.ts` wrapper and migrate tests to
FTR Retry\r\nservice\r\n3) Adds descriptions to Retry service functions
explaining the default\r\nuse case\r\n\r\nHow the failures look
like:\r\n\r\n- when reached timeout before retry count limit \r\n```\r\n
Error: retry.tryWithRetries reached timeout 11000 ms waiting for 'run
console request'\r\n Error: expected '# Click the Variables button,
above, to create your own variables.\\nGET ${exampleVariable1} //
_search\\n{\\n \"query\": {\\n \"${exampleVariable2}\": {} //
match_all\\n }\\n}' to sort of equal '5'\r\n at Assertion.assert
(expect.js:100:11)\r\n at Assertion.eql (expect.js:244:8)\r\n at
_console.ts:47:41\r\n at processTicksAndRejections
(node:internal/process/task_queues:95:5)\r\n at runAttempt
(retry_with_retries.ts:29:15)\r\n at retryWithRetries
(retry_with_retries.ts:97:21)\r\n at RetryService.tryForTime
(retry.ts:38:12)\r\n at Context.<anonymous> (_console.ts:44:7)\r\n at
Object.apply (wrap_function.js:73:16)\r\n```\r\n- when reached retry
count limit before timeout\r\n```\r\n Error: retry.tryWithRetries
reached the limit of attempts waiting for 'run console request': 2 out
of 2\r\n Error: expected '# Click the Variables button, above, to create
your own variables.\\nGET ${exampleVariable1} // _search\\n{\\n
\"query\": {\\n \"${exampleVariable2}\": {} // match_all\\n }\\n}' to
sort of equal '5'\r\n at Assertion.assert (expect.js:100:11)\r\n at
Assertion.eql (expect.js:244:8)\r\n at _console.ts:47:41\r\n at
processTicksAndRejections (node:internal/process/task_queues:95:5)\r\n
at runAttempt (retry_for_success.ts:29:15)\r\n at retryForSuccess
(retry_for_success.ts:97:21)\r\n at RetryService.tryWithRetries
(retry.ts:115:12)\r\n at Context.<anonymous> (_console.ts:44:7)\r\n at
Object.apply
(wrap_function.js:73:16)\r\n```","sha":"277b3fbc24889bedd512f23674e768d2f4c43294"}},{"branch":"8.13","label":"v8.13.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Dzmitry Lemechko <[email protected]>
  • Loading branch information
kibanamachine and dmlemeshko authored Mar 19, 2024
1 parent 87f05fd commit 1f4f125
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,31 @@ import { FtrService } from '../ftr_provider_context';
import { retryForSuccess } from './retry_for_success';
import { retryForTruthy } from './retry_for_truthy';

interface TryWithRetriesOptions {
retryCount: number;
retryDelay?: number;
timeout?: number;
}

export class RetryService extends FtrService {
private readonly config = this.ctx.getService('config');
private readonly log = this.ctx.getService('log');

/**
* Use to retry block within {timeout} period and return block result.
* @param timeout retrying timeout
* @param block retriable action
* @param onFailureBlock optional action to run before the new retriable action attempt
* @param retryDelay optional delay before the new attempt
* @returns result from retriable action
*/
public async tryForTime<T>(
timeout: number,
block: () => Promise<T>,
onFailureBlock?: () => Promise<T>,
retryDelay?: number
) {
return await retryForSuccess(this.log, {
return await retryForSuccess<T>(this.log, {
timeout,
methodName: 'retry.tryForTime',
block,
Expand All @@ -43,6 +57,13 @@ export class RetryService extends FtrService {
});
}

/**
* Use to wait for block condition to be true
* @param description description for retriable action
* @param timeout retrying timeout
* @param block retriable action
* @param onFailureBlock optional action to run before the new retriable action attempt
*/
public async waitForWithTimeout(
description: string,
timeout: number,
Expand Down Expand Up @@ -71,4 +92,31 @@ export class RetryService extends FtrService {
onFailureBlock,
});
}

/**
* Use to retry block {options.retryCount} times within {options.timeout} period and return block result
* @param description description for retriable action
* @param block retriable action
* @param options options.retryCount for how many attempts to retry
* @param onFailureBlock optional action to run before the new retriable action attempt
* @returns result from retriable action
*/
public async tryWithRetries<T>(
description: string,
block: () => Promise<T>,
options: TryWithRetriesOptions,
onFailureBlock?: () => Promise<T>
): Promise<T> {
const { retryCount, timeout = this.config.get('timeouts.try'), retryDelay = 200 } = options;

return await retryForSuccess<T>(this.log, {
description,
timeout,
methodName: 'retry.tryWithRetries',
block,
onFailureBlock,
retryDelay,
retryCount,
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

const returnTrue = () => true;

const defaultOnFailure = (methodName: string) => (lastError: Error | undefined) => {
const defaultOnFailure = (methodName: string) => (lastError: Error | undefined, reason: string) => {
throw new Error(
`${methodName} timeout${lastError ? `: ${lastError.stack || lastError.message}` : ''}`
`${methodName} ${reason}\n${lastError ? `${lastError.stack || lastError.message}` : ''}`
);
};

Expand Down Expand Up @@ -44,32 +44,51 @@ interface Options<T> {
onFailureBlock?: () => Promise<T>;
onFailure?: ReturnType<typeof defaultOnFailure>;
accept?: (v: T) => boolean;
description?: string;
retryDelay?: number;
retryCount?: number;
}

export async function retryForSuccess<T>(log: ToolingLog, options: Options<T>) {
const {
description,
timeout,
methodName,
block,
onFailureBlock,
onFailure = defaultOnFailure(methodName),
accept = returnTrue,
retryDelay = 502,
retryCount,
} = options;
const { onFailure = defaultOnFailure(methodName) } = options;

const start = Date.now();
const criticalWebDriverErrors = ['NoSuchSessionError', 'NoSuchWindowError'];
let lastError;
let attemptCounter = 0;
const addText = (str: string | undefined) => (str ? ` waiting for '${str}'` : '');

while (true) {
// Aborting if no retry attempts are left (opt-in)
if (retryCount && ++attemptCounter > retryCount) {
onFailure(
lastError,
// optionally extend error message with description
`reached the limit of attempts${addText(description)}: ${
attemptCounter - 1
} out of ${retryCount}`
);
}
// Aborting if timeout is reached
if (Date.now() - start > timeout) {
await onFailure(lastError);
throw new Error('expected onFailure() option to throw an error');
} else if (lastError && criticalWebDriverErrors.includes(lastError.name)) {
// Aborting retry since WebDriver session is invalid or browser window is closed
onFailure(lastError, `reached timeout ${timeout} ms${addText(description)}`);
}
// Aborting if WebDriver session is invalid or browser window is closed
if (lastError && criticalWebDriverErrors.includes(lastError.name)) {
throw new Error('WebDriver session is invalid, retry was aborted');
} else if (lastError && onFailureBlock) {
}
// Run opt-in onFailureBlock before the next attempt
if (lastError && onFailureBlock) {
const before = await runAttempt(onFailureBlock);
if ('error' in before) {
log.debug(`--- onRetryBlock error: ${before.error.message}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
'99.0.0',
retry,
log
retry
);

// As opposed to "registry"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ export default ({ getService }: FtrProviderContext): void => {
const fleetPackageInstallationResponse = await installPrebuiltRulesPackageViaFleetAPI(
es,
supertest,
retry,
log
retry
);

expect(fleetPackageInstallationResponse.items.length).toBe(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export default ({ getService }: FtrProviderContext): void => {
supertest,
overrideExistingPackage: true,
retryService: retry,
log,
});

// Verify that status is updated after package installation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
previousVersion,
retry,
log
retry
);

expect(installPreviousPackageResponse._meta.install_source).toBe('registry');
Expand Down Expand Up @@ -161,8 +160,7 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
currentVersion,
retry,
log
retry
);
expect(installLatestPackageResponse.items.length).toBeGreaterThanOrEqual(0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,4 @@ export * from './wait_for_index_to_populate';
export * from './get_stats';
export * from './get_detection_metrics_from_body';
export * from './get_stats_url';
export * from './retry';
export * from './combine_to_ndjson';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import type SuperTest from 'supertest';
import { InstallPackageResponse } from '@kbn/fleet-plugin/common/types';
import { epmRouteService } from '@kbn/fleet-plugin/common';
import { RetryService } from '@kbn/ftr-common-functional-services';
import type { ToolingLog } from '@kbn/tooling-log';
import expect from 'expect';
import { retry } from '../../retry';
import { refreshSavedObjectIndices } from '../../refresh_index';

const MAX_RETRIES = 2;
Expand All @@ -29,11 +27,11 @@ const ATTEMPT_TIMEOUT = 120000;
export const installPrebuiltRulesPackageViaFleetAPI = async (
es: Client,
supertest: SuperTest.SuperTest<SuperTest.Test>,
retryService: RetryService,
log: ToolingLog
retryService: RetryService
): Promise<InstallPackageResponse> => {
const fleetResponse = await retry<InstallPackageResponse>({
test: async () => {
const fleetResponse = await retryService.tryWithRetries<InstallPackageResponse>(
installPrebuiltRulesPackageViaFleetAPI.name,
async () => {
const testResponse = await supertest
.post(`/api/fleet/epm/packages/security_detection_engine`)
.set('kbn-xsrf', 'xxxx')
Expand All @@ -46,12 +44,11 @@ export const installPrebuiltRulesPackageViaFleetAPI = async (

return testResponse.body;
},
utilityName: installPrebuiltRulesPackageViaFleetAPI.name,
retryService,
retries: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
log,
});
{
retryCount: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
}
);

await refreshSavedObjectIndices(es);

Expand All @@ -71,11 +68,11 @@ export const installPrebuiltRulesPackageByVersion = async (
es: Client,
supertest: SuperTest.SuperTest<SuperTest.Test>,
version: string,
retryService: RetryService,
log: ToolingLog
retryService: RetryService
): Promise<InstallPackageResponse> => {
const fleetResponse = await retry<InstallPackageResponse>({
test: async () => {
const fleetResponse = await retryService.tryWithRetries<InstallPackageResponse>(
installPrebuiltRulesPackageByVersion.name,
async () => {
const testResponse = await supertest
.post(epmRouteService.getInstallPath('security_detection_engine', version))
.set('kbn-xsrf', 'xxxx')
Expand All @@ -88,12 +85,11 @@ export const installPrebuiltRulesPackageByVersion = async (

return testResponse.body;
},
utilityName: installPrebuiltRulesPackageByVersion.name,
retryService,
retries: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
log,
});
{
retryCount: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
}
);

await refreshSavedObjectIndices(es);

Expand Down
Loading

0 comments on commit 1f4f125

Please sign in to comment.