From dfd7ee122f4b7a856534e9add55b60d834bcaa25 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Fri, 5 Jan 2018 11:35:29 -0800 Subject: [PATCH] Clean up error logs and messages in temp mount Signed-off-by: Derek McGowan --- container_opts_unix.go | 6 +++--- diff/walking/differ.go | 6 +++--- mount/mount.go | 19 +++++++++---------- oci/spec_opts_unix.go | 4 ++-- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/container_opts_unix.go b/container_opts_unix.go index 6e9ed1ce2..9d5225a91 100644 --- a/container_opts_unix.go +++ b/container_opts_unix.go @@ -165,7 +165,7 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool if err != nil { return err } - if err := remapRootFS(mounts, uid, gid); err != nil { + if err := remapRootFS(ctx, mounts, uid, gid); err != nil { snapshotter.Remove(ctx, usernsID) return err } @@ -186,8 +186,8 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool } } -func remapRootFS(mounts []mount.Mount, uid, gid uint32) error { - return mount.WithTempMount(mounts, func(root string) error { +func remapRootFS(ctx context.Context, mounts []mount.Mount, uid, gid uint32) error { + return mount.WithTempMount(ctx, mounts, func(root string) error { return filepath.Walk(root, incrementFS(root, uid, gid)) }) } diff --git a/diff/walking/differ.go b/diff/walking/differ.go index 2aab318fa..eb6c2df70 100644 --- a/diff/walking/differ.go +++ b/diff/walking/differ.go @@ -90,7 +90,7 @@ func (s *walkingDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts } var ocidesc ocispec.Descriptor - if err := mount.WithTempMount(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") @@ -158,8 +158,8 @@ func (s *walkingDiff) DiffMounts(ctx context.Context, lower, upper []mount.Mount } var ocidesc ocispec.Descriptor - if err := mount.WithTempMount(lower, func(lowerRoot string) error { - return mount.WithTempMount(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/mount.go b/mount/mount.go index 0d06cf934..323e6e2fb 100644 --- a/mount/mount.go +++ b/mount/mount.go @@ -1,6 +1,7 @@ package mount import ( + "context" "io/ioutil" "os" @@ -34,20 +35,21 @@ func All(mounts []Mount, target string) error { // 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(mounts []Mount, f func(root string) error) (err error) { +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 for %v", mounts) + 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, if we use RemoveAll, We will incorrectly delete data from mounted dir. - // if we use Remove,even though we won't successfully delete the temp dir, - // but we only leak a temp dir, we don't loss data from mounted dir. + // 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.L.Errorf("Failed to remove the temp dir %s: %v", root, uerr) + log.G(ctx).WithError(uerr).WithField("dir", root).Errorf("failed to remove mount temp dir") } }() @@ -66,8 +68,5 @@ func WithTempMount(mounts []Mount, f func(root string) error) (err error) { return errors.Wrapf(uerr, "failed to mount %s", root) } - if uerr = f(root); uerr != nil { - return errors.Wrapf(uerr, "failed to f(%s)", root) - } - return nil + return errors.Wrapf(f(root), "mount callback failed on %s", root) } diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go index c344a1bd9..ebc423d3e 100644 --- a/oci/spec_opts_unix.go +++ b/oci/spec_opts_unix.go @@ -271,7 +271,7 @@ func WithUserID(uid uint32) SpecOpts { return err } - return mount.WithTempMount(mounts, func(root string) error { + return mount.WithTempMount(ctx, mounts, func(root string) error { ppath, err := fs.RootPath(root, "/etc/passwd") if err != nil { return err @@ -319,7 +319,7 @@ func WithUsername(username string) SpecOpts { if err != nil { return err } - return mount.WithTempMount(mounts, func(root string) error { + return mount.WithTempMount(ctx, mounts, func(root string) error { ppath, err := fs.RootPath(root, "/etc/passwd") if err != nil { return err