libnetwork: Sandbox.ResolveName: refactor ordering of endpoints

When resolving names in swarm mode, services with exposed ports are
connected to user overlay network, ingress network, and local (docker_gwbridge)
networks. Name resolution should prioritize returning the VIP/IPs on user
overlay network over ingress and local networks.

Sandbox.ResolveName implemented this by taking the list of endpoints,
splitting the list into 3 separate lists based on the type of network
that the endpoint was attached to (dynamic, ingress, local), and then
creating a new list, applying the networks in that order.

This patch refactors that logic to use a custom sorter (sort.Interface),
which makes the code more transparent, and prevents iterating over the
list of endpoints multiple times.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

libnetwork/endpoint.go

@@ -17,6 +17,47 @@ import (
 	"github.com/docker/docker/libnetwork/types"
 )
 
+// ByNetworkType sorts a [Endpoint] slice based on the network-type
+// they're attached to. It implements [sort.Interface] and can be used
+// with [sort.Stable] or [sort.Sort]. It is used by [Sandbox.ResolveName]
+// when resolving names in swarm mode. In swarm mode, services with exposed
+// ports are connected to user overlay network, ingress network, and local
+// ("docker_gwbridge") networks. Name resolution should prioritize returning
+// the VIP/IPs on user overlay network over ingress and local networks.
+//
+// ByNetworkType re-orders the endpoints based on the network-type they
+// are attached to:
+//
+//  1. dynamic networks (user overlay networks)
+//  2. ingress network(s)
+//  3. local networks ("docker_gwbridge")
+type ByNetworkType []*Endpoint
+
+func (ep ByNetworkType) Len() int      { return len(ep) }
+func (ep ByNetworkType) Swap(i, j int) { ep[i], ep[j] = ep[j], ep[i] }
+func (ep ByNetworkType) Less(i, j int) bool {
+	return getNetworkType(ep[i].getNetwork()) < getNetworkType(ep[j].getNetwork())
+}
+
+// Define the order in which resolution should happen if an endpoint is
+// attached to multiple network-types. It is used by [ByNetworkType].
+const (
+	typeDynamic = iota
+	typeIngress
+	typeLocal
+)
+
+func getNetworkType(nw *Network) int {
+	switch {
+	case nw.ingress:
+		return typeIngress
+	case nw.dynamic:
+		return typeDynamic
+	default:
+		return typeLocal
+	}
+}
+
 // EndpointOption is an option setter function type used to pass various options to Network
 // and Endpoint interfaces methods. The various setter functions of type EndpointOption are
 // provided by libnetwork, they look like <Create|Join|Leave>Option[...](...)

libnetwork/endpoint_test.go

@@ -0,0 +1,43 @@
+package libnetwork
+
+import (
+	"sort"
+	"testing"
+
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestSortByNetworkType(t *testing.T) {
+	nws := []*Network{
+		{name: "local2"},
+		{name: "ovl2", dynamic: true},
+		{name: "local3"},
+		{name: "ingress", ingress: true},
+		{name: "ovl3", dynamic: true},
+		{name: "local1"},
+		{name: "ovl1", dynamic: true},
+	}
+	eps := make([]*Endpoint, 0, len(nws))
+	for _, nw := range nws {
+		eps = append(eps, &Endpoint{
+			name:    "ep-" + nw.name,
+			network: nw,
+		})
+	}
+	sort.Sort(ByNetworkType(eps))
+	actual := make([]string, 0, len(eps))
+	for _, ep := range eps {
+		actual = append(actual, ep.name)
+	}
+	expected := []string{
+		"ep-ovl2",
+		"ep-ovl3",
+		"ep-ovl1",
+		"ep-ingress",
+		"ep-local2",
+		"ep-local3",
+		"ep-local1",
+	}
+	assert.Check(t, is.DeepEqual(actual, expected))
+}

libnetwork/sandbox.go

@@ -392,38 +392,6 @@ func (sb *Sandbox) ResolveService(ctx context.Context, name string) ([]*net.SRV,
 	return nil, nil
 }
 
-func getDynamicNwEndpoints(epList []*Endpoint) []*Endpoint {
-	eps := []*Endpoint{}
-	for _, ep := range epList {
-		n := ep.getNetwork()
-		if n.dynamic && !n.ingress {
-			eps = append(eps, ep)
-		}
-	}
-	return eps
-}
-
-func getIngressNwEndpoint(epList []*Endpoint) *Endpoint {
-	for _, ep := range epList {
-		n := ep.getNetwork()
-		if n.ingress {
-			return ep
-		}
-	}
-	return nil
-}
-
-func getLocalNwEndpoints(epList []*Endpoint) []*Endpoint {
-	eps := []*Endpoint{}
-	for _, ep := range epList {
-		n := ep.getNetwork()
-		if !n.dynamic && !n.ingress {
-			eps = append(eps, ep)
-		}
-	}
-	return eps
-}
-
 func (sb *Sandbox) ResolveName(ctx context.Context, name string, ipType int) ([]net.IP, bool) {
 	// Embedded server owns the docker network domain. Resolution should work
 	// for both container_name and container_name.network_name
@@ -455,18 +423,17 @@ func (sb *Sandbox) ResolveName(ctx context.Context, name string, ipType int) ([]
 
 	epList := sb.Endpoints()
 
-	// In swarm mode services with exposed ports are connected to user overlay
+	// In swarm mode, services with exposed ports are connected to user overlay
-	// network, ingress network and docker_gwbridge network. Name resolution
+	// network, ingress network and docker_gwbridge networks. Name resolution
 	// should prioritize returning the VIP/IPs on user overlay network.
-	newList := []*Endpoint{}
+	//
+	// Re-order the endpoints based on the network-type they're attached to;
+	//
+	//  1. dynamic networks (user overlay networks)
+	//  2. ingress network(s)
+	//  3. local networks ("docker_gwbridge")
 	if sb.controller.isSwarmNode() {
-		newList = append(newList, getDynamicNwEndpoints(epList)...)
+		sort.Sort(ByNetworkType(epList))
-		ingressEP := getIngressNwEndpoint(epList)
-		if ingressEP != nil {
-			newList = append(newList, ingressEP)
-		}
-		newList = append(newList, getLocalNwEndpoints(epList)...)
-		epList = newList
 	}
 
 	for i := 0; i < len(reqName); i++ {