diff --git a/pkg/controller/endpointslice/reconciler.go b/pkg/controller/endpointslice/reconciler.go index 9055b52c14a..bd1d0c5653c 100644 --- a/pkg/controller/endpointslice/reconciler.go +++ b/pkg/controller/endpointslice/reconciler.go @@ -86,30 +86,32 @@ func (r *reconciler) reconcile(service *corev1.Service, pods []*corev1.Pod, exis numDesiredEndpoints := 0 for _, pod := range pods { - if endpointutil.ShouldPodBeInEndpoints(pod) { - endpointPorts := getEndpointPorts(service, pod) - epHash := endpointutil.NewPortMapKey(endpointPorts) - if _, ok := desiredEndpointsByPortMap[epHash]; !ok { - desiredEndpointsByPortMap[epHash] = endpointSet{} - } + if !endpointutil.ShouldPodBeInEndpoints(pod, service.Spec.PublishNotReadyAddresses) { + continue + } - if _, ok := desiredMetaByPortMap[epHash]; !ok { - desiredMetaByPortMap[epHash] = &endpointMeta{ - AddressType: addressType, - Ports: endpointPorts, - } - } + endpointPorts := getEndpointPorts(service, pod) + epHash := endpointutil.NewPortMapKey(endpointPorts) + if _, ok := desiredEndpointsByPortMap[epHash]; !ok { + desiredEndpointsByPortMap[epHash] = endpointSet{} + } - node, err := r.nodeLister.Get(pod.Spec.NodeName) - if err != nil { - return err - } - endpoint := podToEndpoint(pod, node, service) - if len(endpoint.Addresses) > 0 { - desiredEndpointsByPortMap[epHash].Insert(&endpoint) - numDesiredEndpoints++ + if _, ok := desiredMetaByPortMap[epHash]; !ok { + desiredMetaByPortMap[epHash] = &endpointMeta{ + AddressType: addressType, + Ports: endpointPorts, } } + + node, err := r.nodeLister.Get(pod.Spec.NodeName) + if err != nil { + return err + } + endpoint := podToEndpoint(pod, node, service) + if len(endpoint.Addresses) > 0 { + desiredEndpointsByPortMap[epHash].Insert(&endpoint) + numDesiredEndpoints++ + } } spMetrics := metrics.NewServicePortCache() diff --git a/pkg/controller/util/endpoint/controller_utils.go b/pkg/controller/util/endpoint/controller_utils.go index 4d597ba66aa..e38e65668d7 100644 --- a/pkg/controller/util/endpoint/controller_utils.go +++ b/pkg/controller/util/endpoint/controller_utils.go @@ -126,12 +126,16 @@ func DeepHashObjectToString(objectToWrite interface{}) string { } // ShouldPodBeInEndpoints returns true if a specified pod should be in an -// endpoints object. -func ShouldPodBeInEndpoints(pod *v1.Pod) bool { +// endpoints object. Terminating pods are only included if publishNotReady is true. +func ShouldPodBeInEndpoints(pod *v1.Pod, publishNotReady bool) bool { if len(pod.Status.PodIP) == 0 && len(pod.Status.PodIPs) == 0 { return false } + if !publishNotReady && pod.DeletionTimestamp != nil { + return false + } + if pod.Spec.RestartPolicy == v1.RestartPolicyNever { return pod.Status.Phase != v1.PodFailed && pod.Status.Phase != v1.PodSucceeded } diff --git a/pkg/controller/util/endpoint/controller_utils_test.go b/pkg/controller/util/endpoint/controller_utils_test.go index 90ce9aec2ba..474842b43da 100644 --- a/pkg/controller/util/endpoint/controller_utils_test.go +++ b/pkg/controller/util/endpoint/controller_utils_test.go @@ -102,9 +102,10 @@ func TestDetermineNeededServiceUpdates(t *testing.T) { // 12 true cases. func TestShouldPodBeInEndpoints(t *testing.T) { testCases := []struct { - name string - pod *v1.Pod - expected bool + name string + pod *v1.Pod + publishNotReady bool + expected bool }{ // Pod should not be in endpoints: { @@ -160,6 +161,23 @@ func TestShouldPodBeInEndpoints(t *testing.T) { }, expected: false, }, + { + name: "Terminating Pod", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: &metav1.Time{ + Time: time.Now(), + }, + }, + Spec: v1.PodSpec{}, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + PodIP: "1.2.3.4", + }, + }, + publishNotReady: false, + expected: false, + }, // Pod should be in endpoints: { name: "Failed pod with Always RestartPolicy", @@ -226,11 +244,28 @@ func TestShouldPodBeInEndpoints(t *testing.T) { }, expected: true, }, + { + name: "Terminating Pod with publish not ready", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: &metav1.Time{ + Time: time.Now(), + }, + }, + Spec: v1.PodSpec{}, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + PodIP: "1.2.3.4", + }, + }, + publishNotReady: true, + expected: true, + }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - result := ShouldPodBeInEndpoints(test.pod) + result := ShouldPodBeInEndpoints(test.pod, test.publishNotReady) if result != test.expected { t.Errorf("expected: %t, got: %t", test.expected, result) }