Skip to content

feat(fleet-controller): skip config re-apply and callbacks for unmodified reloads - #5452

Open
d3flex wants to merge 1 commit into
rancher:mainfrom
d3flex:feat/config_flag
Open

feat(fleet-controller): skip config re-apply and callbacks for unmodified reloads#5452
d3flex wants to merge 1 commit into
rancher:mainfrom
d3flex:feat/config_flag

Conversation

@d3flex

@d3flex d3flex commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

The version guard added in the previous commit kept a running controller's
config in memory when a mismatched-version ConfigMap was detected, but it
returned no signal to the caller. Without that signal the live reload paths
kept doing their work even for a rejected ConfigMap: reloadConfig re-fired
every OnChange callback via SetAndTrigger, and Reconcile re-applied the
retained config, when nothing was actually adopted.

ReadConfig and Lookup now report whether the config was adopted, and both
live reload paths skip re-applying and re-triggering callbacks when it was
not.

Depends-on: #5397
Refers to #3547

@d3flex
d3flex force-pushed the feat/config_flag branch from 9ec8f2a to 9d1df21 Compare July 28, 2026 07:40
@d3flex
d3flex marked this pull request as ready for review July 29, 2026 16:07
@d3flex
d3flex requested a review from a team as a code owner July 29, 2026 16:07
Copilot AI review requested due to automatic review settings July 29, 2026 16:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refines Fleet controller hot-reload behavior so that when a ConfigMap reload is rejected by the version guard, downstream reload paths do not re-apply the existing in-memory config or re-trigger OnChange callbacks.

Changes:

  • Extend ReadConfig / Lookup to return an “adopted” boolean alongside the config and error.
  • Skip SetAndTrigger (wrangler OnChange path) and config.Set (controller-runtime reconciler path) when the new ConfigMap was not adopted.
  • Update unit tests to validate the new boolean return value for accepted vs rejected reloads.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/config/config.go Adds an adoption boolean to ReadConfig/Lookup so callers can detect rejected reloads.
internal/config/config_test.go Updates tests to assert adoption behavior (accepted vs rejected).
internal/cmd/controller/reconciler/config_controller.go Skips re-applying config in reconciler when reload is rejected by version guard.
internal/cmd/controller/agentmanagement/controllers/config/controller.go Skips re-triggering callbacks on rejected reloads in wrangler OnChange path.
internal/cmd/controller/agentmanagement/agent/config.go Updates Lookup call sites for new return signature.
internal/cmd/controller/agentmanagement/agent/agent.go Updates Lookup call sites for new return signature.
internal/cmd/agent/register/register.go Updates Lookup call sites for new return signature.
internal/cmd/agent/operator.go Updates ReadConfig call site for new return signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/cmd/controller/reconciler/config_controller.go Outdated
Comment thread internal/cmd/controller/agentmanagement/controllers/config/controller.go Outdated

@0xavi0 0xavi0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the Copilot comments are valid suggestions.

Aside from that.. looks good to me

…ed reloads

The version guard in the previous commit kept a running controller's config in
memory when it saw a mismatched-version ConfigMap, but it gave the caller no
signal back. So the live reload paths kept working on a rejected ConfigMap
anyway: reloadConfig re-fired every OnChange callback through SetAndTrigger, and
Reconcile re-applied the retained config, even though nothing was adopted.

ReadConfig and Lookup now return whether the config was adopted. Both live
reload paths skip the re-apply and the callbacks when it was not, which slightly
improves performance and avoids unnecessary overload.

Refers to rancher#3547

Signed-off-by: Ioannis Bonatakis <ybonatakis@suse.com>
@d3flex

d3flex commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

I think the Copilot comments are valid suggestions.

Aside from that.. looks good to me

both accepted and done

@d3flex
d3flex requested a review from 0xavi0 August 13, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants