Merge pull request #3310 from gabibeyer/rootlessKata

rootless: Rearrange setup of rootless containers ***CIRRUS: TEST IMAGES***

8 files changed, 103 insertions, 37 deletions

diff --git a/libpod/container.go b/libpod/container.go
index 2d96b1120..9c01d2adf 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -377,6 +377,9 @@ type ContainerConfig struct {
 	RestartRetries uint `json:"restart_retries,omitempty"`
 	// TODO log options for log drivers
 
+	// PostConfigureNetNS needed when a user namespace is created by an OCI runtime
+	// if the network namespace is created before the user namespace it will be
+	// owned by the wrong user namespace.
 	PostConfigureNetNS bool `json:"postConfigureNetNS"`
 
 	// OCIRuntime used to create the container
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 6dbd53fbf..57c2854ea 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -63,12 +63,12 @@ func (c *Container) unmountSHM(mount string) error {
 // namespaces
 func (c *Container) prepare() (err error) {
 	var (
-		wg                              sync.WaitGroup
-		netNS                           ns.NetNS
-		networkStatus                   []*cnitypes.Result
-		createNetNSErr, mountStorageErr error
-		mountPoint                      string
-		tmpStateLock                    sync.Mutex
+		wg                                                sync.WaitGroup
+		netNS                                             ns.NetNS
+		networkStatus                                     []*cnitypes.Result
+		createNetNSErr, mountStorageErr, rootlessSetupErr error
+		mountPoint                                        string
+		tmpStateLock                                      sync.Mutex
 	)
 
 	wg.Add(2)
@@ -87,6 +87,11 @@ func (c *Container) prepare() (err error) {
 				c.state.NetNS = netNS
 				c.state.NetworkStatus = networkStatus
 			}
+
+			// Setup rootless networking, requires c.state.NetNS to be set
+			if rootless.IsRootless() {
+				rootlessSetupErr = c.runtime.setupRootlessNetNS(c)
+			}
 		}
 	}()
 	// Mount storage if not mounted
@@ -132,6 +137,10 @@ func (c *Container) prepare() (err error) {
 		return mountStorageErr
 	}
 
+	if rootlessSetupErr != nil {
+		return rootlessSetupErr
+	}
+
 	// Save the container
 	return c.save()
 }
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index bef3f7739..73e839bda 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -103,9 +103,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())
@@ -123,7 +120,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
 }
 
