From bc974a7a326474d34a2c1eb6744d5da699a6e3f2 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 7 Feb 2018 11:10:35 -0500 Subject: [PATCH 1/2] Create temp mount location manager Signed-off-by: Michael Crosby --- container_opts_unix.go | 2 +- diff/apply/apply.go | 2 +- diff/walking/differ.go | 4 +- mount/mount.go | 48 ----------------- mount/temp.go | 118 +++++++++++++++++++++++++++++++++++++++++ oci/spec_opts_unix.go | 4 +- 6 files changed, 124 insertions(+), 54 deletions(-) create mode 100644 mount/temp.go diff --git a/container_opts_unix.go b/container_opts_unix.go index 5a281918d..fbf4299c1 100644 --- a/container_opts_unix.go +++ b/container_opts_unix.go @@ -188,7 +188,7 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool } func remapRootFS(ctx context.Context, mounts []mount.Mount, uid, gid uint32) error { - return mount.WithTempMount(ctx, mounts, func(root string) error { + return mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { return filepath.Walk(root, incrementFS(root, uid, gid)) }) } diff --git a/diff/apply/apply.go b/diff/apply/apply.go index 57f0ae953..57f1f902d 100644 --- a/diff/apply/apply.go +++ b/diff/apply/apply.go @@ -56,7 +56,7 @@ func (s *fsApplier) Apply(ctx context.Context, desc ocispec.Descriptor, mounts [ } var ocidesc ocispec.Descriptor - if err := mount.WithTempMount(ctx, mounts, func(root string) error { + if err := mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { ra, err := s.store.ReaderAt(ctx, desc.Digest) if err != nil { return errors.Wrap(err, "failed to get reader from content store") diff --git a/diff/walking/differ.go b/diff/walking/differ.go index 115ad7a98..95238358a 100644 --- a/diff/walking/differ.go +++ b/diff/walking/differ.go @@ -62,8 +62,8 @@ func (s *walkingDiff) Compare(ctx context.Context, lower, upper []mount.Mount, o } var ocidesc ocispec.Descriptor - if err := mount.WithTempMount(ctx, lower, func(lowerRoot string) error { - return mount.WithTempMount(ctx, upper, func(upperRoot string) error { + if err := mount.DefaultTempLocation.Mount(ctx, lower, func(lowerRoot string) error { + return mount.DefaultTempLocation.Mount(ctx, upper, func(upperRoot string) error { var newReference bool if config.Reference == "" { newReference = true diff --git a/mount/mount.go b/mount/mount.go index 323e6e2fb..94c2c9f92 100644 --- a/mount/mount.go +++ b/mount/mount.go @@ -1,14 +1,5 @@ package mount -import ( - "context" - "io/ioutil" - "os" - - "github.com/containerd/containerd/log" - "github.com/pkg/errors" -) - // Mount is the lingua franca of containerd. A mount represents a // serialized mount syscall. Components either emit or consume mounts. type Mount struct { @@ -31,42 +22,3 @@ func All(mounts []Mount, target string) error { } return nil } - -// WithTempMount mounts the provided mounts to a temp dir, and pass the temp dir to f. -// The mounts are valid during the call to the f. -// Finally we will unmount and remove the temp dir regardless of the result of f. -func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) error) (err error) { - root, uerr := ioutil.TempDir("", "containerd-WithTempMount") - if uerr != nil { - return errors.Wrapf(uerr, "failed to create temp dir") - } - // We use Remove here instead of RemoveAll. - // The RemoveAll will delete the temp dir and all children it contains. - // When the Unmount fails, RemoveAll will incorrectly delete data from - // the mounted dir. However, if we use Remove, even though we won't - // successfully delete the temp dir and it may leak, we won't loss data - // from the mounted dir. - // For details, please refer to #1868 #1785. - defer func() { - if uerr = os.Remove(root); uerr != nil { - log.G(ctx).WithError(uerr).WithField("dir", root).Errorf("failed to remove mount temp dir") - } - }() - - // We should do defer first, if not we will not do Unmount when only a part of Mounts are failed. - defer func() { - if uerr = UnmountAll(root, 0); uerr != nil { - uerr = errors.Wrapf(uerr, "failed to unmount %s", root) - if err == nil { - err = uerr - } else { - err = errors.Wrap(err, uerr.Error()) - } - } - }() - if uerr = All(mounts, root); uerr != nil { - return errors.Wrapf(uerr, "failed to mount %s", root) - } - - return errors.Wrapf(f(root), "mount callback failed on %s", root) -} diff --git a/mount/temp.go b/mount/temp.go new file mode 100644 index 000000000..9d59fe974 --- /dev/null +++ b/mount/temp.go @@ -0,0 +1,118 @@ +package mount + +import ( + "context" + "io/ioutil" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/containerd/containerd/log" + "github.com/pkg/errors" +) + +func init() { + t, err := TempLocation("/tmp") + if err != nil { + panic(err) + } + DefaultTempLocation = t +} + +var DefaultTempLocation TempMounts + +func TempLocation(root string) (TempMounts, error) { + root, err := filepath.Abs(root) + if err != nil { + return DefaultTempLocation, err + } + if err := os.MkdirAll(root, 0700); err != nil { + return DefaultTempLocation, err + } + return TempMounts{ + root: root, + }, nil +} + +type TempMounts struct { + root string +} + +// WithTempMount mounts the provided mounts to a temp dir, and pass the temp dir to f. +// The mounts are valid during the call to the f. +// Finally we will unmount and remove the temp dir regardless of the result of f. +func (t TempMounts) Mount(ctx context.Context, mounts []Mount, f func(root string) error) (err error) { + root, uerr := ioutil.TempDir(t.root, "containerd-WithTempMount") + if uerr != nil { + return errors.Wrapf(uerr, "failed to create temp dir") + } + // We use Remove here instead of RemoveAll. + // The RemoveAll will delete the temp dir and all children it contains. + // When the Unmount fails, RemoveAll will incorrectly delete data from + // the mounted dir. However, if we use Remove, even though we won't + // successfully delete the temp dir and it may leak, we won't loss data + // from the mounted dir. + // For details, please refer to #1868 #1785. + defer func() { + if uerr = os.Remove(root); uerr != nil { + log.G(ctx).WithError(uerr).WithField("dir", root).Errorf("failed to remove mount temp dir") + } + }() + + // We should do defer first, if not we will not do Unmount when only a part of Mounts are failed. + defer func() { + if uerr = UnmountAll(root, 0); uerr != nil { + uerr = errors.Wrapf(uerr, "failed to unmount %s", root) + if err == nil { + err = uerr + } else { + err = errors.Wrap(err, uerr.Error()) + } + } + }() + if uerr = All(mounts, root); uerr != nil { + return errors.Wrapf(uerr, "failed to mount %s", root) + } + return errors.Wrapf(f(root), "mount callback failed on %s", root) +} + +// Unmount all temp mounts and remove the directories +func (t TempMounts) Unmount(flags int) error { + mounts, err := PID(os.Getpid()) + if err != nil { + return err + } + var toUnmount []string + for _, m := range mounts { + if strings.HasPrefix(m.Mountpoint, t.root) { + toUnmount = append(toUnmount, m.Mountpoint) + } + } + sort.Sort(sort.Reverse(mountSorter(toUnmount))) + for _, path := range toUnmount { + if err := UnmountAll(path, flags); err != nil { + return err + } + if err := os.Remove(path); err != nil { + return err + } + } + return nil +} + +type mountSorter []string + +func (by mountSorter) Len() int { + return len(by) +} + +func (by mountSorter) Less(i, j int) bool { + is := strings.Split(by[i], string(os.PathSeparator)) + js := strings.Split(by[j], string(os.PathSeparator)) + return len(is) < len(js) +} + +func (by mountSorter) Swap(i, j int) { + by[i], by[j] = by[j], by[i] +} diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go index 8bbdd78b1..c34b79454 100644 --- a/oci/spec_opts_unix.go +++ b/oci/spec_opts_unix.go @@ -287,7 +287,7 @@ func WithUserID(uid uint32) SpecOpts { if err != nil { return err } - return mount.WithTempMount(ctx, mounts, func(root string) error { + return mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { uuid, ugid, err := getUIDGIDFromPath(root, func(u user.User) bool { return u.Uid == int(uid) }) @@ -334,7 +334,7 @@ func WithUsername(username string) SpecOpts { if err != nil { return err } - return mount.WithTempMount(ctx, mounts, func(root string) error { + return mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { uid, gid, err := getUIDGIDFromPath(root, func(u user.User) bool { return u.Name == username }) From b2ec177bb2f37aea43172feb4c77385ad0eb4b4e Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 7 Feb 2018 11:16:15 -0500 Subject: [PATCH 2/2] Call temp mounts and unmount in containerd server Fixes #2004 Signed-off-by: Michael Crosby --- cmd/containerd/command/main.go | 9 +++++ container_opts_unix.go | 2 +- diff/apply/apply.go | 2 +- diff/walking/differ.go | 4 +- mount/temp.go | 74 ++-------------------------------- mount/temp_unix.go | 47 +++++++++++++++++++++ mount/temp_unsupported.go | 13 ++++++ oci/spec_opts_unix.go | 4 +- 8 files changed, 78 insertions(+), 77 deletions(-) create mode 100644 mount/temp_unix.go create mode 100644 mount/temp_unsupported.go diff --git a/cmd/containerd/command/main.go b/cmd/containerd/command/main.go index 9c169c97a..b7815ee01 100644 --- a/cmd/containerd/command/main.go +++ b/cmd/containerd/command/main.go @@ -12,6 +12,7 @@ import ( "time" "github.com/containerd/containerd/log" + "github.com/containerd/containerd/mount" "github.com/containerd/containerd/server" "github.com/containerd/containerd/sys" "github.com/containerd/containerd/version" @@ -95,6 +96,14 @@ func App() *cli.App { if err := applyFlags(context, config); err != nil { return err } + // cleanup temp mounts + if err := mount.SetTempMountLocation(filepath.Join(config.Root, "tmpmounts")); err != nil { + return errors.Wrap(err, "creating temp mount location") + } + // unmount all temp mounts on boot for the server + if err := mount.CleanupTempMounts(0); err != nil { + return errors.Wrap(err, "unmounting temp mounts") + } address := config.GRPC.Address if address == "" { return errors.New("grpc address cannot be empty") diff --git a/container_opts_unix.go b/container_opts_unix.go index fbf4299c1..5a281918d 100644 --- a/container_opts_unix.go +++ b/container_opts_unix.go @@ -188,7 +188,7 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool } func remapRootFS(ctx context.Context, mounts []mount.Mount, uid, gid uint32) error { - return mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { + return mount.WithTempMount(ctx, mounts, func(root string) error { return filepath.Walk(root, incrementFS(root, uid, gid)) }) } diff --git a/diff/apply/apply.go b/diff/apply/apply.go index 57f1f902d..57f0ae953 100644 --- a/diff/apply/apply.go +++ b/diff/apply/apply.go @@ -56,7 +56,7 @@ func (s *fsApplier) Apply(ctx context.Context, desc ocispec.Descriptor, mounts [ } var ocidesc ocispec.Descriptor - if err := mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { + if err := mount.WithTempMount(ctx, mounts, func(root string) error { ra, err := s.store.ReaderAt(ctx, desc.Digest) if err != nil { return errors.Wrap(err, "failed to get reader from content store") diff --git a/diff/walking/differ.go b/diff/walking/differ.go index 95238358a..115ad7a98 100644 --- a/diff/walking/differ.go +++ b/diff/walking/differ.go @@ -62,8 +62,8 @@ func (s *walkingDiff) Compare(ctx context.Context, lower, upper []mount.Mount, o } var ocidesc ocispec.Descriptor - if err := mount.DefaultTempLocation.Mount(ctx, lower, func(lowerRoot string) error { - return mount.DefaultTempLocation.Mount(ctx, upper, func(upperRoot string) error { + if err := mount.WithTempMount(ctx, lower, func(lowerRoot string) error { + return mount.WithTempMount(ctx, upper, func(upperRoot string) error { var newReference bool if config.Reference == "" { newReference = true diff --git a/mount/temp.go b/mount/temp.go index 9d59fe974..ec7a06bcb 100644 --- a/mount/temp.go +++ b/mount/temp.go @@ -4,46 +4,18 @@ import ( "context" "io/ioutil" "os" - "path/filepath" - "sort" - "strings" "github.com/containerd/containerd/log" "github.com/pkg/errors" ) -func init() { - t, err := TempLocation("/tmp") - if err != nil { - panic(err) - } - DefaultTempLocation = t -} - -var DefaultTempLocation TempMounts - -func TempLocation(root string) (TempMounts, error) { - root, err := filepath.Abs(root) - if err != nil { - return DefaultTempLocation, err - } - if err := os.MkdirAll(root, 0700); err != nil { - return DefaultTempLocation, err - } - return TempMounts{ - root: root, - }, nil -} - -type TempMounts struct { - root string -} +var tempMountLocation = os.TempDir() // WithTempMount mounts the provided mounts to a temp dir, and pass the temp dir to f. // The mounts are valid during the call to the f. // Finally we will unmount and remove the temp dir regardless of the result of f. -func (t TempMounts) Mount(ctx context.Context, mounts []Mount, f func(root string) error) (err error) { - root, uerr := ioutil.TempDir(t.root, "containerd-WithTempMount") +func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) error) (err error) { + root, uerr := ioutil.TempDir(tempMountLocation, "containerd-mount") if uerr != nil { return errors.Wrapf(uerr, "failed to create temp dir") } @@ -76,43 +48,3 @@ func (t TempMounts) Mount(ctx context.Context, mounts []Mount, f func(root strin } return errors.Wrapf(f(root), "mount callback failed on %s", root) } - -// Unmount all temp mounts and remove the directories -func (t TempMounts) Unmount(flags int) error { - mounts, err := PID(os.Getpid()) - if err != nil { - return err - } - var toUnmount []string - for _, m := range mounts { - if strings.HasPrefix(m.Mountpoint, t.root) { - toUnmount = append(toUnmount, m.Mountpoint) - } - } - sort.Sort(sort.Reverse(mountSorter(toUnmount))) - for _, path := range toUnmount { - if err := UnmountAll(path, flags); err != nil { - return err - } - if err := os.Remove(path); err != nil { - return err - } - } - return nil -} - -type mountSorter []string - -func (by mountSorter) Len() int { - return len(by) -} - -func (by mountSorter) Less(i, j int) bool { - is := strings.Split(by[i], string(os.PathSeparator)) - js := strings.Split(by[j], string(os.PathSeparator)) - return len(is) < len(js) -} - -func (by mountSorter) Swap(i, j int) { - by[i], by[j] = by[j], by[i] -} diff --git a/mount/temp_unix.go b/mount/temp_unix.go new file mode 100644 index 000000000..770631362 --- /dev/null +++ b/mount/temp_unix.go @@ -0,0 +1,47 @@ +// +build !windows + +package mount + +import ( + "os" + "path/filepath" + "sort" + "strings" +) + +// SetTempMountLocation sets the temporary mount location +func SetTempMountLocation(root string) error { + root, err := filepath.Abs(root) + if err != nil { + return err + } + if err := os.MkdirAll(root, 0700); err != nil { + return err + } + tempMountLocation = root + return nil +} + +// CleanupTempMounts all temp mounts and remove the directories +func CleanupTempMounts(flags int) error { + mounts, err := Self() + if err != nil { + return err + } + var toUnmount []string + for _, m := range mounts { + if strings.HasPrefix(m.Mountpoint, tempMountLocation) { + toUnmount = append(toUnmount, m.Mountpoint) + } + } + sort.Sort(sort.Reverse(sort.StringSlice(toUnmount))) + for _, path := range toUnmount { + if err := UnmountAll(path, flags); err != nil { + return err + } + if err := os.Remove(path); err != nil { + return err + } + } + return nil +} diff --git a/mount/temp_unsupported.go b/mount/temp_unsupported.go new file mode 100644 index 000000000..d3a188fc0 --- /dev/null +++ b/mount/temp_unsupported.go @@ -0,0 +1,13 @@ +// +build windows + +package mount + +// SetTempMountLocation sets the temporary mount location +func SetTempMountLocation(root string) error { + return nil +} + +// CleanupTempMounts all temp mounts and remove the directories +func CleanupTempMounts(flags int) error { + return nil +} diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go index c34b79454..8bbdd78b1 100644 --- a/oci/spec_opts_unix.go +++ b/oci/spec_opts_unix.go @@ -287,7 +287,7 @@ func WithUserID(uid uint32) SpecOpts { if err != nil { return err } - return mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { + return mount.WithTempMount(ctx, mounts, func(root string) error { uuid, ugid, err := getUIDGIDFromPath(root, func(u user.User) bool { return u.Uid == int(uid) }) @@ -334,7 +334,7 @@ func WithUsername(username string) SpecOpts { if err != nil { return err } - return mount.DefaultTempLocation.Mount(ctx, mounts, func(root string) error { + return mount.WithTempMount(ctx, mounts, func(root string) error { uid, gid, err := getUIDGIDFromPath(root, func(u user.User) bool { return u.Name == username })