Skip to content

Commit dfd7ee1

Browse files
dmcgowandmcgowan
committed
Clean up error logs and messages in temp mount
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
1 parent 002c0e2 commit dfd7ee1

File tree

4 files changed

+17
-18
lines changed

4 files changed

+17
-18
lines changed

‎container_opts_unix.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool
165165
if err != nil {
166166
return err
167167
}
168-
if err := remapRootFS(mounts, uid, gid); err != nil {
168+
if err := remapRootFS(ctx, mounts, uid, gid); err != nil {
169169
snapshotter.Remove(ctx, usernsID)
170170
return err
171171
}
@@ -186,8 +186,8 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool
186186
}
187187
}
188188

189-
func remapRootFS(mounts []mount.Mount, uid, gid uint32) error {
190-
return mount.WithTempMount(mounts, func(root string) error {
189+
func remapRootFS(ctx context.Context, mounts []mount.Mount, uid, gid uint32) error {
190+
return mount.WithTempMount(ctx, mounts, func(root string) error {
191191
return filepath.Walk(root, incrementFS(root, uid, gid))
192192
})
193193
}

‎diff/walking/differ.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func (s *walkingDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts
9090
}
9191

9292
var ocidesc ocispec.Descriptor
93-
if err := mount.WithTempMount(mounts, func(root string) error {
93+
if err := mount.WithTempMount(ctx, mounts, func(root string) error {
9494
ra, err := s.store.ReaderAt(ctx, desc.Digest)
9595
if err != nil {
9696
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
158158
}
159159

160160
var ocidesc ocispec.Descriptor
161-
if err := mount.WithTempMount(lower, func(lowerRoot string) error {
162-
return mount.WithTempMount(upper, func(upperRoot string) error {
161+
if err := mount.WithTempMount(ctx, lower, func(lowerRoot string) error {
162+
return mount.WithTempMount(ctx, upper, func(upperRoot string) error {
163163
var newReference bool
164164
if config.Reference == "" {
165165
newReference = true

‎mount/mount.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package mount
22

33
import (
4+
"context"
45
"io/ioutil"
56
"os"
67

@@ -34,20 +35,21 @@ func All(mounts []Mount, target string) error {
3435
// WithTempMount mounts the provided mounts to a temp dir, and pass the temp dir to f.
3536
// The mounts are valid during the call to the f.
3637
// Finally we will unmount and remove the temp dir regardless of the result of f.
37-
func WithTempMount(mounts []Mount, f func(root string) error) (err error) {
38+
func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) error) (err error) {
3839
root, uerr := ioutil.TempDir("", "containerd-WithTempMount")
3940
if uerr != nil {
40-
return errors.Wrapf(uerr, "failed to create temp dir for %v", mounts)
41+
return errors.Wrapf(uerr, "failed to create temp dir")
4142
}
4243
// We use Remove here instead of RemoveAll.
4344
// The RemoveAll will delete the temp dir and all children it contains.
44-
// When the Unmount fails, if we use RemoveAll, We will incorrectly delete data from mounted dir.
45-
// if we use Remove,even though we won't successfully delete the temp dir,
46-
// but we only leak a temp dir, we don't loss data from mounted dir.
45+
// When the Unmount fails, RemoveAll will incorrectly delete data from
46+
// the mounted dir. However, if we use Remove, even though we won't
47+
// successfully delete the temp dir and it may leak, we won't loss data
48+
// from the mounted dir.
4749
// For details, please refer to #1868 #1785.
4850
defer func() {
4951
if uerr = os.Remove(root); uerr != nil {
50-
log.L.Errorf("Failed to remove the temp dir %s: %v", root, uerr)
52+
log.G(ctx).WithError(uerr).WithField("dir", root).Errorf("failed to remove mount temp dir")
5153
}
5254
}()
5355

@@ -66,8 +68,5 @@ func WithTempMount(mounts []Mount, f func(root string) error) (err error) {
6668
return errors.Wrapf(uerr, "failed to mount %s", root)
6769
}
6870

69-
if uerr = f(root); uerr != nil {
70-
return errors.Wrapf(uerr, "failed to f(%s)", root)
71-
}
72-
return nil
71+
return errors.Wrapf(f(root), "mount callback failed on %s", root)
7372
}

‎oci/spec_opts_unix.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ func WithUserID(uid uint32) SpecOpts {
271271
return err
272272
}
273273

274-
return mount.WithTempMount(mounts, func(root string) error {
274+
return mount.WithTempMount(ctx, mounts, func(root string) error {
275275
ppath, err := fs.RootPath(root, "/etc/passwd")
276276
if err != nil {
277277
return err
@@ -319,7 +319,7 @@ func WithUsername(username string) SpecOpts {
319319
if err != nil {
320320
return err
321321
}
322-
return mount.WithTempMount(mounts, func(root string) error {
322+
return mount.WithTempMount(ctx, mounts, func(root string) error {
323323
ppath, err := fs.RootPath(root, "/etc/passwd")
324324
if err != nil {
325325
return err

0 commit comments

Comments
 (0)