shimv2: move container rootfs mounted flag to container level

It is in fact a container specific info not sandbox level info. We are assuming that all containers use the same snapshotter but it may not be the fact in reality. Fixes: #2532 Signed-off-by: Peng Tao <[email protected]>
kata-containers · Mar 23, 2020 · d0a730c · d0a730c
1 parent 2d89766
commit d0a730c
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 50 deletions.
diff --git a/containerd-shim-v2/container.go b/containerd-shim-v2/container.go
@@ -33,9 +33,10 @@ type container struct {
 	exit     uint32
 	status   task.Status
 	terminal bool
+	mounted  bool
 }
 
-func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.ContainerType, spec *specs.Spec) (*container, error) {
+func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.ContainerType, spec *specs.Spec, mounted bool) (*container, error) {
 	if r == nil {
 		return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, " CreateTaskRequest points to nil")
 	}
@@ -59,6 +60,7 @@ func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.Con
 		status:   task.StatusCreated,
 		exitIOch: make(chan struct{}),
 		exitCh:   make(chan uint32, 1),
+		mounted:  mounted,
 	}
 	return c, nil
 }
diff --git a/containerd-shim-v2/container_test.go b/containerd-shim-v2/container_test.go
@@ -6,15 +6,16 @@
 package containerdshim
 
 import (
+	"testing"
+
 	taskAPI "github.com/containerd/containerd/runtime/v2/task"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 func TestNewContainer(t *testing.T) {
 	assert := assert.New(t)
 
-	_, err := newContainer(nil, nil, "", nil)
+	_, err := newContainer(nil, nil, "", nil, false)
 
 	assert.Error(err)
 }
@@ -24,7 +25,7 @@ func TestGetExec(t *testing.T) {
 
 	r := &taskAPI.CreateTaskRequest{}
 
-	c, err := newContainer(nil, r, "", nil)
+	c, err := newContainer(nil, r, "", nil, true)
 	assert.NoError(err)
 
 	_, err = c.getExec("")

diff --git a/containerd-shim-v2/create.go b/containerd-shim-v2/create.go
@@ -20,6 +20,7 @@ import (
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+
 	// only register the proto type
 	_ "github.com/containerd/containerd/runtime/linux/runctypes"
 	crioption "github.com/containerd/cri-containerd/pkg/api/runtimeoptions/v1"
@@ -31,7 +32,7 @@ import (
 )
 
 func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*container, error) {
-	rootFs := vc.RootFs{Mounted: s.mount}
+	rootFs := vc.RootFs{}
 	if len(r.Rootfs) == 1 {
 		m := r.Rootfs[0]
 		rootFs.Source = m.Source
@@ -64,21 +65,18 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con
 			return nil, err
 		}
 
+		if rootFs.Mounted, err = checkAndMount(s, r); err != nil {
+			return nil, err
+		}
+
 		defer func() {
-			if err != nil && s.mount {
+			if err != nil && rootFs.Mounted {
 				if err2 := mount.UnmountAll(rootfs, 0); err2 != nil {
 					logrus.WithError(err2).Warn("failed to cleanup rootfs mount")
 				}
 			}
 		}()
 
-		s.mount = true
-		if err = checkAndMount(s, r); err != nil {
-			return nil, err
-		}
-
-		rootFs.Mounted = s.mount
-
 		katautils.HandleFactory(ctx, vci, s.config)
 
 		// Pass service's context instead of local ctx to CreateSandbox(), since local
@@ -96,27 +94,25 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con
 			return nil, fmt.Errorf("BUG: Cannot start the container, since the sandbox hasn't been created")
 		}
 
-		if s.mount {
-			defer func() {
-				if err != nil {
-					if err2 := mount.UnmountAll(rootfs, 0); err2 != nil {
-						logrus.WithError(err2).Warn("failed to cleanup rootfs mount")
-					}
-				}
-			}()
+		if rootFs.Mounted, err = checkAndMount(s, r); err != nil {
+			return nil, err
+		}
 
-			if err = doMount(r.Rootfs, rootfs); err != nil {
-				return nil, err
+		defer func() {
+			if err != nil && rootFs.Mounted {
+				if err2 := mount.UnmountAll(rootfs, 0); err2 != nil {
+					logrus.WithError(err2).Warn("failed to cleanup rootfs mount")
+				}
 			}
-		}
+		}()
 
 		_, err = katautils.CreateContainer(ctx, vci, s.sandbox, *ociSpec, rootFs, r.ID, bundlePath, "", disableOutput, true)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	container, err := newContainer(s, r, containerType, ociSpec)
+	container, err := newContainer(s, r, containerType, ociSpec, rootFs.Mounted)
 	if err != nil {
 		return nil, err
 	}
@@ -184,20 +180,19 @@ func loadRuntimeConfig(s *service, r *taskAPI.CreateTaskRequest, anno map[string
 	return &runtimeConfig, nil
 }
 
-func checkAndMount(s *service, r *taskAPI.CreateTaskRequest) error {
+func checkAndMount(s *service, r *taskAPI.CreateTaskRequest) (bool, error) {
 	if len(r.Rootfs) == 1 {
 		m := r.Rootfs[0]
 
 		if katautils.IsBlockDevice(m.Source) && !s.config.HypervisorConfig.DisableBlockDeviceUse {
-			s.mount = false
-			return nil
+			return false, nil
 		}
 	}
 	rootfs := filepath.Join(r.Bundle, "rootfs")
 	if err := doMount(r.Rootfs, rootfs); err != nil {
-		return err
+		return false, err
 	}
-	return nil
+	return true, nil
 }
 
 func doMount(mounts []*containerd_types.Mount, rootfs string) error {

diff --git a/containerd-shim-v2/delete.go b/containerd-shim-v2/delete.go
@@ -39,7 +39,7 @@ func deleteContainer(ctx context.Context, s *service, c *container) error {
 		return err
 	}
 
-	if s.mount {
+	if c.mounted {
 		rootfs := path.Join(c.bundle, "rootfs")
 		if err := mount.UnmountAll(rootfs, 0); err != nil {
 			logrus.WithError(err).Warn("failed to cleanup rootfs mount")

diff --git a/containerd-shim-v2/delete_test.go b/containerd-shim-v2/delete_test.go
@@ -40,7 +40,7 @@ func TestDeleteContainerSuccessAndFail(t *testing.T) {
 	reqCreate := &taskAPI.CreateTaskRequest{
 		ID: testContainerID,
 	}
-	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil)
+	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil, true)
 	assert.NoError(err)
 }
 

diff --git a/containerd-shim-v2/exec_test.go b/containerd-shim-v2/exec_test.go
@@ -36,7 +36,7 @@ func TestExecNoSpecFail(t *testing.T) {
 	}
 
 	var err error
-	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil)
+	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil, false)
 	assert.NoError(err)
 
 	reqExec := &taskAPI.ExecProcessRequest{

diff --git a/containerd-shim-v2/pause_test.go b/containerd-shim-v2/pause_test.go
@@ -57,7 +57,7 @@ func TestPauseContainerSuccess(t *testing.T) {
 	reqCreate := &taskAPI.CreateTaskRequest{
 		ID: testContainerID,
 	}
-	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil)
+	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil, true)
 	assert.NoError(err)
 
 	reqPause := &taskAPI.PauseRequest{
@@ -149,7 +149,7 @@ func TestResumeContainerSuccess(t *testing.T) {
 	reqCreate := &taskAPI.CreateTaskRequest{
 		ID: testContainerID,
 	}
-	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil)
+	s.containers[testContainerID], err = newContainer(s, reqCreate, "", nil, true)
 	assert.NoError(err)
 
 	reqResume := &taskAPI.ResumeRequest{

diff --git a/containerd-shim-v2/service.go b/containerd-shim-v2/service.go
@@ -82,7 +82,6 @@ func New(ctx context.Context, id string, publisher events.Publisher) (cdshim.Shi
 		events:     make(chan interface{}, chSize),
 		ec:         make(chan exit, bufferSize),
 		cancel:     cancel,
-		mount:      false,
 	}
 
 	go s.processExits()
@@ -110,10 +109,6 @@ type service struct {
 	// pid directly.
 	pid uint32
 
-	// if the container's rootfs is block device backed, kata shimv2
-	// will not do the rootfs mount.
-	mount bool
-
 	ctx        context.Context
 	sandbox    vc.VCSandbox
 	containers map[string]*container

diff --git a/containerd-shim-v2/start_test.go b/containerd-shim-v2/start_test.go
@@ -50,7 +50,7 @@ func TestStartStartSandboxSuccess(t *testing.T) {
 	reqCreate := &taskAPI.CreateTaskRequest{
 		ID: testSandboxID,
 	}
-	s.containers[testSandboxID], err = newContainer(s, reqCreate, vc.PodSandbox, nil)
+	s.containers[testSandboxID], err = newContainer(s, reqCreate, vc.PodSandbox, nil, false)
 	assert.NoError(err)
 
 	reqStart := &taskAPI.StartRequest{
@@ -98,7 +98,7 @@ func TestStartMissingAnnotation(t *testing.T) {
 	reqCreate := &taskAPI.CreateTaskRequest{
 		ID: testSandboxID,
 	}
-	s.containers[testSandboxID], err = newContainer(s, reqCreate, "", nil)
+	s.containers[testSandboxID], err = newContainer(s, reqCreate, "", nil, false)
 	assert.NoError(err)
 
 	reqStart := &taskAPI.StartRequest{
@@ -164,7 +164,7 @@ func TestStartStartContainerSucess(t *testing.T) {
 	reqCreate := &taskAPI.CreateTaskRequest{
 		ID: testContainerID,
 	}
-	s.containers[testContainerID], err = newContainer(s, reqCreate, vc.PodContainer, nil)
+	s.containers[testContainerID], err = newContainer(s, reqCreate, vc.PodContainer, nil, false)
 	assert.NoError(err)
 
 	reqStart := &taskAPI.StartRequest{

diff --git a/containerd-shim-v2/wait.go b/containerd-shim-v2/wait.go
@@ -112,13 +112,14 @@ func watchSandbox(s *service) {
 		logrus.WithError(err).Warn("delete sandbox failed")
 	}
 
-	if s.mount {
-		for _, c := range s.containers {
-			rootfs := path.Join(c.bundle, "rootfs")
-			logrus.WithField("rootfs", rootfs).WithField("id", c.id).Debug("container umount rootfs")
-			if err := mount.UnmountAll(rootfs, 0); err != nil {
-				logrus.WithError(err).Warn("failed to cleanup rootfs mount")
-			}
+	for _, c := range s.containers {
+		if !c.mounted {
+			continue
+		}
+		rootfs := path.Join(c.bundle, "rootfs")
+		logrus.WithField("rootfs", rootfs).WithField("id", c.id).Debug("container umount rootfs")
+		if err := mount.UnmountAll(rootfs, 0); err != nil {
+			logrus.WithError(err).Warn("failed to cleanup rootfs mount")
 		}
 	}