From e0fce54d02531a6c30d836b5786055d801dc82c6 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 13 Jun 2024 17:10:18 +0200 Subject: [PATCH] DRA: fix indexing of generated parameters 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]' --- .../plugins/dynamicresources/dynamicresources.go | 10 +++++----- .../plugins/dynamicresources/dynamicresources_test.go | 11 ++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go index f01f89feb9c..a4da1f7780d 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go @@ -381,12 +381,12 @@ func New(ctx context.Context, plArgs runtime.Object, fh framework.Handle, fts fe return pl, nil } -func claimParametersReferenceKeyFunc(ref *resourcev1alpha2.ResourceClaimParametersReference) string { - return ref.APIGroup + "/" + ref.Kind + "/" + ref.Name +func claimParametersReferenceKeyFunc(namespace string, ref *resourcev1alpha2.ResourceClaimParametersReference) string { + return ref.APIGroup + "/" + ref.Kind + "/" + namespace + "/" + ref.Name } // claimParametersGeneratedFromIndexFunc is an index function that returns other resource keys -// (= apiGroup/kind/name) for ResourceClaimParametersReference in a given claim parameters. +// (= apiGroup/kind/namespace/name) for ResourceClaimParametersReference in a given claim parameters. func claimParametersGeneratedFromIndexFunc(obj interface{}) ([]string, error) { parameters, ok := obj.(*resourcev1alpha2.ResourceClaimParameters) if !ok { @@ -395,7 +395,7 @@ func claimParametersGeneratedFromIndexFunc(obj interface{}) ([]string, error) { if parameters.GeneratedFrom == nil { return nil, nil } - return []string{claimParametersReferenceKeyFunc(parameters.GeneratedFrom)}, nil + return []string{claimParametersReferenceKeyFunc(parameters.Namespace, parameters.GeneratedFrom)}, nil } func classParametersReferenceKeyFunc(ref *resourcev1alpha2.ResourceClassParametersReference) string { @@ -1105,7 +1105,7 @@ func (pl *dynamicResources) lookupClaimParameters(logger klog.Logger, class *res return parameters, nil } - objs, err := pl.claimParametersIndexer.ByIndex(generatedFromIndex, claimParametersReferenceKeyFunc(claim.Spec.ParametersRef)) + objs, err := pl.claimParametersIndexer.ByIndex(generatedFromIndex, claimParametersReferenceKeyFunc(claim.Namespace, claim.Spec.ParametersRef)) if err != nil { return nil, statusError(logger, fmt.Errorf("listing claim parameters failed: %v", err)) } diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go index 566610e891d..18b6b832a1b 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go @@ -140,6 +140,15 @@ var ( APIGroup: "example.com", }). Obj() + claimParametersOtherNamespace = st.MakeClaimParameters().Name(claimName).Namespace(namespace+"-2"). + NamedResourcesRequests("some-driver", "true"). + Shareable(true). + GeneratedFrom(&resourcev1alpha2.ResourceClaimParametersReference{ + Name: claimName, + Kind: "ResourceClaimParameters", + APIGroup: "example.com", + }). + Obj() classParameters = st.MakeClassParameters().Name(className).Namespace(namespace). NamedResourcesFilters("some-driver", "true"). GeneratedFrom(&resourcev1alpha2.ResourceClassParametersReference{ @@ -631,7 +640,7 @@ func TestPlugin(t *testing.T) { pod: podWithClaimName, claims: []*resourcev1alpha2.ResourceClaim{claimWithCRD(pendingDelayedClaimWithParams)}, classes: []*resourcev1alpha2.ResourceClass{classWithCRD(structuredResourceClassWithCRD)}, - objs: []apiruntime.Object{claimParameters, classParameters, workerNodeSlice}, + objs: []apiruntime.Object{claimParameters, claimParametersOtherNamespace /* must be ignored */, classParameters, workerNodeSlice}, want: want{ reserve: result{ inFlightClaim: claimWithCRD(structuredAllocatedClaimWithParams),