Exclude terminated Pods from group members

pkg/controller/egress/controller.go

@@ -44,6 +44,7 @@ import (
 	"antrea.io/antrea/pkg/controller/externalippool"
 	"antrea.io/antrea/pkg/controller/grouping"
 	antreatypes "antrea.io/antrea/pkg/controller/types"
+	"antrea.io/antrea/pkg/util/k8s"
 )
 
 const (
@@ -371,9 +372,9 @@ func (c *EgressController) syncEgress(key string) error {
 	egressGroup := egressGroupObj.(*antreatypes.EgressGroup)
 	pods, _ := c.groupingInterface.GetEntities(egressGroupType, key)
 	for _, pod := range pods {
-		// Ignore Pod if it's not scheduled or not running. And Egress does not support HostNetwork Pods, so also ignore
+		// Ignore Pod if it's not scheduled or is already terminated. And Egress does not support HostNetwork Pods, so also ignore
 		// Pod if it's HostNetwork Pod.
-		if pod.Spec.NodeName == "" || pod.Spec.HostNetwork {
+		if pod.Spec.NodeName == "" || pod.Spec.HostNetwork || k8s.IsPodTerminated(pod) {
 			continue
 		}
 		podNum++

pkg/controller/egress/controller_test.go

@@ -203,6 +203,11 @@ func newController(objects, crdObjects []runtime.Object) *egressController {
 }
 
 func TestAddEgress(t *testing.T) {
+	podSucceeded := newPod("default", "succeeded-pod", map[string]string{"app": "foo"}, node1, "1.1.5.1", false)
+	podSucceeded.Status.Phase = v1.PodSucceeded
+	podFailed := newPod("default", "failed-pod", map[string]string{"app": "foo"}, node1, "1.1.5.2", false)
+	podFailed.Status.Phase = v1.PodFailed
+
 	tests := []struct {
 		name                 string
 		inputEgress          *v1beta1.Egress
@@ -347,7 +352,7 @@ func TestAddEgress(t *testing.T) {
 			defer close(stopCh)
 			var fakeObjects []runtime.Object
 			fakeObjects = append(fakeObjects, nsDefault, nsOther)
-			fakeObjects = append(fakeObjects, podFoo1, podFoo2, podBar1, podFoo1InOtherNamespace, podUnscheduled, podNonIP, podWithHostNetwork)
+			fakeObjects = append(fakeObjects, podFoo1, podFoo2, podBar1, podFoo1InOtherNamespace, podUnscheduled, podNonIP, podWithHostNetwork, podSucceeded, podFailed)
 			var fakeCRDObjects []runtime.Object
 			fakeCRDObjects = append(fakeCRDObjects, eipFoo1)
 			controller := newController(fakeObjects, fakeCRDObjects)

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -1184,9 +1184,9 @@ func (n *NetworkPolicyController) getMemberSetForGroupType(groupType grouping.Gr
 	groupMemberSet := controlplane.GroupMemberSet{}
 	pods, externalEntities := n.groupingInterface.GetEntities(groupType, name)
 	for _, pod := range pods {
-		// HostNetwork Pods should be excluded from group members
-		// https://github.com/antrea-io/antrea/issues/3078
-		if pod.Spec.HostNetwork == true || len(pod.Status.PodIPs) == 0 {
+		// HostNetwork Pods should be excluded from group members: https://github.com/antrea-io/antrea/issues/3078.
+		// Terminated Pods should be excluded as their IPs can be recycled and used by other Pods.
+		if pod.Spec.HostNetwork || k8s.IsPodTerminated(pod) || len(pod.Status.PodIPs) == 0 {
 			continue
 		}
 		groupMemberSet.Insert(podToGroupMember(pod, true))
@@ -1329,8 +1329,8 @@ func (n *NetworkPolicyController) syncAppliedToGroup(key string) error {
 		} else {
 			scheduledPodNum, scheduledExtEntityNum := 0, 0
 			for _, pod := range pods {
-				if pod.Spec.NodeName == "" || pod.Spec.HostNetwork == true {
-					// No need to process Pod when it's not scheduled.
+				if pod.Spec.NodeName == "" || pod.Spec.HostNetwork || k8s.IsPodTerminated(pod) {
+					// No need to process Pod when it's not scheduled or is already terminated.
 					// HostNetwork Pods will not be applied to by policies.
 					continue
 				}

pkg/controller/networkpolicy/networkpolicy_controller_test.go

@@ -614,6 +614,72 @@ func TestAddPod(t *testing.T) {
 			outAddressGroupMatch: false,
 			groupMatch:           false,
 		},
+		{
+			name: "match-all-selectors-succeeded",
+			addedPod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "podA",
+					Namespace: "nsA",
+					Labels: map[string]string{
+						"role":         "app",
+						"group":        "appliedTo",
+						"inGroup":      "inAddress",
+						"outGroup":     "outAddress",
+						"clustergroup": "yes",
+					},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name: "container-1",
+					}},
+					NodeName: "nodeA",
+				},
+				Status: corev1.PodStatus{
+					Phase: corev1.PodSucceeded,
+					PodIP: "1.2.3.4",
+					PodIPs: []corev1.PodIP{
+						{IP: "1.2.3.4"},
+					},
+				},
+			},
+			appGroupMatch:        false,
+			inAddressGroupMatch:  false,
+			outAddressGroupMatch: false,
+			groupMatch:           false,
+		},
+		{
+			name: "match-all-selectors-failed",
+			addedPod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "podA",
+					Namespace: "nsA",
+					Labels: map[string]string{
+						"role":         "app",
+						"group":        "appliedTo",
+						"inGroup":      "inAddress",
+						"outGroup":     "outAddress",
+						"clustergroup": "yes",
+					},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name: "container-1",
+					}},
+					NodeName: "nodeA",
+				},
+				Status: corev1.PodStatus{
+					Phase: corev1.PodFailed,
+					PodIP: "1.2.3.4",
+					PodIPs: []corev1.PodIP{
+						{IP: "1.2.3.4"},
+					},
+				},
+			},
+			appGroupMatch:        false,
+			inAddressGroupMatch:  false,
+			outAddressGroupMatch: false,
+			groupMatch:           false,
+		},
 		{
 			name: "match-spec-podselector-no-podip",
 			addedPod: &corev1.Pod{

pkg/util/k8s/pod.go

@@ -0,0 +1,22 @@
+// Copyright 2024 Antrea 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 k8s
+
+import v1 "k8s.io/api/core/v1"
+
+// IsPodTerminated returns true if a pod is terminated, all containers are stopped and cannot ever regress.
+func IsPodTerminated(pod *v1.Pod) bool {
+	return pod.Status.Phase == v1.PodFailed || pod.Status.Phase == v1.PodSucceeded
+}