From 10fafb06eb3bd66d10dfbd3274be96ea8c5a764d Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Tue, 12 May 2015 23:39:30 +0000 Subject: [PATCH] Fixed an intermittent issue in the libnetwork test The libnetwork test does not need to run inside a namespace when inside a container. This results in unpredictable behavior when the sandbox code unlocks the go routine from the OS thread while the test code still wants it locked in the OS thread. This will result in unreachable interfaces when the go routine migrates to a different OS thread. Fixed by passing a special test flag which is only set to true when the test is run inside a container. Signed-off-by: Jana Radhakrishnan --- libnetwork/Makefile | 10 +- libnetwork/client/client_test.go | 2 + libnetwork/libnetwork_test.go | 109 ++++++++++++++---- libnetwork/netutils/test_utils.go | 8 ++ libnetwork/pkg/etchosts/etchosts_test.go | 2 + libnetwork/pkg/iptables/iptables_test.go | 2 + libnetwork/pkg/options/options_test.go | 2 + .../pkg/portallocator/portallocator_test.go | 2 + libnetwork/pkg/resolvconf/resolvconf_test.go | 2 + 9 files changed, 112 insertions(+), 27 deletions(-) diff --git a/libnetwork/Makefile b/libnetwork/Makefile index 956e6eb3e9..bf15342ad5 100644 --- a/libnetwork/Makefile +++ b/libnetwork/Makefile @@ -2,8 +2,9 @@ SHELL=/bin/bash build_image=libnetwork-build dockerargs = --privileged -v $(shell pwd):/go/src/github.com/docker/libnetwork -w /go/src/github.com/docker/libnetwork -docker = docker run --rm ${dockerargs} ${build_image} -ciargs = -e "COVERALLS_TOKEN=$$COVERALLS_TOKEN" +container_env = -e "INSIDECONTAINER=-incontainer=true" +docker = docker run --rm ${dockerargs} ${container_env} ${build_image} +ciargs = -e "COVERALLS_TOKEN=$$COVERALLS_TOKEN" -e "INSIDECONTAINER=-incontainer=true" cidocker = docker run ${ciargs} ${dockerargs} golang:1.4 all: ${build_image}.created @@ -42,8 +43,11 @@ run-tests: @echo "mode: count" > coverage.coverprofile @for dir in $$(find . -maxdepth 10 -not -path './.git*' -not -path '*/_*' -type d); do \ if ls $$dir/*.go &> /dev/null; then \ - $(shell which godep) go test -test.parallel 3 -test.v -covermode=count -coverprofile=$$dir/profile.tmp $$dir ; \ + pushd . &> /dev/null ; \ + cd $$dir ; \ + $(shell which godep) go test ${INSIDECONTAINER} -test.parallel 3 -test.v -covermode=count -coverprofile=./profile.tmp ; \ if [ $$? -ne 0 ]; then exit $$?; fi ;\ + popd &> /dev/null; \ if [ -f $$dir/profile.tmp ]; then \ cat $$dir/profile.tmp | tail -n +2 >> coverage.coverprofile ; \ rm $$dir/profile.tmp ; \ diff --git a/libnetwork/client/client_test.go b/libnetwork/client/client_test.go index d0a2b613f8..a07ca93f5f 100644 --- a/libnetwork/client/client_test.go +++ b/libnetwork/client/client_test.go @@ -4,6 +4,8 @@ import ( "bytes" "io" "testing" + + _ "github.com/docker/libnetwork/netutils" ) // nopCloser is used to provide a dummy CallFunc for Cmd() diff --git a/libnetwork/libnetwork_test.go b/libnetwork/libnetwork_test.go index c965bee1e5..2e443ee28f 100644 --- a/libnetwork/libnetwork_test.go +++ b/libnetwork/libnetwork_test.go @@ -138,7 +138,10 @@ func TestHost(t *testing.T) { } func TestBridge(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + ip, subnet, err := net.ParseCIDR("192.168.100.1/24") if err != nil { t.Fatal(err) @@ -209,7 +212,9 @@ func TestBridge(t *testing.T) { } func TestUnknownDriver(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } _, err := createTestNetwork("unknowndriver", "testnetwork", options.Generic{}, options.Generic{}) if err == nil { @@ -256,7 +261,10 @@ func TestNoInitDriver(t *testing.T) { } func TestDuplicateNetwork(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + controller, err := libnetwork.New() if err != nil { t.Fatal(err) @@ -287,7 +295,9 @@ func TestDuplicateNetwork(t *testing.T) { } func TestNetworkName(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } _, err := createTestNetwork(bridgeNetType, "", options.Generic{}, options.Generic{}) if err == nil { @@ -309,7 +319,10 @@ func TestNetworkName(t *testing.T) { } func TestNetworkType(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { t.Fatal(err) @@ -321,7 +334,9 @@ func TestNetworkType(t *testing.T) { } func TestNetworkID(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -334,7 +349,10 @@ func TestNetworkID(t *testing.T) { } func TestDeleteNetworkWithActiveEndpoints(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + option := options.Generic{ "BridgeName": bridgeName, "AllowNonDefaultBridge": true} @@ -369,7 +387,10 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) { } func TestUnknownNetwork(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + option := options.Generic{ "BridgeName": bridgeName, "AllowNonDefaultBridge": true} @@ -395,7 +416,10 @@ func TestUnknownNetwork(t *testing.T) { } func TestUnknownEndpoint(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + ip, subnet, err := net.ParseCIDR("192.168.100.1/24") if err != nil { t.Fatal(err) @@ -446,7 +470,10 @@ func TestUnknownEndpoint(t *testing.T) { } func TestNetworkEndpointsWalkers(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + controller, err := libnetwork.New() if err != nil { t.Fatal(err) @@ -527,7 +554,10 @@ func TestNetworkEndpointsWalkers(t *testing.T) { } func TestControllerQuery(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + controller, err := libnetwork.New() if err != nil { t.Fatal(err) @@ -590,7 +620,10 @@ func TestControllerQuery(t *testing.T) { } func TestNetworkQuery(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } + controller, err := libnetwork.New() if err != nil { t.Fatal(err) @@ -656,7 +689,9 @@ func TestNetworkQuery(t *testing.T) { const containerID = "valid_container" func TestEndpointJoin(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -685,7 +720,9 @@ func TestEndpointJoin(t *testing.T) { } func TestEndpointJoinInvalidContainerId(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -708,7 +745,9 @@ func TestEndpointJoinInvalidContainerId(t *testing.T) { } func TestEndpointDeleteWithActiveContainer(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -750,7 +789,9 @@ func TestEndpointDeleteWithActiveContainer(t *testing.T) { } func TestEndpointMultipleJoins(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -788,7 +829,9 @@ func TestEndpointMultipleJoins(t *testing.T) { } func TestEndpointInvalidLeave(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -847,7 +890,9 @@ func TestEndpointInvalidLeave(t *testing.T) { } func TestEndpointUpdateParent(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } n, err := createTestNetwork("bridge", "testnetwork", options.Generic{}, options.Generic{}) if err != nil { @@ -899,7 +944,9 @@ func TestEndpointUpdateParent(t *testing.T) { } func TestEnableIPv6(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } tmpResolvConf := []byte("search pommesfrites.fr\nnameserver 12.34.56.78\nnameserver 2001:4860:4860::8888") //take a copy of resolv.conf for restoring after test completes @@ -914,8 +961,17 @@ func TestEnableIPv6(t *testing.T) { } }() + ip, cidrv6, err := net.ParseCIDR("fe80::1/64") + if err != nil { + t.Fatal(err) + } + cidrv6.IP = ip + netOption := options.Generic{ netlabel.EnableIPv6: true, + netlabel.GenericData: options.Generic{ + "FixedCIDRv6": cidrv6, + }, } n, err := createTestNetwork("bridge", "testnetwork", options.Generic{}, netOption) @@ -962,7 +1018,9 @@ func TestEnableIPv6(t *testing.T) { } func TestNoEnableIPv6(t *testing.T) { - defer netutils.SetupTestNetNS(t)() + if !netutils.IsRunningInContainer() { + defer netutils.SetupTestNetNS(t)() + } tmpResolvConf := []byte("search pommesfrites.fr\nnameserver 12.34.56.78\nnameserver 2001:4860:4860::8888") expectedResolvConf := []byte("search pommesfrites.fr\nnameserver 12.34.56.78\n") @@ -1046,10 +1104,13 @@ func createGlobalInstance(t *testing.T) { t.Fatal(err) } - //testns = origns - testns, err = netns.New() - if err != nil { - t.Fatal(err) + if netutils.IsRunningInContainer() { + testns = origns + } else { + testns, err = netns.New() + if err != nil { + t.Fatal(err) + } } ctrlr, err = libnetwork.New() diff --git a/libnetwork/netutils/test_utils.go b/libnetwork/netutils/test_utils.go index 841b49ec71..d0a2fab789 100644 --- a/libnetwork/netutils/test_utils.go +++ b/libnetwork/netutils/test_utils.go @@ -1,11 +1,19 @@ package netutils import ( + "flag" "runtime" "syscall" "testing" ) +var runningInContainer = flag.Bool("incontainer", false, "Indicates if the test is running in a container") + +// IsRunningInContainer returns whether the test is running inside a container. +func IsRunningInContainer() bool { + return (*runningInContainer) +} + // SetupTestNetNS joins a new network namespace, and returns its associated // teardown function. // diff --git a/libnetwork/pkg/etchosts/etchosts_test.go b/libnetwork/pkg/etchosts/etchosts_test.go index c033904c31..8c8b87c016 100644 --- a/libnetwork/pkg/etchosts/etchosts_test.go +++ b/libnetwork/pkg/etchosts/etchosts_test.go @@ -5,6 +5,8 @@ import ( "io/ioutil" "os" "testing" + + _ "github.com/docker/libnetwork/netutils" ) func TestBuildDefault(t *testing.T) { diff --git a/libnetwork/pkg/iptables/iptables_test.go b/libnetwork/pkg/iptables/iptables_test.go index ced4262ce2..f5f79c7096 100644 --- a/libnetwork/pkg/iptables/iptables_test.go +++ b/libnetwork/pkg/iptables/iptables_test.go @@ -6,6 +6,8 @@ import ( "strconv" "strings" "testing" + + _ "github.com/docker/libnetwork/netutils" ) const chainName = "DOCKERTEST" diff --git a/libnetwork/pkg/options/options_test.go b/libnetwork/pkg/options/options_test.go index f1ab7ea495..ecd3b3b311 100644 --- a/libnetwork/pkg/options/options_test.go +++ b/libnetwork/pkg/options/options_test.go @@ -4,6 +4,8 @@ import ( "reflect" "strings" "testing" + + _ "github.com/docker/libnetwork/netutils" ) func TestGenerate(t *testing.T) { diff --git a/libnetwork/pkg/portallocator/portallocator_test.go b/libnetwork/pkg/portallocator/portallocator_test.go index 76ba37601d..6b55456c57 100644 --- a/libnetwork/pkg/portallocator/portallocator_test.go +++ b/libnetwork/pkg/portallocator/portallocator_test.go @@ -3,6 +3,8 @@ package portallocator import ( "net" "testing" + + _ "github.com/docker/libnetwork/netutils" ) func resetPortAllocator() { diff --git a/libnetwork/pkg/resolvconf/resolvconf_test.go b/libnetwork/pkg/resolvconf/resolvconf_test.go index dfb0a053f9..a21c7afb3e 100644 --- a/libnetwork/pkg/resolvconf/resolvconf_test.go +++ b/libnetwork/pkg/resolvconf/resolvconf_test.go @@ -5,6 +5,8 @@ import ( "io/ioutil" "os" "testing" + + _ "github.com/docker/libnetwork/netutils" ) func TestGet(t *testing.T) {