Merge pull request #3756 from gabibeyer/rootlessOrdering

rootless: Rearrange setup of rootless containers

4 files changed, 61 insertions, 30 deletions

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 230b5b888..e7b4b2b22 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -78,15 +78,21 @@ func (c *Container) prepare() (Err error) {
 		// Set up network namespace if not already set up
 		if c.config.CreateNetNS && c.state.NetNS == nil && !c.config.PostConfigureNetNS {
 			netNS, networkStatus, createNetNSErr = c.runtime.createNetNS(c)
+			if createNetNSErr != nil {
+				return
+			}
 
 			tmpStateLock.Lock()
 			defer tmpStateLock.Unlock()
 
 			// Assign NetNS attributes to container
-			if createNetNSErr == nil {
-				c.state.NetNS = netNS
-				c.state.NetworkStatus = networkStatus
-			}
+			c.state.NetNS = netNS
+			c.state.NetworkStatus = networkStatus
+		}
+
+		// handle rootless network namespace setup
+		if c.state.NetNS != nil && c.config.NetMode == "slirp4netns" && !c.config.PostConfigureNetNS {
+			createNetNSErr = c.runtime.setupRootlessNetNS(c)
 		}
 	}()
 	// Mount storage if not mounted
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 67dd0150b..d854a2de6 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -90,9 +90,6 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re
 
 // Create and configure a new network namespace for a container
 func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q []*cnitypes.Result, err error) {
-	if rootless.IsRootless() {
-		return nil, nil, errors.New("cannot configure a new network namespace in rootless mode, only --network=slirp4netns is supported")
-	}
 	ctrNS, err := netns.NewNS()
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "error creating network namespace for container %s", ctr.ID())
@@ -110,7 +107,10 @@ func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q []*cnitypes.Result,
 
 	logrus.Debugf("Made network namespace at %s for container %s", ctrNS.Path(), ctr.ID())
 
-	networkStatus, err := r.configureNetNS(ctr, ctrNS)
+	networkStatus := []*cnitypes.Result{}
+	if !rootless.IsRootless() {
+		networkStatus, err = r.configureNetNS(ctr, ctrNS)
+	}
 	return ctrNS, networkStatus, err
 }
 
@@ -138,9 +138,6 @@ func checkSlirpFlags(path string) (bool, bool, bool, error) {
 
 // Configure the network namespace for a rootless container
 func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) {
-	defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR)
-	defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncW)
-
 	path := r.config.NetworkCmdPath
 
 	if path == "" {
@@ -164,7 +161,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) {
 
 	cmdArgs := []string{}
 	if havePortMapping {
-		cmdArgs = append(cmdArgs, "--api-socket", apiSocket, fmt.Sprintf("%d", ctr.state.PID))
+		cmdArgs = append(cmdArgs, "--api-socket", apiSocket)
 	}
 	dhp, mtu, sandbox, err := checkSlirpFlags(path)
 	if err != nil {
@@ -179,13 +176,32 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) {
 	if sandbox {
 		cmdArgs = append(cmdArgs, "--enable-sandbox")
 	}
-	cmdArgs = append(cmdArgs, "-c", "-e", "3", "-r", "4", fmt.Sprintf("%d", ctr.state.PID), "tap0")
 
-	cmd := exec.Command(path, cmdArgs...)
+	// the slirp4netns arguments being passed are describes as follows:
+	// from the slirp4netns documentation: https://github.com/rootless-containers/slirp4netns
+	// -c, --configure Brings up the tap interface
+	// -e, --exit-fd=FD specify the FD for terminating slirp4netns
+	// -r, --ready-fd=FD specify the FD to write to when the initialization steps are finished
+	cmdArgs = append(cmdArgs, "-c", "-e", "3", "-r", "4")
+	if !ctr.config.PostConfigureNetNS {
+		ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe()
+		if err != nil {
+			return errors.Wrapf(err, "failed to create rootless network sync pipe")
+		}
+		cmdArgs = append(cmdArgs, "--netns-type=path", ctr.state.NetNS.Path(), "tap0")
+	} else {
+		defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR)
+		defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncW)
+		cmdArgs = append(cmdArgs, fmt.Sprintf("%d", ctr.state.PID), "tap0")
+	}
 
