[yansun1996]

A GPU node selected by a custom DeviceConfig selector can lack both AMD NFD discovery labels. With driver management disabled, the operator currently schedules the node labeller and then deletes its valid GPU labels during cleanup.

Prevent this deletion in both single- and multi-DeviceConfig deployments:

• Run NFD-based stale-label cleanup only for DeviceConfigs that explicitly select on feature.node.kubernetes.io/amd-gpu or feature.node.kubernetes.io/amd-vgpu.
• Exclude nodes matching any other DeviceConfig's current selector across all namespaces. A default NFD config's broad cleanup query must not delete labels belonging to a custom-selected node managed by another config.
• Protect matching nodes even before the other config becomes Ready or enters nodeAssignments. Namespace and name identify a config; disabled/deleting configs retain responsibility for their own cleanup. If listing configs fails, preserve the candidate whose ownership could not be checked and return an error for retry.
• Recheck discovery, readiness, pool, and other-config selectors on the freshly fetched Node inside every cleanup attempt. For automatic cleanup, use a resource-version precondition so concurrent Node updates cause a retry and another ownership check. Explicit disable/delete cleanup uses ordinary merge patches to avoid conflicts on unrelated Node updates.
• Enqueue other enabled NFD configs on DeviceConfig creation, selector changes, and deletion. A custom config releasing its last node therefore triggers stale cleanup even when its own reconciliation exits early. Status-only changes do not fan out.
• Retain NFD/KMM readiness protections and explicit labeller-disable/DeviceConfig-delete cleanup. Finalization also cleans labels with kmm.watch=false; cleanup failures keep that finalizer for retry.
• Fix the e2e client to issue a named DELETE with DeleteOptions and reject an empty name, protecting unrelated DeviceConfigs during teardown.

Fixes #665. The labeller dependency upgrade in ROCm/k8s-device-plugin#255 is a separate fix.

Validation:

• 36 cleanup unit scenarios cover custom/nil/empty selectors, GPU/vGPU discovery, stale nodes, unrelated pools, KMM readiness, explicit disablement, two configs, namespace identity, unreconciled/disabled/deleting configs, and list failures. Added cases simulate discovery/ownership changes between reads, conflicting Node updates, refreshed config selectors on retry, and permanent patch errors.
• Four additional ordered two-node tests verify that skipping a protected node, an eligibility error, a patch error, or a conflict followed by protection does not stop cleanup of later nodes. Patch-body assertions verify optimistic locking is used only for automatic cleanup.
• Additional unit tests verify finalization with KMM watching disabled (success and cleanup failure), dependent-watch filtering/mapping, and the actual named HTTP DELETE request. The client regression runs in make unit-test.
• make unit-test passed on this ROCm branch, including go vet ./.... Full lint reports the existing cmd/prepareimage/main.go:61 unused useSourceImage field, which this PR does not modify. Lint restricted to changes from public/main passes.
• Five e2e cases cover custom, GPU, and vGPU selectors plus GPU/custom and vGPU/custom config pairs. Paired cases create same-named configs in different namespaces with disjoint node sets, verify labels survive both reconciliations, confirm unselected stale labels still get removed, and verify each config's explicit disablement cleanup. Both paired cases now change only the custom selector to release its node, wait for stale cleanup, restore the selector, and verify labels are preserved again.
• E2e compilation passed. The e2e Makefile automatically discovers all five cases in the node-labeller chunk (make -C tests/e2e chunk-node-labeller). The file is now 03_node_labeller_test.go, placing it after driver-install chunks and before slower feature suites. Build, unit-test, and other public checks passed at the previous head 0c296b8f. The matching internal simulation run failed before reaching the labeller chunk, so the selector-transition assertions still require a fresh CI run.

The e2e cases use synthetic unschedulable Node objects with the real deployed controller and API watches. They test operator cleanup, not GPU hardware discovery or the labeller binary.

NFD and selector behavior matrix

The deciding factor is whether the node actually has an AMD NFD discovery label, not who installed NFD.

