Conversation
fleetcli apply, and the agentmanagement and cleanup fleet-controller subcommands, never called ctrl.SetLogger. Since logrus was replaced with the controller-runtime log.Log delegating logger, calls to it in these entrypoints are silently dropped after a 30s timeout, hiding messages such as the helmRepoURLRegex credential-stripping warning and any logging from the agentmanagement and cleanup controllers. Wire up zap.Options and ctrl.SetLogger in these entrypoints the same way gitops and helmops already do, and drop the now-unused registerKubeconfigFlags helper.
Add an AST-based test per fixed entrypoint (fleetcli apply, the agentmanagement and cleanup fleet-controller subcommands) asserting their Run method calls SetLogger. Without it, log.Log output is silently dropped rather than printed.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Configures controller-runtime logging for apply, cleanup, and agentmanagement commands so controller logs are emitted correctly.
Changes:
- Initializes zap logging in affected entrypoints.
- Passes shared zap options to controller subcommands.
- Removes the obsolete kubeconfig helper and adds regression tests.
| File | Description |
|---|---|
internal/cmd/controller/root.go |
Passes zap options to subcommands |
internal/cmd/controller/cleanup/root.go |
Configures cleanup logging |
internal/cmd/controller/cleanup/root_test.go |
Tests cleanup logger setup |
internal/cmd/controller/agentmanagement/root.go |
Configures agentmanagement logging |
internal/cmd/controller/agentmanagement/root_test.go |
Tests agentmanagement logger setup |
internal/cmd/cli/logging_setup_test.go |
Tests apply logger setup |
internal/cmd/cli/kubeconfig.go |
Removes the obsolete helper and updates documentation |
internal/cmd/cli/apply.go |
Registers zap flags and configures logging |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
weyfonk
approved these changes
Sep 22, 2026
| @@ -0,0 +1,50 @@ | |||
| package agentmanagement | |||
Contributor
There was a problem hiding this comment.
nit: this regression is now in place for a few locations, but if say, the bundlediff CLI were modified not to call ctrl.SetLogger anymore, it would not be covered.
d3flex
approved these changes
Sep 22, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fleetcli
apply, and the agentmanagement and cleanup fleet-controller subcommands, never calledctrl.SetLogger. Sincelogruswas replaced with the controller-runtimelog.Logdelegating logger, calls to it in these entrypoints are silently dropped after a 30s timeout, hiding messages such as thehelmRepoURLRegexcredential-stripping warning and any logging from the agentmanagement and cleanup controllers.Wire up
zap.Optionsandctrl.SetLoggerin these entrypoints the same way gitops and helmops already do, and drop the now-unusedregisterKubeconfigFlagshelper.Refers #3554