Improve network endpoints order, keep the order even container has been

restarted

Signed-off-by: DrAuYueng <ouyang1204@gmail.com>

integration-cli/docker_cli_network_unix_test.go

@@ -1189,7 +1189,7 @@ func (s *DockerNetworkSuite) TestDockerNetworkConnectDisconnectWithPortMapping(c
 
 	// Connect to a network which does not cause the container's default gw switch
 	dockerCmd(c, "network", "connect", "ccc", cnt)
-	verifyPortMap(c, cnt, "70", curPortMap, true)
+	verifyPortMap(c, cnt, "70", curPortMap, false)
 	verifyPortMap(c, cnt, "90", curExplPortMap, true)
 }
 
@@ -1220,6 +1220,33 @@ func (s *DockerNetworkSuite) TestDockerNetworkRestartWithMultipleNetworks(c *tes
 	networks := inspectField(c, "foo", "NetworkSettings.Networks")
 	assert.Assert(c, strings.Contains(networks, "bridge"), "Should contain 'bridge' network")
 	assert.Assert(c, strings.Contains(networks, "test"), "Should contain 'test' network")
+
+	cli.DockerCmd(c, "network", "create", "-d", "bridge", "--subnet", "172.28.0.0/16", "mynet1")
+	cli.DockerCmd(c, "network", "create", "-d", "bridge", "--subnet", "172.29.0.0/16", "mynet2")
+
+	cli.DockerCmd(c, "run", "--name=foo", "--net", "mynet2", "-d", "busybox", "top")
+	assert.Assert(c, waitRun("foo") == nil)
+
+	cli.DockerCmd(c, "network", "connect", "mynet1", "foo")
+	out, _, err := dockerCmdWithError("exec", "foo", "ip", "route")
+	assert.NilError(c, err)
+	assert.Assert(c, strings.Contains(out, "default via 172.29.0.1 dev eth0"), "Default network should be mynet2")
+
+	cli.DockerCmd(c, "restart", "foo")
+	out, _, err = dockerCmdWithError("exec", "foo", "ip", "route")
+	assert.NilError(c, err)
+	assert.Assert(c, strings.Contains(out, "default via 172.29.0.1 dev eth0"), "Default network should still be mynet2")
+
+	cli.DockerCmd(c, "network", "disconnect", "mynet2", "foo")
+	out, _, err = dockerCmdWithError("exec", "foo", "ip", "route")
+	assert.NilError(c, err)
+	assert.Assert(c, strings.Contains(out, "default via 172.28.0.1 dev eth1"), "Default network should be mynet1")
+
+	cli.DockerCmd(c, "network", "connect", "mynet2", "foo")
+	cli.DockerCmd(c, "restart", "foo")
+	out, _, err = dockerCmdWithError("exec", "foo", "ip", "route")
+	assert.NilError(c, err)
+	assert.Assert(c, strings.Contains(out, "default via 172.28.0.1 dev eth0"), "Default network should still be mynet1")
 }
 
 func (s *DockerNetworkSuite) TestDockerNetworkConnectDisconnectToStoppedContainer(c *testing.T) {

libnetwork/default_gateway.go

@@ -175,15 +175,38 @@ func (c *controller) defaultGwNetwork() (Network, error) {
 	return n, err
 }
 
+// Returns the highest priority endpoint which is providing external connectivity to the sandbox
+func (sb *sandbox) getHighestPriorityGatewayEndpoint() *endpoint {
+	for _, ep := range sb.getConnectedEndpoints() {
+		if sb.isGatewayEndpoint(ep) {
+			return ep
+		}
+	}
+	return nil
+}
+
 // Returns the endpoint which is providing external connectivity to the sandbox
 func (sb *sandbox) getGatewayEndpoint() *endpoint {
-	for _, ep := range sb.getConnectedEndpoints() {
-		if ep.getNetwork().Type() == "null" || ep.getNetwork().Type() == "host" {
-			continue
+	internalOrInGWNetworkEndpoints, reversedOtherNetworkEndpoints := sb.getGroupedEndpoints()
+	for _, ep := range reversedOtherNetworkEndpoints {
+		if sb.isGatewayEndpoint(ep) {
+			return ep
 		}
-		if len(ep.Gateway()) != 0 {
+	}
+
+	for _, ep := range internalOrInGWNetworkEndpoints {
+		if sb.isGatewayEndpoint(ep) {
 			return ep
 		}
 	}
+
 	return nil
 }
+
+func (sb *sandbox) isGatewayEndpoint(ep *endpoint) bool {
+	if ep.getNetwork().Type() != "null" && ep.getNetwork().Type() != "host" && len(ep.Gateway()) != 0 {
+		return true
+	}
+
+	return false
+}

libnetwork/sandbox.go

@@ -345,10 +345,17 @@ func (sb *sandbox) Endpoints() []Endpoint {
 	sb.Lock()
 	defer sb.Unlock()
 
-	endpoints := make([]Endpoint, len(sb.endpoints))
-	for i, ep := range sb.endpoints {
-		endpoints[i] = ep
+	endpoints := make([]Endpoint, 0, len(sb.endpoints))
+
+	internalOrInGWNetworkEndpoints, reversedOtherNetworkEndpoints := sb.getGroupedEndpoints()
+	for _, ep := range reversedOtherNetworkEndpoints {
+		endpoints = append(endpoints, ep)
+	}
+
+	for _, ep := range internalOrInGWNetworkEndpoints {
+		endpoints = append(endpoints, ep)
 	}
+
 	return endpoints
 }
 
@@ -362,6 +369,28 @@ func (sb *sandbox) getConnectedEndpoints() []*endpoint {
 	return eps
 }
 
+func (sb *sandbox) getGroupedEndpoints() ([]*endpoint, []*endpoint) {
+	internalOrInGWNetworkEndpoints := make([]*endpoint, 0)
+	otherNetworkEndpoints := make([]*endpoint, 0)
+	eps := make([]*endpoint, len(sb.endpoints))
+	copy(eps, sb.endpoints)
+	for i, ep := range eps {
+		if ep.getNetwork().Internal() || ep.endpointInGWNetwork() {
+			internalOrInGWNetworkEndpoints = append(internalOrInGWNetworkEndpoints, eps[i])
+			continue
+		}
+
+		otherNetworkEndpoints = append(otherNetworkEndpoints, eps[i])
+	}
+
+	reversedOtherNetworkEndpoints := make([]*endpoint, 0, len(otherNetworkEndpoints))
+	for i := len(otherNetworkEndpoints) - 1; i >= 0; i-- {
+		reversedOtherNetworkEndpoints = append(reversedOtherNetworkEndpoints, otherNetworkEndpoints[i])
+	}
+
+	return internalOrInGWNetworkEndpoints, reversedOtherNetworkEndpoints
+}
+
 func (sb *sandbox) addEndpoint(ep *endpoint) {
 	sb.Lock()
 	defer sb.Unlock()
@@ -686,6 +715,7 @@ func (sb *sandbox) SetKey(basePath string) error {
 		}
 	}
 
+	//for _, ep := range sb.getSortedEndpoints() {
 	for _, ep := range sb.getConnectedEndpoints() {
 		if err = sb.populateNetworkResources(ep); err != nil {
 			return err
@@ -895,7 +925,9 @@ func (sb *sandbox) populateNetworkResources(ep *endpoint) error {
 		}
 	}
 
-	if ep == sb.getGatewayEndpoint() {
+	logrus.Infof("populateNetworkResources")
+	if ep == sb.getHighestPriorityGatewayEndpoint() {
+		logrus.Infof("populateNetworkResources getHighestPriorityGatewayEndpoint")
 		if err := sb.updateGateway(ep); err != nil {
 			return err
 		}
@@ -1200,19 +1232,18 @@ func OptionLoadBalancer(nid string) SandboxOption {
 // epi.gw <=> epj.gw               # non-gw < gw
 // epi.internal <=> epj.internal   # non-internal < internal
 // epi.joininfo <=> epj.joininfo   # ipv6 < ipv4
-// epi.name <=> epj.name           # bar < foo
+// epi.name <=> epj.name           # bar != foo
 func (epi *endpoint) Less(epj *endpoint) bool {
 	var (
 		prioi, prioj int
 	)
-
 	sbi, _ := epi.getSandbox()
 	sbj, _ := epj.getSandbox()
 
-	// Prio defaults to 0
 	if sbi != nil {
 		prioi = sbi.epPriority[epi.ID()]
 	}
+
 	if sbj != nil {
 		prioj = sbj.epPriority[epj.ID()]
 	}
@@ -1257,7 +1288,11 @@ func (epi *endpoint) Less(epj *endpoint) bool {
 		return jii > jij
 	}
 
-	return epi.network.Name() < epj.network.Name()
+	if epi.network.Name() != epj.network.Name() {
+		return epj.getNetwork().Internal()
+	}
+
+	return false
 }
 
 func (sb *sandbox) NdotsSet() bool {

libnetwork/sandbox_test.go

@@ -244,9 +244,9 @@ func TestSandboxAddSamePrio(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// 'test_nw_0' has precedence over 'test_nw_1'
-	if ctrlr.sandboxes[sid].endpoints[0].ID() != epNw0.ID() {
-		t.Fatal("Expected epNw0 to be at the top of the heap after removing epIPv6. But did not find epNw0 at the top of the heap")
+	// 'test_nw_1' has precedence over 'test_nw_0'
+	if ctrlr.sandboxes[sid].endpoints[0].ID() != epNw1.ID() {
+		t.Fatal("Expected epNw1 to be at the top of the heap after removing epIPv6. But did not find epNw1 at the top of the heap")
 	}
 
 	if err := epNw1.Leave(sbx); err != nil {