Skip to content
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.

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

fix get device ids when a specific number of devices are to be replicated #763

Closed
wants to merge 1 commit into from

Conversation

KubeKyrie
Copy link
Contributor

@KubeKyrie KubeKyrie commented Dec 30, 2024

What type of PR is this?
/kind bug

What this PR does / why we need it:

When a specific number of devices for this resource type are to be replicated, every time we get device ids different. Because the traversal map is unordered.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Copy link

codecov bot commented Dec 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
unittests 44.00% <ø> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@wawa0210
Copy link
Member

wawa0210 commented Jan 3, 2025

@lengrongfu Can you help review this pr ?

@lengrongfu
Copy link
Member

lengrongfu commented Jan 4, 2025

There is no logical problem with the code, but I still do not recommend modifying it.

  1. The code in the pkg/device-plugin directory comes from the repository https://github.com/NVIDIA/k8s-device-plugin. The upstream logic is also handled in this way. Do not modify the code in this directory unless necessary to facilitate subsequent tracking of the upstream code.
  2. In the current logic, the order of maps does not affect the results.

Mainly because of the first point I do not recommend making any changes.

@KubeKyrie

@KubeKyrie KubeKyrie closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants