Merge pull request #1968 from dmcgowan/mount-temp-dir-error-cleanup

mount: clean up error logs and messages in temp mount
This commit is contained in:
Michael Crosby 2018-01-08 10:39:57 -05:00 committed by GitHub
commit 1df6287150
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 17 additions and 18 deletions

View File

@ -165,7 +165,7 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool
if err != nil { if err != nil {
return err return err
} }
if err := remapRootFS(mounts, uid, gid); err != nil { if err := remapRootFS(ctx, mounts, uid, gid); err != nil {
snapshotter.Remove(ctx, usernsID) snapshotter.Remove(ctx, usernsID)
return err 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 { func remapRootFS(ctx context.Context, mounts []mount.Mount, uid, gid uint32) error {
return mount.WithTempMount(mounts, func(root string) error { return mount.WithTempMount(ctx, mounts, func(root string) error {
return filepath.Walk(root, incrementFS(root, uid, gid)) return filepath.Walk(root, incrementFS(root, uid, gid))
}) })
} }

View File

@ -90,7 +90,7 @@ func (s *walkingDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts
} }
var ocidesc ocispec.Descriptor 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) ra, err := s.store.ReaderAt(ctx, desc.Digest)
if err != nil { if err != nil {
return errors.Wrap(err, "failed to get reader from content store") 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 var ocidesc ocispec.Descriptor
if err := mount.WithTempMount(lower, func(lowerRoot string) error { if err := mount.WithTempMount(ctx, lower, func(lowerRoot string) error {
return mount.WithTempMount(upper, func(upperRoot string) error { return mount.WithTempMount(ctx, upper, func(upperRoot string) error {
var newReference bool var newReference bool
if config.Reference == "" { if config.Reference == "" {
newReference = true newReference = true

View File

@ -1,6 +1,7 @@
package mount package mount
import ( import (
"context"
"io/ioutil" "io/ioutil"
"os" "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. // 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. // 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. // 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") root, uerr := ioutil.TempDir("", "containerd-WithTempMount")
if uerr != nil { 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. // We use Remove here instead of RemoveAll.
// The RemoveAll will delete the temp dir and all children it contains. // 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. // When the Unmount fails, RemoveAll will incorrectly delete data from
// if we use Remove,even though we won't successfully delete the temp dir, // the mounted dir. However, if we use Remove, even though we won't
// but we only leak a temp dir, we don't loss data from mounted dir. // 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. // For details, please refer to #1868 #1785.
defer func() { defer func() {
if uerr = os.Remove(root); uerr != nil { 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) return errors.Wrapf(uerr, "failed to mount %s", root)
} }
if uerr = f(root); uerr != nil { return errors.Wrapf(f(root), "mount callback failed on %s", root)
return errors.Wrapf(uerr, "failed to f(%s)", root)
}
return nil
} }

View File

@ -271,7 +271,7 @@ func WithUserID(uid uint32) SpecOpts {
return err 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") ppath, err := fs.RootPath(root, "/etc/passwd")
if err != nil { if err != nil {
return err return err
@ -319,7 +319,7 @@ func WithUsername(username string) SpecOpts {
if err != nil { if err != nil {
return err 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") ppath, err := fs.RootPath(root, "/etc/passwd")
if err != nil { if err != nil {
return err return err