[7.10][Ingest Manager][EPM] Moving reinstall function outside of promise.all to avoid race

[jonathan-buttner]

This is to address this issue: #82614

There is a potential for a race condition during the /setup API call if one of the required packages is in the installing state. A required package can get in this state if a failure occurred to install/upgrade that package and we were unable to rollback the package for some reason.

Upon the next call to /setup, if the package can be upgraded, the upgrade code will be initiated in addition to attempting to reinstall the failed package version because both the

ensureInstalledDefaultPackages and the ensurePackagesCompletedInstall calls happen in parallel in the Promise.all.

This PR tries to mitigate that by moving the reinstall functionality outside of the Promise.all so that it is done after the upgrade attempt.

I'm not sure how we would test this with integration tests 🤔 or if it's worth writing unit tests for this. Let me know what you think.

[elasticmachine]

Pinging @elastic/ingest-management (Team:Ingest Management)

[neptunian]

I was able to consistently reproduce the saved object conflict locally by having only one required package that /setup checks for, force installing an older version of it and throwing an error during the installation so the package became stuck in "installing" status, and then running setup again. It tries to upgrade and reinstall causing the conflict. With this change, I cannot reproduce it following the same steps.

[jfsiii]

Approving since a) worst case "should" be longer install time b) @neptunian can repro error & fix.

Can we open a ticket to create a test based on @neptunian instructions? e.g. mock ensure* to throw and/or put saved object in a certain state?

We could move the function back into Promise.all, create the failure case which causes the failure, then move it back to this position and change the expectation.

[jonathan-buttner]

Issue for adding tests: #82670

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 63f6632

Metrics [docs]

✅ unchanged

History

• 💚 Build #85960 succeeded 63f6632

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