From 97b84dedf3806a9e87c04ccfb51212992785d2c8 Mon Sep 17 00:00:00 2001 From: baude Date: Tue, 6 Aug 2019 09:49:03 -0500 Subject: Revert "rootless: Rearrange setup of rootless containers" This reverts commit 80dcd4bebcdc8e280f6b43228561d09c194c328b. Signed-off-by: baude --- libpod/container_internal_linux.go | 21 ++++--------- libpod/networking_linux.go | 60 +++++++++++++++----------------------- libpod/oci_internal_linux.go | 12 ++------ libpod/oci_linux.go | 8 ----- libpod/runtime_pod_infra_linux.go | 4 +-- pkg/netns/netns_linux.go | 30 ++----------------- pkg/spec/createconfig.go | 2 +- 7 files changed, 37 insertions(+), 100 deletions(-) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index d06c19b8c..047b73d65 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, rootlessSetupErr error - mountPoint string - tmpStateLock sync.Mutex + wg sync.WaitGroup + netNS ns.NetNS + networkStatus []*cnitypes.Result + createNetNSErr, mountStorageErr error + mountPoint string + tmpStateLock sync.Mutex ) wg.Add(2) @@ -87,11 +87,6 @@ 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 @@ -137,10 +132,6 @@ 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 73e839bda..bef3f7739 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -103,6 +103,9 @@ 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()) @@ -120,10 +123,7 @@ 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 := []*cnitypes.Result{} - if !rootless.IsRootless() { - networkStatus, err = r.configureNetNS(ctr, ctrNS) - } + networkStatus, err := r.configureNetNS(ctr, ctrNS) return ctrNS, networkStatus, err } @@ -151,6 +151,9 @@ 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 == "" { @@ -174,7 +177,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { cmdArgs := []string{} if havePortMapping { - cmdArgs = append(cmdArgs, "--api-socket", apiSocket) + cmdArgs = append(cmdArgs, "--api-socket", apiSocket, fmt.Sprintf("%d", ctr.state.PID)) } dhp, mtu, err := checkSlirpFlags(path) if err != nil { @@ -186,27 +189,13 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { if mtu { cmdArgs = append(cmdArgs, "--mtu", "65520") } - - 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") - } + cmdArgs = append(cmdArgs, "-c", "-e", "3", "-r", "4", 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 { @@ -421,25 +410,22 @@ func (r *Runtime) teardownNetNS(ctr *Container) error { } } - logrus.Debugf("Tearing down network namespace for container %s", ctr.ID()) + logrus.Debugf("Tearing down network namespace at %s for container %s", ctr.state.NetNS.Path(), ctr.ID()) - // 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 - } + 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 28e4b5b82..52cebefab 100644 --- a/libpod/oci_internal_linux.go +++ b/libpod/oci_internal_linux.go @@ -130,16 +130,10 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Containe } if ctr.config.NetMode.IsSlirp4netns() { - 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) + ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe() + if err != nil { + return errors.Wrapf(err, "failed to create rootless network sync pipe") } - // 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 4d8e36516..45365203e 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -405,14 +405,6 @@ 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 20b841296..da35b7f93 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -95,9 +95,7 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID if isRootless { netmode = "slirp4netns" } - // 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)) + options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, isRootless, netmode, networks)) return r.newContainer(ctx, g.Config, options...) } diff --git a/pkg/netns/netns_linux.go b/pkg/netns/netns_linux.go index e8388055a..1d6fb873c 100644 --- a/pkg/netns/netns_linux.go +++ b/pkg/netns/netns_linux.go @@ -23,42 +23,23 @@ 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" ) -// 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 -} +const nsRunDir = "/var/run/netns" // 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) } @@ -146,7 +127,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.Warnf("unable to set namespace: %q", err) + logrus.Errorf("unable to set namespace: %q", err) } }() @@ -169,11 +150,6 @@ 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 289634a0d..f21ae2831 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 := hasUserns && !c.UsernsMode.IsHost() + postConfigureNetNS := c.NetMode.IsSlirp4netns() || (hasUserns && !c.UsernsMode.IsHost()) options = append(options, libpod.WithNetNS(portBindings, postConfigureNetNS, string(c.NetMode), networks)) } -- cgit v1.2.3-54-g00ecf