Skip to content

fix: prevent panic in device manager when allocating devices in parallel #136022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ERJavier wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: prevent panic in device manager when allocating devices in parallel #136022

ERJavier wants to merge 1 commit into from
+87 −10

Conversation

@ERJavier
Copy link

@ERJavier ERJavier commented Jan 5, 2026 •
edited
Loading

What this PR does

Fixes race condition in device manager's Allocate() function that causes a panic when multiple pods allocate devices in parallel.

Problem

When allocating devices in parallel:

  • Pod1 creates m.devicesToReuse[pod1.uid] and makes an RPC call
  • Pod2 deletes m.devicesToReuse[pod1.uid] while Pod1's RPC is still running
  • Pod1 tries to access m.devicesToReuse[pod1.uid] after RPC returns, causing a nil pointer panic

Solution

Save a local reference to devicesToReuse for the current pod before the RPC call and use it throughout the function, preventing the race condition.

Testing

  • Added TestDeviceAllocateParallel test case that reproduces the issue
  • Test verifies no panic occurs when allocating devices concurrently
    -note
    Fix race condition in device manager that caused panic when multiple pods allocate devices in parallel
    Fixes kubelet panic when allcoate gpu parallelly  #136021

/sig node
/kind bug

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 5, 2026
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 5, 2026

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@k8s-ci-robot k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. sig/node Categorizes an issue or PR as relevant to SIG Node. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. kind/bug Categorizes issue or PR as related to a bug. labels Jan 5, 2026
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 5, 2026
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 5, 2026 •
edited
Loading

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: ERJavier / name: Javier Cepeda (6d73c62)

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 5, 2026

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 5, 2026

Welcome @ERJavier!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 5, 2026
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 5, 2026

Hi @ERJavier. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 5, 2026
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 5, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ERJavier
Once this PR has been reviewed and has the lgtm label, please assign ffromani for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@ERJavier
fix: prevent panic in device manager when allocating devices in parallel
6d73c62 
Fix race condition in Allocate() where m.devicesToReuse[podUID] could be
deleted by another goroutine while still being accessed after the RPC call.

The issue occurs when:
- Pod1 starts allocation and sets m.devicesToReuse[pod1.uid]
- Pod1 makes RPC call (which can take time)
- Pod2 starts allocation and deletes m.devicesToReuse[pod1.uid]
- Pod1 tries to access m.devicesToReuse[pod1.uid] after RPC, causing panic

The fix saves a local reference to devicesToReuse before the RPC call
and uses it throughout the function, preventing the panic.

Added TestDeviceAllocateParallel to reproduce and verify the fix.
@ERJavier ERJavier force-pushed the fix-device-manager-race-condition-136021 branch from 59227c7 to 6d73c62 Compare January 5, 2026 05:20
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 5, 2026
AutuSnow
AutuSnow reviewed Jan 5, 2026
View reviewed changes
pkg/kubelet/cm/devicemanager/manager.go
m.devicesToReuse[string(pod.UID)] = make(map[string]sets.Set[string])
podUID := string(pod.UID)
if _, ok := m.devicesToReuse[podUID]; !ok {
m.devicesToReuse[podUID] = make(map[string]sets.Set[string])
Copy link

@AutuSnow AutuSnow Jan 5, 2026

Choose a reason for hiding this comment

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

The modification of this place is meaningless

Copy link
Author

@ERJavier ERJavier Jan 5, 2026

Choose a reason for hiding this comment

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

i apologize for my misunderstanding, i will be better next time.

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

Reviewers

@klueska klueska Awaiting requested review from klueska

@wzshiming wzshiming Awaiting requested review from wzshiming

1 more reviewer

@AutuSnow AutuSnow AutuSnow left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/node Categorizes an issue or PR as relevant to SIG Node. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

SIG Node: code and documentation PRs
Status: Triage

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

kubelet panic when allcoate gpu parallelly

3 participants

@ERJavier @k8s-ci-robot @AutuSnow