Skip to content

[Fleet] Fix agent status badge when stuck upgrading #226893

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[Fleet] Fix agent status badge when stuck upgrading #226893

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e627c68
changed logic to show failed when stuck or in bad state
Supplementing Jul 7, 2025
19ed087
Merge branch 'main' into fix-agent-status-table-fix
elasticmachine Jul 7, 2025
a03dd6b
changed status component to only use the failed upgrade state, rather…
Supplementing Jul 7, 2025
2d93f31
Merge branch 'fix-agent-status-table-fix' of https://github.com/Suppl…
Supplementing Jul 7, 2025
029160e
updated getStatusComponent to use entire agent object rather than spe…
Supplementing Jul 8, 2025
fb23062
fix error related to type
Supplementing Jul 8, 2025
ed924d8
added union typing to props to make sure that at least one of the pro…
Supplementing Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion ...lugins/shared/fleet/public/applications/fleet/sections/agents/components/agent_health.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ type Props = EuiBadgeProps & {

function getStatusComponent({
status,
inFailedUpgradeState,
...restOfProps
}: {
status: Agent['status'];
inFailedUpgradeState: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

it feels a bit odd to introduce a new prop just for this specific sub-state.. I wonder if we can just accept the agent upgrade details information like this:

Suggested change
status: Agent['status'];
inFailedUpgradeState: boolean;
status: Agent['status'];
upgradeDetails?: Agent['upgrade_details'];

and then use isAgentInFailedUpgradeState() directly in this component to derive the badge. WDYT?

then this component could show other upgrade sub-states in the future if we choose to (I see 9 of them in AgentUpgradeStateType!)

Copy link
Contributor Author

@Supplementing Supplementing Jul 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I agree, using something more extensible rather than a specific prop makes more sense in this case. However, that isAgentInFailedUpgradeState() function takes an Agent rather than upgrade_details, so maybe it makes sense to simplify further and just pass the entire Agent into this function and then we can access the status and upgrade_details inside as needed. WDYT?

EDIT: I went ahead and made the changes in the latest commits. However, there was a small implication in that, we are passing in only a string for the previousToOfflineStatus rather than an agent, so I needed to include that as a prop of its own.

Copy link
Contributor

Choose a reason for hiding this comment

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

that isAgentInFailedUpgradeState() function takes an Agent rather than upgrade_details

this is true, but it is only using upgrade_details in its logic so can be easily refactored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is true, but it is only using upgrade_details in its logic so can be easily refactored

Should we prefer refactoring that function then over my latest changes (passing in the Agent and using a union for the props type)?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO status+upgradeDetails makes more sense as props for this component than agent+previousStatus

} & EuiBadgeProps): React.ReactElement {
switch (status) {
case 'error':
Expand Down Expand Up @@ -97,7 +99,14 @@ function getStatusComponent({
case 'unenrolling':
case 'enrolling':
case 'updating':
return (
return inFailedUpgradeState ? (
<EuiBadge color="danger" {...restOfProps}>
<FormattedMessage
id="xpack.fleet.agentHealth.upgradingFailedStatusText"
defaultMessage="Upgrade failed"
/>
</EuiBadge>
) : (
<EuiBadge color="primary" {...restOfProps}>
<FormattedMessage
id="xpack.fleet.agentHealth.updatingStatusText"
Expand Down Expand Up @@ -206,6 +215,7 @@ export const AgentHealth: React.FunctionComponent<Props> = ({
<div className="eui-textNoWrap">
{getStatusComponent({
status: agent.status,
inFailedUpgradeState: isAgentInFailedUpgradeState(agent),
...restOfProps,
})}
&nbsp;
Expand All @@ -215,11 +225,13 @@ export const AgentHealth: React.FunctionComponent<Props> = ({
<>
{getStatusComponent({
status: agent.status,
inFailedUpgradeState: isAgentInFailedUpgradeState(agent),
...restOfProps,
})}
{previousToOfflineStatus
? getStatusComponent({
status: previousToOfflineStatus,
inFailedUpgradeState: isAgentInFailedUpgradeState(agent),
...restOfProps,
})
: null}
Expand Down