Ensure bundle removal is atomic

crosbymichael · crosbymichael · commit 36e4dc603e01 · 2018-12-20T13:45:18.000-05:00
This makes bundle removal atomic by first renaming the bundle and working directories to a hidden path before removing the underlying directories. Closes containerd#2567 Closes containerd#2327 Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
diff --git a/runtime/v1/linux/bundle.go b/runtime/v1/linux/bundle.go
@@ -20,6 +20,7 @@ package linux
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -114,12 +115,12 @@ func (b *bundle) NewShimClient(ctx context.Context, namespace string, getClientO
 
 // Delete deletes the bundle from disk
 func (b *bundle) Delete() error {
-	err := os.RemoveAll(b.path)
+	err := atomicDelete(b.path)
 	if err == nil {
-		return os.RemoveAll(b.workDir)
+		return atomicDelete(b.workDir)
 	}
 	// error removing the bundle path; still attempt removing work dir
-	err2 := os.RemoveAll(b.workDir)
+	err2 := atomicDelete(b.workDir)
 	if err2 == nil {
 		return err
 	}
@@ -152,3 +153,13 @@ func (b *bundle) shimConfig(namespace string, c *Config, runcOptions *runctypes.
 		SystemdCgroup: systemdCgroup,
 	}
 }
+
+// atomicDelete renames the path to a hidden file before removal
+func atomicDelete(path string) error {
+	// create a hidden dir for an atomic removal
+	atomicPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".%s", filepath.Base(path)))
+	if err := os.Rename(path, atomicPath); err != nil {
+		return err
+	}
+	return os.RemoveAll(atomicPath)
+}
diff --git a/runtime/v1/linux/runtime.go b/runtime/v1/linux/runtime.go
@@ -290,6 +290,10 @@ func (r *Runtime) restoreTasks(ctx context.Context) ([]*Task, error) {
 			continue
 		}
 		name := namespace.Name()
+		// skip hidden directories
+		if len(name) > 0 && name[0] == '.' {
+			continue
+		}
 		log.G(ctx).WithField("namespace", name).Debug("loading tasks in namespace")
 		tasks, err := r.loadTasks(ctx, name)
 		if err != nil {
diff --git a/runtime/v2/bundle.go b/runtime/v2/bundle.go
@@ -18,6 +18,7 @@ package v2
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -114,20 +115,30 @@ type Bundle struct {
 // Delete a bundle atomically
 func (b *Bundle) Delete() error {
 	work, werr := os.Readlink(filepath.Join(b.Path, "work"))
-	err := os.RemoveAll(b.Path)
+	err := atomicDelete(b.Path)
 	if err == nil {
 		if werr == nil {
-			return os.RemoveAll(work)
+			return atomicDelete(work)
 		}
 		return nil
 	}
 	// error removing the bundle path; still attempt removing work dir
 	var err2 error
 	if werr == nil {
-		err2 = os.RemoveAll(work)
+		err2 = atomicDelete(work)
 		if err2 == nil {
 			return err
 		}
 	}
 	return errors.Wrapf(err, "failed to remove both bundle and workdir locations: %v", err2)
 }
+
+// atomicDelete renames the path to a hidden file before removal
+func atomicDelete(path string) error {
+	// create a hidden dir for an atomic removal
+	atomicPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".%s", filepath.Base(path)))
+	if err := os.Rename(path, atomicPath); err != nil {
+		return err
+	}
+	return os.RemoveAll(atomicPath)
+}
diff --git a/runtime/v2/manager.go b/runtime/v2/manager.go
@@ -145,6 +145,10 @@ func (m *TaskManager) loadExistingTasks(ctx context.Context) error {
 			continue
 		}
 		ns := nsd.Name()
+		// skip hidden directories
+		if len(ns) > 0 && ns[0] == '.' {
+			continue
+		}
 		log.G(ctx).WithField("namespace", ns).Debug("loading tasks in namespace")
 		if err := m.loadTasks(namespaces.WithNamespace(ctx, ns)); err != nil {
 			log.G(ctx).WithField("namespace", ns).WithError(err).Error("loading tasks in namespace")
