-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Force unenroll in Agent activity #141208
Conversation
Pinging @elastic/fleet (Team:Fleet) |
(options.revoke && agent.unenrolled_at) || | ||
(!options.revoke && (agent.unenrollment_started_at || agent.unenrolled_at)) | ||
) { | ||
outgoingErrors[agent.id] = new FleetError(`Agent ${agent.id} already unenrolled`); |
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.
Changed the implementation of unenroll to be consistent with reassign: if an agent is already unenrolled, the action is set to failed for the agent.
Previously the unenrolled agents were filtered out, so the action didn't include them. This is problematic in agent activity, as those agents don't have any trace of being actioned.
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.
Not tested locally but the code looks good to me 🚀
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.
Code LGTM and works as expected when testing locally. 🚀
…tions that do not have results yet
const commonAgents = intersection(action.agents, agentIds); | ||
if (commonAgents.length > 0) { | ||
// filtering out agents with action results | ||
const agentsToUpdate = await getAgentsWithoutActionResults( |
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.
Added one more check to only update action results for those agents that do not yet have action results. This is to avoid having multiple result entries for the same action.
Did some measurements on 100k agent entries, and the unenroll took 24s without revoke and 31s with revoke.
@@ -143,7 +143,7 @@ export const AgentActivityFlyout: React.FunctionComponent<{ | |||
body={ | |||
<FormattedMessage | |||
id="xpack.fleet.agentActivityFlyout.noActivityDescription" | |||
defaultMessage="Activity feed will appear here as agents get enrolled, upgraded, or configured." | |||
defaultMessage="Activity feed will appear here as agents are reassigned, upgraded, or unenrolled." |
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.
tweaked description to fix #141186
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
* fix for force unenroll * fixed tests * fixed checks, solve for one more edge case: only updating unenroll actions that do not have results yet * added more tests * added try catch * updated description on flyout Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 3433f5d)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
* fix for force unenroll * fixed tests * fixed checks, solve for one more edge case: only updating unenroll actions that do not have results yet * added more tests * added try catch * updated description on flyout Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 3433f5d) Co-authored-by: Julia Bardi <[email protected]>
Summary
Fix #140794 and #140923
To verify:
Checklist