Remove SubnetPort when Pod is in terminating state (#1441)

Previously, the logic ignored Pods in a terminating state to allow for graceful
shutdown. However, if a Namespace (NS) or StatefulSet (STS) is deleted and
immediately recreated with the same name, the lingering terminating Pods can
interfere with garbage collection (GC) or cause resource conflicts.

This change ensures that the SubnetPort is promptly removed when a Pod enters
the terminating state, preventing GC failures during rapid delete-and-recreate
scenarios.

Check the pod sts ID to avoid pod belonging to the new sts

Author: TaoZou1

pkg/controllers/common/utils.go

@@ -614,5 +614,6 @@ func PodIsDeleted(pod *v1.Pod) bool {
 		return false
 	}
 	return pod.Status.Phase == v1.PodSucceeded ||
-		pod.Status.Phase == v1.PodFailed
+		pod.Status.Phase == v1.PodFailed ||
+		!pod.ObjectMeta.DeletionTimestamp.IsZero()
 }

pkg/controllers/common/utils_test.go

@@ -1164,7 +1164,7 @@ func TestPodIsDeleted(t *testing.T) {
 		{"terminating_running", &v1.Pod{
 			ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now, Finalizers: []string{"x"}},
 			Status:     v1.PodStatus{Phase: v1.PodRunning},
-		}, false},
+		}, true},
 		{"succeeded", &v1.Pod{Status: v1.PodStatus{Phase: v1.PodSucceeded}}, true},
 		{"failed", &v1.Pod{Status: v1.PodStatus{Phase: v1.PodFailed}}, true},
 	}

pkg/controllers/statefulset/statefulset_controller.go

@@ -322,8 +322,10 @@ func (r *StatefulSetReconciler) CollectGarbage(ctx context.Context) error {
 			if podName != "" && namespace != "" {
 				pod := &corev1.Pod{}
 				if err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: podName}, pod); err == nil && !common.PodIsDeleted(pod) {
-					log.Debug("GC: pod still exists, skipping orphaned port deletion", "pod", podName)
-					continue
+					if isPodBelongToStatefulSet(pod, types.UID(stsID)) {
+						log.Debug("GC: pod still exists and belongs to StatefulSet, skipping orphaned port deletion", "pod", podName)
+						continue
+					}
 				}
 			}
 			log.Debug("Found orphaned subnet port for deleted StatefulSet", "port", *port.Id, "stsUID", stsID)
@@ -339,6 +341,15 @@ func (r *StatefulSetReconciler) CollectGarbage(ctx context.Context) error {
 	return nil
 }
 
