Skip to content

Pass --log-level to NodeAgent DaemonSet container args#2139

Open
kaovilai wants to merge 1 commit intoopenshift:oadp-devfrom
kaovilai:pass-log-level-to-nodeagent
Open

Pass --log-level to NodeAgent DaemonSet container args#2139
kaovilai wants to merge 1 commit intoopenshift:oadp-devfrom
kaovilai:pass-log-level-to-nodeagent

Conversation

@kaovilai
Copy link
Copy Markdown
Member

@kaovilai kaovilai commented Mar 27, 2026 •
edited
Loading

Summary

  • Pass --log-level from dpa.Spec.Configuration.Velero.LogLevel to NodeAgent DaemonSet container args
  • This allows the Velero exposer to inherit log level for data mover pods (via getInheritedPodInfo in pkg/exposer/image.go)
  • Previously, LogLevel was only passed to the Velero server deployment but not to NodeAgent

Request: https://redhat-internal.slack.com/archives/C0144ECKUJ0/p1772820634393599

Test plan

  • Unit test added for LogLevel=debug in NodeAgent DaemonSet build
  • go test ./internal/controller/ -run TestDPAReconciler_buildNodeAgentDaemonset passes
  • E2E: set logLevel: debug in DPA and verify node-agent pod has --log-level=debug in args
  • E2E: verify data mover pods spawned during backup/restore inherit the log level

🤖 Generated with Claude Code
via Happy

@kaovilai @claude @happy-otter
Pass --log-level to NodeAgent DaemonSet container args
61e70f4 
The DPA LogLevel field was only passed to the Velero server deployment
but not to the NodeAgent DaemonSet. This meant data mover pods spawned
by the exposer could not inherit the log level from the node-agent,
since the exposer reads --log-level from the node-agent container args.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Copilot AI review requested due to automatic review settings March 27, 2026 13:42
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2df97b97-c4c9-4272-999b-9d1e6b3e2f4f

📥 Commits

Reviewing files that changed from the base of the PR and between f2f8c5e and 61e70f4.

📒 Files selected for processing (2)
  • internal/controller/nodeagent.go
  • internal/controller/nodeagent_test.go

Walkthrough

Added conditional injection of --log-level argument into the NodeAgent container CLI arguments based on the dpa.Spec.Configuration.Velero.LogLevel configuration value, with corresponding test updates to validate the functionality.

Changes

Cohort / File(s) Summary
NodeAgent log-level configuration
internal/controller/nodeagent.go		 Added conditional logic to inject --log-level=<value> CLI argument into the NodeAgent container when LogLevel is configured, placed alongside existing --log-format argument handling.
NodeAgent test coverage
internal/controller/nodeagent_test.go		 Extended test infrastructure with optional logLevel field and added a new test case validating that Configuration.Velero.LogLevel set to "debug" results in the --log-level=debug argument in the built DaemonSet.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 27, 2026
@weshayutin
Copy link
Copy Markdown
Contributor

weshayutin commented Mar 27, 2026

@kaovilai isn't there a jira on this ???

Copilot AI reviewed Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 propagates spec.configuration.velero.logLevel from the DataProtectionApplication (DPA) CR into the NodeAgent DaemonSet container args, aligning node-agent logging behavior with the Velero server deployment.

Changes:

  • Append --log-level=<value> to the node-agent container args when dpa.Spec.Configuration.Velero.LogLevel is set.
  • Extend node-agent daemonset unit test scaffolding to support asserting log-level.
  • Add a unit test case validating LogLevel=debug results in --log-level=debug on the NodeAgent DaemonSet.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/controller/nodeagent.go Passes DPA Velero logLevel into the node-agent DaemonSet container args.
internal/controller/nodeagent_test.go Adds test builder support and a unit test case asserting node-agent receives --log-level.

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

internal/controller/nodeagent.go
Comment on lines +697 to +699
if dpa.Spec.Configuration.Velero.LogLevel != "" {
nodeAgentContainer.Args = append(nodeAgentContainer.Args, fmt.Sprintf("--log-level=%s", dpa.Spec.Configuration.Velero.LogLevel))
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

PR description references getInheritedPodInfo in pkg/exposer/image.go, but there is no pkg/exposer package (and no getInheritedPodInfo symbol) in this repository. Please update the PR description to point at the correct code path in this repo that consumes the node-agent --log-level arg, or clarify that it’s referring to an external component/repo.

Copilot uses AI. Check for mistakes.
@kaovilai
Copy link
Copy Markdown
Member Author

kaovilai commented Mar 28, 2026

no jira yet.. thread

@weshayutin
Copy link
Copy Markdown
Contributor

weshayutin commented Mar 31, 2026

/retest

mpryc
mpryc reviewed Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mpryc mpryc left a comment

Choose a reason for hiding this comment

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

@kaovilai is there any validation against improper log levels somewhere?

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Mar 31, 2026

@kaovilai: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

weshayutin
weshayutin approved these changes Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@weshayutin weshayutin left a comment

Choose a reason for hiding this comment

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

/LGTM

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2026
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Mar 31, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, sseago, weshayutin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kaovilai
Copy link
Copy Markdown
Member Author

kaovilai commented Mar 31, 2026

is there any validation against improper log levels somewhere?

nodeagent will error out if that happens. Our CRD also have these markers to block invalid values.

oadp-operator/api/v1alpha1/dataprotectionapplication_types.go

Lines 322 to 323 in 3cceb1e

// +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic
LogLevel string `json:"logLevel,omitempty"`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mpryc mpryc mpryc left review comments

Copilot code review Copilot Copilot left review comments

@weshayutin weshayutin weshayutin approved these changes

@sseago sseago sseago approved these changes

@shubham-pampattiwar shubham-pampattiwar Awaiting requested review from shubham-pampattiwar

Assignees

@weshayutin weshayutin

Labels

ai-gen-bugfix ai-generated-test approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kaovilai @weshayutin @sseago @mpryc