Merge pull request #7852 from omerap12/migrate-ClaimOwningPod

migrate ClaimOwningPod to use upstream IsForPod

Author: k8s-ci-robot

cluster-autoscaler/simulator/clustersnapshot/predicate/predicate_snapshot.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/clustersnapshot"
 	drautils "k8s.io/autoscaler/cluster-autoscaler/simulator/dynamicresources/utils"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/framework"
+	"k8s.io/dynamic-resource-allocation/resourceclaim"
 	schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework"
 )
 
@@ -245,7 +246,7 @@ func (s *PredicateSnapshot) modifyResourceClaimsForNewPod(podInfo *framework.Pod
 	// so we don't add them. The claims should already be allocated in the provided PodInfo.
 	var podOwnedClaims []*resourceapi.ResourceClaim
 	for _, claim := range podInfo.NeededResourceClaims {
-		if ownerName, _ := drautils.ClaimOwningPod(claim); ownerName != "" {
+		if err := resourceclaim.IsForPod(podInfo.Pod, claim); err == nil {
 			podOwnedClaims = append(podOwnedClaims, claim)
 		}
 	}

cluster-autoscaler/simulator/dynamicresources/snapshot/snapshot.go

@@ -178,7 +178,7 @@ func (s Snapshot) RemovePodOwnedClaims(pod *apiv1.Pod) {
 			// The claim isn't tracked in the snapshot for some reason. Nothing to remove/modify, so continue to the next claim.
 			continue
 		}
-		if ownerName, ownerUid := drautils.ClaimOwningPod(claim); ownerName == pod.Name && ownerUid == pod.UID {
+		if err := resourceclaim.IsForPod(pod, claim); err == nil {
 			delete(s.resourceClaimsById, claimId)
 		} else {
 			drautils.ClearPodReservationInPlace(claim, pod)
@@ -214,9 +214,7 @@ func (s Snapshot) UnreservePodClaims(pod *apiv1.Pod) error {
 		return err
 	}
 	for _, claim := range claims {
-		ownerPodName, ownerPodUid := drautils.ClaimOwningPod(claim)
-		podOwnedClaim := ownerPodName == pod.Name && ownerPodUid == ownerPodUid
-
+		podOwnedClaim := resourceclaim.IsForPod(pod, claim) == nil
 		drautils.ClearPodReservationInPlace(claim, pod)
 		if podOwnedClaim || !drautils.ClaimInUse(claim) {
 			drautils.DeallocateClaimInPlace(claim)

cluster-autoscaler/simulator/dynamicresources/utils/resource_claims.go

@@ -22,23 +22,10 @@ import (
 	apiv1 "k8s.io/api/core/v1"
 	resourceapi "k8s.io/api/resource/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/component-helpers/scheduling/corev1"
 	resourceclaim "k8s.io/dynamic-resource-allocation/resourceclaim"
-	"k8s.io/utils/ptr"
 )
 
-// ClaimOwningPod returns the name and UID of the Pod owner of the provided claim. If the claim isn't
-// owned by a Pod, empty strings are returned.
-func ClaimOwningPod(claim *resourceapi.ResourceClaim) (string, types.UID) {
-	for _, owner := range claim.OwnerReferences {
-		if ptr.Deref(owner.Controller, false) && owner.APIVersion == "v1" && owner.Kind == "Pod" {
-			return owner.Name, owner.UID
-		}
-	}
-	return "", ""
-}
-
 // ClaimAllocated returns whether the provided claim is allocated.
 func ClaimAllocated(claim *resourceapi.ResourceClaim) bool {
 	return claim.Status.Allocation != nil

cluster-autoscaler/simulator/dynamicresources/utils/resource_claims_test.go

@@ -25,84 +25,8 @@ import (
 	apiv1 "k8s.io/api/core/v1"
 	resourceapi "k8s.io/api/resource/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 )
 
-func TestClaimOwningPod(t *testing.T) {
-	truePtr := true
-	for _, tc := range []struct {
-		testName string
-		claim    *resourceapi.ResourceClaim
-		wantName string
-		wantUid  types.UID
-	}{
-		{
-			testName: "claim with no owners",
-			claim: &resourceapi.ResourceClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "claim", UID: "claim", Namespace: "default",
-				},
-			},
-			wantName: "",
-			wantUid:  "",
-		},
-		{
-			testName: "claim with non-Pod owners",
-			claim: &resourceapi.ResourceClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "claim", UID: "claim", Namespace: "default",
-					OwnerReferences: []metav1.OwnerReference{
-						{Name: "owner1", UID: "owner1uid", APIVersion: "v1", Kind: "ReplicationController", Controller: &truePtr},
-						{Name: "owner2", UID: "owner2uid", APIVersion: "v1", Kind: "ConfigMap"},
-					},
-				},
-			},
-			wantName: "",
-			wantUid:  "",
-		},
-		{
-			testName: "claim with a Pod non-controller owner",
-			claim: &resourceapi.ResourceClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "claim", UID: "claim", Namespace: "default",
-					OwnerReferences: []metav1.OwnerReference{
-						{Name: "owner1", UID: "owner1uid", APIVersion: "v1", Kind: "ReplicationController"},
-						{Name: "owner2", UID: "owner2uid", APIVersion: "v1", Kind: "ConfigMap"},
-						{Name: "owner3", UID: "owner3uid", APIVersion: "v1", Kind: "Pod"},
-					},
-				},
-			},
-			wantName: "",
-			wantUid:  "",
-		},
-		{
-			testName: "claim with a Pod controller owner",
-			claim: &resourceapi.ResourceClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "claim", UID: "claim", Namespace: "default",
-					OwnerReferences: []metav1.OwnerReference{
-						{Name: "owner1", UID: "owner1uid", APIVersion: "v1", Kind: "ReplicationController"},
-						{Name: "owner2", UID: "owner2uid", APIVersion: "v1", Kind: "ConfigMap"},
-						{Name: "owner3", UID: "owner3uid", APIVersion: "v1", Kind: "Pod", Controller: &truePtr},
-					},
-				},
-			},
-			wantName: "owner3",
-			wantUid:  "owner3uid",
-		},
-	} {
-		t.Run(tc.testName, func(t *testing.T) {
-			name, uid := ClaimOwningPod(tc.claim)
-			if tc.wantName != name {
-				t.Errorf("ClaimOwningPod(): unexpected output name: want %s, got %s", tc.wantName, name)
-			}
-			if tc.wantUid != uid {
-				t.Errorf("ClaimOwningPod(): unexpected output UID: want %v, got %v", tc.wantUid, uid)
-			}
-		})
-	}
-}
-
 func TestClaimAllocated(t *testing.T) {
 	for _, tc := range []struct {
 		testName      string

cluster-autoscaler/simulator/dynamicresources/utils/sanitize.go

@@ -23,6 +23,7 @@ import (
 	resourceapi "k8s.io/api/resource/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/dynamic-resource-allocation/resourceclaim"
 	"k8s.io/utils/set"
 )
 
@@ -61,7 +62,7 @@ func SanitizedNodeResourceSlices(nodeLocalSlices []*resourceapi.ResourceSlice, n
 func SanitizedPodResourceClaims(newOwner, oldOwner *v1.Pod, claims []*resourceapi.ResourceClaim, nameSuffix, newNodeName, oldNodeName string, oldNodePoolNames set.Set[string]) ([]*resourceapi.ResourceClaim, error) {
 	var result []*resourceapi.ResourceClaim
 	for _, claim := range claims {
-		if ownerName, ownerUid := ClaimOwningPod(claim); ownerName != oldOwner.Name || ownerUid != oldOwner.UID {
+		if err := resourceclaim.IsForPod(oldOwner, claim); err != nil {
 			// Only claims owned by the pod are bound to its lifecycle. The lifecycle of other claims is independent, and they're most likely shared
 			// by multiple pods. They shouldn't be sanitized or duplicated - just add unchanged to the result.
 			result = append(result, claim)

cluster-autoscaler/simulator/node_info_utils_test.go

@@ -40,6 +40,7 @@ import (
 	"k8s.io/autoscaler/cluster-autoscaler/utils/labels"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/taints"
 	. "k8s.io/autoscaler/cluster-autoscaler/utils/test"
+	"k8s.io/dynamic-resource-allocation/resourceclaim"
 )
 
 var (
@@ -601,7 +602,7 @@ func verifySanitizedPods(initialPods, sanitizedPods []*framework.PodInfo, wantNo
 			return fmt.Errorf("sanitized Pod unexpected diff (-want +got): %s", diff)
 		}
 
-		if err := verifySanitizedPodResourceClaims(initialPod.NeededResourceClaims, sanitizedPod.NeededResourceClaims, nameSuffix); err != nil {
+		if err := verifySanitizedPodResourceClaims(initialPod, sanitizedPod, nameSuffix); err != nil {
 			return err
 		}
 	}
@@ -633,7 +634,11 @@ func verifySanitizedNodeResourceSlices(initialSlices, sanitizedSlices []*resourc
 	return nil
 }
 
-func verifySanitizedPodResourceClaims(initialClaims, sanitizedClaims []*resourceapi.ResourceClaim, nameSuffix string) error {
+func verifySanitizedPodResourceClaims(initialPod, sanitizedPod *framework.PodInfo, nameSuffix string) error {
+	initialClaims := initialPod.NeededResourceClaims
+	sanitizedClaims := sanitizedPod.NeededResourceClaims
+	owningPod := initialPod.Pod
+
 	if len(initialClaims) != len(sanitizedClaims) {
 		return fmt.Errorf("want %d NeededResourceClaims in sanitized NodeInfo, got %d", len(initialClaims), len(sanitizedClaims))
 	}
@@ -642,7 +647,9 @@ func verifySanitizedPodResourceClaims(initialClaims, sanitizedClaims []*resource
 		initialClaim := initialClaims[i]
 
 		// Pod-owned claims should be sanitized, other claims shouldn't.
-		if owningPod, _ := drautils.ClaimOwningPod(initialClaim); owningPod != "" {
+		err := resourceclaim.IsForPod(owningPod, initialClaim)
+		isPodOwned := err == nil
+		if isPodOwned {
 			// Pod-owned claim, verify that it was sanitized.
 			if sanitizedClaim.Name == initialClaim.Name || !strings.HasSuffix(sanitizedClaim.Name, nameSuffix) {
 				return fmt.Errorf("sanitized ResourceClaim name unexpected: want (different than %q, ending in %q), got %q", initialClaim.Name, nameSuffix, sanitizedClaim.Name)