libnetwork_test: isolate tests from each other

Reusing the same "OS context" (read: network namespace) and
NetworkController across multiple tests risks tests interfering with
each other, or worse: _depending on_ other tests to set up
preconditions. Construct a new controller for every test which needs
one, and run every test which mutates or inspects the host environment
in a fresh OS context.

The only outlier is runParallelTests. It is the only remaining test
implementation which references the "global" package-scoped controller,
so the global controller instance is effectively private to that one
test.

Signed-off-by: Cory Snider <csnider@mirantis.com>

libnetwork/firewall_linux_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/docker/docker/libnetwork/iptables"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/options"
+	"github.com/docker/docker/libnetwork/testutils"
 	"gotest.tools/v3/assert"
 )
 
@@ -19,9 +20,6 @@ const (
 func TestUserChain(t *testing.T) {
 	iptable := iptables.GetIptable(iptables.IPv4)
 
-	nc, err := New()
-	assert.NilError(t, err)
-
 	tests := []struct {
 		iptables  bool
 		insert    bool // insert other rules to FORWARD
@@ -47,10 +45,15 @@ func TestUserChain(t *testing.T) {
 		},
 	}
 
-	resetIptables(t)
 	for _, tc := range tests {
 		tc := tc
 		t.Run(fmt.Sprintf("iptables=%v,insert=%v", tc.iptables, tc.insert), func(t *testing.T) {
+			defer testutils.SetupTestOSContext(t)()
+			defer resetIptables(t)
+
+			nc, err := New()
+			assert.NilError(t, err)
+			defer nc.Stop()
 			c := nc.(*controller)
 			c.cfg.DriverCfg["bridge"] = map[string]interface{}{
 				netlabel.GenericData: options.Generic{
@@ -75,7 +78,6 @@ func TestUserChain(t *testing.T) {
 				assert.Assert(t, err != nil, "chain %v: created unexpectedly", usrChainName)
 			}
 		})
-		resetIptables(t)
 	}
 }
 

libnetwork/libnetwork_internal_test.go

@@ -312,6 +312,8 @@ func compareNwLists(a, b []*net.IPNet) bool {
 }
 
 func TestAuxAddresses(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+
 	c, err := New()
 	if err != nil {
 		t.Fatal(err)
@@ -349,6 +351,8 @@ func TestAuxAddresses(t *testing.T) {
 func TestSRVServiceQuery(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")
 
+	defer testutils.SetupTestOSContext(t)()
+
 	c, err := New()
 	if err != nil {
 		t.Fatal(err)
@@ -447,6 +451,8 @@ func TestSRVServiceQuery(t *testing.T) {
 func TestServiceVIPReuse(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")
 
+	defer testutils.SetupTestOSContext(t)()
+
 	c, err := New()
 	if err != nil {
 		t.Fatal(err)

libnetwork/libnetwork_linux_test.go

@@ -31,22 +31,17 @@ const (
 	bridgeNetType = "bridge"
 )
 
+// Shared state for createGlobalInstance() and runParallelTests().
 var (
 	origins = netns.None()
 	testns  = netns.None()
-)
 
-var createTesthostNetworkOnce sync.Once
+	controller libnetwork.NetworkController
+)
 
-func getTesthostNetwork(t *testing.T) libnetwork.Network {
+func makeTesthostNetwork(t *testing.T, c libnetwork.NetworkController) libnetwork.Network {
 	t.Helper()
-	createTesthostNetworkOnce.Do(func() {
-		_, err := createTestNetwork(controller, "host", "testhost", options.Generic{}, nil, nil)
-		if err != nil {
-			t.Fatal(err)
-		}
-	})
-	n, err := controller.NetworkByName("testhost")
+	n, err := createTestNetwork(c, "host", "testhost", options.Generic{}, nil, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -67,13 +62,16 @@ func createGlobalInstance(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	controller = newController(t)
+	t.Cleanup(controller.Stop)
+
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
 			"BridgeName": "network",
 		},
 	}
 
-	net1 := getTesthostNetwork(t)
+	net1 := makeTesthostNetwork(t, controller)
 	net2, err := createTestNetwork(controller, "bridge", "network2", netOption, nil, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -105,6 +103,9 @@ func createGlobalInstance(t *testing.T) {
 }
 
 func TestHost(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
+
 	sbx1, err := controller.NewSandbox("host_c1",
 		libnetwork.OptionHostname("test1"),
 		libnetwork.OptionDomainname("docker.io"),
@@ -133,7 +134,7 @@ func TestHost(t *testing.T) {
 		}
 	}()
 
-	network := getTesthostNetwork(t)
+	network := makeTesthostNetwork(t, controller)
 	ep1, err := network.CreateEndpoint("testep1")
 	if err != nil {
 		t.Fatal(err)
@@ -204,6 +205,7 @@ func TestHost(t *testing.T) {
 // Testing IPV6 from MAC address
 func TestBridgeIpv6FromMac(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -278,6 +280,7 @@ func checkSandbox(t *testing.T, info libnetwork.EndpointInfo) {
 
 func TestEndpointJoin(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	// Create network 1 and add 2 endpoint: ep11, ep12
 	netOption := options.Generic{
@@ -452,6 +455,7 @@ func TestExternalKey(t *testing.T) {
 
 func externalKeyTest(t *testing.T, reexec bool) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -612,6 +616,7 @@ func reexecSetKey(key string, containerID string, controllerID string) error {
 
 func TestEnableIPv6(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	tmpResolvConf := []byte("search pommesfrites.fr\nnameserver 12.34.56.78\nnameserver 2001:4860:4860::8888\n")
 	expectedResolvConf := []byte("search pommesfrites.fr\nnameserver 127.0.0.11\nnameserver 2001:4860:4860::8888\noptions ndots:0\n")
@@ -688,6 +693,7 @@ func TestEnableIPv6(t *testing.T) {
 
 func TestResolvConfHost(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	tmpResolvConf := []byte("search localhost.net\nnameserver 127.0.0.1\nnameserver 2001:4860:4860::8888\n")
 
@@ -703,7 +709,7 @@ func TestResolvConfHost(t *testing.T) {
 		}
 	}()
 
-	n := getTesthostNetwork(t)
+	n := makeTesthostNetwork(t, controller)
 	ep1, err := n.CreateEndpoint("ep1", libnetwork.CreateOptionDisableResolution())
 	if err != nil {
 		t.Fatal(err)
@@ -762,6 +768,7 @@ func TestResolvConfHost(t *testing.T) {
 
 func TestResolvConf(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	tmpResolvConf1 := []byte("search pommesfrites.fr\nnameserver 12.34.56.78\nnameserver 2001:4860:4860::8888\n")
 	tmpResolvConf2 := []byte("search pommesfrites.fr\nnameserver 112.34.56.78\nnameserver 2001:4860:4860::8888\n")
@@ -980,7 +987,10 @@ func runParallelTests(t *testing.T, thrNumber int) {
 		}
 	}()
 
-	net1 := getTesthostNetwork(t)
+	net1, err := controller.NetworkByName("testhost")
+	if err != nil {
+		t.Fatal(err)
+	}
 	if net1 == nil {
 		t.Fatal("Could not find testhost")
 	}
@@ -1054,6 +1064,7 @@ func TestParallel2(t *testing.T) {
 
 func TestBridge(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		netlabel.EnableIPv6: true,
@@ -1144,6 +1155,9 @@ func TestParallel3(t *testing.T) {
 }
 
 func TestNullIpam(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
+
 	_, err := controller.NewNetwork(bridgeNetType, "testnetworkinternal", "", libnetwork.NetworkOptionIpam(ipamapi.NullIPAM, "", nil, nil, nil))
 	if err == nil || err.Error() != "ipv4 pool is empty" {
 		t.Fatal("bridge network should complain empty pool")

libnetwork/libnetwork_test.go

@@ -28,8 +28,6 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-var controller libnetwork.NetworkController
-
 func TestMain(m *testing.M) {
 	if runtime.GOOS == "windows" {
 		logrus.Info("Test suite does not currently support windows")
@@ -39,35 +37,30 @@ func TestMain(m *testing.M) {
 		return
 	}
 
-	if err := createController(); err != nil {
-		logrus.Errorf("Error creating controller: %v", err)
-		os.Exit(1)
-	}
+	// Cleanup local datastore file
+	_ = os.Remove(datastore.DefaultScopes("")[datastore.LocalScope].Client.Address)
 
-	x := m.Run()
-	controller.Stop()
-	os.Exit(x)
+	os.Exit(m.Run())
 }
 
-func createController() error {
-	var err error
-
-	// Cleanup local datastore file
-	os.Remove(datastore.DefaultScopes("")[datastore.LocalScope].Client.Address)
-
-	option := options.Generic{
-		"EnableIPForwarding": true,
+func newController(t *testing.T) libnetwork.NetworkController {
+	t.Helper()
+	genericOption := map[string]interface{}{
+		netlabel.GenericData: options.Generic{
+			"EnableIPForwarding": true,
+		},
 	}
 
-	genericOption := make(map[string]interface{})
-	genericOption[netlabel.GenericData] = option
-
 	cfgOptions, err := libnetwork.OptionBoltdbWithRandomDBFile()
 	if err != nil {
-		return err
+		t.Fatal(err)
+	}
+	c, err := libnetwork.New(append(cfgOptions, config.OptionDriverConfig(bridgeNetType, genericOption))...)
+	if err != nil {
+		t.Fatal(err)
 	}
-	controller, err = libnetwork.New(append(cfgOptions, config.OptionDriverConfig(bridgeNetType, genericOption))...)
-	return err
+	t.Cleanup(c.Stop)
+	return c
 }
 
 func createTestNetwork(c libnetwork.NetworkController, networkType, networkName string, netOption options.Generic, ipamV4Configs, ipamV6Configs []*libnetwork.IpamConf) (libnetwork.Network, error) {
@@ -98,6 +91,9 @@ func isNotFound(err error) bool {
 }
 
 func TestNull(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
+
 	cnt, err := controller.NewSandbox("null_container",
 		libnetwork.OptionHostname("test"),
 		libnetwork.OptionDomainname("docker.io"),
@@ -146,6 +142,7 @@ func TestNull(t *testing.T) {
 
 func TestUnknownDriver(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	_, err := createTestNetwork(controller, "unknowndriver", "testnetwork", options.Generic{}, nil, nil)
 	if err == nil {
@@ -158,6 +155,9 @@ func TestUnknownDriver(t *testing.T) {
 }
 
 func TestNilRemoteDriver(t *testing.T) {
+	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
+
 	_, err := controller.NewNetwork("framerelay", "dummy", "",
 		libnetwork.NetworkOptionGeneric(getEmptyGenericOption()))
 	if err == nil {
@@ -171,6 +171,7 @@ func TestNilRemoteDriver(t *testing.T) {
 
 func TestNetworkName(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -205,6 +206,7 @@ func TestNetworkName(t *testing.T) {
 
 func TestNetworkType(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -229,6 +231,7 @@ func TestNetworkType(t *testing.T) {
 
 func TestNetworkID(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -253,6 +256,7 @@ func TestNetworkID(t *testing.T) {
 
 func TestDeleteNetworkWithActiveEndpoints(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		"BridgeName": "testnetwork",
@@ -292,6 +296,7 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) {
 
 func TestNetworkConfig(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	// Verify config network cannot inherit another config network
 	_, err := controller.NewNetwork("bridge", "config_network0", "",
@@ -392,6 +397,7 @@ func TestNetworkConfig(t *testing.T) {
 
 func TestUnknownNetwork(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		"BridgeName": "testnetwork",
@@ -422,6 +428,7 @@ func TestUnknownNetwork(t *testing.T) {
 
 func TestUnknownEndpoint(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		"BridgeName": "testnetwork",
@@ -462,6 +469,7 @@ func TestUnknownEndpoint(t *testing.T) {
 
 func TestNetworkEndpointsWalkers(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	// Create network 1 and add 2 endpoint: ep11, ep12
 	netOption := options.Generic{
@@ -590,6 +598,7 @@ func TestNetworkEndpointsWalkers(t *testing.T) {
 
 func TestDuplicateEndpoint(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -637,6 +646,7 @@ func TestDuplicateEndpoint(t *testing.T) {
 
 func TestControllerQuery(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	// Create network 1
 	netOption := options.Generic{
@@ -737,6 +747,7 @@ func TestControllerQuery(t *testing.T) {
 
 func TestNetworkQuery(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	// Create network 1 and add 2 endpoint: ep11, ep12
 	netOption := options.Generic{
@@ -884,6 +895,7 @@ func (f *fakeSandbox) DisableService() error {
 
 func TestEndpointDeleteWithActiveContainer(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -957,6 +969,7 @@ func TestEndpointDeleteWithActiveContainer(t *testing.T) {
 
 func TestEndpointMultipleJoins(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	n, err := createTestNetwork(controller, bridgeNetType, "testmultiple", options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -1029,6 +1042,7 @@ func TestEndpointMultipleJoins(t *testing.T) {
 
 func TestLeaveAll(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -1092,6 +1106,7 @@ func TestLeaveAll(t *testing.T) {
 
 func TestContainerInvalidLeave(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -1156,6 +1171,7 @@ func TestContainerInvalidLeave(t *testing.T) {
 
 func TestEndpointUpdateParent(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+	controller := newController(t)
 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
 		netlabel.GenericData: options.Generic{
@@ -1300,6 +1316,7 @@ func TestValidRemoteDriver(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	controller := newController(t)
 	n, err := controller.NewNetwork("valid-network-driver", "dummy", "",
 		libnetwork.NetworkOptionGeneric(getEmptyGenericOption()))
 	if err != nil {

libnetwork/resolver_test.go

@@ -77,6 +77,7 @@ func checkDNSRRType(t *testing.T, actual, expected uint16) {
 func TestDNSIPQuery(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")
 
+	defer testutils.SetupTestOSContext(t)()
 	c, err := New()
 	if err != nil {
 		t.Fatal(err)

libnetwork/service_common_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/libnetwork/resolvconf"
+	"github.com/docker/docker/libnetwork/testutils"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
@@ -14,6 +15,7 @@ import (
 func TestCleanupServiceDiscovery(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")
 
+	defer testutils.SetupTestOSContext(t)()
 	c, err := New()
 	assert.NilError(t, err)
 	defer c.Stop()