Assumptions: the node labeller is enabled, driver.enable=false (as reported in #665), and the node matches any custom selector constraints. An installed, working AMD NFD rule labels the GPU node; without the rule, the node has neither feature.node.kubernetes.io/amd-gpu nor feature.node.kubernetes.io/amd-vgpu.

NFD installed by	AMD NFD rule	DeviceConfig selector	Labeller scheduled?	Current operator deletes labeller labels?	With this PR
Operator	Installed	AMD NFD label	Yes	No	No
Operator	Installed	Custom user label	Yes	No	No
Operator	Not installed	AMD NFD label	No¹	Stops before labeller handling¹	Same
Operator	Not installed	Custom user label	Yes	Yes — bug	No — fixed
Separately / external	Installed	AMD NFD label	Yes	No	No
Separately / external	Installed	Custom user label	Yes	No	No
Separately / external	Not installed	AMD NFD label	No¹	Stops before labeller handling¹	Same
Separately / external	Not installed	Custom user label	Yes	Yes — bug	No — fixed
Not deployed	Not installed	Custom user label	Yes	Yes — bug	No — fixed

¹ If no nodes match the DeviceConfig's NFD selector, reconciliation stops before labeller handling. If some nodes still match, reconciliation proceeds and can clean stale labels from other nodes missing both AMD discovery labels, subject to the remaining selector constraints. With this PR, nodes matching another DeviceConfig’s selector are excluded from that cleanup.

Additional distinctions:

• Installing the rule does not guarantee a label. If discovery fails or the GPU is not recognized, the node behaves like the “rule not installed” rows.
• Disabling bundled NFD does not automatically disable the AMD rule; installdefaultNFDRule controls that separately.
• Explicitly disabling the node labeller removes its labels from selected nodes, both before and after this change.
• With driver.enable=true, existing KMM readiness labels provide additional protection against cleanup.

The failing combination is a custom selector plus missing AMD NFD labels. Both the custom config itself and any coexisting NFD-based config must preserve that node's labels.

Coexisting DeviceConfigs

Cleanup context	Current operator	With this PR
A uses an AMD NFD selector; B selects a separate node with custom labels and no AMD NFD labels	A can delete B's labeller labels	B's matching node is excluded from A's cleanup
B has not reconciled yet, or has the same name as A in another namespace	No selector-based protection	B's current selector still protects its node
Candidate matches no other DeviceConfig and meets the stale-node checks	Cleaned	Cleaned
Listing other DeviceConfigs fails	No cross-config check	Preserves the candidate that could not be checked and returns an error
B explicitly disables its labeller	B cleans its selected nodes	B cleans its selected nodes
B changes its selector away from a node	A is not necessarily requeued	Enabled NFD configs are requeued; unowned stale nodes are reconsidered
Node gains discovery or ownership labels during cleanup	Snapshot check can become stale	Recheck the fetched Node; concurrent Node updates conflict and retry
B is deleted with KMM watches disabled	Labels can survive deletion	Clean selected nodes before removing the finalizer

Why retain automatic cleanup?

Removing the NFD-based cleanup block while keeping explicit disable/delete cleanup is a possible alternative, but changes stale-label behavior when a node stops being selected. The current labeller removes old keys before writing startup-generated labels; it ignores node Update/Delete events and has no shutdown cleanup. Its refresh logic does not replace lifecycle cleanup. This PR retains that behavior while preventing deletion of custom-selected or other-config-selected nodes. Ownership reads use the controller cache; the Node resource-version precondition protects concurrent Node changes, but does not make Node and DeviceConfig changes a single atomic transaction.

Review clarifications

• Returning nil for an ineligible node exits only the RetryOnConflict callback; the outer node loop continues. The new ordered two-node regressions verify this directly, including failure and conflict cases.
• Optimistic locking is limited to automatic stale-label cleanup, whose eligibility decision depends on the fetched Node. Explicit disablement and finalization retain ordinary merge patches.
• Cross-namespace requeueing is intentional: Nodes are cluster-scoped, and an NFD config's stale-node query can reach nodes selected by a custom config in another namespace. Restricting the watch to the changed config's namespace would miss released nodes. Requeueing does not assign ownership or bypass the selector checks. Unit tests assert namespace-unrestricted listing, and paired e2e tests change selectors across namespaces.
• All five earlier Copilot findings remain covered: cross-config protection, named DELETE, cleanup without KMM watches, dependent requeueing, and eligibility rechecks with optimistic locking for automatic cleanup.

Cross-validation: six deliberately broken variants were rejected by regression tests (whole-loop early exit, namespace-filtered watch, locking explicit cleanup, unlocking automatic cleanup, ignoring other-config ownership, and collection DELETE). These checks used temporary Go overlays; no broken variant was committed.