From 906e6d4670695d6e1e91553c3ab8d24ba24781ec Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Thu, 17 Feb 2022 14:10:49 +0800 Subject: [PATCH] Stop publishing Pod ResourceVersion in Endpoints and EndpointSlice API The field is not used anywhere and its value may be stale as Endpoints and EndpointSlice won't be updated if there is only Pod ResourceVersion change.. --- .../endpoint/endpoints_controller.go | 9 +- .../endpoint/endpoints_controller_test.go | 17 +-- pkg/controller/endpointslice/utils.go | 9 +- pkg/controller/endpointslice/utils_test.go | 109 ++++++++---------- .../util/endpoint/controller_utils_test.go | 30 +++++ 5 files changed, 96 insertions(+), 78 deletions(-) diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go index fbfe40b67ed..209cad19cfe 100644 --- a/pkg/controller/endpoint/endpoints_controller.go +++ b/pkg/controller/endpoint/endpoints_controller.go @@ -255,11 +255,10 @@ func podToEndpointAddressForService(svc *v1.Service, pod *v1.Pod) (*v1.EndpointA IP: endpointIP, NodeName: &pod.Spec.NodeName, TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: pod.ObjectMeta.Namespace, - Name: pod.ObjectMeta.Name, - UID: pod.ObjectMeta.UID, - ResourceVersion: pod.ObjectMeta.ResourceVersion, + Kind: "Pod", + Namespace: pod.ObjectMeta.Namespace, + Name: pod.ObjectMeta.Name, + UID: pod.ObjectMeta.UID, }, }, nil } diff --git a/pkg/controller/endpoint/endpoints_controller_test.go b/pkg/controller/endpoint/endpoints_controller_test.go index a6a2fe5cf58..72b7f6813dd 100644 --- a/pkg/controller/endpoint/endpoints_controller_test.go +++ b/pkg/controller/endpoint/endpoints_controller_test.go @@ -63,9 +63,10 @@ func testPod(namespace string, id int, nPorts int, isReady bool, ipFamilies []v1 p := &v1.Pod{ TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: fmt.Sprintf("pod%d", id), - Labels: map[string]string{"foo": "bar"}, + Namespace: namespace, + Name: fmt.Sprintf("pod%d", id), + Labels: map[string]string{"foo": "bar"}, + ResourceVersion: fmt.Sprint(id), }, Spec: v1.PodSpec{ Containers: []v1.Container{{Ports: []v1.ContainerPort{}}}, @@ -1430,16 +1431,16 @@ func TestPodToEndpointAddressForService(t *testing.T) { t.Fatalf("TargetRef.Kind: expected: %s, got: %s", "Pod", epa.TargetRef.Kind) } if epa.TargetRef.Namespace != pod.ObjectMeta.Namespace { - t.Fatalf("TargetRef.Kind: expected: %s, got: %s", pod.ObjectMeta.Namespace, epa.TargetRef.Namespace) + t.Fatalf("TargetRef.Namespace: expected: %s, got: %s", pod.ObjectMeta.Namespace, epa.TargetRef.Namespace) } if epa.TargetRef.Name != pod.ObjectMeta.Name { - t.Fatalf("TargetRef.Kind: expected: %s, got: %s", pod.ObjectMeta.Name, epa.TargetRef.Name) + t.Fatalf("TargetRef.Name: expected: %s, got: %s", pod.ObjectMeta.Name, epa.TargetRef.Name) } if epa.TargetRef.UID != pod.ObjectMeta.UID { - t.Fatalf("TargetRef.Kind: expected: %s, got: %s", pod.ObjectMeta.UID, epa.TargetRef.UID) + t.Fatalf("TargetRef.UID: expected: %s, got: %s", pod.ObjectMeta.UID, epa.TargetRef.UID) } - if epa.TargetRef.ResourceVersion != pod.ObjectMeta.ResourceVersion { - t.Fatalf("TargetRef.Kind: expected: %s, got: %s", pod.ObjectMeta.ResourceVersion, epa.TargetRef.ResourceVersion) + if epa.TargetRef.ResourceVersion != "" { + t.Fatalf("TargetRef.ResourceVersion: expected empty, got: %s", epa.TargetRef.ResourceVersion) } }) } diff --git a/pkg/controller/endpointslice/utils.go b/pkg/controller/endpointslice/utils.go index 3344e2ab4a4..ab347eff77d 100644 --- a/pkg/controller/endpointslice/utils.go +++ b/pkg/controller/endpointslice/utils.go @@ -53,11 +53,10 @@ func podToEndpoint(pod *corev1.Pod, node *corev1.Node, service *corev1.Service, Ready: &ready, }, TargetRef: &corev1.ObjectReference{ - Kind: "Pod", - Namespace: pod.ObjectMeta.Namespace, - Name: pod.ObjectMeta.Name, - UID: pod.ObjectMeta.UID, - ResourceVersion: pod.ObjectMeta.ResourceVersion, + Kind: "Pod", + Namespace: pod.ObjectMeta.Namespace, + Name: pod.ObjectMeta.Name, + UID: pod.ObjectMeta.UID, }, } diff --git a/pkg/controller/endpointslice/utils_test.go b/pkg/controller/endpointslice/utils_test.go index f5f2b26bae8..22c70964891 100644 --- a/pkg/controller/endpointslice/utils_test.go +++ b/pkg/controller/endpointslice/utils_test.go @@ -262,11 +262,10 @@ func TestPodToEndpoint(t *testing.T) { Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, }, @@ -279,11 +278,10 @@ func TestPodToEndpoint(t *testing.T) { Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, }, @@ -296,11 +294,10 @@ func TestPodToEndpoint(t *testing.T) { Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(false)}, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, }, @@ -313,11 +310,10 @@ func TestPodToEndpoint(t *testing.T) { Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)}, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, }, @@ -332,11 +328,10 @@ func TestPodToEndpoint(t *testing.T) { Zone: utilpointer.StringPtr("us-central1-a"), NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, }, @@ -351,11 +346,10 @@ func TestPodToEndpoint(t *testing.T) { Zone: utilpointer.StringPtr("us-central1-a"), NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, }, @@ -371,11 +365,10 @@ func TestPodToEndpoint(t *testing.T) { Zone: utilpointer.StringPtr("us-central1-a"), NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPodHostname.Name, - UID: readyPodHostname.UID, - ResourceVersion: readyPodHostname.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPodHostname.Name, + UID: readyPodHostname.UID, }, }, }, @@ -392,11 +385,10 @@ func TestPodToEndpoint(t *testing.T) { }, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, terminatingGateEnabled: true, @@ -412,11 +404,10 @@ func TestPodToEndpoint(t *testing.T) { }, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, terminatingGateEnabled: false, @@ -434,11 +425,10 @@ func TestPodToEndpoint(t *testing.T) { }, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, terminatingGateEnabled: true, @@ -454,11 +444,10 @@ func TestPodToEndpoint(t *testing.T) { }, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, terminatingGateEnabled: false, @@ -476,11 +465,10 @@ func TestPodToEndpoint(t *testing.T) { }, NodeName: utilpointer.StringPtr("node-1"), TargetRef: &v1.ObjectReference{ - Kind: "Pod", - Namespace: ns, - Name: readyPod.Name, - UID: readyPod.UID, - ResourceVersion: readyPod.ResourceVersion, + Kind: "Pod", + Namespace: ns, + Name: readyPod.Name, + UID: readyPod.UID, }, }, terminatingGateEnabled: true, @@ -937,6 +925,7 @@ func newPod(n int, namespace string, ready bool, nPorts int, terminating bool) * Name: fmt.Sprintf("pod%d", n), Labels: map[string]string{"foo": "bar"}, DeletionTimestamp: deletionTimestamp, + ResourceVersion: fmt.Sprint(n), }, Spec: v1.PodSpec{ Containers: []v1.Container{{ diff --git a/pkg/controller/util/endpoint/controller_utils_test.go b/pkg/controller/util/endpoint/controller_utils_test.go index acc50e9bd01..bf4d199dd95 100644 --- a/pkg/controller/util/endpoint/controller_utils_test.go +++ b/pkg/controller/util/endpoint/controller_utils_test.go @@ -851,6 +851,28 @@ func TestEndpointsEqualBeyondHash(t *testing.T) { }, expected: true, }, + { + name: "Pod resourceVersion removed", + ep1: &discovery.Endpoint{ + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + }, + Addresses: []string{"10.0.0.1"}, + TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0", ResourceVersion: "1"}, + Zone: utilpointer.StringPtr("zone-1"), + NodeName: utilpointer.StringPtr("node-1"), + }, + ep2: &discovery.Endpoint{ + Conditions: discovery.EndpointConditions{ + Ready: utilpointer.BoolPtr(true), + }, + Addresses: []string{"10.0.0.1"}, + TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0", ResourceVersion: ""}, + Zone: utilpointer.StringPtr("zone-1"), + NodeName: utilpointer.StringPtr("node-1"), + }, + expected: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -933,6 +955,14 @@ func TestEndpointSubsetsEqualIgnoreResourceVersion(t *testing.T) { })}, expected: true, }, + { + name: "Pod ResourceVersion removed", + subsets1: []v1.EndpointSubset{*es1, *es2}, + subsets2: []v1.EndpointSubset{*es1, *copyAndMutateEndpointSubset(es2, func(es *v1.EndpointSubset) { + es.Addresses[0].TargetRef.ResourceVersion = "" + })}, + expected: true, + }, { name: "Ports changed", subsets1: []v1.EndpointSubset{*es1, *es2},