From 990cd1a950955e25dc8935a6aca61906307a0851 Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 20 Jul 2021 20:53:33 -0700 Subject: Don't kill container when volume is unmounted The gofer session is killed when a gofer backed volume is unmounted. The gofer monitor catches the disconnect and kills the container. This changes the gofer monitor to only care about the rootfs connections, which cannot be unmounted. Fixes #6259 PiperOrigin-RevId: 385929039 --- runsc/boot/loader.go | 27 +++++++++++++++++---------- test/e2e/integration_test.go | 33 +++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 898692219..ec9188021 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -742,8 +742,11 @@ func (l *Loader) createContainerProcess(root bool, cid string, info *containerIn // ours either way. info.procArgs.FDTable = fdTable - // Setup the child container file system. - l.startGoferMonitor(cid, info.goferFDs) + // Gofer FDs must be ordered and the first FD is always the rootfs. + if len(info.goferFDs) < 1 { + return nil, nil, nil, fmt.Errorf("rootfs gofer FD not found") + } + l.startGoferMonitor(cid, int32(info.goferFDs[0].FD())) mntr := newContainerMounter(info, l.k, l.mountHints, kernel.VFS2Enabled) if root { @@ -816,17 +819,21 @@ func (l *Loader) createContainerProcess(root bool, cid string, info *containerIn } // startGoferMonitor runs a goroutine to monitor gofer's health. It polls on -// the gofer FDs looking for disconnects, and kills the container processes if a -// disconnect occurs in any of the gofer FDs. -func (l *Loader) startGoferMonitor(cid string, goferFDs []*fd.FD) { +// the gofer FD looking for disconnects, and kills the container processes if +// the rootfs FD disconnects. +// +// Note that other gofer mounts are allowed to be unmounted and disconnected. +func (l *Loader) startGoferMonitor(cid string, rootfsGoferFD int32) { + if rootfsGoferFD < 0 { + panic(fmt.Sprintf("invalid FD: %d", rootfsGoferFD)) + } go func() { log.Debugf("Monitoring gofer health for container %q", cid) - var events []unix.PollFd - for _, goferFD := range goferFDs { - events = append(events, unix.PollFd{ - Fd: int32(goferFD.FD()), + events := []unix.PollFd{ + { + Fd: rootfsGoferFD, Events: unix.POLLHUP | unix.POLLRDHUP, - }) + }, } _, _, err := specutils.RetryEintr(func() (uintptr, uintptr, error) { // Use ppoll instead of poll because it's already whilelisted in seccomp. diff --git a/test/e2e/integration_test.go b/test/e2e/integration_test.go index f53417cab..9e22c9a7d 100644 --- a/test/e2e/integration_test.go +++ b/test/e2e/integration_test.go @@ -44,6 +44,12 @@ import ( // defaultWait is the default wait time used for tests. const defaultWait = time.Minute +func TestMain(m *testing.M) { + dockerutil.EnsureSupportedDockerVersion() + flag.Parse() + os.Exit(m.Run()) +} + // httpRequestSucceeds sends a request to a given url and checks that the status is OK. func httpRequestSucceeds(client http.Client, server string, port int) error { url := fmt.Sprintf("http://%s:%d", server, port) @@ -712,8 +718,27 @@ func TestStdiosChown(t *testing.T) { } } -func TestMain(m *testing.M) { - dockerutil.EnsureSupportedDockerVersion() - flag.Parse() - os.Exit(m.Run()) +func TestUnmount(t *testing.T) { + ctx := context.Background() + d := dockerutil.MakeContainer(ctx, t) + defer d.CleanUp(ctx) + + dir, err := ioutil.TempDir(testutil.TmpDir(), "sub-mount") + if err != nil { + t.Fatalf("TempDir(): %v", err) + } + opts := dockerutil.RunOpts{ + Image: "basic/alpine", + Privileged: true, // Required for umount + Mounts: []mount.Mount{ + { + Type: mount.TypeBind, + Source: dir, + Target: "/foo", + }, + }, + } + if _, err := d.Run(ctx, opts, "umount", "/foo"); err != nil { + t.Fatalf("docker run failed: %v", err) + } } -- cgit v1.2.3