create endpoints on a predictable order

Endpoints (internal haproxy ingress struct, not the k8s one) are part of
the building blocks to configure a backend or a tcp listener. HAProxy
Ingress uses deep equals on some structs in order to identify equality,
hence the ability to skip a reload event. When two endpoint arrays have
the same content but on distinct order, they are not considered equals,
although it should from the haproxy and haproxy ingress perspective.
Because of that we're sorting them.

The former update (which we're reverting here) was sorting the
endpoints after they get named, so the order is right but some names
might be changed. Now we're sorting the endpoints on its source.

pkg/converters/utils/services.go

@@ -18,6 +18,7 @@ package utils
 
 import (
 	"fmt"
+	"sort"
 	"strconv"
 
 	api "k8s.io/api/core/v1"
@@ -69,32 +70,40 @@ func FindContainerPort(pod *api.Pod, svcPort *api.ServicePort) int {
 type Endpoint struct {
 	IP        string
 	Port      int
+	Target    string
 	TargetRef string
 }
 
 // CreateEndpoints ...
 func CreateEndpoints(cache types.Cache, svc *api.Service, svcPort *api.ServicePort) (ready, notReady []*Endpoint, err error) {
 	if svc.Spec.Type == api.ServiceTypeExternalName {
-		ready, err := createEndpointsExternalName(cache, svc, svcPort)
-		return ready, nil, err
-	}
-	endpoints, err := cache.GetEndpoints(svc)
-	if err != nil {
-		return nil, nil, err
-	}
-	for _, subset := range endpoints.Subsets {
-		for _, epPort := range subset.Ports {
-			if matchPort(svcPort, &epPort) {
-				port := int(epPort.Port)
-				for _, addr := range subset.Addresses {
-					ready = append(ready, newEndpointAddr(&addr, port))
-				}
-				for _, addr := range subset.NotReadyAddresses {
-					notReady = append(notReady, newEndpointAddr(&addr, port))
+		ready, err = createEndpointsExternalName(cache, svc, svcPort)
+	} else {
+		endpoints, err1 := cache.GetEndpoints(svc)
+		if err1 != nil {
+			return nil, nil, err1
+		}
+		for _, subset := range endpoints.Subsets {
+			for _, epPort := range subset.Ports {
+				if matchPort(svcPort, &epPort) {
+					port := int(epPort.Port)
+					for _, addr := range subset.Addresses {
+						ready = append(ready, newEndpoint(addr.IP, port, addr.TargetRef))
+					}
+					for _, addr := range subset.NotReadyAddresses {
+						notReady = append(notReady, newEndpoint(addr.IP, port, addr.TargetRef))
+					}
 				}
 			}
 		}
 	}
+	// ensures predictable result, allowing to compare old and new states
+	sort.Slice(ready, func(i, j int) bool {
+		return ready[i].Target < ready[j].Target
+	})
+	sort.Slice(notReady, func(i, j int) bool {
+		return notReady[i].Target < notReady[j].Target
+	})
 	return ready, notReady, nil
 }
 
@@ -111,7 +120,7 @@ func CreateSvcEndpoint(svc *api.Service, svcPort *api.ServicePort) (endpoint *En
 	if port <= 0 {
 		return nil, fmt.Errorf("invalid port number: %d", port)
 	}
-	return newEndpointIP(svc.Spec.ClusterIP, int(port)), nil
+	return newEndpoint(svc.Spec.ClusterIP, int(port), nil), nil
 }
 
 func createEndpointsExternalName(cache types.Cache, svc *api.Service, svcPort *api.ServicePort) (endpoints []*Endpoint, err error) {
@@ -126,30 +135,22 @@ func createEndpointsExternalName(cache types.Cache, svc *api.Service, svcPort *a
 	}
 	endpoints = make([]*Endpoint, len(addr))
 	for i, ip := range addr {
-		endpoints[i] = newEndpointIP(ip.String(), port)
+		endpoints[i] = newEndpoint(ip.String(), port, nil)
 	}
 	return endpoints, nil
 }
 
-func newEndpointAddr(addr *api.EndpointAddress, port int) *Endpoint {
-	return &Endpoint{
-		IP:        addr.IP,
-		Port:      port,
-		TargetRef: targetRefToString(addr.TargetRef),
-	}
-}
+func newEndpoint(ip string, port int, targetRef *api.ObjectReference) *Endpoint {
+	var targetRefStr string
+	if targetRef != nil {
+		targetRefStr = fmt.Sprintf("%s/%s", targetRef.Namespace, targetRef.Name)
 
-func targetRefToString(targetRef *api.ObjectReference) string {
-	if targetRef == nil {
-		return ""
 	}
-	return fmt.Sprintf("%s/%s", targetRef.Namespace, targetRef.Name)
-}
-
-func newEndpointIP(ip string, port int) *Endpoint {
 	return &Endpoint{
-		IP:   ip,
-		Port: port,
+		IP:        ip,
+		Port:      port,
+		Target:    ip + ":" + strconv.Itoa(port),
+		TargetRef: targetRefStr,
 	}
 }
 

pkg/converters/utils/services_test.go

@@ -39,12 +39,14 @@ func TestCreateEndpointsExternalName(t *testing.T) {
 	ready, notReady, err := CreateEndpoints(cache, svc, svcPort)
 	expected := []*Endpoint{
 		{
-			IP:   "10.0.1.10",
-			Port: 8080,
+			IP:     "10.0.1.10",
+			Port:   8080,
+			Target: "10.0.1.10:8080",
 		},
 		{
-			IP:   "10.0.1.11",
-			Port: 8080,
+			IP:     "10.0.1.11",
+			Port:   8080,
+			Target: "10.0.1.11:8080",
 		},
 	}
 	if !reflect.DeepEqual(ready, expected) {
@@ -71,8 +73,8 @@ func TestCreateEndpoints(t *testing.T) {
 			declarePort: "svcport:8080:http",
 			findPort:    "8080",
 			expected: []*Endpoint{
-				{IP: "172.17.0.11", Port: 8080},
-				{IP: "172.17.0.12", Port: 8080},
+				{IP: "172.17.0.11", Port: 8080, Target: "172.17.0.11:8080"},
+				{IP: "172.17.0.12", Port: 8080, Target: "172.17.0.12:8080"},
 			},
 		},
 		// 1
@@ -81,7 +83,7 @@ func TestCreateEndpoints(t *testing.T) {
 			declarePort: "svcport:8080:http",
 			findPort:    "svcport",
 			expected: []*Endpoint{
-				{IP: "172.17.0.11", Port: 8080},
+				{IP: "172.17.0.11", Port: 8080, Target: "172.17.0.11:8080"},
 			},
 		},
 		// 2
@@ -90,7 +92,7 @@ func TestCreateEndpoints(t *testing.T) {
 			declarePort: "svcport:8000:http",
 			findPort:    "http",
 			expected: []*Endpoint{
-				{IP: "172.17.0.12", Port: 8000},
+				{IP: "172.17.0.12", Port: 8000, Target: "172.17.0.12:8000"},
 			},
 		},
 	}

pkg/haproxy/types/tcpbackend.go

@@ -18,7 +18,6 @@ package types
 
 import (
 	"fmt"
-	"sort"
 )
 
 // AddEndpoint ...
@@ -30,9 +29,5 @@ func (b *TCPBackend) AddEndpoint(ip string, port int) *TCPEndpoint {
 		Target: fmt.Sprintf("%s:%d", ip, port),
 	}
 	b.Endpoints = append(b.Endpoints, ep)
-	// ensures predictable result, so Changed() works properly
-	sort.Slice(b.Endpoints, func(i, j int) bool {
-		return b.Endpoints[i].Target < b.Endpoints[j].Target
-	})
 	return ep
 }