The claim parameter key didn't include the namespace of the claim. In the case
where two namespaces used the exact same parameter reference, the "too many
generated parameters" case got triggered incorrectly and lookup could have
returned an object from the wrong namespace.
Found while running the E2E tests in parallel:
message: 'running PreFilter plugin "DynamicResources": multiple generated claim
parameters for ConfigMap. dra-8794/parameters-3 found: [dra-4729/parameters-4
dra-7328/parameters-4 dra-8794/parameters-4 dra-3402/parameters-4 dra-6156/parameters-4
dra-1839/parameters-4 dra-7434/parameters-4 dra-6504/parameters-4]'
There's no reason for having the interface because there is only one
implementation. Makes the implementation of the test functions a bit
simpler (no casting). They are still stand-alone functions instead of methods
because they should not be considered part of the "normal" API.
This is now used by both the volumebinding and dynamicresources plugin, so
promoting it to a common helper package is better.
In terms of functionality, nothing was changed. Documentation got
updated (warns about storing locally modified objects, clarifies what the Get
parameters are). Code coverage should be a bit better than before (tested with
and without indexer, exercises event handlers, more error paths).
Checking for specific errors can now be done via errors.Is.
Clearing some irrelevant fields in objects caused a flaky data race alert
because in some cases, the objects were pointers into a shared cache. A better
solution is to treat the objects as read-only and ignore the irrelevant fields.
Coverage was checked with a cover profile. The biggest remaining gap is for
isSchedulableAfterClaimParametersChange and
isSchedulableAfterClassParametersChange which will get handled when refactoring
the
foreachPodResourceClaim (https://github.com/kubernetes/kubernetes/issues/123697).
The code was incorrectly checking for a controller, but only the boolean
is set for allocated claims. As a result, deallocation was requested from
a non-existent control plane controller.
While at it, let's also clear the driver name. It's not needed when the
claim is deallocated.
Without this, the scheduler was crashing in newClaimController() in
pkg/scheduler/framework/plugins/dynamicresources/structuredparameters.go
The code in newClaimController() assumes that the parameters are not nil.
Furthermore it assumes that there is at least one DriverRequest populated in
order to allocate any resources to a claim.
This PR adds logic to define default claim/class parameters that will allow
allocation to proceed even if an end user doesn't provide any class or claim
parameters themselves.
Signed-off-by: Kevin Klues <kklues@nvidia.com>
