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.

Sign up for GitHub

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

Jump to bottom

[Ingest Manager] Internal action for policy reassign #78493

Merged
merged 3 commits into from
Oct 6, 2020
Merged

[Ingest Manager] Internal action for policy reassign #78493

merged 3 commits into from
Oct 6, 2020

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Sep 24, 2020 •
edited
Loading

Summary

Resolves #77613

When we introduced long polling for agent checkin, we introduced a bug for policy reassignment.

Agent policy reassignment is triggered by removing the field policy_revision and updating the field policy_id on the agent, so during the next checkin we will send a new POLICY_CHANGE action that the agent will use.

With long polling the agent is going to wait until the long polling timeout duration to do a new checkin, this cause policy reassign to be extremely long.

I think we can fix that by introducing new kind of actions INTERNAL_* and INTERNAL_POLICY_REASSIGN.

When we reassign an agent we can create a new action INTERNAL_POLICY_REASSIGN that will be get during the long polling (we check for new agent actions every 3 seconds) and we can use that action to refresh the agent and distribute the POLICY_CHANGE action.

Not sure it's the best solution to that problem.

In progress

  • Add some tests

@nchaulet nchaulet self-assigned this Sep 28, 2020
@nchaulet nchaulet added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 28, 2020
@nchaulet nchaulet mentioned this pull request Oct 1, 2020
Closed
@nchaulet
Copy link
Member Author

nchaulet commented Oct 5, 2020

@jen-huang what do you think of this approach? I am considering moving forward with that and adding this for bulk actions if it sounds good to you?

@jen-huang
Copy link
Contributor

@nchaulet I am in favor of this approach, I spent some time thinking about about alternatives but couldn't think of anything better that wouldn't require major restructuring. I think it would be good to add some code comments here to say that INTERNAL_* actions are not sent down to the agent and are just for internal use by Fleet. I'll do a full code review when this is ready.

@nchaulet nchaulet added Ingest Management:beta2 v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Oct 5, 2020
@nchaulet nchaulet marked this pull request as ready for review October 5, 2020 22:10
@nchaulet nchaulet requested a review from a team October 5, 2020 22:10
@elasticmachine
Copy link
Contributor

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

jen-huang
jen-huang approved these changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

Tested locally with latest 7.10 snapshot of agent, policy reassignment got picked up super fast and regular policy change worked like normal. Code LGTM 👍

@jen-huang
Copy link
Contributor

One thing I noticed in my testing is that unenrollment still takes a long time to be acknowledged. I wonder if it is possible to use the same kind of INTERNAL_* pattern to speed that up?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@nchaulet
Copy link
Member Author

nchaulet commented Oct 6, 2020

One thing I noticed in my testing is that unenrollment still takes a long time to be acknowledged. I wonder if it is possible to use the same kind of INTERNAL_* pattern to speed that up?

Yes the force unenroll will benefits to use the same pattern, I will try to get another PR for that or at least an issue

@nchaulet nchaulet merged commit 71fef96 into elastic:master Oct 6, 2020
@nchaulet nchaulet deleted the feature-internal-actions branch October 6, 2020 13:21
@nchaulet nchaulet mentioned this pull request Oct 6, 2020
Merged
nchaulet added a commit to nchaulet/kibana that referenced this pull request Oct 6, 2020
@nchaulet
[Ingest Manager] Internal action for policy reassign (elastic#78493)
75d9b6e
nchaulet added a commit that referenced this pull request Oct 6, 2020
@nchaulet
[Ingest Manager] Internal action for policy reassign (#78493) (#79657)
ef824c0
gmmorris added a commit that referenced this pull request Oct 8, 2020
@gmmorris
Merge branch 'feature/task_manager_429' of github.com:elastic/kibana …
574e523 
…into feature/task_manager_429

* 'feature/task_manager_429' of github.com:elastic/kibana: (158 commits)
  Add license check to direct package upload handler. (#79653)
  [Ingest Manager] Rename API /api/ingest_manager => /api/fleet (#79193)
  [Security Solution][Resolver] Simplify CopyableField styling and add comments (#79594)
  Fine-tunes ML related text on Metrics UI (#79425)
  [ML] DF Analytics creation wizard: ensure job creation possible when model memory lower than estimate (#79229)
  Add new "Add Data" tutorials (#77237)
  Update APM telemetry docs (#79583)
  Revert "Add support for runtime field types to mappings editor. (#77420)" (#79611)
  Kibana request headers (#79218)
  ensure missing indexPattern error is bubbled up to error callout (#79378)
  Missing space fix (#79585)
  remove duplicate tab states (#79501)
  [data.ui] Lazy load UI components in data plugin. (#78889)
  Add generic type params to search dependency. (#79608)
  [Ingest Manager] Internal action for policy reassign (#78493)
  [ILM] Add index_codec to forcemerge action in hot and warm phases (#78175)
  [Ingest Manager] Update open API spec and add condition to agent upgrade endpoint (#79579)
  [ML] Hide Data Grid column options when histogram charts are enabled. (#79459)
  [Telemetry] Synchronous `setup` and `start` methods (#79457)
  [Observability] Persist time range across apps (#79258)
  ...
@EricDavisX EricDavisX mentioned this pull request Oct 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jen-huang jen-huang jen-huang approved these changes

Assignees

@nchaulet nchaulet

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Ingest Manager] Assigning policy/config takes too long
4 participants
@nchaulet @jen-huang @elasticmachine @kibanamachine