Skip to content
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] Add global component template to all fleet index templates #102225

Merged

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Jun 15, 2021

Description

Resolve #101309

With that PR event.ingested should be correctly mapped for all Fleet managed index template.

This PR introduce a new global component template, that is added to all the index template installed by Fleet.
This component template set the final_pipeline to be a global fleet final pipeline that verify agent id against the used API key, and add the mappings for the fields added by the fleet final pipeline.

Assets naming, I choose to name the two assets like this (open to better naming, and to discuss the convention here)

  • .fleet_component_template-{version} for the component template
  • .fleet_final_pipeline-{version} for the pipeline that check agent status

The feature is expose behind a config flag xpack.fleet.agentIdVerificationEnabled by default set to true.

How it works

The pipeline and the component template name contains a version so we can upgrade them in the future.

During the fleet setup, if the component template or the pipeline is not here we are going to:

  • create the pipeline and the component template
  • Apply the component template to all the installed integrations (for this I force reinstall the package, it's probably too much work but it used a well known code path and I think it avoid bug risk)
    • this is going to update the index template and update write indices

Future upgrade (not implemented) can

  • create new pipeline and new component template
  • Apply this to existing integrations
  • Delete unused old pipeline and component template

@nchaulet nchaulet added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 labels Jun 15, 2021
@nchaulet nchaulet self-assigned this Jun 15, 2021
@nchaulet nchaulet added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 15, 2021
@nchaulet nchaulet force-pushed the feature-fleet-global-component-template branch 2 times, most recently from dac3864 to 21c2a69 Compare June 15, 2021 17:04
@nchaulet nchaulet marked this pull request as ready for review June 15, 2021 17:10
@nchaulet nchaulet requested a review from a team as a code owner June 15, 2021 17:10
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet force-pushed the feature-fleet-global-component-template branch from 21c2a69 to b9b4144 Compare June 15, 2021 18:41
@nchaulet nchaulet requested review from ruflin and jfsiii June 15, 2021 18:57
@ruflin
Copy link
Contributor

ruflin commented Jun 16, 2021

Could we put this installation behind a config option? It should be turned on by default but I think we should offer users to turn it off. This also helps in the case we discover any issues with the installation mechanism, there is an opt out.

On the upgrade path, can you make sure we follow up on this? I have the suspicion we need it sooner then we like.

There is a downside to having the pipeline versioned. If it is not versioned, we could just overwrite it all the processing would use the new one. If we version it, we need to update all data streams. Maybe if the pipeline keeps the data the same, we just keep the version? But how will then the upgrade mechanism know to overwrite?

@nchaulet
Copy link
Member Author

Could we put this installation behind a config option? It should be turned on by default but I think we should offer users to turn it off. This also helps in the case we discover any issues with the installation mechanism, there is an opt out.

Just added the xpack.fleet.agentIdVerificationEnabled flag

For the migration

If we want to only update the pipeline we can use the version property to know if we should upgrade it or not, we will have 3 version to track the version in the pipeline name, the version in the pipeline, and the version of the component template, it start to add some complexity.

I am wondering this will be a good idea. If we have a migration system for Fleet close to the one we use for Saved object, with a version saved in one of our saved object, and based on that we can run migration function where we can do whatever we want, it probably can help for other things like endpoint migration between version too. (but there is a risk to have the ES assets out of sync with what we have if a user manually change them)

@ruflin
Copy link
Contributor

ruflin commented Jun 18, 2021

Thanks for adding the config. Will this always install the assets but not just enable them? What if a user had it enabled and hits issues and wants to disable it, how does this work?

For the upgrade: Happy to discuss it in a separate thread.

@nchaulet
Copy link
Member Author

Disabling the flag will not install the assets and not enable.

What if a user had it enabled and want to disable

new package installation will not have it but old packages will have to be reinstalled to disable the component template.

@ruflin
Copy link
Contributor

ruflin commented Jun 18, 2021

I'm a bit worried about the part that after disabling it is not really disabled. Is there an easy way for users to "reinstall" a package?

@nchaulet
Copy link
Member Author

Is there an easy way for users to "reinstall" a package?

Not really an easy way it's possible to reinstall the package via calling the API to install the package with force.

If you think it's problematic I can do it in the setup call if we detect the flag is off and there is a component template we can clean everything.

@ruflin
Copy link
Contributor

ruflin commented Jun 21, 2021

@nchaulet @jen-huang I suggest for now we get this PR in quickly so we have enough time to test and see if it causes any issue. If it does not have any issues we don't need to worry about the disable part. There is also a "hacky" way to disable it which is overwrite the pipeline with an empty pipeline in case of "emergency" ;-)

@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. Left two grammar-related suggestions. I'm w/ @ruflin re: merging and confirming behavior

x-pack/plugins/fleet/server/services/setup.ts Outdated Show resolved Hide resolved
x-pack/plugins/fleet/server/services/setup.ts Outdated Show resolved Hide resolved
Co-authored-by: John Schulz <john.schulz@elastic.co>
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@nchaulet nchaulet requested a review from a team as a code owner June 22, 2021 18:51
@nchaulet
Copy link
Member Author

@elastic/ml-ui I add to modify an API test as it's required to setup fleet before installing a package.

@nchaulet nchaulet force-pushed the feature-fleet-global-component-template branch from e345d60 to e7c0e23 Compare June 22, 2021 19:46
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@jfsiii
Copy link
Contributor

jfsiii commented Jun 23, 2021

@nchaulet I just merged #101769 which means this PR needs the update you mentioned to prevent the global template from being removed.

Hopefully the change is clear and simple enough, but please let me know if you have any questions or if I can help at all.

@nchaulet nchaulet requested a review from pheyos June 23, 2021 11:22
@nchaulet nchaulet force-pushed the feature-fleet-global-component-template branch from f7c5b6f to 27b292a Compare June 23, 2021 12:42
Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ML test changes LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
fleet 1014 1015 +1
Unknown metric groups

API count

id before after diff
fleet 1106 1107 +1

History

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

cc @nchaulet

@nchaulet nchaulet merged commit 3864fe1 into elastic:master Jun 23, 2021
@nchaulet nchaulet deleted the feature-fleet-global-component-template branch June 23, 2021 17:18
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 102225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] mapping for event.agent_id_status and event.ingested
6 participants