Enable rootless network connect/disconnect

With the new rootless cni supporting network connect/disconnect is easy.
Combine common setps into extra functions to prevent code duplication.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>

2 files changed, 62 insertions, 65 deletions

diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 287742706..a5056d834 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -310,13 +310,36 @@ func (r *Runtime) getRootlessCNINetNs(new bool) (*rootlessCNI, error) {
 	return rootlessCNINS, nil
 }
 
-// Create and configure a new network namespace for a container
-func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Result, error) {
+// setUpOCICNIPod will set up the cni networks, on error it will also tear down the cni
+// networks. If rootless it will join/create the rootless cni namespace.
+func (r *Runtime) setUpOCICNIPod(podNetwork ocicni.PodNetwork) ([]ocicni.NetResult, error) {
 	rootlessCNINS, err := r.getRootlessCNINetNs(true)
 	if err != nil {
 		return nil, err
 	}
+	var results []ocicni.NetResult
+	setUpPod := func() error {
+		results, err = r.netPlugin.SetUpPod(podNetwork)
+		if err != nil {
+			if err2 := r.netPlugin.TearDownPod(podNetwork); err2 != nil {
+				logrus.Errorf("Error tearing down partially created network namespace for container %s: %v", podNetwork.ID, err2)
+			}
+			return errors.Wrapf(err, "error configuring network namespace for container %s", podNetwork.ID)
+		}
+		return nil
+	}
+	// rootlessCNINS is nil if we are root
+	if rootlessCNINS != nil {
+		// execute the cni setup in the rootless net ns
+		err = rootlessCNINS.Do(setUpPod)
+	} else {
+		err = setUpPod()
+	}
+	return results, err
+}
 
+// Create and configure a new network namespace for a container
+func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Result, error) {
 	var requestedIP net.IP
 	if ctr.requestedIP != nil {
 		requestedIP = ctr.requestedIP
@@ -360,28 +383,7 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re
 		podNetwork.Aliases = aliases
 	}
 
-	var results []ocicni.NetResult
-	setUpPod := func() error {
-		results, err = r.netPlugin.SetUpPod(podNetwork)
-		if err != nil {
-			return errors.Wrapf(err, "error configuring network namespace for container %s", ctr.ID())
-		}
-		defer func() {
-			if err != nil {
-				if err2 := r.netPlugin.TearDownPod(podNetwork); err2 != nil {
-					logrus.Errorf("Error tearing down partially created network namespace for container %s: %v", ctr.ID(), err2)
-				}
-			}
-		}()
-		return nil
-	}
-	// rootlessCNINS is nil if we are root
-	if rootlessCNINS != nil {
-		// execute the cni setup in the rootless net ns
-		err = rootlessCNINS.Do(setUpPod)
-	} else {
-		err = setUpPod()
-	}
+	results, err := r.setUpOCICNIPod(podNetwork)
 	if err != nil {
 		return nil, err
 	}
@@ -514,6 +516,28 @@ func (r *Runtime) closeNetNS(ctr *Container) error {
 	return nil
 }
 
+// Tear down a container's CNI network configuration and joins the
+// rootless net ns as rootless user
+func (r *Runtime) teardownOCICNIPod(podNetwork ocicni.PodNetwork) error {
+	rootlessCNINS, err := r.getRootlessCNINetNs(false)
+	if err != nil {
+		return err
+	}
+	tearDownPod := func() error {
+		err := r.netPlugin.TearDownPod(podNetwork)
+		return errors.Wrapf(err, "error tearing down CNI namespace configuration for container %s", podNetwork.ID)
+	}
+
+	// rootlessCNINS is nil if we are root
+	if rootlessCNINS != nil {
+		// execute the cni setup in the rootless net ns
+		err = rootlessCNINS.Do(tearDownPod)
+	} else {
+		err = tearDownPod()
+	}
+	return err
+}
+
 // Tear down a container's CNI network configuration, but do not tear down the
 // namespace itself.
 func (r *Runtime) teardownCNI(ctr *Container) error {
@@ -530,10 +554,6 @@ func (r *Runtime) teardownCNI(ctr *Container) error {
 	}
 
 	if !ctr.config.NetMode.IsSlirp4netns() && len(networks) > 0 {
-		rootlessCNINS, err := r.getRootlessCNINetNs(false)
-		if err != nil {
-			return err
-		}
 		var requestedIP net.IP
 		if ctr.requestedIP != nil {
 			requestedIP = ctr.requestedIP
@@ -553,21 +573,7 @@ func (r *Runtime) teardownCNI(ctr *Container) error {
 		}
 
 		podNetwork := r.getPodNetwork(ctr.ID(), ctr.Name(), ctr.state.NetNS.Path(), networks, ctr.config.PortMappings, requestedIP, requestedMAC, ctr.state.NetInterfaceDescriptions)
-
-		tearDownPod := func() error {
-			if err := r.netPlugin.TearDownPod(podNetwork); err != nil {
-				return errors.Wrapf(err, "error tearing down CNI namespace configuration for container %s", ctr.ID())
-			}
-			return nil
-		}
-
-		// rootlessCNINS is nil if we are root
-		if rootlessCNINS != nil {
-			// execute the cni setup in the rootless net ns
-			err = rootlessCNINS.Do(tearDownPod)
-		} else {
-			err = tearDownPod()
-		}
+		err = r.teardownOCICNIPod(podNetwork)
 		return err
 	}
 	return nil
@@ -920,7 +926,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 	}
 
 	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 {
+	if err := c.runtime.teardownOCICNIPod(podConfig); err != nil {
 		return err
 	}
 
@@ -984,7 +990,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 	podConfig := c.runtime.getPodNetwork(c.ID(), c.Name(), c.state.NetNS.Path(), []string{netName}, c.config.PortMappings, nil, nil, c.state.NetInterfaceDescriptions)
 	podConfig.Aliases = make(map[string][]string, 1)
 	podConfig.Aliases[netName] = aliases
-	results, err := c.runtime.netPlugin.SetUpPod(podConfig)
+	results, err := c.runtime.setUpOCICNIPod(podConfig)
 	if err != nil {
 		return err
 	}
@@ -1031,9 +1037,6 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 
 // DisconnectContainerFromNetwork removes a container from its CNI network
 func (r *Runtime) DisconnectContainerFromNetwork(nameOrID, netName string, force bool) error {
-	if rootless.IsRootless() {
-		return errors.New("network connect is not enabled for rootless containers")
-	}
 	ctr, err := r.LookupContainer(nameOrID)
 	if err != nil {
 		return err
@@ -1043,9 +1046,6 @@ func (r *Runtime) DisconnectContainerFromNetwork(nameOrID, netName string, force
 
 // ConnectContainerToNetwork connects a container to a CNI network
 func (r *Runtime) ConnectContainerToNetwork(nameOrID, netName string, aliases []string) error {
-	if rootless.IsRootless() {
-		return errors.New("network disconnect is not enabled for rootless containers")
-	}
 	ctr, err := r.LookupContainer(nameOrID)
 	if err != nil {
 		return err
diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go
index e9a7b421f..6974c7614 100644
--- a/test/e2e/network_connect_disconnect_test.go
+++ b/test/e2e/network_connect_disconnect_test.go
@@ -33,14 +33,12 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("bad network name in disconnect should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		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 rootful")
 		netName := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -72,7 +70,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("podman network disconnect", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		netName := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -102,14 +99,12 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("bad network name in connect should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		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 rootful")
 		netName := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -141,7 +136,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("podman connect on a container that already is connected to the network should error", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		netName := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -159,7 +153,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 
 	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 rootful")
 		netName := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -203,18 +196,23 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("podman network connect when not running", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
-		netName := "aliasTest" + stringid.GenerateNonCryptoID()
-		session := podmanTest.Podman([]string{"network", "create", netName})
+		netName1 := "connect1" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName1})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(BeZero())
-		defer podmanTest.removeCNINetwork(netName)
+		defer podmanTest.removeCNINetwork(netName1)
 
-		ctr := podmanTest.Podman([]string{"create", "--name", "test", ALPINE, "top"})
+		netName2 := "connect2" + stringid.GenerateNonCryptoID()
+		session = podmanTest.Podman([]string{"network", "create", netName2})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName2)
+
+		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName1, ALPINE, "top"})
 		ctr.WaitWithDefaultTimeout()
 		Expect(ctr.ExitCode()).To(BeZero())
 
-		dis := podmanTest.Podman([]string{"network", "connect", netName, "test"})
+		dis := podmanTest.Podman([]string{"network", "connect", netName2, "test"})
 		dis.WaitWithDefaultTimeout()
 		Expect(dis.ExitCode()).To(BeZero())
 
@@ -286,7 +284,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("podman network disconnect when not running", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		netName1 := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName1})
 		session.WaitWithDefaultTimeout()