Apply PR feedback

* Rootfs dir is created during container creation not during bundle
  creation
* Add support for v2
* UnmountAll is a no-op when the path to unmount (i.e. the rootfs dir)
  does not exist or is invalid

Co-authored-by: Danail Branekov <danailster@gmail.com>
Signed-off-by: Georgi Sabev <georgethebeatle@gmail.com>

mount/mount_linux.go

@@ -112,6 +112,9 @@ func unmount(target string, flags int) error {
 // are no mounts remaining (EINVAL is returned by mount), which is
 // useful for undoing a stack of mounts on the same mount point.
 func UnmountAll(mount string, flags int) error {
+	if _, err := os.Stat(mount); err != nil {
+		return nil
+	}
 	for {
 		if err := unmount(mount, flags); err != nil {
 			// EINVAL is returned if the target is not a

runtime/v1/linux/bundle.go

@@ -65,9 +65,6 @@ func newBundle(id, path, workDir string, spec []byte) (b *bundle, err error) {
 			os.RemoveAll(workDir)
 		}
 	}()
-	if err := os.Mkdir(filepath.Join(path, "rootfs"), 0711); err != nil {
-		return nil, err
-	}
 	err = ioutil.WriteFile(filepath.Join(path, configFilename), spec, 0666)
 	return &bundle{
 		id:      id,

runtime/v1/linux/proc/init.go

@@ -304,12 +304,10 @@ func (p *Init) delete(ctx context.Context) error {
 		}
 		p.io.Close()
 	}
-	if p.Rootfs != "" {
+	if err2 := mount.UnmountAll(p.Rootfs, 0); err2 != nil {
-		if err2 := mount.UnmountAll(p.Rootfs, 0); err2 != nil {
+		log.G(ctx).WithError(err2).Warn("failed to cleanup rootfs mount")
-			log.G(ctx).WithError(err2).Warn("failed to cleanup rootfs mount")
+		if err == nil {
-			if err == nil {
+			err = errors.Wrap(err2, "failed rootfs umount")
-				err = errors.Wrap(err2, "failed rootfs umount")
-			}
 		}
 	}
 	return err

runtime/v1/shim/service.go

@@ -124,6 +124,14 @@ func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (_ *
 		})
 	}
 
+	rootfs := ""
+	if len(mounts) > 0 {
+		rootfs = filepath.Join(r.Bundle, "rootfs")
+		if err := os.Mkdir(rootfs, 0711); err != nil {
+			return nil, err
+		}
+	}
+
 	config := &proc.CreateConfig{
 		ID:               r.ID,
 		Bundle:           r.Bundle,
@@ -137,14 +145,13 @@ func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (_ *
 		ParentCheckpoint: r.ParentCheckpoint,
 		Options:          r.Options,
 	}
-	rootfs := filepath.Join(r.Bundle, "rootfs")
+	defer func() {
-	defer func(rootfs string) {
 		if err != nil {
 			if err2 := mount.UnmountAll(rootfs, 0); err2 != nil {
 				log.G(ctx).WithError(err2).Warn("Failed to cleanup rootfs mount")
 			}
 		}
-	}(rootfs)
+	}()
 	for _, rm := range mounts {
 		m := &mount.Mount{
 			Type:    rm.Type,
@@ -159,10 +166,6 @@ func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (_ *
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	if len(mounts) == 0 {
-		rootfs = ""
-	}
-
 	process, err := newInit(
 		ctx,
 		s.config.Path,

runtime/v2/bundle.go

@@ -89,10 +89,6 @@ func NewBundle(ctx context.Context, root, state, id string, spec []byte) (b *Bun
 		}
 	}
 	paths = append(paths, work)
-	// create rootfs dir
-	if err := os.Mkdir(filepath.Join(b.Path, "rootfs"), 0711); err != nil {
-		return nil, err
-	}
 	// symlink workdir
 	if err := os.Symlink(work, filepath.Join(b.Path, "work")); err != nil {
 		return nil, err

runtime/v2/runc/container.go

@@ -21,6 +21,7 @@ package runc
 import (
 	"context"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"sync"
 
@@ -63,6 +64,15 @@ func NewContainer(ctx context.Context, platform rproc.Platform, r *task.CreateTa
 			Options: m.Options,
 		})
 	}
+
+	rootfs := ""
+	if len(mounts) > 0 {
+		rootfs = filepath.Join(r.Bundle, "rootfs")
+		if err := os.Mkdir(rootfs, 0711); err != nil {
+			return nil, err
+		}
+	}
+
 	config := &proc.CreateConfig{
 		ID:               r.ID,
 		Bundle:           r.Bundle,
@@ -80,7 +90,6 @@ func NewContainer(ctx context.Context, platform rproc.Platform, r *task.CreateTa
 	if err := WriteRuntime(r.Bundle, opts.BinaryName); err != nil {
 		return nil, err
 	}
-	rootfs := filepath.Join(r.Bundle, "rootfs")
 	defer func() {
 		if err != nil {
 			if err2 := mount.UnmountAll(rootfs, 0); err2 != nil {
@@ -107,6 +116,7 @@ func NewContainer(ctx context.Context, platform rproc.Platform, r *task.CreateTa
 		platform,
 		config,
 		&opts,
+		rootfs,
 	)
 	if err != nil {
 		return nil, errdefs.ToGRPC(err)
@@ -146,8 +156,7 @@ func WriteRuntime(path, runtime string) error {
 }
 
 func newInit(ctx context.Context, path, workDir, namespace string, platform rproc.Platform,
-	r *proc.CreateConfig, options *options.Options) (*proc.Init, error) {
+	r *proc.CreateConfig, options *options.Options, rootfs string) (*proc.Init, error) {
-	rootfs := filepath.Join(path, "rootfs")
 	runtime := proc.NewRunc(options.Root, path, namespace, options.BinaryName, options.CriuPath, options.SystemdCgroup)
 	p := proc.New(r.ID, runtime, rproc.Stdio{
 		Stdin:    r.Stdin,