OADP-7665: Pass --log-level to NodeAgent DaemonSet container args by kaovilai · Pull Request #2139 · openshift/oadp-operator

kaovilai · 2026-03-27T13:42:15Z

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

Summary by CodeRabbit

New Features
- NodeAgent resource labels and annotations are now applied consistently to ConfigMaps, DaemonSets, and pod templates.
- Additional NodeAgent arguments and Velero log-level settings are supported with predictable precedence.
- Load affinity now supports storage-class constraints.
Bug Fixes
- Unbounded pod resource values are normalized before configuration serialization.
- DaemonSet cleanup succeeds when the resource is already absent.
- Pod labels and annotations now require the appropriate NodeAgent ConfigMap configuration.

coderabbitai · 2026-03-27T13:42:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c0c4d35e-a9ec-4ca6-b699-9cf7b666890d

📥 Commits

Reviewing files that changed from the base of the PR and between 6449f95 and ec10bbf.

📒 Files selected for processing (2)

internal/controller/nodeagent.go
internal/controller/nodeagent_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

NodeAgent reconciliation now handles Pod metadata, normalized resource values, storage-class affinity, DaemonSet metadata, missing DaemonSets, and argument precedence. Tests cover these behaviors and verify that ConfigMap generation does not mutate the DPA specification.

Changes

NodeAgent configuration and reconciliation

Layer / File(s)	Summary
ConfigMap resources and affinity `internal/controller/nodeagent.go`, `internal/controller/nodeagent_test.go`	ConfigMap generation requires Pod labels or annotations, normalizes unbounded resources, preserves annotations, and includes storage classes in load affinity. Tests cover resource defaults and DPA specification immutability.
DaemonSet metadata and deletion `internal/controller/nodeagent.go`, `internal/controller/nodeagent_test.go`	Resource labels and annotations apply to the DaemonSet and pod template. Missing DaemonSets are treated as successfully deleted. Test DaemonSet constraints now use node affinity that excludes Windows nodes.
NodeAgent argument precedence `internal/controller/nodeagent.go`, `internal/controller/nodeagent_test.go`	Configured `Velero.LogLevel` adds a log-level argument. `ExtraArgs` merge after generated log settings and before unsupported server-argument overrides. Tests verify argument precedence and log-level output.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to ec10b

This localized change passes the stated unit test and adds the configured log level to NodeAgent arguments; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: msfrucht

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: passing --log-level to the NodeAgent DaemonSet container arguments.
Description check	✅ Passed	The description explains why the change was made and provides a test plan with a targeted unit test and command. It also identifies the pending E2E validations. The headings differ from the repository…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PASS. The pull request adds one table-test name: "valid DPA CR with LogLevel set to debug, NodeAgent DaemonSet is built with LogLevel set to debug". This is a fixed descriptive string. It contains no …
Test Structure And Quality	✅ Passed	PASS: The pull request adds a case to an existing Go table test, not a new Ginkgo `It` block. The case checks only `LogLevel` argument propagation and uses the existing fake-client pattern. It creates…
Microshift Test Compatibility	✅ Passed	PASS: The pull request adds a standard Go table-test case to `TestDPAReconciler_buildNodeAgentDaemonset` and extends its test helper. It does not add a Ginkgo `It`, `Describe`, `Context`, or `When` te…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request changes only `internal/controller/nodeagent.go` and `internal/controller/nodeagent_test.go`. It adds no test under `tests/e2e` and adds no new `It`, `Describe`, `Context`, or `W…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The pull request changes only NodeAgent container arguments and adds a matching unit-test case. The controller diff adds `--log-level=<value>`; it does not add or change `nodeSelector`, affinity…
Ote Binary Stdout Contract	✅ Passed	PASS: The pull request changes only `internal/controller/nodeagent.go` and `internal/controller/nodeagent_test.go`. The added production code appends a `--log-level` argument to a DaemonSet container.…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS. The pull request changes only `internal/controller/nodeagent.go` and the table-driven unit test `TestDPAReconciler_buildNodeAgentDaemonset`. The added test case checks `LogLevel: "debug"` and do…
No-Weak-Crypto	✅ Passed	PASS: The pull request adds only `--log-level` argument handling and its test support. The exact added lines contain no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token c…
Container-Privileges	✅ Passed	PASS: The PR changes only NodeAgent argument handling and test setup for `--log-level`. The exact parent-to-PR diff adds no `privileged`, `hostPID`, `hostNetwork`, `hostIPC`, `SYS_ADMIN`, `allowPrivil…
No-Sensitive-Data-In-Logs	✅ Passed	The pull-request diff adds only `--log-level=<validated DPA value>` to the NodeAgent container arguments and matching test data. It adds no logging call and does not include passwords, tokens, API key…

Full details: Description check

Explanation

The description explains why the change was made and provides a test plan with a targeted unit test and command. It also identifies the pending E2E validations. The headings differ from the repository template, and no pictures are included, but the required information is mostly present.

