Merge pull request #1382 from vrothberg/del-ctr

container deletion - EBUSY improvement

pkg/mount/unmount_unix.go

@@ -1,16 +1,30 @@
+//go:build !windows
 // +build !windows
 
 package mount
 
-import "golang.org/x/sys/unix"
+import (
+	"time"
+
+	"golang.org/x/sys/unix"
+)
 
 func unmount(target string, flags int) error {
-	err := unix.Unmount(target, flags)
-	if err == nil || err == unix.EINVAL {
-		// Ignore "not mounted" error here. Note the same error
-		// can be returned if flags are invalid, so this code
-		// assumes that the flags value is always correct.
-		return nil
+	var err error
+	for i := 0; i < 50; i++ {
+		err = unix.Unmount(target, flags)
+		switch err {
+		case unix.EBUSY:
+			time.Sleep(50 * time.Millisecond)
+			continue
+		case unix.EINVAL, nil:
+			// Ignore "not mounted" error here. Note the same error
+			// can be returned if flags are invalid, so this code
+			// assumes that the flags value is always correct.
+			return nil
+		default:
+			break
+		}
 	}
 
 	return &mountError{

pkg/system/rm.go

@@ -3,7 +3,6 @@ package system
 import (
 	"fmt"
 	"os"
-	"syscall"
 	"time"
 
 	"github.com/containers/storage/pkg/mount"
@@ -65,7 +64,7 @@ func EnsureRemoveAll(dir string) error {
 			continue
 		}
 
-		if pe.Err != syscall.EBUSY {
+		if !IsEBUSY(pe.Err) {
 			return err
 		}
 

store.go

@@ -2504,68 +2504,72 @@ func (s *store) DeleteContainer(id string) error {
 		return err
 	}
 
-	if rcstore.Exists(id) {
-		if container, err := rcstore.Get(id); err == nil {
-			errChan := make(chan error)
-			var wg sync.WaitGroup
+	if !rcstore.Exists(id) {
+		return ErrNotAContainer
+	}
 
-			if rlstore.Exists(container.LayerID) {
-				wg.Add(1)
-				go func() {
-					errChan <- rlstore.Delete(container.LayerID)
-					wg.Done()
-				}()
-			}
-			wg.Add(1)
-			go func() {
-				errChan <- rcstore.Delete(id)
-				wg.Done()
-			}()
-
-			middleDir := s.graphDriverName + "-containers"
-			gcpath := filepath.Join(s.GraphRoot(), middleDir, container.ID)
-			wg.Add(1)
-			go func() {
-				defer wg.Done()
-				// attempt a simple rm -rf first
-				err := os.RemoveAll(gcpath)
-				if err == nil {
-					errChan <- nil
-					return
-				}
-				// and if it fails get to the more complicated cleanup
-				errChan <- system.EnsureRemoveAll(gcpath)
-			}()
-
-			rcpath := filepath.Join(s.RunRoot(), middleDir, container.ID)
-			wg.Add(1)
-			go func() {
-				defer wg.Done()
-				// attempt a simple rm -rf first
-				err := os.RemoveAll(rcpath)
-				if err == nil {
-					errChan <- nil
-					return
-				}
-				// and if it fails get to the more complicated cleanup
-				errChan <- system.EnsureRemoveAll(rcpath)
-			}()
+	container, err := rcstore.Get(id)
+	if err != nil {
+		return ErrNotAContainer
+	}
 
-			go func() {
-				wg.Wait()
-				close(errChan)
-			}()
+	errChan := make(chan error)
+	var wg sync.WaitGroup
 
-			var errors []error
-			for err := range errChan {
-				if err != nil {
-					errors = append(errors, err)
-				}
-			}
-			return multierror.Append(nil, errors...).ErrorOrNil()
+	if rlstore.Exists(container.LayerID) {
+		wg.Add(1)
+		go func() {
+			errChan <- rlstore.Delete(container.LayerID)
+			wg.Done()
+		}()
+	}
+	wg.Add(1)
+	go func() {
+		errChan <- rcstore.Delete(id)
+		wg.Done()
+	}()
+
+	middleDir := s.graphDriverName + "-containers"
+	gcpath := filepath.Join(s.GraphRoot(), middleDir, container.ID)
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		// attempt a simple rm -rf first
+		err := os.RemoveAll(gcpath)
+		if err == nil {
+			errChan <- nil
+			return
+		}
+		// and if it fails get to the more complicated cleanup
+		errChan <- system.EnsureRemoveAll(gcpath)
+	}()
+
+	rcpath := filepath.Join(s.RunRoot(), middleDir, container.ID)
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		// attempt a simple rm -rf first
+		err := os.RemoveAll(rcpath)
+		if err == nil {
+			errChan <- nil
+			return
+		}
+		// and if it fails get to the more complicated cleanup
+		errChan <- system.EnsureRemoveAll(rcpath)
+	}()
+
+	go func() {
+		wg.Wait()
+		close(errChan)
+	}()
+
+	var errors []error
+	for err := range errChan {
+		if err != nil {
+			errors = append(errors, err)
 		}
 	}
-	return ErrNotAContainer
+	return multierror.Append(nil, errors...).ErrorOrNil()
 }
 
 func (s *store) Delete(id string) error {