[Security Solution] Unskip flaky tests in Prebuilt Rules FTR Integration tests

[jpdjere]

Addresses:
#172107
#171380

Summary

Unskip skipped tests in:

1. x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/bundled_prebuilt_rules_package/install_latest_bundled_prebuilt_rules.ts
2. x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/fleet_integration.ts

• Makes the retryDelay in the RetryService in packages/kbn-ftr-common-functional-services/services/retry/retry.ts a configurable parameter - used in our retry util to shorten the wait period to 200ms.
• Creates retry wrapper util for our FTR Integration tests, that wraps retry.try from the RetryService, to implement maximum attempts.
• Uses this retry wrapper in test utils that install the security_detection_engine Fleet package, and asserts that rules have been succesfully installed - and retries if they haven't.
• Creates refreshSavedObjectIndices reusable util that cleans cache and refreshes indices. Centralizes comment spread around file into this single file.
• Use this new util to clean the cache in util that install the Fleet package, and utils that read the prebuilt rule status.

Flaky test runner

Before changes:

• For both bundled_prebuilt_rules_package and management: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4688 🟢 (250 and 250 runs)
• bundled_prebuilt_rules_package:
  • ESS: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4805 (500 runs)
  • Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4807 (500 runs)
• management
  • ESS: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4806 (500 runs)
  • Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4808 (500 runs)

After changes:

• bundled_prebuilt_rules_package:
  • ESS: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4825 🟢 (500 runs)
  • Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4826 🟢 (500 runs)
• management
  • ESS: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4827 🟢 (500 runs)
  • Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4828 🟢 (500 runs)

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[jpdjere]

@banderror I'd like your opinion on this, whenever you can take a look.

I wasn't able to reproduce the flake reported in either of the two linked tickets - I've run the flaky test runner with 500 runs for each, and all were green. So the issue has been tough to investigate, but I have two theories/proposed solutions.

1. Cache for ES reading requests needs to be cleared

This is what's currently implemented in this PR.

I noticed that the issue in both tickets pops up in test suites where we attempt to read the status of the prebuilt rules before and after installing our prebuilt rules. In both cases, we first check that no rules are available for install, then install prebuilt rule assets, and then read the status again to assert that rules are now available for install.

I'm not completely sure how cacheing in ES works, but I suspect that in very specific cases (one in thousands) the cache for the request is not completely cleared after the first read, and even though we correctly install the prebuilt rule assets and refresh the indices, the subsequent read operations simply responds the cached value - which is still 0 rules to be installed.

If that is the case, we can use await es.indices.clearCache({ index: ALL_SAVED_OBJECT_INDICES }); to clean the request caches in a similar way to how we refresh the indices. This PR adds that logic to our getPrebuiltRulesStatus and the legacy getPrebuiltRulesAndTimelinesStatus helper.

2. Implement retry logic for security_detection_engine package installation

There's a retry tool that is used extensively in the FTR Integration tests that we can make use of in the helpers that install the package. In all cases for these utils, we want to install at least one asset. If no assets are present, that means that the installation failed, and that we should retry it.

So we can modify our helper to something like:

export const installPrebuiltRulesPackageByVersion = async (
  es: Client,
  supertest: SuperTest.SuperTest<SuperTest.Test>,
  version: string,
  retry: RetryService
): Promise<InstallPackageResponse> => {
  return retry.try(async () => {
    const fleetResponse = await supertest
      .post(epmRouteService.getInstallPath('security_detection_engine', version))
      .set('kbn-xsrf', 'xxxx')
      .set('elastic-api-version', '2023-10-31')
      .type('application/json')
      .send({ force: true })
      .expect(200);

    // This new expect clause makes sure that if no assets were installed, then
    // then we should retry - as this assertion will fail and cause the whole block to be retried until it passes
    expect((fleetResponse.body as InstallPackageResponse).items.length).toBeGreaterThan(0);

    await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

    return fleetResponse.body as InstallPackageResponse;
  });
};

This is not implemented in this PR; I'd like your opinion first.

But my suggestion is: go with option 1 (as the PR stands) and monitor these tests. If they reappear, then the cache was not the problem, and we can try with the retry logic. I can have a PR ready for that case, so we'd only need to review it and merge it as soon (and if) the flake reappears.

