Add operator set-log-level CLI command#2008
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesSet Log Level CLI Command
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5b8f686 to
2258bd0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/operator-log-level.md`:
- Around line 75-85: The example output in the operator log level doc uses an
unlabeled fenced block, which will trip markdownlint MD040. Update the second
fenced example under the set-log-level output to use a Bash info tag in the
fence, or remove the fence entirely, so the example remains valid and consistent
with the existing `nb operator set-log-level` snippet.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d73f908e-861f-49f0-a2ae-d233f6724251
📒 Files selected for processing (2)
doc/operator-log-level.mdpkg/operator/operator.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/operator/operator.go
2258bd0 to
98fc0d0
Compare
Expose a runtime CLI for changing OPERATOR_LOG_LEVEL in noobaa-config, matching the existing nb system set-debug-level flow for core/endpoints. Signed-off-by: Kajal Pareek <pareekkajal97@gmail.com>
fd5146a to
7b040ca
Compare
| -p '{"data":{"OPERATOR_LOG_LEVEL":"info"}}' | ||
| ``` | ||
|
|
||
| The operator picks up the change on the next reconcile cycle (no restart needed). |
There was a problem hiding this comment.
I don't think you need to remove the examples; they were fine.
|
|
||
| ```bash | ||
| noobaa-operator operator --operator-log-level=warn | ||
| ``` |
There was a problem hiding this comment.
This part is where you need to update.
The CLI in this doc doesn't work noobaa-operator operator --operator-log-level=warn.
| time="2025-10-10T13:20:54Z" level=info msg="Update event detected for noobaa (openshift-storage), queuing Reconcile" | ||
| time="2025-10-10T13:20:54Z" level=info msg="Update event detected for noobaa-default-backing-store (openshift-storage), queuing Reconcile" | ||
| ```bash | ||
| nb operator set-log-level debug -n <namespace> |
There was a problem hiding this comment.
I think that in docs they prefer it to be noobaa and not the alias of the binary (if you want, you can add a comment about nb as an alias.
| time="2025-10-10T13:20:57Z" level=info msg="Delete event detected for noobaa-default-backing-store (openshift-storage), queuing Reconcile" | ||
| time="2025-10-10T13:20:57Z" level=debug msg="Generic event detected for noobaa (openshift-storage), queuing Reconcile" | ||
| Operator log level was set to "debug" successfully | ||
| The operator picks up the change on the next reconcile. Core and endpoint pods are not restarted. |
There was a problem hiding this comment.
They are not affected, writing that they are not restarted is confusing.
There was a problem hiding this comment.
Do you need to delete the global --operator-log-level that was added?
| func CmdSetLogLevel() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "set-log-level <level>", | ||
| Short: "Sets the operator log level in noobaa-config. level can be 'warn', 'info', or 'debug'", |
There was a problem hiding this comment.
I think you can add 'Long` and add additional details.
| cm.Namespace = options.Namespace | ||
| cm.Name = "noobaa-config" | ||
|
|
||
| _, _, err := util.KubeGet(cm) |
There was a problem hiding this comment.
I think that usually we use KubeCheck and then you don't need to use blank identifier (_).
|
|
||
| _, _, err := util.KubeGet(cm) | ||
| if err != nil { | ||
| log.Fatalf(`❌ ConfigMap "noobaa-config" not found in namespace %q. Create a NooBaa system first.`, options.Namespace) |
There was a problem hiding this comment.
You have the name "noobaa-config" in variable cm.Name; you can reuse it.
| cm.Data["OPERATOR_LOG_LEVEL"] = level | ||
|
|
||
| if !util.KubeUpdate(cm) { | ||
| log.Fatal(`❌ Failed to update ConfigMap "noobaa-config"`) |
There was a problem hiding this comment.
You have the name "noobaa-config" in variable cm.Name; you can reuse it.
|
|
||
| fmt.Println("") | ||
| fmt.Printf("Operator log level was set to %q successfully\n", level) | ||
| fmt.Println("The operator picks up the change on the next reconcile. Core and endpoint pods are not restarted.") |
There was a problem hiding this comment.
Again, core and endpoint are not affected, so it is clear that they should not be restarted.
Summary
nb operator set-log-level <warn|info|debug>to updateOPERATOR_LOG_LEVELinnoobaa-configat runtimenb system set-debug-levelfor operator logging after installdoc/operator-log-level.mdTest plan
nb operator set-log-level debug -n <namespace>updatesOPERATOR_LOG_LEVELinnoobaa-confignb operator set-log-level warn -n <namespace>reduces operator log noiseoc logs -n <namespace> -l noobaa-operator=deployment -fshows expected verbositySummary by CodeRabbit
New Features
operator set-log-level <level>CLI subcommand supportingwarn,info, anddebug.Documentation
nb operator set-log-level debug -n <namespace>command).