From 697ea476e27f4fb47cc3b009b3d4a840824d20df Mon Sep 17 00:00:00 2001 From: Enrique Llorente Date: Mon, 20 Feb 2023 13:47:34 +0100 Subject: [PATCH] svc: Support pods with same address If different pods with same address are exposed by the same service if some of the endpointslices endpoints are overwriten. This change add the pod name to the hash function to ensure that all the endpoints are in place. Signed-off-by: Enrique Llorente --- .../endpointslice_controller_test.go | 262 ++++++++++++++++++ .../util/endpointslice/endpointset.go | 8 +- 2 files changed, 269 insertions(+), 1 deletion(-) diff --git a/pkg/controller/endpointslice/endpointslice_controller_test.go b/pkg/controller/endpointslice/endpointslice_controller_test.go index 093f83fa04c..f1b14b193dc 100644 --- a/pkg/controller/endpointslice/endpointslice_controller_test.go +++ b/pkg/controller/endpointslice/endpointslice_controller_test.go @@ -947,6 +947,268 @@ func TestSyncService(t *testing.T) { }, }, }, + { + name: "Ready and Complete pods with same IPs", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foobar", + Namespace: "default", + CreationTimestamp: creationTimestamp, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + {Name: "tcp-example", TargetPort: intstr.FromInt(80), Protocol: v1.ProtocolTCP}, + {Name: "udp-example", TargetPort: intstr.FromInt(161), Protocol: v1.ProtocolUDP}, + {Name: "sctp-example", TargetPort: intstr.FromInt(3456), Protocol: v1.ProtocolSCTP}, + }, + Selector: map[string]string{"foo": "bar"}, + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }, + }, + pods: []*v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pod0", + Labels: map[string]string{"foo": "bar"}, + DeletionTimestamp: nil, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "container-1", + }}, + NodeName: "node-1", + }, + Status: v1.PodStatus{ + PodIP: "10.0.0.1", + PodIPs: []v1.PodIP{{ + IP: "10.0.0.1", + }}, + Conditions: []v1.PodCondition{ + { + Type: v1.PodInitialized, + Status: v1.ConditionTrue, + }, + { + Type: v1.PodReady, + Status: v1.ConditionTrue, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pod1", + Labels: map[string]string{"foo": "bar"}, + DeletionTimestamp: nil, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "container-1", + }}, + NodeName: "node-1", + }, + Status: v1.PodStatus{ + PodIP: "10.0.0.1", + PodIPs: []v1.PodIP{ + { + IP: "10.0.0.1", + }, + }, + Conditions: []v1.PodCondition{ + { + Type: v1.PodInitialized, + Status: v1.ConditionTrue, + }, + { + Type: v1.PodReady, + Status: v1.ConditionFalse, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + }, + expectedEndpointPorts: []discovery.EndpointPort{ + { + Name: pointer.StringPtr("sctp-example"), + Protocol: protoPtr(v1.ProtocolSCTP), + Port: pointer.Int32Ptr(int32(3456)), + }, + { + Name: pointer.StringPtr("udp-example"), + Protocol: protoPtr(v1.ProtocolUDP), + Port: pointer.Int32Ptr(int32(161)), + }, + { + Name: pointer.StringPtr("tcp-example"), + Protocol: protoPtr(v1.ProtocolTCP), + Port: pointer.Int32Ptr(int32(80)), + }, + }, + expectedEndpoints: []discovery.Endpoint{ + { + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(true), + Serving: pointer.BoolPtr(true), + Terminating: pointer.BoolPtr(false), + }, + Addresses: []string{"10.0.0.1"}, + TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0"}, + NodeName: pointer.StringPtr("node-1"), + }, + { + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(false), + Serving: pointer.BoolPtr(false), + Terminating: pointer.BoolPtr(false), + }, + Addresses: []string{"10.0.0.1"}, + TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod1"}, + NodeName: pointer.StringPtr("node-1"), + }, + }, + }, + { + // Any client reading EndpointSlices already has to handle deduplicating endpoints by IP address. + // If 2 pods are ready, something has gone wrong further up the stack, we shouldn't try to hide that. + name: "Two Ready pods with same IPs", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foobar", + Namespace: "default", + CreationTimestamp: creationTimestamp, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + {Name: "tcp-example", TargetPort: intstr.FromInt(80), Protocol: v1.ProtocolTCP}, + {Name: "udp-example", TargetPort: intstr.FromInt(161), Protocol: v1.ProtocolUDP}, + {Name: "sctp-example", TargetPort: intstr.FromInt(3456), Protocol: v1.ProtocolSCTP}, + }, + Selector: map[string]string{"foo": "bar"}, + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }, + }, + pods: []*v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pod0", + Labels: map[string]string{"foo": "bar"}, + DeletionTimestamp: nil, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "container-1", + }}, + NodeName: "node-1", + }, + Status: v1.PodStatus{ + PodIP: "10.0.0.1", + PodIPs: []v1.PodIP{{ + IP: "10.0.0.1", + }}, + Conditions: []v1.PodCondition{ + { + Type: v1.PodInitialized, + Status: v1.ConditionTrue, + }, + { + Type: v1.PodReady, + Status: v1.ConditionTrue, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pod1", + Labels: map[string]string{"foo": "bar"}, + DeletionTimestamp: nil, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "container-1", + }}, + NodeName: "node-1", + }, + Status: v1.PodStatus{ + PodIP: "10.0.0.1", + PodIPs: []v1.PodIP{ + { + IP: "10.0.0.1", + }, + }, + Conditions: []v1.PodCondition{ + { + Type: v1.PodInitialized, + Status: v1.ConditionTrue, + }, + { + Type: v1.PodReady, + Status: v1.ConditionTrue, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + }, + expectedEndpointPorts: []discovery.EndpointPort{ + { + Name: pointer.StringPtr("sctp-example"), + Protocol: protoPtr(v1.ProtocolSCTP), + Port: pointer.Int32Ptr(int32(3456)), + }, + { + Name: pointer.StringPtr("udp-example"), + Protocol: protoPtr(v1.ProtocolUDP), + Port: pointer.Int32Ptr(int32(161)), + }, + { + Name: pointer.StringPtr("tcp-example"), + Protocol: protoPtr(v1.ProtocolTCP), + Port: pointer.Int32Ptr(int32(80)), + }, + }, + expectedEndpoints: []discovery.Endpoint{ + { + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(true), + Serving: pointer.BoolPtr(true), + Terminating: pointer.BoolPtr(false), + }, + Addresses: []string{"10.0.0.1"}, + TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod0"}, + NodeName: pointer.StringPtr("node-1"), + }, + { + Conditions: discovery.EndpointConditions{ + Ready: pointer.BoolPtr(true), + Serving: pointer.BoolPtr(true), + Terminating: pointer.BoolPtr(false), + }, + Addresses: []string{"10.0.0.1"}, + TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "default", Name: "pod1"}, + NodeName: pointer.StringPtr("node-1"), + }, + }, + }, } for _, testcase := range testcases { diff --git a/pkg/controller/util/endpointslice/endpointset.go b/pkg/controller/util/endpointslice/endpointset.go index f6c7df669b7..82a0744847a 100644 --- a/pkg/controller/util/endpointslice/endpointset.go +++ b/pkg/controller/util/endpointslice/endpointset.go @@ -25,11 +25,13 @@ import ( // endpointHash is used to uniquely identify endpoints. Only including addresses // and hostnames as unique identifiers allows us to do more in place updates -// should attributes such as topology, conditions, or targetRef change. +// should attributes such as topology or conditions change. type endpointHash string type endpointHashObj struct { Addresses []string Hostname string + Namespace string + Name string } func hashEndpoint(endpoint *discovery.Endpoint) endpointHash { @@ -38,6 +40,10 @@ func hashEndpoint(endpoint *discovery.Endpoint) endpointHash { if endpoint.Hostname != nil { hashObj.Hostname = *endpoint.Hostname } + if endpoint.TargetRef != nil { + hashObj.Namespace = endpoint.TargetRef.Namespace + hashObj.Name = endpoint.TargetRef.Name + } return endpointHash(endpointutil.DeepHashObjectToString(hashObj)) }