Skip to content

Commit 7ecf09e

Browse files
committed
Fix a race condition in TestNodeLoadFromExistingTaints
TestNodeLoadFromExistingTaints creates a currentTime variable set to time.Now(), and a bunch of test objects with time values offset from that variable. This is all standard practice, but then the test iterates over test cases, calls t.Parallel(), and overwrites currentTime with time.Now() again. This makes go test -race fail, because multiple goroutines are writing currentTime at once. It also doesn't seem to make sense in the context of the test, because the other test objects are still offset from the original value. Removing the second write to currentTime seems to be the correct fix here. Also renamed one import because it collided with a local variable name used throughout this test file.
1 parent cef695b commit 7ecf09e

File tree

1 file changed

+2
-5
lines changed

1 file changed

+2
-5
lines changed

cluster-autoscaler/core/scaledown/unneeded/nodes_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
"k8s.io/autoscaler/cluster-autoscaler/core/scaledown/resource"
3131
"k8s.io/autoscaler/cluster-autoscaler/core/scaledown/status"
3232
. "k8s.io/autoscaler/cluster-autoscaler/core/test"
33-
"k8s.io/autoscaler/cluster-autoscaler/processors/nodes"
33+
nodeprocessors "k8s.io/autoscaler/cluster-autoscaler/processors/nodes"
3434
"k8s.io/autoscaler/cluster-autoscaler/simulator"
3535
"k8s.io/autoscaler/cluster-autoscaler/utils/kubernetes"
3636
kube_util "k8s.io/autoscaler/cluster-autoscaler/utils/kubernetes"
@@ -205,7 +205,7 @@ func TestRemovableAt(t *testing.T) {
205205

206206
n := NewNodes(&fakeScaleDownTimeGetter{}, &resource.LimitsFinder{})
207207
n.Update(removableNodes, time.Now())
208-
gotEmptyToRemove, gotDrainToRemove, _ := n.RemovableAt(&ctx, nodes.ScaleDownContext{
208+
gotEmptyToRemove, gotDrainToRemove, _ := n.RemovableAt(&ctx, nodeprocessors.ScaleDownContext{
209209
ActuationStatus: as,
210210
ResourcesLeft: resource.Limits{},
211211
ResourcesWithLimits: []string{},
@@ -218,7 +218,6 @@ func TestRemovableAt(t *testing.T) {
218218
}
219219

220220
func TestNodeLoadFromExistingTaints(t *testing.T) {
221-
222221
deletionCandidateTaint := taints.DeletionCandidateTaint()
223222
currentTime := time.Now()
224223

@@ -275,8 +274,6 @@ func TestNodeLoadFromExistingTaints(t *testing.T) {
275274
t.Run(tc.name, func(t *testing.T) {
276275
t.Parallel()
277276

278-
currentTime = time.Now()
279-
280277
nodes := NewNodes(nil, nil)
281278

282279
allNodeLister := kubernetes.NewTestNodeLister(nil)

0 commit comments

Comments
 (0)