fix: check errors.IsNotFound before failing DaemonSet delete in NodeAgent reconcile

[kaovilai]

Summary

The DaemonSet deletion path in ReconcileNodeAgentDaemonset didn't check errors.IsNotFound(err) before treating a delete failure as a real error — unlike the adjacent Get() call and the ConfigMap deletion path in the same file, which both handle this correctly.

if err := r.Delete(deleteContext, ds, &client.DeleteOptions{...}); err != nil {
    if errors.IsNotFound(err) {
        return true, nil
    }
    r.EventRecorder.Event(ds, corev1.EventTypeNormal, "DeleteDaemonSetFailed", "...")
    return false, err
}

How this was found

Surfaced by this repo's own e2e log-analysis tooling on ci/prow/4.23-e2e-test-aws for PR #2206 (oadp-1.6): the AfterEach cleanup of "Configuration testing for DPA Custom Resource" hit a DeleteDaemonSetFailed event while the velero pod was stuck PodInitializing. Confirmed to be a pre-existing bug present across oadp-dev/oadp-1.6/oadp-1.5/oadp-1.4 (this repo tracks it via #2372, opened for all four branches; this PR addresses oadp-dev).

Test plan

• gofmt -l clean
• go build ./internal/controller/...
• go vet ./internal/controller/...

Part of #2372

Note

Responses generated with Claude

Summary by CodeRabbit

• Bug Fixes
  • Improved Node Agent reconciliation when the DaemonSet has already been deleted.
  • Prevents unnecessary reconciliation failures for resources that are no longer found.

[coderabbitai]

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: 595ba5b9-0011-4efb-8636-07efa7d163a8

📥 Commits

Reviewing files that changed from the base of the PR and between 110aad0 and fa752e4.

📒 Files selected for processing (1)

• internal/controller/nodeagent.go

---

Walkthrough

The node agent reconciler now treats a NotFound error during DaemonSet deletion as successful reconciliation.

Changes

Node agent deletion

Layer / File(s)	Summary
Handle missing DaemonSets as success internal/controller/nodeagent.go	ReconcileNodeAgentDaemonset returns (true, nil) when DaemonSet deletion reports NotFound. It no longer records a reconciliation failure for an already-absent DaemonSet.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

• openshift/oadp-operator#2372 — Describes the same DaemonSet deletion NotFound handling addressed by this change.

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the DaemonSet deletion error-handling fix.
Description check	✅ Passed	The description explains the bug, rationale, discovery context, fix, and validation steps, although it uses different headings from the template.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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	The PR diff changes only nodeagent.go and adds deletion error handling; it introduces no Ginkgo test titles or dynamic test-name values.
Test Structure And Quality	✅ Passed	The PR changes only internal/controller/nodeagent.go (+3 lines) and changes no Ginkgo test files; the custom test-structure check is therefore inapplicable.
Microshift Test Compatibility	✅ Passed	The PR changes only internal/controller/nodeagent.go; it adds no new Ginkgo e2e tests, so MicroShift API compatibility checks do not apply.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The diff changes only internal/controller/nodeagent.go and adds no Ginkgo e2e tests, so no SNO multi-node assumption is introduced.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff only adds NotFound handling to DaemonSet deletion; it introduces no scheduling constraints, topology selectors, affinity, replicas, spreads, or PDB changes.
Ote Binary Stdout Contract	✅ Passed	The PR only adds an errors.IsNotFound return branch in nodeagent.go; process entrypoints are unchanged, and the changed file has no stdout-writing APIs.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The diff changes only controller reconciliation logic and adds no Ginkgo e2e tests, IPv4 assumptions, or external connectivity requirements.
No-Weak-Crypto	✅ Passed	The PR adds only errors.IsNotFound handling for DaemonSet deletion; the diff introduces no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparison.
Container-Privileges	✅ Passed	The PR changes only DaemonSet deletion error handling. Added lines contain no privilege settings, and existing privileged/runAsUser/allowPrivilegeEscalation code is unchanged from the parent.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds only an IsNotFound branch returning true, nil. It adds no logging or sensitive data; the existing error event remains unchanged.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai

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]

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

[openshift-ci]

@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/4.23-e2e-test-aws	fa752e4	link	false	/test 4.23-e2e-test-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.