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

[Fleet] Replace INTERNAL_POLICY_REASSIGN by POLICY_REASSIGN #94116

Merged
merged 6 commits into from
Mar 25, 2021
Merged

[Fleet] Replace INTERNAL_POLICY_REASSIGN by POLICY_REASSIGN #94116

merged 6 commits into from
Mar 25, 2021

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Mar 9, 2021 •
edited
Loading

Context

The way we reassign an agent to a new policy is a little different than other fleet actions:

we update the agent doc

  • We currently use an action called INTERNAL_POLICY_REASSIGN that is handler in kibana checkin to refresh the agent and send a new policy to the agent
  • This is not handled by Fleet server and to simplify that we are going to create a POLICY_REASSIGN action that will be sent to the agent

we update the agent doc

  • we send a POLICY_REASSIGN action to the agent
  • the agent ack the action than checkin and get the new policy

How to test

Without fleet server

  • Enroll an agent through Kibana
  • reassign this agent to another policy

With Fleet server it's still a little hard to setup Fleet server and there is a few bugs currently, I think there is not major difference here that need a test with Fleet Server.

  • enroll an agent throught fleet server
  • reassign this agent

@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.13.0 labels Mar 9, 2021
@nchaulet nchaulet requested a review from a team as a code owner March 9, 2021 15:20
@nchaulet nchaulet self-assigned this Mar 9, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet mentioned this pull request Mar 9, 2021
Closed
@afgomez afgomez self-requested a review March 18, 2021 10:59
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@nchaulet nchaulet requested a review from jen-huang March 18, 2021 17:16
afgomez
afgomez reviewed Mar 22, 2021
View reviewed changes
x-pack/plugins/fleet/server/services/agents/checkin/state_new_actions.ts
@@ -261,25 +261,6 @@ export function agentCheckinStateNewActionsFactory() {
return EMPTY;
}

const hasConfigReassign = newActions.some(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you help me understand why this is no longer needed?

Copy link
Member Author

@nchaulet nchaulet Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, previously we were relying on Kibana to handle an action INTERNAL_POLICY_REASSIGN and transform this to a POLICY_CHANGE action so the agent get the new policy.

With the move to fleet server we are going to simplify that, we are introducting a new action POLICY_REASSIGN that will be send to the agent, then the agent will handle that, then checkin again and get the new POLICY_CHANGE action.

Does it make things clearer?

@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@nchaulet nchaulet requested a review from afgomez March 23, 2021 13:20
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @nchaulet

jen-huang
jen-huang approved these changes Mar 24, 2021
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.

I tested reassigning agents to another policy locally and it seems to have worked as expected, I confirmed that corresponding fleet-agent-actions SO that is created has "type": "POLICY_REASSIGN". I have a question regarding need for migration, but otherwise code LGTM.

x-pack/plugins/fleet/common/types/models/agent.ts
@@ -36,8 +36,7 @@ export type AgentActionType =
| 'UNENROLL'
| 'UPGRADE'
| 'SETTINGS'
// INTERNAL* actions are mean to interupt long polling calls these actions will not be distributed to the agent
| 'INTERNAL_POLICY_REASSIGN';
| 'POLICY_REASSIGN';
Copy link
Contributor

Choose a reason for hiding this comment

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

do you think we would need to add a migration script for existing fleet-agent-actions SOs to transform INTERNAL_POLICY_REASSIGN to POLICY_REASSIGN?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's okay actions are not mean to stay for a long time, so I guess we are okay moreover with the move to fleet server we should start from a fresh ground and do not have any agent already enrolled.

jen-huang
jen-huang reviewed Mar 24, 2021
View reviewed changes
x-pack/plugins/fleet/common/types/models/agent.ts
@@ -36,8 +36,7 @@ export type AgentActionType =
| 'UNENROLL'
| 'UPGRADE'
| 'SETTINGS'
// INTERNAL* actions are mean to interupt long polling calls these actions will not be distributed to the agent
| 'INTERNAL_POLICY_REASSIGN';
| 'POLICY_REASSIGN';
Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot to mention, do we have any integration tests for these agent actions that would need to get updated to cover this?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's well covered by the e2e tests

@nchaulet nchaulet added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 25, 2021
@nchaulet nchaulet merged commit 00c53c5 into elastic:master Mar 25, 2021
@nchaulet nchaulet deleted the feature-internal-policy-reassign branch March 25, 2021 13:16
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 25, 2021
@nchaulet @kibanamachine
[Fleet] Replace INTERNAL_POLICY_REASSIGN by POLICY_REASSIGN (elastic#…
9a64e44 
…94116)
@kibanamachine kibanamachine mentioned this pull request Mar 25, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #95406

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 25, 2021
@kibanamachine @nchaulet
[Fleet] Replace INTERNAL_POLICY_REASSIGN by POLICY_REASSIGN (#94116) (#…
25c89a6 
…95406)

Co-authored-by: Nicolas Chaulet <[email protected]>
@nchaulet nchaulet mentioned this pull request Mar 26, 2021
Closed
@nchaulet
Copy link
Member Author

Resolve elastic/fleet-server#84

@nchaulet nchaulet mentioned this pull request Apr 27, 2021
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

@afgomez afgomez Awaiting requested review from afgomez

Assignees

@nchaulet nchaulet

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.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nchaulet @elasticmachine @kibanamachine @afgomez @jen-huang