-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fleet] Improve performance of Fleet setup #102219
Conversation
Pinging @elastic/fleet (Feature:Fleet) |
Pinging @elastic/fleet (Team:Fleet) |
As discussed with the Endpoint team, let's remove Endpoint from the list of packages that are auto-installed, but allow it to be auto-upgraded when visiting Fleet. I reviewed our current code here for default vs required packages, but their meaning doesn't suit our needs:
I think we will need to create a third list:
Default packages are presently auto-upgraded as well. With the creation of the third list, we'll have to adjust the logic so that auto-install acts on the list of default packages, and auto-upgrade acts on the list of auto-upgrade packages. What do you think? |
Makes sense, but I would collapse the first two lists into one. If a package cannot be uninstalled it needs to be auto-installed. We need to also consider how this should work when the user specifies the default packages/policies in their I'll play with it and push something today |
We want to keep the assumption that the lists contain the same packages only in `epm/constants.ts`
I do agree with this logic, but per some offline discussion we felt it would be safer to mimic the existing functionality of the Endpoint package as much as possible for this iteration. So the Endpoint package needs to:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I benchmarked /setup
in this PR against main and saw ~6 seconds reduced loading time (20% reduction!) 🔥 I tested the non-auto-install, but auto-upgrade nature of Endpoint package and it seems to work well too.
There is probably more we could optimize in the actual install process to reduce the time even more, but I'm very happy to get this PR in as it is a significant improvement.
</h2> | ||
} | ||
titleSize="m" | ||
body={<EuiLoadingSpinner size="xl" />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after #101334 is merged, we can take advantage of the new empty prompt loading pattern that was recently added to EUI which looks a bit nicer. not a blocker for this PR, we can come back to adjust it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to use that, but Kibana doesn't use yet the right version of EUI. Let's keep track of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good, we'll just need the endpoint tests to install the endpoint package as part of their setup and they should pass 👍
FleetServer: FLEET_SERVER_PACKAGE, | ||
} as const; | ||
/* | ||
Package rules: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the table! Does requiredPackages
just mean that they are not removable? I think that's what I'm seeing from the code. Maybe we should change it to unremovable
or something. required
makes me think that it must be installed by just reading the name. The table helps clarify it though 😄
@jonathan-buttner Thanks for testing! I made the Endpoint package be installed by default on tests by passing it in start options (b400b9d). Hopefully that approach works for you? |
Sounds good to me! |
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @afgomez |
* Remove endpoint from the default packages * Change the default spinner for the initial load * Export fleet endpoint package as a constant * Use constants for special packages * Simplify type signature of `isRequiredPackage` * Remove unused types * Simplify required and default package definitions * Treat REQUIRED_PACKAGES as independent from DEFAULT_PACKAGES We want to keep the assumption that the lists contain the same packages only in `epm/constants.ts` * Install all default packages, not only the required ones * Document the purpose of each package list * Handle auto-update for non-default packages * Make `endpoint` non-removable * Make endpoint package be installed by default in tests * Rename requiredPackages to unremovablePackages * Fix type check * Add Endpoint to be installed by default on Fleet tests too Co-authored-by: Jen Huang <its.jenetic@gmail.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* Remove endpoint from the default packages * Change the default spinner for the initial load * Export fleet endpoint package as a constant * Use constants for special packages * Simplify type signature of `isRequiredPackage` * Remove unused types * Simplify required and default package definitions * Treat REQUIRED_PACKAGES as independent from DEFAULT_PACKAGES We want to keep the assumption that the lists contain the same packages only in `epm/constants.ts` * Install all default packages, not only the required ones * Document the purpose of each package list * Handle auto-update for non-default packages * Make `endpoint` non-removable * Make endpoint package be installed by default in tests * Rename requiredPackages to unremovablePackages * Fix type check * Add Endpoint to be installed by default on Fleet tests too Co-authored-by: Jen Huang <its.jenetic@gmail.com> Co-authored-by: Alejandro Fernández Gómez <alejandro.fernandez@elastic.co> Co-authored-by: Jen Huang <its.jenetic@gmail.com>
…ets-tab * 'master' of github.com:elastic/kibana: (93 commits) [ML] Remove blank job definition as it is unused and out-of-sync with Elasticsearch (elastic#102506) [Lens] Fix wrong error detection on transition to Top values operation (elastic#102384) [ML] Anomaly detection job custom_settings improvements (elastic#102099) [Cases] Route: Get all alerts attach to a case (elastic#101878) Fixes wrong list exception type when creating endpoint event filters list (elastic#102522) remove search bar that's not working yet (elastic#102550) Migrated Ingest Node Pipeline Functional Tests to use test_user (elastic#102409) [Maps] clean up feature editing name space to avoid conflicts with layer settings editing (elastic#102516) [canvas] Refactor Storybook from bespoke to standard configuration (elastic#101962) [Security Solution] adds wrapSequences method (RAC) (elastic#102106) [FTR] Stabilize SSLP functional tests (elastic#102553) [K8] Added `Inter` font files for new theme (elastic#102359) [Workplace Search] Convert Groups pages to new page template (elastic#102449) [DOC] Add experimental disclaimer to rollup jobs (elastic#95624) [Security Solution][Endpoint] Suppress some of the jest console.error noise created by endpoint list middelware (elastic#102535) [Fleet] Improve performance of Fleet setup (elastic#102219) [Alerting] Add event log entry when a rule starts executing (elastic#102001) [Fleet] Update docker image of registry used in integration tests (elastic#101911) [Asset Management] Osquery telemetry updates (elastic#100754) Converts saved object tagging to new management layout (elastic#102284) ... # Conflicts: # x-pack/plugins/fleet/kibana.json
Hi @jen-huang We have validated this PR on 7.14 Snapshot build as per test steps mentioned in ticket summary and found changes are working fine.
Followed above steps and observed it is working fine.
Build details:
cc @EricDavisX Please let us know if any further tests are required. Thanks |
Summary
Closes #96026. See the issue's comments for details of what we have tried and measured.
This PR attempts to improve the setup time in Fleet. We are doing two things for it:
endpoint
from the list of default packages. The package is not in any of the default policies, so it can be safely removed.Some functionality depends on the endpoint package being auto-updated, so we have introduced a new constant
AUTO_UPDATE_PACKAGES
withendpoint
in it. Fleet will ensure the packages in this list will be updated to the last version, even if they are not installed by default.Currently the loader is briefly shown even if Fleet is already set up, so we cannot do much regarding the text of that screen. We would need to first change how that loader is shown, and make it dependent of the actual setup status.
Testing
To test the new auto-update,
yarn es snapshot -E xpack.security.authc.api_key.enabled=true
endpoint
package in yourkibana.dev.yml
kibana.dev.yml
Checklist
Delete any items that are not applicable to this PR.