Full details: Stable And Deterministic Test Names

Explanation

PASS. The pull request adds one table-test name: "valid DPA CR with LogLevel set to debug, NodeAgent DaemonSet is built with LogLevel set to debug". This is a fixed descriptive string. It contains no generated value, timestamp, node name, namespace, IP address, or runtime interpolation. The Ginkgo Describe and Entry titles were not changed.

Full details: Test Structure And Quality

Explanation

PASS: The pull request adds a case to an existing Go table test, not a new Ginkgo It block. The case checks only LogLevel argument propagation and uses the existing fake-client pattern. It creates no cluster resources, adds no Eventually or Consistently calls, and adds no cleanup or timeout requirement. The existing Ginkgo setup and assertions were not changed by this pull request.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request adds a standard Go table-test case to TestDPAReconciler_buildNodeAgentDaemonset and extends its test helper. It does not add a Ginkgo It, Describe, Context, or When test, and it does not add an e2e test. The existing Ginkgo suite and existing config.openshift.io Infrastructure fixture are unchanged. Therefore, the MicroShift compatibility failure condition does not apply.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request changes only internal/controller/nodeagent.go and internal/controller/nodeagent_test.go. It adds no test under tests/e2e and adds no new It, Describe, Context, or When declaration. The new LogLevel coverage is a case in the existing TestDPAReconciler_buildNodeAgentDaemonset(t *testing.T) table and only compares constructed DaemonSet arguments. It does not schedule pods, inspect nodes, or assume cluster topology. SNO compatibility is therefore not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only NodeAgent container arguments and adds a matching unit-test case. The controller diff adds --log-level=<value>; it does not add or change nodeSelector, affinity, topology spread constraints, tolerations, replica logic, or a PDB. Existing DaemonSet settings retain MaxUnavailable: 1 and MaxSurge: 0. Therefore, no topology constraint from the custom check is introduced.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only internal/controller/nodeagent.go and internal/controller/nodeagent_test.go. The added production code appends a --log-level argument to a DaemonSet container. The added test code constructs expected arguments and adds a test case. No added line contains fmt.Print*, log.Print*, klog, os.Stdout, suite setup, TestMain, main, or init output. The existing fmt.Printf in the test file is unchanged and is not process-level code.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS. The pull request changes only internal/controller/nodeagent.go and the table-driven unit test TestDPAReconciler_buildNodeAgentDaemonset. The added test case checks LogLevel: "debug" and does not create a Ginkgo e2e test, use IP addresses, or connect to external services. The existing Ginkgo declarations and the existing 1.1.1.1 test value are unchanged.

Full details: No-Weak-Crypto

Explanation

PASS: The pull request adds only --log-level argument handling and its test support. The exact added lines contain no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom cryptography, or secret/token comparisons. The changed code only appends a configured log-level string to NodeAgent arguments.

Full details: Container-Privileges

Explanation

PASS: The PR changes only NodeAgent argument handling and test setup for --log-level. The exact parent-to-PR diff adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or security-context fields. Existing NodeAgent privileged and root behavior is unchanged from the parent revision.

Full details: No-Sensitive-Data-In-Logs

Explanation

The pull-request diff adds only --log-level=<validated DPA value> to the NodeAgent container arguments and matching test data. It adds no logging call and does not include passwords, tokens, API keys, PII, hostnames, or customer data in log output. Existing NodeAgent logging is unchanged.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

weshayutin · 2026-03-27T13:44:20Z

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

Copilot

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.

Copilot · 2026-03-27T13:45:17Z

+			if dpa.Spec.Configuration.Velero.LogLevel != "" {
+				nodeAgentContainer.Args = append(nodeAgentContainer.Args, fmt.Sprintf("--log-level=%s", dpa.Spec.Configuration.Velero.LogLevel))
+			}


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.

kaovilai · 2026-03-28T20:16:41Z

no jira yet.. thread

weshayutin · 2026-03-31T16:00:45Z

/retest

mpryc

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

weshayutin

/LGTM

kaovilai · 2026-03-31T20:08:50Z

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"`

mpryc · 2026-04-01T06:31:01Z

Merge conflict in internal/controller/nodeagent_test.go
Automatic merge failed; fix conflicts and then commit the result.

openshift-ci-robot · 2026-04-01T07:22:12Z

/retest-required

Remaining retests: 0 against base HEAD 3cceb1e and 2 for PR HEAD 61e70f4 in total

weshayutin · 2026-04-07T20:14:22Z

/retest

openshift-ci-robot · 2026-04-07T20:16:55Z

@kaovilai: This pull request references OADP-7665 which is a valid jira issue.

Details

In response to this:

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

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 openshift-eng/jira-lifecycle-plugin repository.

weshayutin · 2026-04-07T20:16:58Z

/cherry-pick oadp-1.6

openshift-cherrypick-robot · 2026-04-07T20:17:02Z

