Fix issue where PolicyEndpoints are not updated correctly when Pods are in a terminal phase (Succeeded or Failed).

Author: Subzidion

internal/eventhandlers/pod.go

@@ -65,7 +65,8 @@ func (h *enqueueRequestForPodEvent) Update(ctx context.Context, e event.UpdateEv
 	if equality.Semantic.DeepEqual(podOld.Annotations, podNew.Annotations) &&
 		equality.Semantic.DeepEqual(podOld.Labels, podNew.Labels) &&
 		equality.Semantic.DeepEqual(podOld.DeletionTimestamp.IsZero(), podNew.DeletionTimestamp.IsZero()) &&
-		equality.Semantic.DeepEqual(podOld.Status.PodIP, podNew.Status.PodIP) {
+		equality.Semantic.DeepEqual(podOld.Status.PodIP, podNew.Status.PodIP) &&
+		equality.Semantic.DeepEqual(podOld.Status.Phase, podNew.Status.Phase) {
 		return
 	}
 	h.enqueueReferredPolicies(ctx, q, podNew, podOld)

pkg/k8s/pod_utils.go

@@ -20,6 +20,13 @@ func GetPodIP(pod *corev1.Pod) string {
 	}
 }
 
+// IsPodNetworkReady returns true if the pod has at least 1 IP address and is in a running state
+func IsPodNetworkReady(pod *corev1.Pod) bool {
+	return len(GetPodIP(pod)) > 0 &&
+		pod.Status.Phase != corev1.PodSucceeded &&
+		pod.Status.Phase != corev1.PodFailed
+}
+
 // LookupContainerPortAndName returns numerical containerPort and portName for specific port and protocol
 func LookupContainerPortAndName(pod *corev1.Pod, port intstr.IntOrString, protocol corev1.Protocol) (int32, string, error) {
 	for _, podContainer := range pod.Spec.Containers {

pkg/k8s/pod_utils_test.go

@@ -185,3 +185,57 @@ func Test_LookupContainerPortAndName(t *testing.T) {
 		})
 	}
 }
+func TestIsPodNetworkReady(t *testing.T) {
+	tests := []struct {
+		name     string
+		pod      *corev1.Pod
+		expected bool
+	}{
+		{
+			name: "running pod with IP",
+			pod: &corev1.Pod{
+				Status: corev1.PodStatus{
+					PodIP: "10.0.0.1",
+					Phase: corev1.PodRunning,
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "succeeded pod with IP should be excluded",
+			pod: &corev1.Pod{
+				Status: corev1.PodStatus{
+					PodIP: "10.0.0.1",
+					Phase: corev1.PodSucceeded,
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "failed pod with IP should be excluded",
+			pod: &corev1.Pod{
+				Status: corev1.PodStatus{
+					PodIP: "10.0.0.1",
+					Phase: corev1.PodFailed,
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "running pod without IP",
+			pod: &corev1.Pod{
+				Status: corev1.PodStatus{
+					Phase: corev1.PodRunning,
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsPodNetworkReady(tt.pod)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

pkg/resolvers/endpoints.go

@@ -116,8 +116,8 @@ func (r *defaultEndpointsResolver) computePodSelectorEndpoints(ctx context.Conte
 		return nil, err
 	}
 	for _, pod := range podList.Items {
-		podIP := k8s.GetPodIP(&pod)
-		if len(podIP) > 0 {
+		if k8s.IsPodNetworkReady(&pod) {
+			podIP := k8s.GetPodIP(&pod)
 			podEndpoints = append(podEndpoints, policyinfo.PodEndpoint{
 				PodIP:     policyinfo.NetworkAddress(podIP),
 				HostIP:    policyinfo.NetworkAddress(pod.Status.HostIP),
@@ -224,6 +224,9 @@ func (r *defaultEndpointsResolver) getIngressRulesPorts(ctx context.Context, pol
 	r.logger.V(2).Info("list pods for ingress", "podList", *podList, "namespace", policyNamespace, "selector", *policyPodSelector)
 	var portList []policyinfo.Port
 	for _, pod := range podList.Items {
+		if !k8s.IsPodNetworkReady(&pod) {
+			continue
+		}
 		portList = append(portList, r.getPortList(pod, ports)...)
 		r.logger.V(1).Info("Got ingress port from pod", "pod", types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name}.String())
 	}
@@ -349,10 +352,10 @@ func (r *defaultEndpointsResolver) getMatchingPodAddresses(ctx context.Context,
 	r.logger.V(1).Info("Got pods for label selector", "count", len(podList.Items), "selector", ls.String())
 
 	for _, pod := range podList.Items {
-		podIP := k8s.GetPodIP(&pod)
-		if len(podIP) == 0 {
+		if !k8s.IsPodNetworkReady(&pod) {
 			continue
 		}
+		podIP := k8s.GetPodIP(&pod)
 
 		if policyType == networking.PolicyTypeEgress {
 			portList = r.getPortList(pod, rulePorts)

pkg/resolvers/endpoints_test.go

@@ -1101,3 +1101,78 @@ func TestEndpointsResolver_ResolveNetworkPeers_NamedIngressPortsIPBlocks(t *test
 		assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[1].Port.StrVal], *ingPE.Ports[1].Port)
 	}
 }
+
+func TestEndpointsResolver_ExcludesTerminalPods(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockClient := mock_client.NewMockClient(ctrl)
+	resolver := NewEndpointsResolver(mockClient, logr.New(&log.NullLogSink{}))
+
+	// Create pods in different phases
+	runningPod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "running-pod",
+			Namespace: "test-ns",
+			Labels:    map[string]string{"app": "test"},
+		},
+		Status: corev1.PodStatus{
+			PodIP: "10.0.0.1",
+			Phase: corev1.PodRunning,
+		},
+	}
+
+	succeededPod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "succeeded-pod",
+			Namespace: "test-ns",
+			Labels:    map[string]string{"app": "test"},
+		},
+		Status: corev1.PodStatus{
+			PodIP: "10.0.0.2",
+			Phase: corev1.PodSucceeded,
+		},
+	}
+
+	failedPod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "failed-pod",
+			Namespace: "test-ns",
+			Labels:    map[string]string{"app": "test"},
+		},
+		Status: corev1.PodStatus{
+			PodIP: "10.0.0.3",
+			Phase: corev1.PodFailed,
+		},
+	}
+
+	podList := &corev1.PodList{
+		Items: []corev1.Pod{*runningPod, *succeededPod, *failedPod},
+	}
+
+	// Mock the List call for pod selector endpoints
+	mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).
+		DoAndReturn(func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
+			list.(*corev1.PodList).Items = podList.Items
+			return nil
+		})
+
+	policy := &networking.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-policy",
+			Namespace: "test-ns",
+		},
+		Spec: networking.NetworkPolicySpec{
+			PodSelector: metav1.LabelSelector{
+				MatchLabels: map[string]string{"app": "test"},
+			},
+		},
+	}
+
+	_, _, podEndpoints, err := resolver.Resolve(context.Background(), policy)
+
+	assert.NoError(t, err)
+	assert.Len(t, podEndpoints, 1, "Should only include running pod in PolicyEndpoints")
+	assert.Equal(t, "10.0.0.1", string(podEndpoints[0].PodIP))
+	assert.Equal(t, "running-pod", podEndpoints[0].Name)
+}