DRA consumable capacity: fix allocation result lookup

Two in-tree places looked for entries with a literal string lookup.  This
compared a QualifiedName from a current slice against a QualifiedName from an
allocation result, which is based on some older slice. The literal lookup
breaks if the slices changed from fully-qualified to un-qualified or vice
versa.

The correct approach is to do what lookupAttribute for constraints does: check
both forms. Following the example set by lookupAttribute, fully-qualified is
given precedence in case of a conflict.

pkg/controller/resourcepoolstatusrequest/views.go

@@ -25,6 +25,7 @@ import (
 	resourcev1 "k8s.io/api/resource/v1"
 	resourcev1alpha3 "k8s.io/api/resource/v1alpha3"
 	"k8s.io/apimachinery/pkg/api/resource"
+	draapi "k8s.io/dynamic-resource-allocation/api"
 	"k8s.io/utils/ptr"
 )
 
@@ -295,7 +296,7 @@ func computeShareableSummary(in poolViewInput) (*resourcev1alpha3.ShareableSumma
 	for _, key := range keys {
 		t := total[key]
 		cons := resource.Quantity{}
-		if q, ok := in.consumedCapacity[key]; ok {
+		if q, ok := draapi.LookupByQualifiedName(in.consumedCapacity, key, in.driver); ok {
 			cons = q.DeepCopy()
 		}
 		avail := nonNegativeDiff(t, cons)

pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	resourcehelper "k8s.io/component-helpers/resource"
 	"k8s.io/component-helpers/scheduling/corev1/nodeaffinity"
+	draapi "k8s.io/dynamic-resource-allocation/api"
 	"k8s.io/dynamic-resource-allocation/resourceclaim"
 	"k8s.io/klog/v2"
 	fwk "k8s.io/kube-scheduler/framework"
@@ -216,7 +217,7 @@ func addDeviceMapping(
 		if result.ConsumedCapacity == nil {
 			return fmt.Errorf("claim %s/%s, device %s: ConsumedCapacity is nil, but Capacity key '%s' is set in NodeAllocatableResources for resource %s", key.Namespace, key.Name, result.Device, capacityKey, resourceName)
 		}
-		if consumed, exists := result.ConsumedCapacity[capacityKey]; exists {
+		if consumed, exists := draapi.LookupByQualifiedName(result.ConsumedCapacity, capacityKey, result.Driver); exists {
 			// If !exists - the capacityKey is not in ConsumedCapacity, this mapping is not relevant for this allocation
 			consumedQuantity := consumed.DeepCopy()
 			quantityOne := resource.MustParse("1")

staging/src/k8s.io/dynamic-resource-allocation/api/qualifiedname.go

@@ -0,0 +1,59 @@
+/*
+Copyright The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+	"strings"
+
+	resourceapi "k8s.io/api/resource/v1"
+)
+
+// LookupByQualifiedName looks up key in m. The default domain
+// qualifies keys which do not already have an explicit domain.
+//
+// The map may contain entries with or without a domain or (worse!) both for the same identifier.
+// The entry with a fully-qualified name is preferred in case of such an ambiguity.
+func LookupByQualifiedName[T any](m map[resourceapi.QualifiedName]T, key resourceapi.QualifiedName, defaultDomain string) (T, bool) {
+	domain, name, hasDomain := strings.Cut(string(key), "/")
+	if hasDomain {
+		// Check with domain first.
+		if v, ok := m[key]; ok {
+			return v, true
+		}
+		// Then without, but only if the explicit domain matches.
+		// Entries in the map without a domain are no match
+		// because they would be qualified with the defaultDomain.
+		if domain == defaultDomain {
+			if v, ok := m[resourceapi.QualifiedName(name)]; ok {
+				return v, true
+			}
+		}
+	} else {
+		// Check with domain first.
+		if v, ok := m[resourceapi.QualifiedName(defaultDomain+"/"+string(key))]; ok {
+			return v, true
+		}
+		// Then without.
+		if v, ok := m[key]; ok {
+			return v, true
+		}
+	}
+
+	// Not found.
+	var zero T
+	return zero, false
+}

staging/src/k8s.io/dynamic-resource-allocation/api/qualifiedname_test.go

@@ -0,0 +1,102 @@
+/*
+Copyright The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+	"testing"
+
+	resourceapi "k8s.io/api/resource/v1"
+)
+
+func TestLookupByQualifiedName(t *testing.T) {
+	const defaultDomain = "example.com"
+
+	for name, tc := range map[string]struct {
+		m         map[resourceapi.QualifiedName]int
+		key       resourceapi.QualifiedName
+		wantValue int
+		wantOK    bool
+	}{
+		"unqualified-key-map-has-unqualified-only": {
+			m:         map[resourceapi.QualifiedName]int{"foo": 1},
+			key:       "foo",
+			wantValue: 1,
+			wantOK:    true,
+		},
+		"unqualified-key-map-has-qualified-only": {
+			m:         map[resourceapi.QualifiedName]int{"example.com/foo": 2},
+			key:       "foo",
+			wantValue: 2,
+			wantOK:    true,
+		},
+		"unqualified-key-map-has-both-prefers-qualified": {
+			m:         map[resourceapi.QualifiedName]int{"foo": 1, "example.com/foo": 2},
+			key:       "foo",
+			wantValue: 2,
+			wantOK:    true,
+		},
+		"unqualified-key-map-has-neither": {
+			m:      map[resourceapi.QualifiedName]int{},
+			key:    "foo",
+			wantOK: false,
+		},
+		"qualified-key-matching-domain-map-has-qualified-only": {
+			m:         map[resourceapi.QualifiedName]int{"example.com/foo": 2},
+			key:       "example.com/foo",
+			wantValue: 2,
+			wantOK:    true,
+		},
+		"qualified-key-matching-domain-map-has-unqualified-only": {
+			m:         map[resourceapi.QualifiedName]int{"foo": 1},
+			key:       "example.com/foo",
+			wantValue: 1,
+			wantOK:    true,
+		},
+		"qualified-key-matching-domain-map-has-both-prefers-qualified": {
+			m:         map[resourceapi.QualifiedName]int{"foo": 1, "example.com/foo": 2},
+			key:       "example.com/foo",
+			wantValue: 2,
+			wantOK:    true,
+		},
+		"qualified-key-different-domain-map-has-unqualified-only-not-found": {
+			m:      map[resourceapi.QualifiedName]int{"foo": 1},
+			key:    "other.com/foo",
+			wantOK: false,
+		},
+		"qualified-key-different-domain-map-has-matching-qualified": {
+			m:         map[resourceapi.QualifiedName]int{"other.com/foo": 3},
+			key:       "other.com/foo",
+			wantValue: 3,
+			wantOK:    true,
+		},
+		"empty-map": {
+			m:      nil,
+			key:    "foo",
+			wantOK: false,
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			value, ok := LookupByQualifiedName(tc.m, tc.key, defaultDomain)
+			if ok != tc.wantOK {
+				t.Fatalf("got ok = %v, want %v", ok, tc.wantOK)
+			}
+			if ok && value != tc.wantValue {
+				t.Fatalf("got value = %v, want %v", value, tc.wantValue)
+			}
+		})
+	}
+}