@weshayutin: once the present PR merges, I will cherry-pick it on top of oadp-1.6 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick oadp-1.6

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.

kaovilai · 2026-04-07T21:07:06Z

/retest

kaovilai · 2026-04-07T21:23:08Z

sounds like rebase needed..

weshayutin · 2026-04-08T15:16:29Z

/retest

kaovilai · 2026-04-30T18:54:46Z

/override ci/prow/4.23-e2e-test-aws

openshift-ci · 2026-04-30T18:55:37Z

@kaovilai: Overrode contexts on behalf of kaovilai: ci/prow/4.23-e2e-test-aws

Details

In response to this:

/override ci/prow/4.23-e2e-test-aws

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.

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>

sseago · 2026-08-26T17:06:27Z

/lgtm

openshift-ci · 2026-08-26T17:06:36Z

[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:

~~OWNERS~~ [kaovilai,sseago]

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

openshift-merge-bot · 2026-08-26T17:12:28Z

/retest-required

Remaining retests: 0 against base HEAD 2409f61 and 2 for PR HEAD ec10bbf in total

openshift-ci · 2026-08-26T17:40:12Z

@kaovilai: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/5.0-e2e-test-cli-aws	`ec10bbf`	link	true	`/test 5.0-e2e-test-cli-aws`

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.

openshift-merge-bot · 2026-08-26T20:14:32Z

/retest-required

Remaining retests: 0 against base HEAD d69edd0 and 1 for PR HEAD ec10bbf in total

Copilot AI review requested due to automatic review settings March 27, 2026 13:42

kaovilai added ai-generated-test ai-gen-bugfix labels Mar 27, 2026

openshift-ci Bot requested review from shubham-pampattiwar and sseago March 27, 2026 13:42

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 27, 2026

Copilot started reviewing on behalf of kaovilai March 27, 2026 13:42 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

mpryc reviewed Mar 31, 2026

View reviewed changes

weshayutin previously approved these changes Mar 31, 2026

View reviewed changes

openshift-ci Bot assigned weshayutin Mar 31, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2026

sseago previously approved these changes Mar 31, 2026

View reviewed changes

weshayutin changed the title ~~Pass --log-level to NodeAgent DaemonSet container args~~ OADP-7665 Pass --log-level to NodeAgent DaemonSet container args Apr 7, 2026

kaovilai changed the title ~~OADP-7665 Pass --log-level to NodeAgent DaemonSet container args~~ OADP-7665: Pass --log-level to NodeAgent DaemonSet container args Apr 7, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 7, 2026

kaovilai dismissed sseago’s stale review via 6449f95 April 7, 2026 21:27

kaovilai dismissed weshayutin’s stale review via 6449f95 April 7, 2026 21:27

kaovilai force-pushed the pass-log-level-to-nodeagent branch from 61e70f4 to 6449f95 Compare April 7, 2026 21:27

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 7, 2026

kaovilai requested review from mpryc, sseago and weshayutin April 7, 2026 21:27

Joeavaikath previously approved these changes Apr 30, 2026

View reviewed changes

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2026

kaovilai dismissed Joeavaikath’s stale review via ec10bbf August 26, 2026 14:56

kaovilai force-pushed the pass-log-level-to-nodeagent branch from 6449f95 to ec10bbf Compare August 26, 2026 14:56

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 26, 2026

sseago approved these changes Aug 26, 2026

View reviewed changes

openshift-ci Bot assigned sseago Aug 26, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2026

Uh oh!

Conversation

kaovilai commented Mar 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

weshayutin commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

kaovilai commented Mar 28, 2026

Uh oh!

weshayutin commented Mar 31, 2026

Uh oh!

mpryc left a comment

Choose a reason for hiding this comment

Uh oh!

weshayutin left a comment

Choose a reason for hiding this comment

Uh oh!

kaovilai commented Mar 31, 2026

Uh oh!

mpryc commented Apr 1, 2026

Uh oh!

openshift-ci-robot commented Apr 1, 2026

Uh oh!

weshayutin commented Apr 7, 2026

Uh oh!

openshift-ci-robot commented Apr 7, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

weshayutin commented Apr 7, 2026

Uh oh!

openshift-cherrypick-robot commented Apr 7, 2026

Uh oh!

kaovilai commented Apr 7, 2026

Uh oh!

kaovilai commented Apr 7, 2026

Uh oh!

weshayutin commented Apr 8, 2026

Uh oh!

kaovilai commented Apr 30, 2026

Uh oh!

openshift-ci Bot commented Apr 30, 2026

Uh oh!

sseago commented Aug 26, 2026

Uh oh!

openshift-ci Bot commented Aug 26, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 26, 2026

Uh oh!

openshift-ci Bot commented Aug 26, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

kaovilai commented Mar 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 27, 2026 •

edited

Loading

openshift-ci-robot commented Apr 7, 2026 •

edited by openshift-ci Bot

Loading