+	cmd := exec.Command(path, cmdArgs...)
+	logrus.Debugf("slirp4netns command: %s", strings.Join(cmd.Args, " "))
 	cmd.SysProcAttr = &syscall.SysProcAttr{
 		Setpgid: true,
 	}
+
+	// Leak one end of the pipe in slirp4netns, the other will be sent to conmon
 	cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessSlirpSyncR, syncW)
 
 	if err := cmd.Start(); err != nil {
@@ -388,20 +404,22 @@ func (r *Runtime) teardownNetNS(ctr *Container) error {
 
 	logrus.Debugf("Tearing down network namespace at %s for container %s", ctr.state.NetNS.Path(), ctr.ID())
 
-	var requestedIP net.IP
-	if ctr.requestedIP != nil {
-		requestedIP = ctr.requestedIP
-		// cancel request for a specific IP in case the container is reused later
-		ctr.requestedIP = nil
-	} else {
-		requestedIP = ctr.config.StaticIP
-	}
+	// rootless containers do not use the CNI plugin
+	if !rootless.IsRootless() {
+		var requestedIP net.IP
+		if ctr.requestedIP != nil {
+			requestedIP = ctr.requestedIP
+			// cancel request for a specific IP in case the container is reused later
+			ctr.requestedIP = nil
+		} else {
+			requestedIP = ctr.config.StaticIP
+		}
 
-	podNetwork := r.getPodNetwork(ctr.ID(), ctr.Name(), ctr.state.NetNS.Path(), ctr.config.Networks, ctr.config.PortMappings, requestedIP)
+		podNetwork := r.getPodNetwork(ctr.ID(), ctr.Name(), ctr.state.NetNS.Path(), ctr.config.Networks, ctr.config.PortMappings, requestedIP)
 
-	// The network may have already been torn down, so don't fail here, just log
-	if err := r.netPlugin.TearDownPod(podNetwork); err != nil {
-		return errors.Wrapf(err, "error tearing down CNI namespace configuration for container %s", ctr.ID())
+		if err := r.netPlugin.TearDownPod(podNetwork); err != nil {
+			return errors.Wrapf(err, "error tearing down CNI namespace configuration for container %s", ctr.ID())
+		}
 	}
 
 	// First unmount the namespace
diff --git a/libpod/oci_internal_linux.go b/libpod/oci_internal_linux.go
index a90af32ed..a5cce795b 100644
--- a/libpod/oci_internal_linux.go
+++ b/libpod/oci_internal_linux.go
@@ -131,9 +131,14 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Containe
 	}
 
 	if ctr.config.NetMode.IsSlirp4netns() {
-		ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe()
-		if err != nil {
-			return errors.Wrapf(err, "failed to create rootless network sync pipe")
+		if ctr.config.PostConfigureNetNS {
+			ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe()
+			if err != nil {
+				return errors.Wrapf(err, "failed to create rootless network sync pipe")
+			}
+		} else {
+			defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR)
+			defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncW)
 		}
 		// Leak one end in conmon, the other one will be leaked into slirp4netns
 		cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessSlirpSyncW)
diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go
index ad6662f03..6a27c2800 100644
--- a/libpod/runtime_pod_infra_linux.go
+++ b/libpod/runtime_pod_infra_linux.go
@@ -99,7 +99,9 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID
 	if isRootless {
 		netmode = "slirp4netns"
 	}
-	options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, isRootless, netmode, networks))
+	// PostConfigureNetNS should not be set since user namespace sharing is not implemented
+	// and rootless networking no longer supports post configuration setup
+	options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, false, netmode, networks))
 
 	return r.newContainer(ctx, g.Config, options...)
 }