Skip to content

Commit 008b075

Browse files
committed
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.
1 parent 87fa400 commit 008b075

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

staging/src/k8s.io/dynamic-resource-allocation/resourceclaim/resourceclaim.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"fmt"
2929
"os"
3030
"strings"
31+
"sync/atomic"
3132

3233
v1 "k8s.io/api/core/v1"
3334
resourcev1alpha2 "k8s.io/api/resource/v1alpha2"
@@ -95,14 +96,15 @@ func NewNameLookup(client kubernetes.Interface) *Lookup {
9596
// Lookup stores the state which is necessary to look up ResourceClaim names.
9697
type Lookup struct {
9798
client kubernetes.Interface
98-
usePodStatus *bool
99+
usePodStatus atomic.Pointer[bool]
99100
}
100101

101102
// Name is a variant of the stand-alone Name with support also for Kubernetes < 1.28.
102103
func (l *Lookup) Name(pod *v1.Pod, podClaim *v1.PodResourceClaim) (name *string, mustCheckOwner bool, err error) {
103-
if l.usePodStatus == nil {
104+
usePodStatus := l.usePodStatus.Load()
105+
if usePodStatus == nil {
104106
if value, _ := os.LookupEnv("DRA_WITH_DETERMINISTIC_RESOURCE_CLAIM_NAMES"); value != "" {
105-
l.usePodStatus = ptr.To(false)
107+
usePodStatus = ptr.To(false)
106108
} else if l.client != nil {
107109
// Check once. This does not detect upgrades or
108110
// 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,
114116
}
115117
if info.Major == "" {
116118
// Fake client...
117-
l.usePodStatus = ptr.To(true)
119+
usePodStatus = ptr.To(true)
118120
} else {
119121
switch strings.Compare(info.Major, "1") {
120122
case -1:
121123
// Huh?
122-
l.usePodStatus = ptr.To(false)
124+
usePodStatus = ptr.To(false)
123125
case 0:
124126
// info.Minor may have a suffix which makes it larger than 28.
125127
// We don't care about pre-releases here.
126-
l.usePodStatus = ptr.To(strings.Compare("28", info.Minor) <= 0)
128+
usePodStatus = ptr.To(strings.Compare("28", info.Minor) <= 0)
127129
case 1:
128130
// Kubernetes 2? Yeah!
129-
l.usePodStatus = ptr.To(true)
131+
usePodStatus = ptr.To(true)
130132
}
131133
}
134+
} else {
135+
// No information. Let's assume recent Kubernetes.
136+
usePodStatus = ptr.To(true)
132137
}
138+
l.usePodStatus.Store(usePodStatus)
133139
}
134140

135-
if *l.usePodStatus {
141+
if *usePodStatus {
136142
return Name(pod, podClaim)
137143
}
138144

0 commit comments

Comments
 (0)