diff --git a/pkg/controller/egress/controller.go b/pkg/controller/egress/controller.go index 5d40662baba..efc08c1e462 100644 --- a/pkg/controller/egress/controller.go +++ b/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++ diff --git a/pkg/controller/egress/controller_test.go b/pkg/controller/egress/controller_test.go index 356774503aa..e29a9debbf9 100644 --- a/pkg/controller/egress/controller_test.go +++ b/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) diff --git a/pkg/controller/networkpolicy/networkpolicy_controller.go b/pkg/controller/networkpolicy/networkpolicy_controller.go index af1edbcf773..44153721065 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller.go @@ -1177,9 +1177,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)) @@ -1322,8 +1322,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 } diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index 2ee35d67d12..e66c3958b44 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/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{ diff --git a/pkg/util/k8s/pod.go b/pkg/util/k8s/pod.go new file mode 100644 index 00000000000..f14c2a73a56 --- /dev/null +++ b/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 +}