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

[Ingest Manager] Agent unenroll #19507

Merged
merged 9 commits into from
Jul 7, 2020

Conversation

michalpristas
Copy link
Contributor

What does this PR do?

This PR works with draft from here: elastic/kibana#70031
Accepts UNENROLL actions and stop all the processes it has started.

Why is it important?

Removing endpoint on unenroll.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@michalpristas michalpristas added enhancement needs_backport PR is waiting to be backported to other branches. Team:Ingest Management Ingest Management:beta1 Group issues for ingest management beta1 labels Jun 30, 2020
@michalpristas michalpristas self-assigned this Jun 30, 2020
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jun 30, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 30, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #19507 updated]

  • Start Time: 2020-07-07T10:23:34.460+0000

  • Duration: 31 min 39 sec

@michalpristas michalpristas marked this pull request as ready for review July 3, 2020 09:16
@michalpristas
Copy link
Contributor Author

Tested with latest kibana branch unpdates, agent ACKs UNENROLL and disapears from the list as expected

@michalpristas michalpristas changed the title Agent unenroll [Ingest Manager] Agent unenroll Jul 3, 2020
Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Overall the code looks good. Seems you still need to add more to clean the action store, assume that is coming in a later branch.

I have more of a question on what happens to Agent after the completion of unenroll. Will it still be able to communicate with the Fleet gateway? I assume that the API keys will be invalidated after ack of unenroll, so then communication will be broken.

If that is the case, should Agent set a configuration setting into the Fleet config so it knows that it is unenrolled? We don't want Fleet to go to standalone mode (that would be weird/wrong). We also don't want it to keep reaching out to Fleet and failing (that would add load both the the client and server, for no benefit). We also don't want to just stop, as the service manager of the system will just keep restarting it.

If I understand unenroll correctly we need some kinda of idle state, that just logs Agent is unenrolled and it just really does nothing (but stays running).

@michalpristas
Copy link
Contributor Author

@blakerouse I was thinking we will use the same mechanism needed for reexec, cancelling everything down and stopping all the loops/processes. for now cancellation is not that good. but it is a good point. in the meantime i will just stop fleet gateway

@michalpristas
Copy link
Contributor Author

Reworked it a bit going to idle state on restart after being unenrolled.

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ingest Management:beta1 Group issues for ingest management beta1 needs_backport PR is waiting to be backported to other branches.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants