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

kubelet dra: add lock to addCDIDevices #116621

Merged
merged 1 commit into from Mar 15, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 30 additions & 27 deletions pkg/kubelet/cm/dra/claiminfo.go
Expand Up @@ -36,18 +36,40 @@ type ClaimInfo struct {
annotations []kubecontainer.Annotation
}

func (res *ClaimInfo) addPodReference(podUID types.UID) {
res.Lock()
defer res.Unlock()
func (info *ClaimInfo) addPodReference(podUID types.UID) {
info.Lock()
defer info.Unlock()

res.PodUIDs.Insert(string(podUID))
info.PodUIDs.Insert(string(podUID))
}

func (res *ClaimInfo) deletePodReference(podUID types.UID) {
res.Lock()
defer res.Unlock()
func (info *ClaimInfo) deletePodReference(podUID types.UID) {
info.Lock()
defer info.Unlock()

res.PodUIDs.Delete(string(podUID))
info.PodUIDs.Delete(string(podUID))
}

func (info *ClaimInfo) addCDIDevices(pluginName string, cdiDevices []string) error {
info.Lock()
defer info.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, put a newline after this.


// NOTE: Passing CDI device names as annotations is a temporary solution
// It will be removed after all runtimes are updated
// to get CDI device names from the ContainerConfig.CDIDevices field
annotations, err := generateCDIAnnotations(info.ClaimUID, info.DriverName, cdiDevices)
if err != nil {
return fmt.Errorf("failed to generate container annotations, err: %+v", err)
}

if info.CDIDevices == nil {
info.CDIDevices = make(map[string][]string)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to lock here, just before the code that changes info?

info.CDIDevices[pluginName] = cdiDevices
info.annotations = append(info.annotations, annotations...)

return nil
}

// claimInfoCache is a cache of processed resource claims keyed by namespace + claim name.
Expand All @@ -72,25 +94,6 @@ func newClaimInfo(driverName, className string, claimUID types.UID, claimName, n
return &claimInfo
}

func (info *ClaimInfo) addCDIDevices(pluginName string, cdiDevices []string) error {
// NOTE: Passing CDI device names as annotations is a temporary solution
// It will be removed after all runtimes are updated
// to get CDI device names from the ContainerConfig.CDIDevices field
annotations, err := generateCDIAnnotations(info.ClaimUID, info.DriverName, cdiDevices)
if err != nil {
return fmt.Errorf("failed to generate container annotations, err: %+v", err)
}

if info.CDIDevices == nil {
info.CDIDevices = make(map[string][]string)
}

info.CDIDevices[pluginName] = cdiDevices
info.annotations = append(info.annotations, annotations...)

return nil
}

// newClaimInfoCache is a function that returns an instance of the claimInfoCache.
func newClaimInfoCache(stateDir, checkpointName string) (*claimInfoCache, error) {
stateImpl, err := state.NewCheckpointState(stateDir, checkpointName)
Expand Down