From e02f35c1fc0de7a9b437c011de97ceb95a5ec0f5 Mon Sep 17 00:00:00 2001 From: Lei Jitang Date: Mon, 12 Jun 2017 21:32:51 -0400 Subject: [PATCH 1/2] libcontainerd: remove fifos on docker exec failed if docker exec failed to start, the fifo for exec will left on system. ``` [root@centos-220 bcc8bc6a080eb859ecd193dc32ea4e1cd0080b070afb2a0259b3ed004aae155e]# docker exec -ti bcc8bc6a080e bash rpc error: code = 2 desc = oci runtime error: exec failed: container_linux.go:247: starting container process caused "exec: \"bash\": executable file not found in $PATH" [root@centos-220 bcc8bc6a080eb859ecd193dc32ea4e1cd0080b070afb2a0259b3ed004aae155e]# ls -l total 4 prwx------. 1 root root 0 Apr 10 11:29 46889b0713827f708949af4f6b30315181e1d96e75ce179c949935ab02db2bd1-stdin prwx------. 1 root root 0 Apr 10 11:29 46889b0713827f708949af4f6b30315181e1d96e75ce179c949935ab02db2bd1-stdout prwx------. 1 root root 0 Apr 6 13:15 4dfc9806a2aef53f72a5d2854ff235714b5f064d10e29b5e59ff67d48a924462-stdin prwx------. 1 root root 0 Apr 6 13:15 4dfc9806a2aef53f72a5d2854ff235714b5f064d10e29b5e59ff67d48a924462-stdout prwx------. 1 root root 0 Apr 6 13:15 cc3af6845394cc60007b49242acd990b28988ef4ddaaa217a26caba1075ab343-stdin prwx------. 1 root root 0 Apr 6 13:15 cc3af6845394cc60007b49242acd990b28988ef4ddaaa217a26caba1075ab343-stdout -rw-r--r--. 1 root root 3353 Apr 6 13:12 config.json prwx------. 1 root root 0 Apr 6 13:14 d35e637104991052261e3afeba96a86b3cc6392dae6bd2226812407c0b92a20c-stdin prwx------. 1 root root 0 Apr 6 13:14 d35e637104991052261e3afeba96a86b3cc6392dae6bd2226812407c0b92a20c-stdout prwx------. 1 root root 0 Apr 6 13:12 init-stdin prwx------. 1 root root 0 Apr 6 13:12 init-stdout ``` Signed-off-by: Lei Jitang (cherry picked from commit 997ec06298081bf616177bf6fb102dc737b321ce) Signed-off-by: Tonis Tiigi --- components/engine/libcontainerd/client_linux.go | 7 +++++++ components/engine/libcontainerd/container_unix.go | 6 +----- components/engine/libcontainerd/process_unix.go | 11 +++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/components/engine/libcontainerd/client_linux.go b/components/engine/libcontainerd/client_linux.go index 6c3460a8c9..95d597a2a5 100644 --- a/components/engine/libcontainerd/client_linux.go +++ b/components/engine/libcontainerd/client_linux.go @@ -117,6 +117,13 @@ func (clnt *client) AddProcess(ctx context.Context, containerID, processFriendly return -1, err } + // clean up fifos if failed to add process + defer func() { + if err != nil { + p.cleanFifos(processFriendlyName) + } + }() + resp, err := clnt.remote.apiClient.AddProcess(ctx, r) if err != nil { p.closeFifos(iopipe) diff --git a/components/engine/libcontainerd/container_unix.go b/components/engine/libcontainerd/container_unix.go index 9a7dbf01cd..63cb2aa5ee 100644 --- a/components/engine/libcontainerd/container_unix.go +++ b/components/engine/libcontainerd/container_unix.go @@ -69,11 +69,7 @@ func (ctr *container) clean() error { // Caller needs to lock container ID before calling this method. func (ctr *container) cleanProcess(id string) { if p, ok := ctr.processes[id]; ok { - for _, i := range []int{unix.Stdin, unix.Stdout, unix.Stderr} { - if err := os.Remove(p.fifo(i)); err != nil && !os.IsNotExist(err) { - logrus.Warnf("libcontainerd: failed to remove %v for process %v: %v", p.fifo(i), id, err) - } - } + p.cleanFifos(id) } delete(ctr.processes, id) } diff --git a/components/engine/libcontainerd/process_unix.go b/components/engine/libcontainerd/process_unix.go index 3b54e325b5..301c2b3468 100644 --- a/components/engine/libcontainerd/process_unix.go +++ b/components/engine/libcontainerd/process_unix.go @@ -9,8 +9,10 @@ import ( "path/filepath" goruntime "runtime" "strings" + "syscall" containerd "github.com/containerd/containerd/api/grpc/types" + "github.com/sirupsen/logrus" "github.com/tonistiigi/fifo" "golang.org/x/net/context" "golang.org/x/sys/unix" @@ -105,3 +107,12 @@ func (r emptyReader) Read(b []byte) (int, error) { func (p *process) fifo(index int) string { return filepath.Join(p.dir, p.friendlyName+"-"+fdNames[index]) } + +func (p *process) cleanFifos(id string) { + for _, i := range []int{syscall.Stdin, syscall.Stdout, syscall.Stderr} { + if err := os.Remove(p.fifo(i)); err != nil && !os.IsNotExist(err) { + logrus.Warnf("failed to remove %v for process %v: %v", p.fifo(i), id, err) + } + } + +} From f1bf9f2c5b479d2e7346b31aa5c862197b55ca72 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 16 Nov 2017 11:01:35 -0800 Subject: [PATCH 2/2] integration-cli: add TestExecStateCleanup test Signed-off-by: Tonis Tiigi --- .../integration-cli/docker_api_exec_test.go | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/components/engine/integration-cli/docker_api_exec_test.go b/components/engine/integration-cli/docker_api_exec_test.go index 6e18df8fee..d88183e6c0 100644 --- a/components/engine/integration-cli/docker_api_exec_test.go +++ b/components/engine/integration-cli/docker_api_exec_test.go @@ -8,6 +8,8 @@ import ( "fmt" "io/ioutil" "net/http" + "os" + "strings" "time" "github.com/docker/docker/api/types" @@ -198,6 +200,45 @@ func (s *DockerSuite) TestExecAPIStartInvalidCommand(c *check.C) { c.Assert(inspectJSON.ExecIDs, checker.IsNil) } +func (s *DockerSuite) TestExecStateCleanup(c *check.C) { + testRequires(c, DaemonIsLinux, SameHostDaemon) + + // This test checks accidental regressions. Not part of stable API. + + name := "exec_cleanup" + cid, _ := dockerCmd(c, "run", "-d", "-t", "--name", name, "busybox", "/bin/sh") + cid = strings.TrimSpace(cid) + + stateDir := "/var/run/docker/libcontainerd/" + cid + + checkReadDir := func(c *check.C) (interface{}, check.CommentInterface) { + fi, err := ioutil.ReadDir(stateDir) + c.Assert(err, checker.IsNil) + return len(fi), nil + } + + fi, err := ioutil.ReadDir(stateDir) + c.Assert(err, checker.IsNil) + c.Assert(len(fi), checker.GreaterThan, 1) + + id := createExecCmd(c, name, "ls") + startExec(c, id, http.StatusOK) + waitForExec(c, id) + + waitAndAssert(c, 5*time.Second, checkReadDir, checker.Equals, len(fi)) + + id = createExecCmd(c, name, "invalid") + startExec(c, id, http.StatusBadRequest) + waitForExec(c, id) + + waitAndAssert(c, 5*time.Second, checkReadDir, checker.Equals, len(fi)) + + dockerCmd(c, "stop", name) + _, err = os.Stat(stateDir) + c.Assert(err, checker.NotNil) + c.Assert(os.IsNotExist(err), checker.True) +} + func createExec(c *check.C, name string) string { return createExecCmd(c, name, "true") }