From aa02b7a434f919e4522fd7f993c80e14399a039e Mon Sep 17 00:00:00 2001 From: Rob Scott Date: Fri, 27 May 2022 21:10:59 +0000 Subject: [PATCH] Updating e2e test to check EndpointSlices and Endpoints as well --- .../endpoint/endpoints_controller.go | 4 +-- pkg/controller/endpointslice/reconciler.go | 2 +- .../util/endpoint/controller_utils.go | 8 +++-- .../util/endpoint/controller_utils_test.go | 4 +-- test/e2e/network/service.go | 29 +++++++++++++++++++ 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go index d0be95b4b7f..61df9eac35a 100644 --- a/pkg/controller/endpoint/endpoints_controller.go +++ b/pkg/controller/endpoint/endpoints_controller.go @@ -413,7 +413,7 @@ func (e *Controller) syncService(ctx context.Context, key string) error { var totalNotReadyEps int for _, pod := range pods { - if !endpointutil.ShouldPodBeInEndpointSlice(pod, service.Spec.PublishNotReadyAddresses) { + if !endpointutil.ShouldPodBeInEndpoints(pod, service.Spec.PublishNotReadyAddresses) { klog.V(5).Infof("Pod %s/%s is not included on endpoints for Service %s/%s", pod.Namespace, pod.Name, service.Namespace, service.Name) continue } @@ -587,7 +587,7 @@ func (e *Controller) checkLeftoverEndpoints() { // addEndpointSubset add the endpoints addresses and ports to the EndpointSubset. // The addresses are added to the corresponding field, ready or not ready, depending // on the pod status and the Service PublishNotReadyAddresses field value. -// The pod passed to this function must have already been filtered through ShouldPodBeInEndpointSlice. +// The pod passed to this function must have already been filtered through ShouldPodBeInEndpoints. func addEndpointSubset(subsets []v1.EndpointSubset, pod *v1.Pod, epa v1.EndpointAddress, epp *v1.EndpointPort, tolerateUnreadyEndpoints bool) ([]v1.EndpointSubset, int, int) { var readyEps int diff --git a/pkg/controller/endpointslice/reconciler.go b/pkg/controller/endpointslice/reconciler.go index 4756a0d3448..9b91566f853 100644 --- a/pkg/controller/endpointslice/reconciler.go +++ b/pkg/controller/endpointslice/reconciler.go @@ -154,7 +154,7 @@ func (r *reconciler) reconcileByAddressType(service *corev1.Service, pods []*cor for _, pod := range pods { includeTerminating := service.Spec.PublishNotReadyAddresses || utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) - if !endpointutil.ShouldPodBeInEndpointSlice(pod, includeTerminating) { + if !endpointutil.ShouldPodBeInEndpoints(pod, includeTerminating) { continue } diff --git a/pkg/controller/util/endpoint/controller_utils.go b/pkg/controller/util/endpoint/controller_utils.go index 391c6fb59c9..addaccc9d68 100644 --- a/pkg/controller/util/endpoint/controller_utils.go +++ b/pkg/controller/util/endpoint/controller_utils.go @@ -135,15 +135,17 @@ func DeepHashObjectToString(objectToWrite interface{}) string { return hex.EncodeToString(hasher.Sum(nil)[0:]) } -// ShouldPodBeInEndpointSlice returns true if a specified pod should be in an Endpoint or EndpointSlice object. -// Terminating pods are only included if includeTerminating is true -func ShouldPodBeInEndpointSlice(pod *v1.Pod, includeTerminating bool) bool { +// ShouldPodBeInEndpoints returns true if a specified pod should be in an +// Endpoints or EndpointSlice resource. Terminating pods are only included if +// includeTerminating is true. +func ShouldPodBeInEndpoints(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 } diff --git a/pkg/controller/util/endpoint/controller_utils_test.go b/pkg/controller/util/endpoint/controller_utils_test.go index 837fb20cb41..e9656b0a8dd 100644 --- a/pkg/controller/util/endpoint/controller_utils_test.go +++ b/pkg/controller/util/endpoint/controller_utils_test.go @@ -99,7 +99,7 @@ func TestDetermineNeededServiceUpdates(t *testing.T) { } } -func TestShouldPodBeInEndpointSlice(t *testing.T) { +func TestShouldPodBeInEndpoints(t *testing.T) { testCases := []struct { name string pod *v1.Pod @@ -263,7 +263,7 @@ func TestShouldPodBeInEndpointSlice(t *testing.T) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - result := ShouldPodBeInEndpointSlice(test.pod, test.includeTerminating) + result := ShouldPodBeInEndpoints(test.pod, test.includeTerminating) if result != test.expected { t.Errorf("expected: %t, got: %t", test.expected, result) } diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index 9d032ef2866..fffef5398a2 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -2080,6 +2080,35 @@ var _ = common.SIGDescribe("Services", func() { e2epod.SetNodeSelection(&pod.Spec, nodeSelection) }) + if epErr := wait.PollImmediate(framework.Poll, e2eservice.ServiceEndpointsTimeout, func() (bool, error) { + endpoints, err := cs.CoreV1().Endpoints(namespace).Get(context.TODO(), serviceName, metav1.GetOptions{}) + if err != nil { + framework.Logf("error fetching '%s/%s' Endpoints: %s", namespace, serviceName, err.Error()) + return false, err + } + if len(endpoints.Subsets) > 0 { + framework.Logf("expected '%s/%s' Endpoints to be empty, got: %v", namespace, serviceName, endpoints.Subsets) + return false, nil + } + epsList, err := cs.DiscoveryV1().EndpointSlices(namespace).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, serviceName)}) + if err != nil { + framework.Logf("error fetching '%s/%s' EndpointSlices: %s", namespace, serviceName, err.Error()) + return false, err + } + if len(epsList.Items) != 1 { + framework.Logf("expected exactly 1 EndpointSlice, got: %d", len(epsList.Items)) + return false, nil + } + endpointSlice := epsList.Items[0] + if len(endpointSlice.Endpoints) > 0 { + framework.Logf("expected EndpointSlice to be empty, got %d endpoints", len(endpointSlice.Endpoints)) + return false, nil + } + return true, nil + }); epErr != nil { + framework.ExpectNoError(epErr) + } + serviceAddress := net.JoinHostPort(serviceName, strconv.Itoa(port)) framework.Logf("waiting up to %v to connect to %v", e2eservice.KubeProxyEndpointLagTimeout, serviceAddress) cmd := fmt.Sprintf("/agnhost connect --timeout=3s %s", serviceAddress)