Test_isSchedulableAfterClaimChange was sensitive to system load because of the
arbitrary delay when waiting for the assume cache to catch up. Running inside
a synctest bubble avoids this. While at it, the unit tests get converted
to ktesting (nicer failure output, no extra indention needed for
tCtx.SyncTest).
TestPlugin/prebind-fail-with-binding-timeout relied on setting up a claim with
certain time stamps and then getting that test case tested within a certain
real-world time window. It's surprising that this didn't flake more often
because test execution order is random. Now the time stamp gets set right
before the test case is about to be tested. Conversion to a synctest would
be nicer, but synctests cannot have sub-tests, which are used here to track
where log output and failures come from within the larger test case.
Inside the plugin itself some log output gets added to explain why a claim is
unavailable on a node in case of a binding timeout or error during Filter.
The script is broken because it relies on grepping the source code.
This has always been fragile (currently it finds the search term
in comments) and stopped working years ago when changing how tests
are labeled.
Instead of fixing the script let's remove it because it's clearly unused.
The tests assumed that instantiating a DRAManager followed by
informerFactory.WaitForCacheSync would be enough to have the manager
up-to-date, but that's not correct: the test only waits for informer *caches*
to be synced, but syncing *event handlers* like the one in the manager may
still be going on. The flake rate is low, though:
$ GOPATH/bin/stress -p 256 ./noderesources.test
5s: 0 runs so far, 0 failures, 256 active
10s: 256 runs so far, 0 failures, 256 active
15s: 256 runs so far, 0 failures, 256 active
20s: 512 runs so far, 0 failures, 256 active
25s: 567 runs so far, 0 failures, 256 active
30s: 771 runs so far, 0 failures, 256 active
/tmp/go-stress-20251226T181044-974980161
--- FAIL: TestCalculateResourceAllocatableRequest (0.81s)
--- FAIL: TestCalculateResourceAllocatableRequest/DRA-backed-resource-with-shared-device-allocation (0.00s)
extendedresourcecache.go:197: I1226 18:11:14.431337] Updated extended resource cache for explicit mapping extendedResource="extended.resource.dra.io/something" deviceClass="device-class-name"
extendedresourcecache.go:204: I1226 18:11:14.431380] Updated extended resource cache for default mapping extendedResource="deviceclass.resource.kubernetes.io/device-class-name" deviceClass="device-class-name"
extendedresourcecache.go:220: I1226 18:11:14.431394] Updated device class mapping deviceClass="device-class-name" extendedResource="extended.resource.dra.io/something"
resource_allocation_test.go:595: Expected requested=2, but got requested=1
FAIL
It becomes higher when changing WaitForCacheSync such that it doesn't poll and
therefore returns more promptly, which is where this flake was first observed.
The fix is to run the test in a syntest bubble where Wait can be used to wait
for all background activity, including event handling, to be finished before
proceeding with the test.
synctest is less forgiving about lingering goroutines. A synctest bubble must
wait for gouroutines to stop, which in this case means that there has to be
a way to wait for the metric recorder shutdown. Event handlers have to be
removed.
This could be done with plain Go, but here test/utils/ktesting is used instead
because it offers some advantages:
- less boilerplate code
- automatic cancellation of the context (i.e. less manual context.WithCancel)
- tCtx.SyncTest is a direct substitute for t.Run, which avoids re-indenting
sub-tests. synctest itself needs another anonymous function, which makes
the line too long and forced re-indention:
t.Run(... func(...) {
synctest.Test(... func() {
})
})
For the sake of consistency all tests get updated.
While at it, some code gets improved:
- t.Fatal(err) is not a good way to report an error because
there is no additional markup in the test output that indicates
that there was an unexpected error. It just logs err.Error(),
which might not be very informative and/or obvious.
- newTestDRAManager aborts in case of a failure instead of
returning an error.
Verify scripts are run such that stderr is captured and included in the JUnit
files. Stdout is not. Therefore the instructions in case of a failure where
only visible by searching the entire job log file, but not in the Prow summary.
The unit test flaked at least once so far in the CI. Can be reproduced
locally by running many instances of the test in parallel.
There were two potential root causes:
- The watch must be set up completely before creating objects (a fake
client-go limitation).
- The one second timeout might be too small on a loaded system.
By running the tests in a synctest bubble with synctest.Wait calls at
the right places both problems are avoided. As a welcome side effect
the test also completes faster.
Before:
$ go test k8s.io/dynamic-resource-allocation/deviceclass/extendedresourcecache
ok k8s.io/dynamic-resource-allocation/deviceclass/extendedresourcecache 9.300s
$ stress -p 512 ./extendedresourcecache.test
5s: 0 runs so far, 0 failures, 512 active
10s: 8 runs so far, 0 failures, 512 active
/tmp/go-stress-20251225T195231-2875181701
--- FAIL: TestExtendedResourceCache (9.14s)
extendedresourcecache_test.go:234: Expected to find device class 'gpu-class' for 'example.com/gpu', got nil
extendedresourcecache_test.go:241: Expected to find device class 'fpga-class' for 'deviceclass.resource.kubernetes.io/fpga-class', got nil
extendedresourcecache_test.go:247: Expected default mapping for gpu-class
extendedresourcecache.go:197: I1225 19:52:36.332170] Updated extended resource cache for explicit mapping extendedResource="example.com/gpu" deviceClass="gpu-class-3"
extendedresourcecache.go:204: I1225 19:52:36.332216] Updated extended resource cache for default mapping extendedResource="deviceclass.resource.kubernetes.io/gpu-class-3" deviceClass="gpu-class-3"
extendedresourcecache.go:220: I1225 19:52:36.332245] Updated device class mapping deviceClass="gpu-class-3" extendedResource="example.com/gpu"
extendedresourcecache_test.go:260: Expected to find device class 'gpu-class' for 'example.com/gpu', got &DeviceClass{ObjectMeta:{gpu-class-3 0 2025-12-24 19:52:35.32560833 +0100 CET m=-86396.691266715 <nil> <nil> map[] map[] [] [] [{unknown Update resource.k8s.io/v1 2025-12-25 18:52:36.332067439 +0000 UTC FieldsV1 {"f:spec":{"f:extendedResourceName":{}}} }]},Spec:DeviceClassSpec{Selectors:[]DeviceSelector{},Config:[]DeviceClassConfiguration{},ExtendedResourceName:*example.com/gpu,},}
extendedresourcecache.go:197: I1225 19:52:37.336135] Updated extended resource cache for explicit mapping extendedResource="example.com/gpu" deviceClass="gpu-class-4"
extendedresourcecache.go:204: I1225 19:52:37.336169] Updated extended resource cache for default mapping extendedResource="deviceclass.resource.kubernetes.io/gpu-class-4" deviceClass="gpu-class-4"
extendedresourcecache.go:220: I1225 19:52:37.336192] Updated device class mapping deviceClass="gpu-class-4" extendedResource="example.com/gpu"
extendedresourcecache.go:197: I1225 19:52:38.340121] Updated extended resource cache for explicit mapping extendedResource="example
After:
ok k8s.io/dynamic-resource-allocation/deviceclass/extendedresourcecache 0.064s
...
2m0s: 7063 runs so far, 0 failures, 512 active
Reading numPreFilterCalled races with writing it in the scheduler, at least as
far as the data race detector is concerned. That the test waits for pod
scheduling is too indirect. enqueuePlugin.called has the same problem,
but hasn't triggered the race detector (yet).
We need to protect against concurrent access. The easiest way to enforce that
is via atomic.Int64. In contrast to a mutex it is impossible to use it wrong.
Shutting down the scheduler first was also tried, but didn't work out because
"teardown" does more than just stopping the scheduler, it also cancels a
context that is needed during test shutdown.
This file was added in 2018 (PR #69238) to ensure Go's netgo DNS
resolver respects /etc/hosts in busybox-based control plane images.
In 2021 (PR #99015), the build switched to on-disk Dockerfiles and
distroless base images. The nsswitch.conf copying was dropped and
the distroless base (Debian-based) already includes /etc/nsswitch.conf.
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
The UnschedulablePods() function iterates over the unschedulablePods.podInfoMap
without holding any lock, while other goroutines may concurrently modify the map
via addOrUpdate(), delete(), or clear().
Other functions like PendingPods() and GetPod() correctly acquire p.lock.RLock()
before accessing unschedulablePods.podInfoMap, but UnschedulablePods() was
missing this.
Fix by adding p.lock.RLock()/RUnlock() to UnschedulablePods(), matching the
pattern used by PendingPods().
