fix: send upgrade action to units before adding it to bkgActions #9634

pkoutsovasilis · 2025-08-28T22:16:16Z

What does this PR do?

This PR refactors the Coordinator.Upgrade API to use functional options (UpgradeOpt) instead of long argument lists, and introduces a preUpgradeCallback.

Specifically:

Moves the action proxying of Endpoint upgrades into a callback passed to the coordinator. This ensures that action dispatch only happens after the initial upgrade checks succeed (already upgrading, agent upgradeable, agent upgrade capability allowed, etc.), which is a more logical sequence.
Reorders checks inside Coordinator.Upgrade to make the flow more consistent (e.g. first verifying that the coordinator is not already upgrading before continuing).
Updates tests and server code to use the new options-based API.

All business logic changes are captured here bcdba7b (+90 -29 lines changed)

Why is it important?

Currently, upgrade actions can remain stuck in bkgActions when tamper protection is enabled (see #9629). This occurs because errors returned from the Endpoint action proxying are not correctly handled, leaving stale entries behind.

By moving this logic into a preUpgradeCallback, any error from proxying is now surfaced through the coordinator’s upgrade flow, which ensures that bkgActions are cleaned up correctly.

I am not fully satisfied with these changes — they are a pragmatic fix until we perform a larger rewrite to centralize upgrade action handling in a single place. Still, they are necessary to resolve the immediate issue of stuck upgrade actions.

Checklist

I have read and understood the pull request guidelines of this project.
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/fragments using the changelog tool
I have added an integration test or an E2E test

Disruptive User Impact

None expected - this change only affects the internal proxying of upgrade actions to Endpoint and does not alter the upgrade request API.

How to test this PR locally

mage unitTest

Related issues

Closes Upgrade actions can be ignored due to stale entries in bkgActions #9629

…de callback to properly handle errs and upgrade details

elasticmachine · 2025-09-08T05:51:09Z

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

cmacknz

The approach looks good to me as a way to fix this without totally rewriting everything. A couple of small comments.

internal/pkg/agent/application/actions/handlers/handler_action_upgrade.go

internal/pkg/agent/application/actions/handlers/handler_action_upgrade_test.go

cmacknz · 2025-09-10T01:09:37Z

After your clarifications my only remaining comments are suggestions for readability or documentation, which I think are necessary but if you do these and get approval from someone else go ahead and merge and don't feel the need to wait for me to review again.

elastic-sonarqube · 2025-09-10T10:17:04Z

Quality Gate passed

Issues
1 New issue
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
64.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

elasticmachine · 2025-09-10T10:18:28Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 9c2f633

Failed CI Steps

History

💔 Build #26254 failed ad27fc6
💔 Build #26253 failed deac63b
💚 Build #26140 succeeded 7e8a2b0
💛 Build #25891 was flaky b3b2c9f

cc @pkoutsovasilis

cmacknz

Thanks for the follow up!

github-actions · 2025-09-10T14:03:16Z

@Mergifyio backport 8.18 8.19 9.0 9.1

mergify · 2025-09-10T14:03:41Z

backport 8.18 8.19 9.0 9.1

✅ Backports have been created

#9859 [8.18] (backport #9634) fix: send upgrade action to units before adding it to bkgActions has been created for branch 8.18 but encountered conflicts
#9860 [8.19] (backport #9634) fix: send upgrade action to units before adding it to bkgActions has been created for branch 8.19 but encountered conflicts
#9861 [9.0] (backport #9634) fix: send upgrade action to units before adding it to bkgActions has been created for branch 9.0 but encountered conflicts
#9862 [9.1] (backport #9634) fix: send upgrade action to units before adding it to bkgActions has been created for branch 9.1 but encountered conflicts

* fix: refactor coordinator Upgrade to use opts and introduce pre-upgrade callback to properly handle errs and upgrade details * ci: add unit-tests * doc: add changelog fragment * fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn * doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion (cherry picked from commit bb98f2a) # Conflicts: # internal/pkg/agent/application/coordinator/coordinator.go

…ng it to bkgActions (#9859) * fix: send upgrade action to units before adding it to bkgActions (#9634) * fix: refactor coordinator Upgrade to use opts and introduce pre-upgrade callback to properly handle errs and upgrade details * ci: add unit-tests * doc: add changelog fragment * fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn * doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion (cherry picked from commit bb98f2a) # Conflicts: # internal/pkg/agent/application/coordinator/coordinator.go * fix: resolve conflicts --------- Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>

…g it to bkgActions (#9861) * fix: send upgrade action to units before adding it to bkgActions (#9634) * fix: refactor coordinator Upgrade to use opts and introduce pre-upgrade callback to properly handle errs and upgrade details * ci: add unit-tests * doc: add changelog fragment * fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn * doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion (cherry picked from commit bb98f2a) # Conflicts: # internal/pkg/agent/application/coordinator/coordinator.go * fix: resolve conflicts --------- Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>

…ng it to bkgActions (#9860) * fix: send upgrade action to units before adding it to bkgActions (#9634) * fix: refactor coordinator Upgrade to use opts and introduce pre-upgrade callback to properly handle errs and upgrade details * ci: add unit-tests * doc: add changelog fragment * fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn * doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion (cherry picked from commit bb98f2a) # Conflicts: # internal/pkg/agent/application/coordinator/coordinator.go * fix: resolve conflicts --------- Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>

…g it to bkgActions (#9862) * fix: send upgrade action to units before adding it to bkgActions (#9634) * fix: refactor coordinator Upgrade to use opts and introduce pre-upgrade callback to properly handle errs and upgrade details * ci: add unit-tests * doc: add changelog fragment * fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn * doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion (cherry picked from commit bb98f2a) # Conflicts: # internal/pkg/agent/application/coordinator/coordinator.go * fix: resolve conflicts --------- Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>

…stic#9634) * fix: refactor coordinator Upgrade to use opts and introduce pre-upgrade callback to properly handle errs and upgrade details * ci: add unit-tests * doc: add changelog fragment * fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn * doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion

pkoutsovasilis self-assigned this Aug 28, 2025

pkoutsovasilis added bug Something isn't working Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team skip-changelog backport-active-all Automated backport with mergify to all the active branches labels Aug 28, 2025

pkoutsovasilis force-pushed the fix/endpoint_upgrade_action_error branch 2 times, most recently from deac63b to ad27fc6 Compare September 5, 2025 10:36

pkoutsovasilis added 3 commits September 5, 2025 16:47

fix: refactor coordinator Upgrade to use opts and introduce pre-upgra…

bcdba7b

…de callback to properly handle errs and upgrade details

ci: add unit-tests

66ebf30

doc: add changelog fragment

eb2f6a5

pkoutsovasilis force-pushed the fix/endpoint_upgrade_action_error branch from ad27fc6 to eb2f6a5 Compare September 5, 2025 13:47

pkoutsovasilis marked this pull request as ready for review September 8, 2025 05:51

pkoutsovasilis requested a review from a team as a code owner September 8, 2025 05:51

pkoutsovasilis requested review from kaanyalti and pchila September 8, 2025 05:51

pkoutsovasilis removed the skip-changelog label Sep 8, 2025

cmacknz reviewed Sep 9, 2025

View reviewed changes

internal/pkg/agent/application/actions/handlers/handler_action_upgrade.go Outdated Show resolved Hide resolved

internal/pkg/agent/application/actions/handlers/handler_action_upgrade_test.go Outdated Show resolved Hide resolved

pkoutsovasilis added 2 commits September 10, 2025 10:10

fix: rename notifyUnitsOfProxiedAction to notifyUnitsOfProxiedActionFn

71181e2

doc: add comment for notifyUnitsOfProxiedActionFn unit-test assertion

9c2f633

cmacknz approved these changes Sep 10, 2025

View reviewed changes

pkoutsovasilis merged commit bb98f2a into elastic:main Sep 10, 2025
23 checks passed

mergify bot mentioned this pull request Sep 10, 2025

[8.18] (backport #9634) fix: send upgrade action to units before adding it to bkgActions #9859

Merged

8 tasks

This was referenced Sep 10, 2025

[8.19] (backport #9634) fix: send upgrade action to units before adding it to bkgActions #9860

Merged

[9.0] (backport #9634) fix: send upgrade action to units before adding it to bkgActions #9861

Merged

mergify bot mentioned this pull request Sep 10, 2025

[9.1] (backport #9634) fix: send upgrade action to units before adding it to bkgActions #9862

Merged

8 tasks

cmacknz mentioned this pull request Sep 15, 2025

Agent is unable to retry upgrade with correct binary on failing once with incorrect binary. #7611

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: send upgrade action to units before adding it to bkgActions #9634

fix: send upgrade action to units before adding it to bkgActions #9634

Uh oh!

pkoutsovasilis commented Aug 28, 2025 •

edited

Loading

elasticmachine commented Sep 8, 2025

cmacknz left a comment

Uh oh!

Uh oh!

cmacknz commented Sep 10, 2025

elastic-sonarqube bot commented Sep 10, 2025

elasticmachine commented Sep 10, 2025

cmacknz left a comment

Uh oh!

github-actions bot commented Sep 10, 2025

mergify bot commented Sep 10, 2025 •

edited

Loading

Labels

3 participants

fix: send upgrade action to units before adding it to bkgActions #9634

fix: send upgrade action to units before adding it to bkgActions #9634

Uh oh!

Conversation

pkoutsovasilis commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

Disruptive User Impact

How to test this PR locally

Related issues

elasticmachine commented Sep 8, 2025

cmacknz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cmacknz commented Sep 10, 2025

elastic-sonarqube bot commented Sep 10, 2025

Quality Gate passed

elasticmachine commented Sep 10, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

History

cmacknz left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 10, 2025

mergify bot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Labels

3 participants

pkoutsovasilis commented Aug 28, 2025 •

edited

Loading

mergify bot commented Sep 10, 2025 •

edited

Loading