[elasticmachine]

Pinging @elastic/security-detections-response (Team:Detections and Resp)

[banderror]

@jpdjere Yea, it seems to be a tough one.

Flaky test runs

First, about that.

I wasn't able to reproduce the flake reported in either of the two linked tickets - I've run the flaky test runner with 500 runs for each, and all were green.

In bundled_prebuilt_rules_package: Serverless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4695 (500 runs) you've got a version conflict error, though. So the flake is not fully fixed, and we need to investigate further before unskipping these tests.

I assume this is the same error as in #171428 -- have you got a response from @MadameSheema? Have you reached out to her elsewhere?

Secondly, why in Before changes: and After changes: we have different flaky test runner configurations? Can we have identical ones? Specifically, if we have 4 separate jobs of 500 runs each in Before, it would be great to do the same After the changes.

1. Cache for ES reading requests needs to be cleared

I suspect that in very specific cases (one in thousands) the cache for the request is not completely cleared after the first read, and even though we correctly install the prebuilt rule assets and refresh the indices, the subsequent read operations simply responds the cached value

This is a good hypothesis and we should take it into account, but it doesn't feel convincing to me from the probabilities perspective. Here there was a second test failure that occurred on the same day as the first one. The probability of that would be 1/1000000 in this case.

Another reason why it's not very convincing is that we don't seem to have the same issue with all our other requests and endpoints, e.g. rule management APIs. When we create custom rules in our tests, wouldn't we have the same issue?..

But my suggestion is: go with option 1 (as the PR stands) and monitor these tests. If they reappear, then the cache was not the problem, and we can try with the retry logic.

I would be ok to try this if we didn't get the version conflict error in that flaky test run. The tests are still flaky. I think we need to continue investigating this and doing more flaky test runs before we unskip them.

2. Implement retry logic for security_detection_engine package installation

This idea looks more robust to me and I'd suggest to implement it in this PR.

A few suggestions:

• Let's first assert that fleetResponse.body.items.length is in the response, and then assert that it's > 0.
• Let's set a small retry limit, such as 2 (1 initial attempt + 2 retries).
• Let's set a short delay between the retries, such as 100 ms.
• Let's refresh the indices after (outside of) the retries -- only if there was a successful attempt.
• Let's also clear the cache after refreshing the indices.
• Let's abstract away the two operations above in a reusable function.

[elasticmachine]

Pinging @elastic/security-solution (Team: SecuritySolution)

[vitaliidm]

Detection engine area LGTM

+++ on exploring option with retries

[banderror]

@jpdjere The flaky runner doesn't seem to be happy...

[banderror]

Changes LGTM, thanks 👍
Approving the PR to not being a blocker for it. Please look into the failed flaky test runs.

[jpdjere]

@jpdjere The flaky runner doesn't seem to be happy...

Ah, seems I hadn't pushed before running them. I'll rerun them and merge if they are green.

Thanks for the review, feedback and discussion.

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 85877d2

Failed CI Steps

• Jest Integration Tests #7
• FTR Configs #56

Test Failures

• [job] [logs] Jest Integration Tests #7 / fake elasticsearch should fail to start Kibana because of the Product Check Error
• [job] [logs] Jest Integration Tests #7 / fake elasticsearch should return unknown product when it cannot perform the Product check (503 response)
• [job] [logs] FTR Configs #56 / links panel links panel create and edit creation can create a new by-reference links panel

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/ftr-common-functional-services	29	31	+2

Unknown metric groups

API count

id	before	after	diff
@kbn/ftr-common-functional-services	29	31	+2

History

• 💛 Build #186510 was flaky f2d95ff
• 💔 Build #186499 failed 8d43ff6
• 💔 Build #186486 failed 02d486e
• 💚 Build #185966 succeeded 496d999ad97e6138ff6157fca16507a4b971d429
• 💔 Build #185935 failed 0c33f0e
• 💚 Build #185864 succeeded 1a9782a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jpdjere

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.12	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 173998

Questions ?

Please refer to the Backport tool documentation

[jpdjere]

💚 All backports created successfully

Status	Branch	Result
✅	8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation