Merge pull request #8515 from baude/netconnectstate

network connect disconnect on non-running containers

5 files changed, 237 insertions, 170 deletions

diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 4e7ffaf81..15e470c80 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -1047,21 +1047,25 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 		return err
 	}
 
+	if err := c.runtime.state.NetworkDisconnect(c, netName); err != nil {
+		return err
+	}
+
+	c.newNetworkEvent(events.NetworkDisconnect, netName)
 	if c.state.State != define.ContainerStateRunning {
-		return errors.Wrapf(define.ErrCtrStateInvalid, "cannot disconnect container %s from networks as it is not running", nameOrID)
+		return nil
 	}
+
 	if c.state.NetNS == nil {
 		return errors.Wrapf(define.ErrNoNetwork, "unable to disconnect %s from %s", nameOrID, netName)
 	}
+
 	podConfig := c.runtime.getPodNetwork(c.ID(), c.Name(), c.state.NetNS.Path(), []string{netName}, c.config.PortMappings, nil, nil, c.state.NetInterfaceDescriptions)
 	if err := c.runtime.netPlugin.TearDownPod(podConfig); err != nil {
 		return err
 	}
-	if err := c.runtime.state.NetworkDisconnect(c, netName); err != nil {
-		return err
-	}
 
-	// update network status
+	// update network status if container is not running
 	networkStatus := c.state.NetworkStatus
 	// clip out the index of the network
 	tmpNetworkStatus := make([]*cnitypes.Result, len(networkStatus)-1)
@@ -1071,7 +1075,6 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 		}
 	}
 	c.state.NetworkStatus = tmpNetworkStatus
-	c.newNetworkEvent(events.NetworkDisconnect, netName)
 	return c.save()
 }
 
@@ -1096,15 +1099,16 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 		return err
 	}
 
+	if err := c.runtime.state.NetworkConnect(c, netName, aliases); err != nil {
+		return err
+	}
+	c.newNetworkEvent(events.NetworkConnect, netName)
 	if c.state.State != define.ContainerStateRunning {
-		return errors.Wrapf(define.ErrCtrStateInvalid, "cannot connect container %s to networks as it is not running", nameOrID)
+		return nil
 	}
 	if c.state.NetNS == nil {
 		return errors.Wrapf(define.ErrNoNetwork, "unable to connect %s to %s", nameOrID, netName)
 	}
-	if err := c.runtime.state.NetworkConnect(c, netName, aliases); err != nil {
-		return err
-	}
 
 	ctrNetworks, _, err := c.networks()
 	if err != nil {
@@ -1159,7 +1163,6 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 		networkStatus[index] = networkResults[0]
 		c.state.NetworkStatus = networkStatus
 	}
-	c.newNetworkEvent(events.NetworkConnect, netName)
 	return c.save()
 }
 
diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go
index f107fe76b..16d8bb770 100644
--- a/test/e2e/common_test.go
+++ b/test/e2e/common_test.go
@@ -788,3 +788,9 @@ func generateNetworkConfig(p *PodmanTestIntegration) (string, string) {
 
 	return name, path
 }
+
+func (p *PodmanTestIntegration) removeCNINetwork(name string) {
+	session := p.Podman([]string{"network", "rm", "-f", name})
+	session.WaitWithDefaultTimeout()
+	Expect(session.ExitCode()).To(BeNumerically("<=", 1))
+}
diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go
new file mode 100644
index 000000000..7cdad9bf2
--- /dev/null
+++ b/test/e2e/network_connect_disconnect_test.go
@@ -0,0 +1,217 @@
+package integration
+
+import (
+	"os"
+
+	. "github.com/containers/podman/v2/test/utils"
+	"github.com/containers/storage/pkg/stringid"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Podman network connect and disconnect", func() {
+	var (
+		tempdir    string
+		err        error
+		podmanTest *PodmanTestIntegration
+	)
+
+	BeforeEach(func() {
+		tempdir, err = CreateTempDirInTempDir()
+		if err != nil {
+			os.Exit(1)
+		}
+		podmanTest = PodmanTestCreate(tempdir)
+		podmanTest.Setup()
+	})
+
+	AfterEach(func() {
+		podmanTest.Cleanup()
+		f := CurrentGinkgoTestDescription()
+		processTestResult(f)
+
+	})
+
+	It("bad network name in disconnect should result in error", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		dis := podmanTest.Podman([]string{"network", "disconnect", "foobar", "test"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).ToNot(BeZero())
+
+	})
+
+	It("bad container name in network disconnect should result in error", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		dis := podmanTest.Podman([]string{"network", "disconnect", netName, "foobar"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).ToNot(BeZero())
+
+	})
+
+	It("podman network disconnect", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netName, ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+
+		dis := podmanTest.Podman([]string{"network", "disconnect", netName, "test"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).To(BeZero())
+
+		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).ToNot(BeZero())
+	})
+
+	It("bad network name in connect should result in error", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		dis := podmanTest.Podman([]string{"network", "connect", "foobar", "test"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).ToNot(BeZero())
+
+	})
+
+	It("bad container name in network connect should result in error", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		dis := podmanTest.Podman([]string{"network", "connect", netName, "foobar"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).ToNot(BeZero())
+
+	})
+
+	It("podman connect on a container that already is connected to the network should error", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName, ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		con := podmanTest.Podman([]string{"network", "connect", netName, "test"})
+		con.WaitWithDefaultTimeout()
+		Expect(con.ExitCode()).ToNot(BeZero())
+	})
+
+	It("podman network connect", func() {
+		SkipIfRemote("This requires a pending PR to be merged before it will work")
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netName, ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+
+		// Create a second network
+		newNetName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session = podmanTest.Podman([]string{"network", "create", newNetName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(newNetName)
+
+		connect := podmanTest.Podman([]string{"network", "connect", newNetName, "test"})
+		connect.WaitWithDefaultTimeout()
+		Expect(connect.ExitCode()).To(BeZero())
+
+		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+	})
+
+	It("podman network connect when not running", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		ctr := podmanTest.Podman([]string{"create", "--name", "test", ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		dis := podmanTest.Podman([]string{"network", "connect", netName, "test"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).To(BeZero())
+
+		start := podmanTest.Podman([]string{"start", "test"})
+		start.WaitWithDefaultTimeout()
+		Expect(start.ExitCode()).To(BeZero())
+
+		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+
+		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+	})
+
+	It("podman network disconnect when not running", func() {
+		SkipIfRootless("network connect and disconnect are only rootfull")
+		netName1 := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName1})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName1)
+
+		netName2 := "aliasTest" + stringid.GenerateNonCryptoID()
+		session2 := podmanTest.Podman([]string{"network", "create", netName2})
+		session2.WaitWithDefaultTimeout()
+		Expect(session2.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName2)
+
+		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName1 + "," + netName2, ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		dis := podmanTest.Podman([]string{"network", "disconnect", netName1, "test"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).To(BeZero())
+
+		start := podmanTest.Podman([]string{"start", "test"})
+		start.WaitWithDefaultTimeout()
+		Expect(start.ExitCode()).To(BeZero())
+
+		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+
+		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).ToNot(BeZero())
+	})
+})
diff --git a/test/e2e/network_create_test.go b/test/e2e/network_create_test.go
index cb997d10a..043046c33 100644
--- a/test/e2e/network_create_test.go
+++ b/test/e2e/network_create_test.go
@@ -55,12 +55,6 @@ func genericPluginsToPortMap(plugins interface{}, pluginType string) (network.Po
 	return portMap, err
 }
 