+func isPodBelongToStatefulSet(pod *corev1.Pod, stsID types.UID) bool {
+	for _, owner := range pod.OwnerReferences {
+		if owner.Kind == "StatefulSet" && owner.UID == stsID {
+			return true
+		}
+	}
+	return false
+}
+
 func (r *StatefulSetReconciler) GetOrdinalRange(sts *appsv1.StatefulSet) (int, int) {
 	start := 0
 	if sts.Spec.Ordinals != nil {

pkg/controllers/statefulset/statefulset_controller_test.go

@@ -1235,8 +1235,8 @@ func TestReleaseSubnetPortsForStatefulSet_PodTerminatingSkipsDeleteWithMatchingU
 
 	pending, err := r.releaseSubnetPortsForStatefulSet(context.Background(), "default", "test-sts")
 	assert.NoError(t, err)
-	assert.True(t, pending, "terminating-but-not-terminal pod should defer delete and surface pending for requeue")
-	assert.Equal(t, 0, deleteCalls, "PodIsDeleted is terminal phase only: Running+DeletionTimestamp must not release port when UID still matches")
+	assert.False(t, pending, "terminating-but-not-terminal pod should defer delete and surface pending for requeue")
+	assert.Equal(t, 1, deleteCalls, "PodIsDeleted is terminal phase only: Running+DeletionTimestamp must not release port when UID still matches")
 }
 
 func TestReleaseSubnetPortsForStatefulSet_PodUIDMismatchDeletesPort(t *testing.T) {
@@ -2077,3 +2077,169 @@ func TestCollectGarbage_GetPodErrorDoesNotSkipDelete(t *testing.T) {
 	require.NoError(t, r.CollectGarbage(context.Background()))
 	assert.GreaterOrEqual(t, deleteCalls, 1)
 }
+
+func TestCollectGarbage_OrphanedPortPodBelongsToSts_SkipsDelete(t *testing.T) {
+	// Setup: orphaned port (stsID not in statefulSetUIDs), pod existed and ownerReference matches stsID
+	fakeClient := fake.NewClientBuilder().WithObjects(&corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-pod-0",
+			Namespace: "default",
+			OwnerReferences: []metav1.OwnerReference{{
+				Kind: "StatefulSet",
+				UID:  "deleted-sts-uid",
+			}},
+		},
+	}).Build()
+	subnetPortService := &subnetportservice.SubnetPortService{
+		Service: servicecommon.Service{
+			NSXClient: &nsx.Client{},
+			NSXConfig: testNSXConfigWithStatefulSetPodEnhance(),
+		},
+		SubnetPortStore: &subnetportservice.SubnetPortStore{},
+	}
+
+	r := &StatefulSetReconciler{
+		Client:            fakeClient,
+		SubnetPortService: subnetPortService,
+	}
+	defer patchNSXClientStatefulSetPodVersion(t, true)()
+
+	// Patch GetByIndex: only orphaned port
+	patches := gomonkey.ApplyFunc(
+		(*subnetportservice.SubnetPortStore).GetByIndex,
+		func(s *subnetportservice.SubnetPortStore, indexKey string, indexValue string) []*model.VpcSubnetPort {
+			if indexKey == servicecommon.IndexKeyAllStsPorts {
+				stsUIDScope := "nsx-op/sts_uid"
+				nsScope := "nsx-op/namespace"
+				podNameScope := "nsx-op/pod_name"
+				return []*model.VpcSubnetPort{
+					{Id: servicecommon.String("port-orphan"),
+						Tags: []model.Tag{
+							{Scope: &stsUIDScope, Tag: servicecommon.String("deleted-sts-uid")},
+							{Scope: &nsScope, Tag: servicecommon.String("default")},
+							{Scope: &podNameScope, Tag: servicecommon.String("test-pod-0")},
+						}},
+				}
+			}
+			if indexKey == servicecommon.TagScopeStatefulSetUID {
+				return []*model.VpcSubnetPort{}
+			}
+			return []*model.VpcSubnetPort{}
+		})
+	var deleteCalls int
+	patches.ApplyFunc(
+		(*subnetportservice.SubnetPortService).DeleteSubnetPort,
+		func(s *subnetportservice.SubnetPortService, port *model.VpcSubnetPort) error {
+			deleteCalls++
+			return nil
+		})
+	defer patches.Reset()
+
+	err := r.CollectGarbage(context.Background())
+	assert.NoError(t, err)
+	assert.Equal(t, 0, deleteCalls, "orphaned port should not be deleted when pod belongs to the deleted sts")
+}
+
+func Test_isPodBelongToStatefulSet(t *testing.T) {
+	targetStsUID := types.UID("sts-uid-111")
+	wrongStsUID := types.UID("sts-uid-222")
+
+	tests := []struct {
+		name     string
+		pod      *corev1.Pod
+		stsUID   types.UID
+		expected bool
+	}{
+		{
+			name: "pod owner matches correct StatefulSet",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Kind:       "StatefulSet",
+							Name:       "my-sts",
+							APIVersion: "apps/v1",
+							UID:        targetStsUID,
+						},
+					},
+				},
+			},
+			stsUID:   targetStsUID,
+			expected: true,
+		},
+		{
+			name: "pod owner is StatefulSet but UID mismatches",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Kind:       "StatefulSet",
+							Name:       "my-sts",
+							APIVersion: "apps/v1",
+							UID:        wrongStsUID,
+						},
+					},
+				},
+			},
+			stsUID:   targetStsUID,
+			expected: false,
+		},
+		{
+			name: "pod owner UID matches but Kind is not StatefulSet",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Kind:       "Deployment",
+							Name:       "my-deploy",
+							APIVersion: "apps/v1",
+							UID:        targetStsUID,
+						},
+					},
+				},
+			},
+			stsUID:   targetStsUID,
+			expected: false,
+		},
+		{
+			name: "pod has multiple owners and one matches",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Kind:       "ReplicaSet",
+							Name:       "my-rs",
+							APIVersion: "apps/v1",
+							UID:        "rs-uid",
+						},
+						{
+							Kind:       "StatefulSet",
+							Name:       "my-sts",
+							APIVersion: "apps/v1",
+							UID:        targetStsUID,
+						},
+					},
+				},
+			},
+			stsUID:   targetStsUID,
+			expected: true,
+		},
+		{
+			name: "pod has no owner references",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					OwnerReferences: nil,
+				},
+			},
+			stsUID:   targetStsUID,
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := isPodBelongToStatefulSet(tt.pod, tt.stsUID)
+			assert.Equal(t, tt.expected, actual)
+		})
+	}
+}