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 <ellorent@redhat.com>

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 {

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