Skip to content

Commit d42dbdd

Browse files
committed
Fix error handling with not-exist errors on remove
Specifically, none of the graphdrivers are supposed to return a not-exist type of error on remove (or at least that's how they are currently handled). Found that AUFS still had one case where a not-exist error could escape, when checking if the directory is mounted we call a `Statfs` on the path. This fixes AUFS to not return an error in this case, but also double-checks at the daemon level on layer remove that the error is not a `not-exist` type of error. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
1 parent 45cad73 commit d42dbdd

2 files changed

Lines changed: 33 additions & 20 deletions

File tree

daemon/delete.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
119119
if container.RWLayer != nil {
120120
metadata, err := daemon.stores[container.Platform].layerStore.ReleaseRWLayer(container.RWLayer)
121121
layer.LogReleaseMetadata(metadata)
122-
if err != nil && err != layer.ErrMountDoesNotExist {
122+
if err != nil && err != layer.ErrMountDoesNotExist && !os.IsNotExist(errors.Cause(err)) {
123123
return errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.Platform), container.ID)
124124
}
125125
}

daemon/graphdriver/aufs/aufs.go

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"github.com/docker/docker/pkg/system"
4747
rsystem "github.com/opencontainers/runc/libcontainer/system"
4848
"github.com/opencontainers/selinux/go-selinux/label"
49+
"github.com/pkg/errors"
4950
"github.com/vbatts/tar-split/tar/storage"
5051
"golang.org/x/sys/unix"
5152
)
@@ -282,30 +283,41 @@ func (a *Driver) Remove(id string) error {
282283
mountpoint = a.getMountpoint(id)
283284
}
284285

286+
logger := logrus.WithFields(logrus.Fields{
287+
"module": "graphdriver",
288+
"driver": "aufs",
289+
"layer": id,
290+
})
291+
285292
var retries int
286293
for {
287294
mounted, err := a.mounted(mountpoint)
288295
if err != nil {
296+
if os.IsNotExist(err) {
297+
break
298+
}
289299
return err
290300
}
291301
if !mounted {
292302
break
293303
}
294304

295-
if err := a.unmount(mountpoint); err != nil {
296-
if err != unix.EBUSY {
297-
return fmt.Errorf("aufs: unmount error: %s: %v", mountpoint, err)
298-
}
299-
if retries >= 5 {
300-
return fmt.Errorf("aufs: unmount error after retries: %s: %v", mountpoint, err)
301-
}
302-
// If unmount returns EBUSY, it could be a transient error. Sleep and retry.
303-
retries++
304-
logrus.Warnf("unmount failed due to EBUSY: retry count: %d", retries)
305-
time.Sleep(100 * time.Millisecond)
306-
continue
305+
err = a.unmount(mountpoint)
306+
if err == nil {
307+
break
307308
}
308-
break
309+
310+
if err != unix.EBUSY {
311+
return errors.Wrapf(err, "aufs: unmount error: %s", mountpoint)
312+
}
313+
if retries >= 5 {
314+
return errors.Wrapf(err, "aufs: unmount error after retries: %s", mountpoint)
315+
}
316+
// If unmount returns EBUSY, it could be a transient error. Sleep and retry.
317+
retries++
318+
logger.Warnf("unmount failed due to EBUSY: retry count: %d", retries)
319+
time.Sleep(100 * time.Millisecond)
320+
continue
309321
}
310322

311323
// Atomically remove each directory in turn by first moving it out of the
@@ -314,21 +326,22 @@ func (a *Driver) Remove(id string) error {
314326
tmpMntPath := path.Join(a.mntPath(), fmt.Sprintf("%s-removing", id))
315327
if err := os.Rename(mountpoint, tmpMntPath); err != nil && !os.IsNotExist(err) {
316328
if err == unix.EBUSY {
317-
logrus.Warn("os.Rename err due to EBUSY")
329+
logger.WithField("dir", mountpoint).WithError(err).Warn("os.Rename err due to EBUSY")
318330
}
319-
return err
331+
return errors.Wrapf(err, "error preparing atomic delete of aufs mountpoint for id: %s", id)
332+
}
333+
if err := system.EnsureRemoveAll(tmpMntPath); err != nil {
334+
return errors.Wrapf(err, "error removing aufs layer %s", id)
320335
}
321-
defer system.EnsureRemoveAll(tmpMntPath)
322336

323337
tmpDiffpath := path.Join(a.diffPath(), fmt.Sprintf("%s-removing", id))
324338
if err := os.Rename(a.getDiffPath(id), tmpDiffpath); err != nil && !os.IsNotExist(err) {
325-
return err
339+
return errors.Wrapf(err, "error preparing atomic delete of aufs diff dir for id: %s", id)
326340
}
327-
defer system.EnsureRemoveAll(tmpDiffpath)
328341

329342
// Remove the layers file for the id
330343
if err := os.Remove(path.Join(a.rootPath(), "layers", id)); err != nil && !os.IsNotExist(err) {
331-
return err
344+
return errors.Wrapf(err, "error removing layers dir for %s", id)
332345
}
333346

334347
a.pathCacheLock.Lock()

0 commit comments

Comments
 (0)