Skip to content

[Bug]: Node labeling stops at the first node patch failure, skipping remaining nodes #3229

Open

Description

@SelvanayakiGurusamy

Motivation
While reviewing the node labeling controller for behavior at large cluster sizes (5,000+ nodes), I found that a single failing node patch stops the whole labeling pass. I found this by reading the code and confirmed it with a unit test, not from a production incident.

  • I would like to work on this issue.

Describe the bug
labelGPUNodes in controllers/nodelabeling_controller.go patches nodes in one loop and returns on the first patch error:

if err := nlc.client.Patch(ctx, &node, client.MergeFrom(original)); err != nil {
    return result, fmt.Errorf("unable to label node %s: %w", node.Name, err)
}

When one node's patch fails:

  • the remaining nodes in that pass are not labeled;
  • Reconcile returns early, so AssignOwners, labelNodesWithOrphanedDriverPods and applyDriverAutoUpgradeAnnotation are skipped for all nodes;
  • if the failure persists (for example, an admission webhook rejecting one node), those steps keep being skipped on every retry.

A node deleted between the List and the Patch returns NotFound and aborts the pass the same way.

At larger cluster sizes, the chance that at least one node is failing at any moment goes up, so one node can hold back labeling for the whole cluster.

To Reproduce
Unit test with the fake client: three GPU nodes (feature.node.kubernetes.io/pci-10de.present=true), with an interceptor that makes the Patch fail for the first node. After labelGPUNodes runs, the other two nodes do not have nvidia.com/gpu.present=true (0 of 2 nodes patched).

Expected behavior
A patch failure on one node should not prevent the other nodes from being labeled. Errors should be collected and returned after all nodes are processed, and nodes deleted after the List (NotFound) should be skipped.

Proposed fix: continue the loop on a patch error, return the collected errors with errors.Join, and ignore NotFound. I have a small fix with a regression test ready and can open a PR if this approach works for you.

Environment (please provide the following information):

  • GPU Operator Version: main at commit 6094104
  • OS / Kernel / Container Runtime: N/A (reproduced with a unit test)
  • Kubernetes Distro and Version: N/A (controller-runtime fake client)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageissue or PR has not been assigned a priority-px label

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions