From 7ddf7be24cbd2a6532a0e2cb5d4023115915f656 Mon Sep 17 00:00:00 2001 From: Joel Smith Date: Tue, 22 Aug 2017 19:19:16 -0600 Subject: [PATCH] UPSTREAM: 51144: Fix unready endpoints bug introduced in #50934 --- .../endpoint/endpoints_controller.go | 15 +++++++--- .../endpoint/endpoints_controller_test.go | 29 ++++++++++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller.go b/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller.go index 0f17c4a51059..0efd748a6e3c 100644 --- a/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller.go +++ b/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller.go @@ -204,7 +204,14 @@ func podToEndpointAddress(pod *v1.Pod) *v1.EndpointAddress { }} } -func podAddressChanged(oldPod, newPod *v1.Pod) bool { +func podChanged(oldPod, newPod *v1.Pod) bool { + // If the pod's readiness has changed, the associated endpoint address + // will move from the unready endpoints set to the ready endpoints. + // So for the purposes of an endpoint, a readiness change on a pod + // means we have a changed pod. + if podutil.IsPodReady(oldPod) != podutil.IsPodReady(newPod) { + return true + } // Convert the pod to an EndpointAddress, clear inert fields, // and see if they are the same. newEndpointAddress := podToEndpointAddress(newPod) @@ -246,7 +253,7 @@ func (e *EndpointController) updatePod(old, cur interface{}) { return } - podChanged := podAddressChanged(oldPod, newPod) + podChangedFlag := podChanged(oldPod, newPod) // Check if the pod labels have changed, indicating a possibe // change in the service membership @@ -257,7 +264,7 @@ func (e *EndpointController) updatePod(old, cur interface{}) { } // If both the pod and labels are unchanged, no update is needed - if !podChanged && !labelsChanged { + if !podChangedFlag && !labelsChanged { return } @@ -273,7 +280,7 @@ func (e *EndpointController) updatePod(old, cur interface{}) { utilruntime.HandleError(fmt.Errorf("Unable to get pod %v/%v's service memberships: %v", oldPod.Namespace, oldPod.Name, err)) return } - services = determineNeededServiceUpdates(oldServices, services, podChanged) + services = determineNeededServiceUpdates(oldServices, services, podChangedFlag) } for key := range services { diff --git a/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller_test.go b/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller_test.go index b4c51a2f2a3e..7af7c41c2834 100644 --- a/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller_test.go +++ b/vendor/k8s.io/kubernetes/pkg/controller/endpoint/endpoints_controller_test.go @@ -656,7 +656,7 @@ func TestPodToEndpointAddress(t *testing.T) { } } -func TestPodAddressChanged(t *testing.T) { +func TestPodChanged(t *testing.T) { podStore := cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc) ns := "test" addPods(podStore, ns, 1, 1, 0) @@ -676,33 +676,40 @@ func TestPodAddressChanged(t *testing.T) { } newPod := copied - if podAddressChanged(oldPod, newPod) { - t.Errorf("Expected address to be unchanged for copied pod") + if podChanged(oldPod, newPod) { + t.Errorf("Expected pod to be unchanged for copied pod") } newPod.Spec.NodeName = "changed" - if !podAddressChanged(oldPod, newPod) { - t.Errorf("Expected address to be changed for pod with NodeName changed") + if !podChanged(oldPod, newPod) { + t.Errorf("Expected pod to be changed for pod with NodeName changed") } newPod.Spec.NodeName = oldPod.Spec.NodeName newPod.ObjectMeta.ResourceVersion = "changed" - if podAddressChanged(oldPod, newPod) { - t.Errorf("Expected address to be unchanged for pod with only ResourceVersion changed") + if podChanged(oldPod, newPod) { + t.Errorf("Expected pod to be unchanged for pod with only ResourceVersion changed") } newPod.ObjectMeta.ResourceVersion = oldPod.ObjectMeta.ResourceVersion newPod.Status.PodIP = "1.2.3.1" - if !podAddressChanged(oldPod, newPod) { - t.Errorf("Expected address to be changed with pod IP address change") + if !podChanged(oldPod, newPod) { + t.Errorf("Expected pod to be changed with pod IP address change") } newPod.Status.PodIP = oldPod.Status.PodIP newPod.ObjectMeta.Name = "wrong-name" - if !podAddressChanged(oldPod, newPod) { - t.Errorf("Expected address to be changed with pod name change") + if !podChanged(oldPod, newPod) { + t.Errorf("Expected pod to be changed with pod name change") } newPod.ObjectMeta.Name = oldPod.ObjectMeta.Name + + saveConditions := oldPod.Status.Conditions + oldPod.Status.Conditions = nil + if !podChanged(oldPod, newPod) { + t.Errorf("Expected pod to be changed with pod readiness change") + } + oldPod.Status.Conditions = saveConditions } func TestDetermineNeededServiceUpdates(t *testing.T) {