-func (p *PodmanTestIntegration) removeCNINetwork(name string) {
-	session := p.Podman([]string{"network", "rm", "-f", name})
-	session.WaitWithDefaultTimeout()
-	Expect(session.ExitCode()).To(BeNumerically("<=", 1))
-}
-
 func removeNetworkDevice(name string) {
 	session := SystemExec("ip", []string{"link", "delete", name})
 	session.WaitWithDefaultTimeout()
diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go
index adcf74f7e..20e1d5b6b 100644
--- a/test/e2e/network_test.go
+++ b/test/e2e/network_test.go
@@ -347,159 +347,6 @@ var _ = Describe("Podman network", func() {
 		Expect(c3.ExitCode()).To(BeZero())
 	})
 
-	It("bad network name in disconnect should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		dis := podmanTest.Podman([]string{"network", "disconnect", "foobar", "test"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).ToNot(BeZero())
-
-	})
-
-	It("bad container name in network disconnect should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		dis := podmanTest.Podman([]string{"network", "disconnect", netName, "foobar"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).ToNot(BeZero())
-
-	})
-
-	It("podman network disconnect with invalid container state should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName, ALPINE, "top"})
-		ctr.WaitWithDefaultTimeout()
-		Expect(ctr.ExitCode()).To(BeZero())
-
-		dis := podmanTest.Podman([]string{"network", "disconnect", netName, "test"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).ToNot(BeZero())
-	})
-
-	It("podman network disconnect", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netName, ALPINE, "top"})
-		ctr.WaitWithDefaultTimeout()
-		Expect(ctr.ExitCode()).To(BeZero())
-
-		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
-		exec.WaitWithDefaultTimeout()
-		Expect(exec.ExitCode()).To(BeZero())
-
-		dis := podmanTest.Podman([]string{"network", "disconnect", netName, "test"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).To(BeZero())
-
-		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
-		exec.WaitWithDefaultTimeout()
-		Expect(exec.ExitCode()).ToNot(BeZero())
-	})
-
-	It("bad network name in connect should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		dis := podmanTest.Podman([]string{"network", "connect", "foobar", "test"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).ToNot(BeZero())
-
-	})
-
-	It("bad container name in network connect should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		dis := podmanTest.Podman([]string{"network", "connect", netName, "foobar"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).ToNot(BeZero())
-
-	})
-
-	It("podman connect on a container that already is connected to the network should error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName, ALPINE, "top"})
-		ctr.WaitWithDefaultTimeout()
-		Expect(ctr.ExitCode()).To(BeZero())
-
-		con := podmanTest.Podman([]string{"network", "connect", netName, "test"})
-		con.WaitWithDefaultTimeout()
-		Expect(con.ExitCode()).ToNot(BeZero())
-	})
-
-	It("podman network connect with invalid container state should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName, ALPINE, "top"})
-		ctr.WaitWithDefaultTimeout()
-		Expect(ctr.ExitCode()).To(BeZero())
-
-		dis := podmanTest.Podman([]string{"network", "connect", netName, "test"})
-		dis.WaitWithDefaultTimeout()
-		Expect(dis.ExitCode()).ToNot(BeZero())
-	})
-
-	It("podman network connect", func() {
-		SkipIfRemote("This requires a pending PR to be merged before it will work")
-		SkipIfRootless("network connect and disconnect are only rootfull")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
-
-		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netName, ALPINE, "top"})
-		ctr.WaitWithDefaultTimeout()
-		Expect(ctr.ExitCode()).To(BeZero())
-
-		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
-		exec.WaitWithDefaultTimeout()
-		Expect(exec.ExitCode()).To(BeZero())
-
-		// Create a second network
-		newNetName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session = podmanTest.Podman([]string{"network", "create", newNetName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(newNetName)
-
-		connect := podmanTest.Podman([]string{"network", "connect", newNetName, "test"})
-		connect.WaitWithDefaultTimeout()
-		Expect(connect.ExitCode()).To(BeZero())
-
-		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"})
-		exec.WaitWithDefaultTimeout()
-		Expect(exec.ExitCode()).To(BeZero())
-	})
-
 	It("podman network create/remove macvlan", func() {
 		net := "macvlan" + stringid.GenerateNonCryptoID()
 		nc := podmanTest.Podman([]string{"network", "create", "--macvlan", "lo", net})