DRA: fix data race in resourceclaim.Lookup

This gets uses concurrently as seen by a data race reported when running
integration tests with race detection enabled. All writes would have written
the same value, but it is a race nonetheless.
This commit is contained in:
Patrick Ohly 2024-02-07 16:27:22 +01:00
parent 87fa400d9d
commit 008b075b46

View File

@ -28,6 +28,7 @@ import (
"fmt" "fmt"
"os" "os"
"strings" "strings"
"sync/atomic"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
resourcev1alpha2 "k8s.io/api/resource/v1alpha2" resourcev1alpha2 "k8s.io/api/resource/v1alpha2"
@ -95,14 +96,15 @@ func NewNameLookup(client kubernetes.Interface) *Lookup {
// Lookup stores the state which is necessary to look up ResourceClaim names. // Lookup stores the state which is necessary to look up ResourceClaim names.
type Lookup struct { type Lookup struct {
client kubernetes.Interface client kubernetes.Interface
usePodStatus *bool usePodStatus atomic.Pointer[bool]
} }
// Name is a variant of the stand-alone Name with support also for Kubernetes < 1.28. // Name is a variant of the stand-alone Name with support also for Kubernetes < 1.28.
func (l *Lookup) Name(pod *v1.Pod, podClaim *v1.PodResourceClaim) (name *string, mustCheckOwner bool, err error) { func (l *Lookup) Name(pod *v1.Pod, podClaim *v1.PodResourceClaim) (name *string, mustCheckOwner bool, err error) {
if l.usePodStatus == nil { usePodStatus := l.usePodStatus.Load()
if usePodStatus == nil {
if value, _ := os.LookupEnv("DRA_WITH_DETERMINISTIC_RESOURCE_CLAIM_NAMES"); value != "" { if value, _ := os.LookupEnv("DRA_WITH_DETERMINISTIC_RESOURCE_CLAIM_NAMES"); value != "" {
l.usePodStatus = ptr.To(false) usePodStatus = ptr.To(false)
} else if l.client != nil { } else if l.client != nil {
// Check once. This does not detect upgrades or // Check once. This does not detect upgrades or
// downgrades, but that is good enough for the simple // downgrades, but that is good enough for the simple
@ -114,25 +116,29 @@ func (l *Lookup) Name(pod *v1.Pod, podClaim *v1.PodResourceClaim) (name *string,
} }
if info.Major == "" { if info.Major == "" {
// Fake client... // Fake client...
l.usePodStatus = ptr.To(true) usePodStatus = ptr.To(true)
} else { } else {
switch strings.Compare(info.Major, "1") { switch strings.Compare(info.Major, "1") {
case -1: case -1:
// Huh? // Huh?
l.usePodStatus = ptr.To(false) usePodStatus = ptr.To(false)
case 0: case 0:
// info.Minor may have a suffix which makes it larger than 28. // info.Minor may have a suffix which makes it larger than 28.
// We don't care about pre-releases here. // We don't care about pre-releases here.
l.usePodStatus = ptr.To(strings.Compare("28", info.Minor) <= 0) usePodStatus = ptr.To(strings.Compare("28", info.Minor) <= 0)
case 1: case 1:
// Kubernetes 2? Yeah! // Kubernetes 2? Yeah!
l.usePodStatus = ptr.To(true) usePodStatus = ptr.To(true)
} }
} }
} else {
// No information. Let's assume recent Kubernetes.
usePodStatus = ptr.To(true)
} }
l.usePodStatus.Store(usePodStatus)
} }
if *l.usePodStatus { if *usePodStatus {
return Name(pod, podClaim) return Name(pod, podClaim)
} }