From b905c2870b726040672247272e69603433a2af55 Mon Sep 17 00:00:00 2001
From: Antonio Ojea <antonio.ojea.garcia@gmail.com>
Date: Wed, 18 May 2022 17:06:11 +0200
Subject: [PATCH] endpointslices: terminal pods doesn't receive enpoints

---
 pkg/controller/util/endpoint/controller_utils.go   | 14 ++++++--------
 .../util/endpoint/controller_utils_test.go         |  7 ++-----
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/pkg/controller/util/endpoint/controller_utils.go b/pkg/controller/util/endpoint/controller_utils.go
index 58c38b3c3c1..411218c5f3b 100644
--- a/pkg/controller/util/endpoint/controller_utils.go
+++ b/pkg/controller/util/endpoint/controller_utils.go
@@ -138,6 +138,12 @@ func DeepHashObjectToString(objectToWrite interface{}) string {
 // ShouldPodBeInEndpointSlice returns true if a specified pod should be in an EndpointSlice object.
 // Terminating pods are only included if includeTerminating is true
 func ShouldPodBeInEndpointSlice(pod *v1.Pod, includeTerminating bool) bool {
+	// "Terminal" describes when a Pod is complete (in a succeeded or failed phase).
+	// This is distinct from the "Terminating" condition which represents when a Pod
+	// is being terminated (metadata.deletionTimestamp is non nil).
+	if podutil.IsPodTerminal(pod) {
+		return false
+	}
 	if len(pod.Status.PodIP) == 0 && len(pod.Status.PodIPs) == 0 {
 		return false
 	}
@@ -146,14 +152,6 @@ func ShouldPodBeInEndpointSlice(pod *v1.Pod, includeTerminating bool) bool {
 		return false
 	}
 
-	if pod.Spec.RestartPolicy == v1.RestartPolicyNever {
-		return pod.Status.Phase != v1.PodFailed && pod.Status.Phase != v1.PodSucceeded
-	}
-
-	if pod.Spec.RestartPolicy == v1.RestartPolicyOnFailure {
-		return pod.Status.Phase != v1.PodSucceeded
-	}
-
 	return true
 }
 
diff --git a/pkg/controller/util/endpoint/controller_utils_test.go b/pkg/controller/util/endpoint/controller_utils_test.go
index bf4d199dd95..837fb20cb41 100644
--- a/pkg/controller/util/endpoint/controller_utils_test.go
+++ b/pkg/controller/util/endpoint/controller_utils_test.go
@@ -99,9 +99,6 @@ func TestDetermineNeededServiceUpdates(t *testing.T) {
 	}
 }
 
-// There are 3*5 possibilities(3 types of RestartPolicy by 5 types of PodPhase).
-// Not listing them all here. Just listing all of the 3 false cases and 3 of the
-// 12 true cases.
 func TestShouldPodBeInEndpointSlice(t *testing.T) {
 	testCases := []struct {
 		name               string
@@ -179,7 +176,6 @@ func TestShouldPodBeInEndpointSlice(t *testing.T) {
 			},
 			expected: false,
 		},
-		// Pod should be in endpoints:
 		{
 			name: "Failed pod with Always RestartPolicy",
 			pod: &v1.Pod{
@@ -191,8 +187,9 @@ func TestShouldPodBeInEndpointSlice(t *testing.T) {
 					PodIP: "1.2.3.4",
 				},
 			},
-			expected: true,
+			expected: false,
 		},
+		// Pod should be in endpoints:
 		{
 			name: "Pending pod with Never RestartPolicy",
 			pod: &v1.Pod{