Skip to content

Commit

Permalink
Merge pull request #9046 from Luap99/fix-network-tests
Browse files Browse the repository at this point in the history 
Fix e2e network test flakes
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Jan 21, 2021
2 parents 7d297dd + 836fa4c commit d102d02
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 64 deletions.
2 changes: 1 addition & 1 deletion libpod/network/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
13 changes: 11 additions & 2 deletions libpod/network/lock.go
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
package network

import (
"os"
"path/filepath"

"github.com/containers/common/pkg/config"
"github.com/containers/storage"
)

// acquireCNILock gets a lock that should be used in create and
// 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
}
Expand Down
3 changes: 1 addition & 2 deletions libpod/network/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
94 changes: 52 additions & 42 deletions test/e2e/network_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -116,40 +117,41 @@ 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))

})

It("podman network create with name and subnet", 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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -255,74 +257,81 @@ 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())
Expect(containerIP.To4()).To(Not(BeNil()))
})

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())
})

Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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())
})

Expand Down
Loading

0 comments on commit d102d02

Please sign in to comment.