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)) }