* DRA resource claim controller: configurable number of workers
It might never be necessary to change the default, but it is hard to be sure.
It's better to have the option, just in case.
* generate files
* resourceclaimcontroller: normalize validation error message
* Update cmd/kube-controller-manager/app/options/resourceclaimcontroller.go
Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
---------
Co-authored-by: Patrick Ohly <patrick.ohly@intel.com>
Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
We need to lock on observedTime reads to avoid races, before we were
only locking to read the observed record.
Use a RW lock since there are much more reads that writes.
Gomega formats errors by first showing Error() (already has all information
after WithError) and then again by dumping the error struct, which is redundant
in this case. We can avoid the latter by providing a GomegaString
implementation which returns nothing.
Being able to call arbitrary functions is useful, even if it means giving up
some compile-time checking. Because we now use reflection,
Eventually/Consistently can be methods.
The "create 100 slices" E2E sometimes flaked with timeouts (e.g. 95 out of 100
slices created). It created too much load for an E2E test.
The same test now uses ktesting as API, which makes it possible to run it as
integration test with the original 100 slices and with more moderate 10 slices
as E2E test.
Manually pairing Being with End is too error prone to be useful. It had the
advantage of keeping variables created between them visible to the following
code, but that doesn't justify using those calls.
By using a callback we can achieve a few things:
- Code using it automatically shadows the parent tCtx, thus enforcing
that within a code block the tCtx with step is used consistently.
- The code block is clearly delineated with curly braces.
- When the code block ends, the unmodified parent tCtx is automatically
in scope again.
Downsides:
- Extra boilerplate for the anonymous function.
Python's `with tCtx.Step(...) as tCtx: ` would be nicer.
As an approximation of that `for tCtx := range tCtx.Step(...)` was
tried with `Step` returning an iterator, but that wasn't very idiomatic.
- Variables created inside the code block are not visible outside of it.
klog calls during init are becoming a problem because now test/e2e/framework
depends in test/utils/ktesting which bumps up the default verbosity during init
to make test output more complete when there is no argument parsing.
For cadvisor, an upstream fix is
needed (https://github.com/google/cadvisor/pull/3778). For kubectl we can make
it silently accept the valid (!) LC_ALL=C.
tCtx.Run and sub-tests make it much simpler to separate the different steps
than with Ginkgo because unless a test runs tCtx.Parallel (which we don't do
here), everything runs sequentially in a deterministic order.
Right now we get:
...
localupcluster.go:285: I1210 12:24:22.067524] bring up v1.34: stopping kubelet
localupcluster.go:285: I1210 12:24:22.067548] bring up v1.34: stopping kube-scheduler
localupcluster.go:285: I1210 12:24:22.067570] bring up v1.34: stopping kube-controller-manager
localupcluster.go:285: I1210 12:24:22.067589] bring up v1.34: stopping kube-apiserver
--- PASS: TestUpgradeDowngrade (94.78s)
--- PASS: TestUpgradeDowngrade/after-cluster-creation (2.07s)
--- PASS: TestUpgradeDowngrade/after-cluster-creation/core_DRA (2.05s)
--- PASS: TestUpgradeDowngrade/after-cluster-creation/ResourceClaim_device_status (0.02s)
--- PASS: TestUpgradeDowngrade/after-cluster-upgrade (4.10s)
--- PASS: TestUpgradeDowngrade/after-cluster-upgrade/core_DRA (4.09s)
--- PASS: TestUpgradeDowngrade/after-cluster-upgrade/ResourceClaim_device_status (0.01s)
--- PASS: TestUpgradeDowngrade/after-cluster-downgrade (1.24s)
--- PASS: TestUpgradeDowngrade/after-cluster-downgrade/core_DRA (1.21s)
--- PASS: TestUpgradeDowngrade/after-cluster-downgrade/ResourceClaim_device_status (0.02s)
PASS
It's even possible to use `-failfast` and
e.g. `-run=TestUpgradeDowngrade/after-cluster-creation/core_DRA`: `go test` then
runs everything up to that sub-test or any failing sub-test, then stops and
cleans up.
This leverages ktesting as wrapper around Ginkgo and testing.T to make all
helper code that is needed to deploy a DRA driver available to Go unit
tests and thus integration tests.
How to proceed with unifying helper code for integration and E2E testing is
open. This is just a minimal first step in that direction. Ideally, such
code should be in separate packages where usage of Ginkgo, e2e/framework
and gomega.Expect/Eventually/Consistently are forbidden.
While at it, the builder gets extended to make cleanup optional.
This will be needed for upgrade/downgrade testing with sub-tests.
Previously it was necessary to use the Ginkgo wrappers when
using any of the custom arguments like WithSlow(). Now the
hook within Ginkgo for modifying arguments is used such that
e.g. the original ginkgo.It also works.
The spaces are unnecessary because Ginkgo adds spaces automatically.
This was detected before only for tests using the wrapper functions,
now it also gets detected for ginkgo methods.