fix: address review - optimize List to O(1) lookup, filter deleting claims

- Extract podClaimedByOthers loop into buildPodClaimIndex() that builds
  a map[podName]claimName once per reconcile, reducing N API calls to 1
- Skip CNClaims with DeletionTimestamp != nil to prevent Pod leak when
  both claims are being deleted simultaneously
- Add holder claim name to log messages for easier debugging
- Add test for deleting claim filtering and buildPodClaimIndex

Author: xzxiong

pkg/controllers/cnclaim/controller.go

@@ -159,13 +159,16 @@ func (r *Actor) selectCN(ctx *recon.Context[*v1alpha1.CNClaim], orphans []corev1
 	}
 
 	sortCNByPriority(c, idleCNs)
+	// build index once: podName -> claimName for all other CNClaims
+	claimIndex, err := buildPodClaimIndex(ctx, c.Namespace, c.Name)
+	if err != nil {
+		return nil, errors.WrapPrefix(err, "error building pod claim index", 0)
+	}
 	for i := range idleCNs {
 		pod := &idleCNs[i]
 		// skip pod already referenced by another CNClaim's spec.podName
-		if claimed, err := podClaimedByOthers(ctx, c.Namespace, pod.Name, c.Name); err != nil {
-			return nil, errors.WrapPrefix(err, "error checking pod claims", 0)
-		} else if claimed {
-			ctx.Log.Info("skip pod claimed by other CNClaim", "podName", pod.Name)
+		if holder, ok := claimIndex[pod.Name]; ok {
+			ctx.Log.Info("skip pod claimed by other CNClaim", "podName", pod.Name, "holder", holder)
 			continue
 		}
 		if err := r.ensureOwnership(ctx, pod); err != nil {
@@ -347,13 +350,16 @@ func (r *Actor) Finalize(ctx *recon.Context[*v1alpha1.CNClaim]) (bool, error) {
 	if len(ownedCNs) == 0 {
 		return true, nil
 	}
+	// build index once: podName -> claimName for all other CNClaims
+	claimIndex, err := buildPodClaimIndex(ctx, c.Namespace, c.Name)
+	if err != nil {
+		return false, errors.WrapPrefix(err, "error building pod claim index", 0)
+	}
 	for i := range ownedCNs {
 		cn := ownedCNs[i]
 		// skip reclaim if another CNClaim still references this pod via spec.podName
-		if claimed, err := podClaimedByOthers(ctx, c.Namespace, cn.Name, c.Name); err != nil {
-			return false, errors.WrapPrefix(err, "error checking pod claims", 0)
-		} else if claimed {
-			ctx.Log.Info("skip reclaim, pod still claimed by other CNClaim", "pod", cn.Name)
+		if holder, ok := claimIndex[cn.Name]; ok {
+			ctx.Log.Info("skip reclaim, pod still claimed by other CNClaim", "pod", cn.Name, "holder", holder)
 			continue
 		}
 		ctx.Log.Info("finalize CNClaim, reclaim bound CN", "cn", cn.Name)
@@ -366,14 +372,15 @@ func (r *Actor) Finalize(ctx *recon.Context[*v1alpha1.CNClaim]) (bool, error) {
 
 // podClaimedByOthers checks if the given pod is referenced by any CNClaim's
 // spec.podName other than excludeClaim in the same namespace.
+// It skips CNClaims that are being deleted (DeletionTimestamp != nil).
 func podClaimedByOthers(cli recon.KubeClient, namespace, podName, excludeClaim string) (bool, error) {
 	claimList := &v1alpha1.CNClaimList{}
 	if err := cli.List(claimList, client.InNamespace(namespace)); err != nil {
 		return false, err
 	}
 	for i := range claimList.Items {
 		claim := &claimList.Items[i]
-		if claim.Name == excludeClaim {
+		if claim.Name == excludeClaim || claim.DeletionTimestamp != nil {
 			continue
 		}
 		if claim.Spec.PodName == podName {
@@ -383,6 +390,25 @@ func podClaimedByOthers(cli recon.KubeClient, namespace, podName, excludeClaim s
 	return false, nil
 }
 
+// buildPodClaimIndex lists all CNClaims in the namespace and returns a map
+// from podName to the claiming CNClaim name, excluding the given claim and
+// CNClaims that are being deleted.
+func buildPodClaimIndex(cli recon.KubeClient, namespace, excludeClaim string) (map[string]string, error) {
+	claimList := &v1alpha1.CNClaimList{}
+	if err := cli.List(claimList, client.InNamespace(namespace)); err != nil {
+		return nil, err
+	}
+	index := make(map[string]string, len(claimList.Items))
+	for i := range claimList.Items {
+		claim := &claimList.Items[i]
+		if claim.Name == excludeClaim || claim.DeletionTimestamp != nil || claim.Spec.PodName == "" {
+			continue
+		}
+		index[claim.Spec.PodName] = claim.Name
+	}
+	return index, nil
+}
+
 func (r *Actor) patchStore(ctx *recon.Context[*v1alpha1.CNClaim], pod *corev1.Pod, req logpb.CNStateLabel) (*metadata.CNService, error) {
 	cs, err := common.ResolveCNSet(ctx, pod)
 	if err != nil {

pkg/controllers/cnclaim/controller_test.go

@@ -141,6 +141,23 @@ func Test_podClaimedByOthers(t *testing.T) {
 			excludeClaim: "claim-a",
 			want:         false,
 		},
+		{
+			name: "deleting claim is ignored",
+			claims: []client.Object{
+				&v1alpha1.CNClaim{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "claim-deleting",
+						Namespace:         "ns",
+						DeletionTimestamp: &metav1.Time{Time: metav1.Now().Time},
+						Finalizers:        []string{"test"},
+					},
+					Spec: v1alpha1.CNClaimSpec{ClaimPodRef: v1alpha1.ClaimPodRef{PodName: "pod-1"}},
+				},
+			},
+			podName:      "pod-1",
+			excludeClaim: "claim-a",
+			want:         false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -153,6 +170,34 @@ func Test_podClaimedByOthers(t *testing.T) {
 	}
 }
 
+func Test_buildPodClaimIndex(t *testing.T) {
+	g := NewGomegaWithT(t)
+	now := metav1.Now()
+	cli := newFakeClient(
+		&v1alpha1.CNClaim{
+			ObjectMeta: metav1.ObjectMeta{Name: "self", Namespace: "ns"},
+			Spec:       v1alpha1.CNClaimSpec{ClaimPodRef: v1alpha1.ClaimPodRef{PodName: "pod-1"}},
+		},
+		&v1alpha1.CNClaim{
+			ObjectMeta: metav1.ObjectMeta{Name: "other", Namespace: "ns"},
+			Spec:       v1alpha1.CNClaimSpec{ClaimPodRef: v1alpha1.ClaimPodRef{PodName: "pod-2"}},
+		},
+		&v1alpha1.CNClaim{
+			ObjectMeta: metav1.ObjectMeta{Name: "deleting", Namespace: "ns",
+				DeletionTimestamp: &now, Finalizers: []string{"test"}},
+			Spec: v1alpha1.CNClaimSpec{ClaimPodRef: v1alpha1.ClaimPodRef{PodName: "pod-3"}},
+		},
+		&v1alpha1.CNClaim{
+			ObjectMeta: metav1.ObjectMeta{Name: "pending", Namespace: "ns"},
+			Spec:       v1alpha1.CNClaimSpec{},
+		},
+	)
+	index, err := buildPodClaimIndex(&fakeKubeClient{cli}, "ns", "self")
+	g.Expect(err).NotTo(HaveOccurred())
+	// "self" excluded, "deleting" filtered, "pending" has no podName
+	g.Expect(index).To(Equal(map[string]string{"pod-2": "other"}))
+}
+
 func Test_sortCNByPriority(t *testing.T) {
 	tests := []struct {
 		name  string