diff --git a/.gitignore b/.gitignore index 418caf8e0..8ba661616 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ coverage.txt .DS_Store bin/ coverage/ +test/testdata/cp/* diff --git a/daemon/mgr/container_copy.go b/daemon/mgr/container_copy.go index 31624acef..231b066e7 100644 --- a/daemon/mgr/container_copy.go +++ b/daemon/mgr/container_copy.go @@ -14,8 +14,11 @@ import ( "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/chrootarchive" + "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/mount" "github.com/go-openapi/strfmt" + pkgerrors "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // StatPath stats the dir info at the specified path in the container. @@ -27,10 +30,14 @@ func (mgr *ContainerManager) StatPath(ctx context.Context, name, path string) (s c.Lock() defer c.Unlock() + if c.State.Dead { + return nil, pkgerrors.Errorf("container(%s) has been deleted", c.ID) + } + running := c.IsRunningOrPaused() err = mgr.Mount(ctx, c, false) if err != nil { - return nil, err + return nil, pkgerrors.Wrapf(err, "failed to mount cid(%s)", c.ID) } defer mgr.Unmount(ctx, c, false, !running) @@ -38,16 +45,16 @@ func (mgr *ContainerManager) StatPath(ctx context.Context, name, path string) (s if !running { err = mgr.attachVolumes(ctx, c) if err != nil { - return nil, err + return nil, pkgerrors.Wrapf(err, "failed to attachVolumes cid(%s)", c.ID) } defer mgr.detachVolumes(ctx, c, false) } - err = c.mountVolumes(!running) + err = c.mountVolumes() if err != nil { - return nil, err + return nil, pkgerrors.Wrapf(err, "failed to mountVolumes cid(%s)", c.ID) } - defer c.unmountVolumes(!running) + defer c.unmountVolumes() resolvedPath, absPath := c.getResolvedPath(path) lstat, err := os.Lstat(resolvedPath) @@ -78,10 +85,14 @@ func (mgr *ContainerManager) ArchivePath(ctx context.Context, name, path string) } }() + if c.State.Dead { + return nil, nil, pkgerrors.Errorf("container(%s) has been deleted", c.ID) + } + running := c.IsRunningOrPaused() err = mgr.Mount(ctx, c, false) if err != nil { - return nil, nil, err + return nil, nil, pkgerrors.Wrapf(err, "failed to mount cid(%s)", c.ID) } defer func() { if err0 != nil { @@ -92,7 +103,7 @@ func (mgr *ContainerManager) ArchivePath(ctx context.Context, name, path string) if !running { err = mgr.attachVolumes(ctx, c) if err != nil { - return nil, nil, err + return nil, nil, pkgerrors.Wrapf(err, "failed to attachVolumes cid(%s)", c.ID) } defer func() { if err0 != nil { @@ -101,13 +112,13 @@ func (mgr *ContainerManager) ArchivePath(ctx context.Context, name, path string) }() } - err = c.mountVolumes(!running) + err = c.mountVolumes() if err != nil { - return nil, nil, err + return nil, nil, pkgerrors.Wrapf(err, "failed to mountVolumes cid(%s)", c.ID) } defer func() { if err0 != nil { - defer c.unmountVolumes(!running) + defer c.unmountVolumes() } }() @@ -140,7 +151,7 @@ func (mgr *ContainerManager) ArchivePath(ctx context.Context, name, path string) if !running { mgr.detachVolumes(ctx, c, false) } - c.unmountVolumes(!running) + c.unmountVolumes() mgr.Unmount(ctx, c, false, !running) c.Unlock() return err @@ -158,26 +169,30 @@ func (mgr *ContainerManager) ExtractToDir(ctx context.Context, name, path string c.Lock() defer c.Unlock() + if c.State.Dead { + return pkgerrors.Errorf("container(%s) has been deleted", c.ID) + } + running := c.IsRunningOrPaused() err = mgr.Mount(ctx, c, false) if err != nil { - return err + return pkgerrors.Wrapf(err, "failed to mount cid(%s)", c.ID) } defer mgr.Unmount(ctx, c, false, !running) if !running { err = mgr.attachVolumes(ctx, c) if err != nil { - return err + return pkgerrors.Wrapf(err, "failed to attachVolumes cid(%s)", c.ID) } defer mgr.detachVolumes(ctx, c, false) } - err = c.mountVolumes(!running) + err = c.mountVolumes() if err != nil { - return err + return pkgerrors.Wrapf(err, "failed to mountVolumes cid(%s)", c.ID) } - defer c.unmountVolumes(!running) + defer c.unmountVolumes() resolvedPath, _ := c.getResolvedPath(path) @@ -229,20 +244,38 @@ func (c *Container) getResolvedPath(path string) (resolvedPath, absPath string) return resolvedPath, absPath } -func (c *Container) mountVolumes(created bool) error { +func (c *Container) mountVolumes() (err0 error) { + rollbackMounts := make([]string, 0, len(c.Mounts)) + + defer func() { + if err0 != nil { + for _, dest := range rollbackMounts { + if err := mount.Unmount(dest); err != nil { + logrus.Warnf("[mountVolumes:rollback] failed to unmount(%s), err(%v)", dest, err) + } else { + logrus.Debugf("[mountVolumes:rollback] unmount(%s)", dest) + } + } + } + }() for _, m := range c.Mounts { dest, _ := c.getResolvedPath(m.Destination) - _, err := os.Stat(m.Source) + logrus.Debugf("try to mount volume(source %s -> dest %s", m.Source, dest) + + stat, err := os.Stat(m.Source) if err != nil { return err } - if created { - if e := os.MkdirAll(dest, 0755); e != nil { - return e - } + // /dev /proc /tmp is mounted by default oci spec. Those mount + // points are mounted by runC. In k8s, the daemonset will mount + // log into /dev/termination-log. Before mount it, we need to + // create folder or empty file for the target. It will not + // impact the running container. + if err = fileutils.CreateIfNotExists(dest, stat.IsDir()); err != nil { + return err } writeMode := "ro" @@ -263,23 +296,20 @@ func (c *Container) mountVolumes(created bool) error { if err := mount.Mount(m.Source, dest, "", opts); err != nil { return err } + + rollbackMounts = append(rollbackMounts, dest) } return nil } -func (c *Container) unmountVolumes(remove bool) error { +func (c *Container) unmountVolumes() error { for _, m := range c.Mounts { dest, _ := c.getResolvedPath(m.Destination) if err := mount.Unmount(dest); err != nil { return err } - if remove { - if err := os.RemoveAll(dest); err != nil { - return err - } - } } return nil } diff --git a/daemon/mgr/container_storage.go b/daemon/mgr/container_storage.go index 16055103b..8fdcd0dd9 100644 --- a/daemon/mgr/container_storage.go +++ b/daemon/mgr/container_storage.go @@ -668,7 +668,19 @@ func (mgr *ContainerManager) detachVolumes(ctx context.Context, c *Container, re return nil } -func (mgr *ContainerManager) attachVolumes(ctx context.Context, c *Container) error { +func (mgr *ContainerManager) attachVolumes(ctx context.Context, c *Container) (err0 error) { + rollbackVolumes := make([]string, 0, len(c.Mounts)) + + defer func() { + if err0 != nil { + for _, name := range rollbackVolumes { + if _, err := mgr.VolumeMgr.Detach(ctx, name, map[string]string{volumetypes.OptionRef: c.ID}); err != nil { + logrus.Warnf("[rollback] failed to detach volume(%s), err(%v)", name, err) + } + } + } + }() + for _, mount := range c.Mounts { name := mount.Name if name == "" { @@ -680,6 +692,8 @@ func (mgr *ContainerManager) attachVolumes(ctx context.Context, c *Container) er logrus.Warnf("failed to attach volume(%s), err(%v)", name, err) return err } + + rollbackVolumes = append(rollbackVolumes, name) } return nil } diff --git a/test/cli_container_cp_test.go b/test/cli_container_cp_test.go index 5d479fb3b..7796c33c9 100644 --- a/test/cli_container_cp_test.go +++ b/test/cli_container_cp_test.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "io/ioutil" "os" "github.com/alibaba/pouch/test/command" @@ -15,8 +16,6 @@ import ( // APIContainerCopySuite is the test suite for container cp CLI. type PouchContainerCopySuite struct{} -var testDataPath = "testdata/cp" - func init() { check.Suite(&PouchContainerCopySuite{}) } @@ -25,18 +24,14 @@ func init() { func (suite *PouchContainerCopySuite) SetUpSuite(c *check.C) { SkipIfFalse(c, environment.IsLinux) PullImage(c, busyboxImage) - err := os.Mkdir(testDataPath, 0755) - c.Assert(err, check.IsNil) -} - -// TearDownSuite does cleanup work in the end of each test suite. -func (suite *PouchContainerCopySuite) TearDownSuite(c *check.C) { - err := os.RemoveAll(testDataPath) - c.Assert(err, check.IsNil) } // Test pouch cp, basic usage func (suite *PouchContainerCopySuite) TestPouchCopy(c *check.C) { + testDataPath, err := ioutil.TempDir("", "test-pouch-copy") + c.Assert(err, check.IsNil) + defer os.RemoveAll(testDataPath) + // test copy from container name := "TestPouchCopy" command.PouchRun("run", @@ -56,7 +51,7 @@ func (suite *PouchContainerCopySuite) TestPouchCopy(c *check.C) { command.PouchRun("cp", localTestPath, containerTestPath).Assert(c, icmd.Success) res := command.PouchRun("exec", name, "cat", "test.txt") res.Assert(c, icmd.Success) - err := util.PartialEqual(res.Stdout(), "test pouch cp") + err = util.PartialEqual(res.Stdout(), "test pouch cp") c.Assert(err, check.IsNil) // test copy to container with non-dir @@ -67,6 +62,10 @@ func (suite *PouchContainerCopySuite) TestPouchCopy(c *check.C) { // Test pouch cp, where dir locate in volume func (suite *PouchContainerCopySuite) TestVolumeCopy(c *check.C) { + testDataPath, err := ioutil.TempDir("", "test-volume-copy") + c.Assert(err, check.IsNil) + defer os.RemoveAll(testDataPath) + // test mount rw and copy from container name := "TestVolumeCopy" command.PouchRun("volume", "create", "--name", name).Assert(c, icmd.Success) @@ -90,7 +89,7 @@ func (suite *PouchContainerCopySuite) TestVolumeCopy(c *check.C) { command.PouchRun("cp", localTestPath, containerTestPath).Assert(c, icmd.Success) res := command.PouchRun("exec", name, "cat", "/test/test.txt") res.Assert(c, icmd.Success) - err := util.PartialEqual(res.Stdout(), "test pouch cp") + err = util.PartialEqual(res.Stdout(), "test pouch cp") c.Assert(err, check.IsNil) // test mount only ro @@ -111,6 +110,10 @@ func (suite *PouchContainerCopySuite) TestVolumeCopy(c *check.C) { // TestStopContainerCopy tests stopped container can work well func (suite *PouchContainerCopySuite) TestStopContainerCopy(c *check.C) { + testDataPath := "testdata/cp/test-stop-container-copy" + c.Assert(os.MkdirAll(testDataPath, 0755), check.IsNil) + defer os.RemoveAll(testDataPath) + name := "TestStopContainerCopy" command.PouchRun("run", "-d", "--name", name,