@@ -151,9 +151,6 @@ func checkSlirpFlags(path string) (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 == "" {
@@ -177,7 +174,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, err := checkSlirpFlags(path)
 	if err != nil {
@@ -189,13 +186,27 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) {
 	if mtu {
 		cmdArgs = append(cmdArgs, "--mtu", "65520")
 	}
-	cmdArgs = append(cmdArgs, "-c", "-e", "3", "-r", "4", fmt.Sprintf("%d", ctr.state.PID), "tap0")
 
-	cmd := exec.Command(path, cmdArgs...)
+	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 {
@@ -410,22 +421,25 @@ func (r *Runtime) teardownNetNS(ctr *Container) error {
 		}
 	}
 
-	logrus.Debugf("Tearing down network namespace at %s for container %s", ctr.state.NetNS.Path(), ctr.ID())
+	logrus.Debugf("Tearing down network namespace for container %s", 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())
+		// 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())
+		}
 	}
 
 	// First unmount the namespace
diff --git a/libpod/oci_internal_linux.go b/libpod/oci_internal_linux.go
index 52cebefab..28e4b5b82 100644
--- a/libpod/oci_internal_linux.go
+++ b/libpod/oci_internal_linux.go
@@ -130,10 +130,16 @@ 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/oci_linux.go b/libpod/oci_linux.go
index 45365203e..4d8e36516 100644
--- a/libpod/oci_linux.go
+++ b/libpod/oci_linux.go
@@ -405,6 +405,14 @@ func (r *OCIRuntime) stopContainer(ctr *Container, timeout uint) error {
 		stopSignal = uint(syscall.SIGTERM)
 	}
 
+	defer func() {
+		// cleanup container networking
+		err = ctr.cleanupNetwork()
+		if err != nil {
+			logrus.Errorf("Error cleaning up container: %s network: %v", ctr.ID(), err)
+		}
+	}()
+
 	if timeout > 0 {
 		if err := r.killContainer(ctr, stopSignal); err != nil {
 			// Is the container gone?
diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go
index da35b7f93..20b841296 100644
--- a/libpod/runtime_pod_infra_linux.go
+++ b/libpod/runtime_pod_infra_linux.go
@@ -95,7 +95,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...)
 }
diff --git a/pkg/netns/netns_linux.go b/pkg/netns/netns_linux.go
index 1d6fb873c..e8388055a 100644
--- a/pkg/netns/netns_linux.go
+++ b/pkg/netns/netns_linux.go
@@ -23,23 +23,42 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
 
 	"github.com/containernetworking/plugins/pkg/ns"
+	"github.com/containers/libpod/pkg/rootless"
+	"github.com/containers/libpod/pkg/util"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
-const nsRunDir = "/var/run/netns"
+// get NSRunDir returns the dir of where to create the netNS. When running
+// rootless, it needs to be at a location writable by user.
+func getNSRunDir() (string, error) {
+	if rootless.IsRootless() {
+		rootlessDir, err := util.GetRootlessRuntimeDir()
+		if err != nil {
+			return "", err
+		}
+		return filepath.Join(rootlessDir, "netns"), nil
+	}
+	return "/var/run/netns", nil
+}
 
 // NewNS creates a new persistent (bind-mounted) network namespace and returns
 // an object representing that namespace, without switching to it.
 func NewNS() (ns.NetNS, error) {
 
+	nsRunDir, err := getNSRunDir()
+	if err != nil {
+		return nil, err
+	}
+
 	b := make([]byte, 16)
-	_, err := rand.Reader.Read(b)
+	_, err = rand.Reader.Read(b)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate random netns name: %v", err)
 	}
@@ -127,7 +146,7 @@ func NewNS() (ns.NetNS, error) {
 		// Put this thread back to the orig ns, since it might get reused (pre go1.10)
 		defer func() {
 			if err := origNS.Set(); err != nil {
-				logrus.Errorf("unable to set namespace: %q", err)
+				logrus.Warnf("unable to set namespace: %q", err)
 			}
 		}()
 
@@ -150,6 +169,11 @@ func NewNS() (ns.NetNS, error) {
 
 // UnmountNS unmounts the NS held by the netns object
 func UnmountNS(ns ns.NetNS) error {
+	nsRunDir, err := getNSRunDir()
+	if err != nil {
+		return err
+	}
+
 	nsPath := ns.Path()
 	// Only unmount if it's been bind-mounted (don't touch namespaces in /proc...)
 	if strings.HasPrefix(nsPath, nsRunDir) {
diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go
index f21ae2831..289634a0d 100644
--- a/pkg/spec/createconfig.go
+++ b/pkg/spec/createconfig.go
@@ -270,7 +270,7 @@ func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *l
 		options = append(options, libpod.WithNetNSFrom(connectedCtr))
 	} else if !c.NetMode.IsHost() && !c.NetMode.IsNone() {
 		hasUserns := c.UsernsMode.IsContainer() || c.UsernsMode.IsNS() || len(c.IDMappings.UIDMap) > 0 || len(c.IDMappings.GIDMap) > 0
-		postConfigureNetNS := c.NetMode.IsSlirp4netns() || (hasUserns && !c.UsernsMode.IsHost())
+		postConfigureNetNS := hasUserns && !c.UsernsMode.IsHost()
 		options = append(options, libpod.WithNetNS(portBindings, postConfigureNetNS, string(c.NetMode), networks))
 	}