From c1cd512cb824c4c470efe7660c91ffeda62327bc Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 20 Jan 2021 22:24:20 +0100 Subject: [PATCH 1/2] Use random network names in the e2e tests Unlike the container storage all e2e test are using the same cni config directory. This causes problems if the network name already exists. Using random names will make the second run pass even if the first failed. This is only done to prevent full CI failures. Signed-off-by: Paul Holzinger --- test/e2e/network_create_test.go | 94 ++++++++++++++++++--------------- test/e2e/network_test.go | 35 ++++++------ 2 files changed, 70 insertions(+), 59 deletions(-) diff --git a/test/e2e/network_create_test.go b/test/e2e/network_create_test.go index 7e9a18ab2b..73e18cbcee 100644 --- a/test/e2e/network_create_test.go +++ b/test/e2e/network_create_test.go @@ -10,6 +10,7 @@ import ( cniversion "github.com/containernetworking/cni/pkg/version" "github.com/containers/podman/v2/libpod/network" . "github.com/containers/podman/v2/test/utils" + "github.com/containers/storage/pkg/stringid" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/pkg/errors" @@ -116,18 +117,19 @@ var _ = Describe("Podman network create", func() { results []network.NcList ) - nc := podmanTest.Podman([]string{"network", "create", "newname"}) + netName := "inspectnet-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", netName}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("newname") - inspect := podmanTest.Podman([]string{"network", "inspect", "newname"}) + inspect := podmanTest.Podman([]string{"network", "inspect", netName}) inspect.WaitWithDefaultTimeout() err := json.Unmarshal([]byte(inspect.OutputToString()), &results) Expect(err).To(BeNil()) result := results[0] - Expect(result["name"]).To(Equal("newname")) + Expect(result["name"]).To(Equal(netName)) }) @@ -135,21 +137,21 @@ var _ = Describe("Podman network create", func() { var ( results []network.NcList ) - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/24", "newnetwork"}) + netName := "subnet-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/24", netName}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("newnetwork") - // Inspect the network configuration - inspect := podmanTest.Podman([]string{"network", "inspect", "newnetwork"}) + inspect := podmanTest.Podman([]string{"network", "inspect", netName}) inspect.WaitWithDefaultTimeout() // JSON the network configuration into something usable err := json.Unmarshal([]byte(inspect.OutputToString()), &results) Expect(err).To(BeNil()) result := results[0] - Expect(result["name"]).To(Equal("newnetwork")) + Expect(result["name"]).To(Equal(netName)) // JSON the bridge info bridgePlugin, err := genericPluginsToBridge(result["plugins"], "bridge") @@ -161,7 +163,7 @@ var _ = Describe("Podman network create", func() { // best we can defer removeNetworkDevice(bridgePlugin.BrName) - try := podmanTest.Podman([]string{"run", "-it", "--rm", "--network", "newnetwork", ALPINE, "sh", "-c", "ip addr show eth0 | awk ' /inet / {print $2}'"}) + try := podmanTest.Podman([]string{"run", "-it", "--rm", "--network", netName, ALPINE, "sh", "-c", "ip addr show eth0 | awk ' /inet / {print $2}'"}) try.WaitWithDefaultTimeout() _, subnet, err := net.ParseCIDR("10.11.12.0/24") @@ -178,21 +180,21 @@ var _ = Describe("Podman network create", func() { var ( results []network.NcList ) - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:1:2:3:4::/64", "newIPv6network"}) + netName := "ipv6-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:1:2:3:4::/64", netName}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("newIPv6network") - // Inspect the network configuration - inspect := podmanTest.Podman([]string{"network", "inspect", "newIPv6network"}) + inspect := podmanTest.Podman([]string{"network", "inspect", netName}) inspect.WaitWithDefaultTimeout() // JSON the network configuration into something usable err := json.Unmarshal([]byte(inspect.OutputToString()), &results) Expect(err).To(BeNil()) result := results[0] - Expect(result["name"]).To(Equal("newIPv6network")) + Expect(result["name"]).To(Equal(netName)) // JSON the bridge info bridgePlugin, err := genericPluginsToBridge(result["plugins"], "bridge") @@ -203,7 +205,7 @@ var _ = Describe("Podman network create", func() { // best we can defer removeNetworkDevice(bridgePlugin.BrName) - try := podmanTest.Podman([]string{"run", "-it", "--rm", "--network", "newIPv6network", ALPINE, "sh", "-c", "ip addr show eth0 | grep global | awk ' /inet6 / {print $2}'"}) + try := podmanTest.Podman([]string{"run", "-it", "--rm", "--network", netName, ALPINE, "sh", "-c", "ip addr show eth0 | grep global | awk ' /inet6 / {print $2}'"}) try.WaitWithDefaultTimeout() _, subnet, err := net.ParseCIDR("fd00:1:2:3:4::/64") @@ -219,21 +221,21 @@ var _ = Describe("Podman network create", func() { var ( results []network.NcList ) - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:4:3:2:1::/64", "--ipv6", "newDualStacknetwork"}) + netName := "dual-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:4:3:2:1::/64", "--ipv6", netName}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("newDualStacknetwork") - // Inspect the network configuration - inspect := podmanTest.Podman([]string{"network", "inspect", "newDualStacknetwork"}) + inspect := podmanTest.Podman([]string{"network", "inspect", netName}) inspect.WaitWithDefaultTimeout() // JSON the network configuration into something usable err := json.Unmarshal([]byte(inspect.OutputToString()), &results) Expect(err).To(BeNil()) result := results[0] - Expect(result["name"]).To(Equal("newDualStacknetwork")) + Expect(result["name"]).To(Equal(netName)) // JSON the bridge info bridgePlugin, err := genericPluginsToBridge(result["plugins"], "bridge") @@ -245,7 +247,7 @@ var _ = Describe("Podman network create", func() { // best we can defer removeNetworkDevice(bridgePlugin.BrName) - try := podmanTest.Podman([]string{"run", "-it", "--rm", "--network", "newDualStacknetwork", ALPINE, "sh", "-c", "ip addr show eth0 | grep global | awk ' /inet6 / {print $2}'"}) + try := podmanTest.Podman([]string{"run", "-it", "--rm", "--network", netName, ALPINE, "sh", "-c", "ip addr show eth0 | grep global | awk ' /inet6 / {print $2}'"}) try.WaitWithDefaultTimeout() _, subnet, err := net.ParseCIDR("fd00:4:3:2:1::/64") @@ -255,7 +257,7 @@ var _ = Describe("Podman network create", func() { // Ensure that the IP the container got is within the subnet the user asked for Expect(subnet.Contains(containerIP)).To(BeTrue()) // verify the container has an IPv4 address too (the IPv4 subnet is autogenerated) - try = podmanTest.Podman([]string{"run", "-it", "--rm", "--network", "newDualStacknetwork", ALPINE, "sh", "-c", "ip addr show eth0 | awk ' /inet / {print $2}'"}) + try = podmanTest.Podman([]string{"run", "-it", "--rm", "--network", netName, ALPINE, "sh", "-c", "ip addr show eth0 | awk ' /inet / {print $2}'"}) try.WaitWithDefaultTimeout() containerIP, _, err = net.ParseCIDR(try.OutputToString()) Expect(err).To(BeNil()) @@ -263,66 +265,73 @@ var _ = Describe("Podman network create", func() { }) It("podman network create with invalid subnet", func() { - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/17000", "fail"}) + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/17000", stringid.GenerateNonCryptoID()}) nc.WaitWithDefaultTimeout() Expect(nc).To(ExitWithError()) }) It("podman network create with ipv4 subnet and ipv6 flag", func() { - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/24", "--ipv6", "fail"}) + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/24", "--ipv6", stringid.GenerateNonCryptoID()}) nc.WaitWithDefaultTimeout() Expect(nc).To(ExitWithError()) }) It("podman network create with empty subnet and ipv6 flag", func() { - nc := podmanTest.Podman([]string{"network", "create", "--ipv6", "fail"}) + nc := podmanTest.Podman([]string{"network", "create", "--ipv6", stringid.GenerateNonCryptoID()}) nc.WaitWithDefaultTimeout() Expect(nc).To(ExitWithError()) }) It("podman network create with invalid IP", func() { - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.0/17000", "fail"}) + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.0/17000", stringid.GenerateNonCryptoID()}) nc.WaitWithDefaultTimeout() Expect(nc).To(ExitWithError()) }) It("podman network create with invalid gateway for subnet", func() { - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/24", "--gateway", "192.168.1.1", "fail"}) + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.12.0/24", "--gateway", "192.168.1.1", stringid.GenerateNonCryptoID()}) nc.WaitWithDefaultTimeout() Expect(nc).To(ExitWithError()) }) It("podman network create two networks with same name should fail", func() { - nc := podmanTest.Podman([]string{"network", "create", "samename"}) + netName := "same-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", netName}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("samename") - ncFail := podmanTest.Podman([]string{"network", "create", "samename"}) + ncFail := podmanTest.Podman([]string{"network", "create", netName}) ncFail.WaitWithDefaultTimeout() Expect(ncFail).To(ExitWithError()) }) It("podman network create two networks with same subnet should fail", func() { - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.13.0/24", "subnet1"}) + netName1 := "sub1-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.13.0/24", netName1}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName1) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("subnet1") - ncFail := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.13.0/24", "subnet2"}) + netName2 := "sub2-" + stringid.GenerateNonCryptoID() + ncFail := podmanTest.Podman([]string{"network", "create", "--subnet", "10.11.13.0/24", netName2}) ncFail.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName2) Expect(ncFail).To(ExitWithError()) }) It("podman network create two IPv6 networks with same subnet should fail", func() { SkipIfRootless("FIXME It needs the ip6tables modules loaded") - nc := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:4:4:4:4::/64", "--ipv6", "subnet1v6"}) + netName1 := "subipv61-" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:4:4:4:4::/64", "--ipv6", netName1}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName1) Expect(nc.ExitCode()).To(BeZero()) - defer podmanTest.removeCNINetwork("subnet1v6") - ncFail := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:4:4:4:4::/64", "--ipv6", "subnet2v6"}) + netName2 := "subipv62-" + stringid.GenerateNonCryptoID() + ncFail := podmanTest.Podman([]string{"network", "create", "--subnet", "fd00:4:4:4:4::/64", "--ipv6", netName2}) ncFail.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(netName2) Expect(ncFail).To(ExitWithError()) }) @@ -333,11 +342,11 @@ var _ = Describe("Podman network create", func() { }) It("podman network create with mtu option", func() { - net := "mtu-test" + net := "mtu-test" + stringid.GenerateNonCryptoID() nc := podmanTest.Podman([]string{"network", "create", "--opt", "mtu=9000", net}) nc.WaitWithDefaultTimeout() - Expect(nc.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(net) + Expect(nc.ExitCode()).To(BeZero()) nc = podmanTest.Podman([]string{"network", "inspect", net}) nc.WaitWithDefaultTimeout() @@ -346,11 +355,11 @@ var _ = Describe("Podman network create", func() { }) It("podman network create with vlan option", func() { - net := "vlan-test" + net := "vlan-test" + stringid.GenerateNonCryptoID() nc := podmanTest.Podman([]string{"network", "create", "--opt", "vlan=9", net}) nc.WaitWithDefaultTimeout() - Expect(nc.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(net) + Expect(nc.ExitCode()).To(BeZero()) nc = podmanTest.Podman([]string{"network", "inspect", net}) nc.WaitWithDefaultTimeout() @@ -359,9 +368,10 @@ var _ = Describe("Podman network create", func() { }) It("podman network create with invalid option", func() { - net := "invalid-test" + net := "invalid-test" + stringid.GenerateNonCryptoID() nc := podmanTest.Podman([]string{"network", "create", "--opt", "foo=bar", net}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(net) Expect(nc).To(ExitWithError()) }) diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go index 98512f01a9..e2080244b7 100644 --- a/test/e2e/network_test.go +++ b/test/e2e/network_test.go @@ -238,11 +238,11 @@ var _ = Describe("Podman network", func() { }) It("podman inspect container single CNI network", func() { - netName := "testNetSingleCNI" + netName := "net-" + stringid.GenerateNonCryptoID() network := podmanTest.Podman([]string{"network", "create", "--subnet", "10.50.50.0/24", netName}) network.WaitWithDefaultTimeout() - Expect(network.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName) + Expect(network.ExitCode()).To(BeZero()) ctrName := "testCtr" container := podmanTest.Podman([]string{"run", "-dt", "--network", netName, "--name", ctrName, ALPINE, "top"}) @@ -268,17 +268,17 @@ var _ = Describe("Podman network", func() { }) It("podman inspect container two CNI networks (container not running)", func() { - netName1 := "testNetThreeCNI1" + netName1 := "net1-" + stringid.GenerateNonCryptoID() network1 := podmanTest.Podman([]string{"network", "create", netName1}) network1.WaitWithDefaultTimeout() - Expect(network1.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName1) + Expect(network1.ExitCode()).To(BeZero()) - netName2 := "testNetThreeCNI2" + netName2 := "net2-" + stringid.GenerateNonCryptoID() network2 := podmanTest.Podman([]string{"network", "create", netName2}) network2.WaitWithDefaultTimeout() - Expect(network2.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName2) + Expect(network2.ExitCode()).To(BeZero()) ctrName := "testCtr" container := podmanTest.Podman([]string{"create", "--network", fmt.Sprintf("%s,%s", netName1, netName2), "--name", ctrName, ALPINE, "top"}) @@ -305,17 +305,17 @@ var _ = Describe("Podman network", func() { }) It("podman inspect container two CNI networks", func() { - netName1 := "testNetTwoCNI1" + netName1 := "net1-" + stringid.GenerateNonCryptoID() network1 := podmanTest.Podman([]string{"network", "create", "--subnet", "10.50.51.0/25", netName1}) network1.WaitWithDefaultTimeout() - Expect(network1.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName1) + Expect(network1.ExitCode()).To(BeZero()) - netName2 := "testNetTwoCNI2" + netName2 := "net2-" + stringid.GenerateNonCryptoID() network2 := podmanTest.Podman([]string{"network", "create", "--subnet", "10.50.51.128/26", netName2}) network2.WaitWithDefaultTimeout() - Expect(network2.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName2) + Expect(network2.ExitCode()).To(BeZero()) ctrName := "testCtr" container := podmanTest.Podman([]string{"run", "-dt", "--network", fmt.Sprintf("%s,%s", netName1, netName2), "--name", ctrName, ALPINE, "top"}) @@ -352,11 +352,11 @@ var _ = Describe("Podman network", func() { }) It("podman network remove --force with pod", func() { - netName := "testnet" + netName := "net-" + stringid.GenerateNonCryptoID() session := podmanTest.Podman([]string{"network", "create", netName}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName) + Expect(session.ExitCode()).To(BeZero()) session = podmanTest.Podman([]string{"pod", "create", "--network", netName}) session.WaitWithDefaultTimeout() @@ -388,17 +388,17 @@ var _ = Describe("Podman network", func() { }) It("podman network remove with two networks", func() { - netName1 := "net1" + netName1 := "net1-" + stringid.GenerateNonCryptoID() session := podmanTest.Podman([]string{"network", "create", netName1}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName1) + Expect(session.ExitCode()).To(BeZero()) - netName2 := "net2" + netName2 := "net2-" + stringid.GenerateNonCryptoID() session = podmanTest.Podman([]string{"network", "create", netName2}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName2) + Expect(session.ExitCode()).To(BeZero()) session = podmanTest.Podman([]string{"network", "rm", netName1, netName2}) session.WaitWithDefaultTimeout() @@ -413,8 +413,8 @@ var _ = Describe("Podman network", func() { netName := "aliasTest" + stringid.GenerateNonCryptoID() session := podmanTest.Podman([]string{"network", "create", netName}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(BeZero()) defer podmanTest.removeCNINetwork(netName) + Expect(session.ExitCode()).To(BeZero()) top := podmanTest.Podman([]string{"run", "-dt", "--name=web", "--network=" + netName, "--network-alias=web1", "--network-alias=web2", nginx}) top.WaitWithDefaultTimeout() @@ -450,6 +450,7 @@ var _ = Describe("Podman network", func() { net := "macvlan" + stringid.GenerateNonCryptoID() nc := podmanTest.Podman([]string{"network", "create", "--macvlan", "lo", net}) nc.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(net) Expect(nc.ExitCode()).To(Equal(0)) nc = podmanTest.Podman([]string{"network", "rm", net}) From 836fa4c493c3809da4bbcbbec0bf5ceb954e7410 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 20 Jan 2021 22:56:13 +0100 Subject: [PATCH 2/2] Move the cni lock file into the cni config dir Commit(fe3faa517e1b) introduced a lock file for network create/rm calls. There is a problem with the location of the lock file. The lock file was stored in the tmpdir. Running multiple podman network create/remove commands in parallel with different tmpdirs made the lockfile inaccessible to the other process, and so parallel read/write operations to the cni config directory continued to occur. This scenario happened frequently during the e2e tests and caused some flakes. Fixes #9041 Signed-off-by: Paul Holzinger --- libpod/network/create.go | 2 +- libpod/network/lock.go | 13 +++++++++++-- libpod/network/network.go | 3 +-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/libpod/network/create.go b/libpod/network/create.go index 094fbe3498..e7f65358bd 100644 --- a/libpod/network/create.go +++ b/libpod/network/create.go @@ -23,7 +23,7 @@ func Create(name string, options entities.NetworkCreateOptions, runtimeConfig *c return nil, err } // Acquire a lock for CNI - l, err := acquireCNILock(filepath.Join(runtimeConfig.Engine.TmpDir, LockFileName)) + l, err := acquireCNILock(runtimeConfig) if err != nil { return nil, err } diff --git a/libpod/network/lock.go b/libpod/network/lock.go index 0395359eb4..037f41efaa 100644 --- a/libpod/network/lock.go +++ b/libpod/network/lock.go @@ -1,6 +1,10 @@ package network import ( + "os" + "path/filepath" + + "github.com/containers/common/pkg/config" "github.com/containers/storage" ) @@ -8,8 +12,13 @@ import ( // delete cases to avoid unwanted collisions in network names. // TODO this uses a file lock and should be converted to shared memory // when we have a more general shared memory lock in libpod -func acquireCNILock(lockPath string) (*CNILock, error) { - l, err := storage.GetLockfile(lockPath) +func acquireCNILock(config *config.Config) (*CNILock, error) { + cniDir := GetCNIConfDir(config) + err := os.MkdirAll(cniDir, 0755) + if err != nil { + return nil, err + } + l, err := storage.GetLockfile(filepath.Join(cniDir, LockFileName)) if err != nil { return nil, err } diff --git a/libpod/network/network.go b/libpod/network/network.go index 89f0b67ac4..0fb878b18f 100644 --- a/libpod/network/network.go +++ b/libpod/network/network.go @@ -6,7 +6,6 @@ import ( "encoding/json" "net" "os" - "path/filepath" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator" @@ -172,7 +171,7 @@ func ValidateUserNetworkIsAvailable(config *config.Config, userNet *net.IPNet) e // RemoveNetwork removes a given network by name. If the network has container associated with it, that // must be handled outside the context of this. func RemoveNetwork(config *config.Config, name string) error { - l, err := acquireCNILock(filepath.Join(config.Engine.TmpDir, LockFileName)) + l, err := acquireCNILock(config) if err != nil { return err }