[pull] master from kata-containers:master

virtcontainers/container.go

@@ -850,7 +850,7 @@ func (c *Container) rollbackFailingContainerCreation() {
 	if err := c.unmountHostMounts(); err != nil {
 		c.Logger().WithError(err).Error("rollback failed unmountHostMounts()")
 	}
-	if err := bindUnmountContainerRootfs(c.ctx, getMountPath(c.sandbox.id), c.id); err != nil {
+	if err := bindUnmountContainerRootfs(c.ctx, getMountPath(c.sandbox.id), c); err != nil {
 		c.Logger().WithError(err).Error("rollback failed bindUnmountContainerRootfs()")
 	}
 }
@@ -1120,7 +1120,7 @@ func (c *Container) stop(force bool) error {
 		return err
 	}
 
-	if err := bindUnmountContainerRootfs(c.ctx, getMountPath(c.sandbox.id), c.id); err != nil && !force {
+	if err := bindUnmountContainerRootfs(c.ctx, getMountPath(c.sandbox.id), c); err != nil && !force {
 		return err
 	}
 

virtcontainers/kata_agent.go

@@ -1290,7 +1290,7 @@ func (k *kataAgent) rollbackFailingContainerCreation(c *Container) {
 			k.Logger().WithError(err2).Error("rollback failed unmountHostMounts()")
 		}
 
-		if err2 := bindUnmountContainerRootfs(k.ctx, getMountPath(c.sandbox.id), c.id); err2 != nil {
+		if err2 := bindUnmountContainerRootfs(k.ctx, getMountPath(c.sandbox.id), c); err2 != nil {
 			k.Logger().WithError(err2).Error("rollback failed bindUnmountContainerRootfs()")
 		}
 	}

virtcontainers/mount.go

@@ -315,13 +315,17 @@ func isSymlink(path string) bool {
 	return stat.Mode()&os.ModeSymlink != 0
 }
 
-func bindUnmountContainerRootfs(ctx context.Context, sharedDir, cID string) error {
+func bindUnmountContainerRootfs(ctx context.Context, sharedDir string, con *Container) error {
 	span, _ := trace(ctx, "bindUnmountContainerRootfs")
 	defer span.Finish()
 
-	rootfsDest := filepath.Join(sharedDir, cID, rootfsDir)
-	if isSymlink(filepath.Join(sharedDir, cID)) || isSymlink(rootfsDest) {
-		logrus.Warnf("container dir %s is a symlink, malicious guest?", cID)
+	if con.state.Fstype != "" && con.state.BlockDeviceID != "" {
+		return nil
+	}
+
+	rootfsDest := filepath.Join(sharedDir, con.id, rootfsDir)
+	if isSymlink(filepath.Join(sharedDir, con.id)) || isSymlink(rootfsDest) {
+		logrus.Warnf("container dir %s is a symlink, malicious guest?", con.id)
 		return nil
 	}
 
@@ -351,7 +355,7 @@ func bindUnmountAllRootfs(ctx context.Context, sharedDir string, sandbox *Sandbo
 		if c.state.Fstype == "" {
 			// even if error found, don't break out of loop until all mounts attempted
 			// to be unmounted, and collect all errors
-			errors = merr.Append(errors, bindUnmountContainerRootfs(c.ctx, sharedDir, c.id))
+			errors = merr.Append(errors, bindUnmountContainerRootfs(c.ctx, sharedDir, c))
 		}
 	}
 	return errors.ErrorOrNil()

virtcontainers/mount_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	ktu "github.com/kata-containers/runtime/pkg/katatestutils"
+	"github.com/kata-containers/runtime/virtcontainers/types"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -378,6 +379,10 @@ func TestBindUnmountContainerRootfsENOENTNotError(t *testing.T) {
 	testMnt := "/tmp/test_mount"
 	sID := "sandIDTest"
 	cID := "contIDTest"
+	container := &Container{
+		id: cID,
+	}
+
 	assert := assert.New(t)
 
 	// check to make sure the file doesn't exist
@@ -386,7 +391,7 @@ func TestBindUnmountContainerRootfsENOENTNotError(t *testing.T) {
 		assert.NoError(os.Remove(testPath))
 	}
 
-	err := bindUnmountContainerRootfs(context.Background(), filepath.Join(testMnt, sID), cID)
+	err := bindUnmountContainerRootfs(context.Background(), filepath.Join(testMnt, sID), container)
 	assert.NoError(err)
 }
 
@@ -398,6 +403,9 @@ func TestBindUnmountContainerRootfsRemoveRootfsDest(t *testing.T) {
 
 	sID := "sandIDTestRemoveRootfsDest"
 	cID := "contIDTestRemoveRootfsDest"
+	container := &Container{
+		id: cID,
+	}
 
 	testPath := filepath.Join(testDir, sID, cID, rootfsDir)
 	syscall.Unmount(testPath, 0)
@@ -407,11 +415,55 @@ func TestBindUnmountContainerRootfsRemoveRootfsDest(t *testing.T) {
 	assert.NoError(err)
 	defer os.RemoveAll(filepath.Join(testDir, sID))
 
-	bindUnmountContainerRootfs(context.Background(), filepath.Join(testDir, sID), cID)
+	bindUnmountContainerRootfs(context.Background(), filepath.Join(testDir, sID), container)
 
 	if _, err := os.Stat(testPath); err == nil {
 		t.Fatal("empty rootfs dest should be removed")
 	} else if !os.IsNotExist(err) {
 		t.Fatal(err)
 	}
 }
+
+func TestBindUnmountContainerRootfsDevicemapper(t *testing.T) {
+	assert := assert.New(t)
+	if tc.NotValid(ktu.NeedRoot()) {
+		t.Skip(ktu.TestDisabledNeedRoot)
+	}
+
+	sID := "sandIDDevicemapper"
+
+	cID1 := "contIDDevicemapper1"
+	container := &Container{
+		id: cID1,
+		state: types.ContainerState{
+			Fstype:        "xfs",
+			BlockDeviceID: "4b073a87f3c9242a",
+		},
+	}
+
+	testPath1 := filepath.Join(testDir, sID, cID1, rootfsDir)
+	err := os.MkdirAll(testPath1, mountPerm)
+	assert.NoError(err)
+
+	bindUnmountContainerRootfs(context.Background(), filepath.Join(testDir, sID), container)
+
+	// expect testPath1 exist, if not exist, it means this case failed.
+	if _, err := os.Stat(testPath1); err != nil && !os.IsExist(err) {
+		t.Fatal("testPath should not be removed")
+	}
+
+	cID2 := "contIDDevicemapper2"
+	container.state.Fstype = ""
+	container.id = cID2
+	testPath2 := filepath.Join(testDir, sID, cID2, rootfsDir)
+	err = os.MkdirAll(testPath2, mountPerm)
+	assert.NoError(err)
+
+	defer os.RemoveAll(filepath.Join(testDir, sID))
+
+	// expect testPath2 not exist, if exist, it means this case failed.
+	bindUnmountContainerRootfs(context.Background(), filepath.Join(testDir, sID), container)
+	if _, err := os.Stat(testPath2); err == nil || os.IsExist(err) {
+		t.Fatal("testPath should be removed")
+	}
+}

virtcontainers/sandbox.go

@@ -728,14 +728,14 @@ func fetchSandbox(ctx context.Context, sandboxID string) (sandbox *Sandbox, err
 
 	var config SandboxConfig
 
-	// Try to load sandbox config from old store at first.
-	c, ctx, err := loadSandboxConfigFromOldStore(ctx, sandboxID)
+	// Try to load sandbox config from new store at first.
+	c, err := loadSandboxConfig(sandboxID)
 	if err != nil {
-		virtLog.Warningf("failed to get sandbox config from old store: %v", err)
-		// If we failed to load sandbox config from old store, try again with new store.
-		c, err = loadSandboxConfig(sandboxID)
+		virtLog.Warningf("failed to get sandbox config from new store: %v", err)
+		// If we failed to load sandbox config from new store, try again with old store.
+		c, ctx, err = loadSandboxConfigFromOldStore(ctx, sandboxID)
 		if err != nil {
-			virtLog.Warningf("failed to get sandbox config from new store: %v", err)
+			virtLog.Warningf("failed to get sandbox config from old store: %v", err)
 			return nil, err
 		}
 	}
@@ -837,7 +837,11 @@ func (s *Sandbox) Delete() error {
 	}
 
 	s.agent.cleanup(s)
-
+	if useOldStore(s.ctx) && s.store != nil {
+		if err := s.store.Delete(); err != nil {
+			s.Logger().WithError(err).Error("store delete failed")
+		}
+	}
 	return s.newStore.Destroy(s.id)
 }
 

virtcontainers/sandbox_test.go

@@ -13,6 +13,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"strings"
 	"sync"
 	"syscall"
 	"testing"
@@ -24,6 +25,7 @@ import (
 	exp "github.com/kata-containers/runtime/virtcontainers/experimental"
 	"github.com/kata-containers/runtime/virtcontainers/persist/fs"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
+	"github.com/kata-containers/runtime/virtcontainers/store"
 	"github.com/kata-containers/runtime/virtcontainers/types"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
@@ -1554,3 +1556,40 @@ func TestGetSandboxCpuSet(t *testing.T) {
 		})
 	}
 }
+
+func TestSandboxStoreClean(t *testing.T) {
+	ctx := context.Background()
+	contID := "SandboxStore"
+	contConfig := newTestContainerConfigNoop(contID)
+	hConfig := newHypervisorConfig(nil, nil)
+	assert := assert.New(t)
+
+	// create a sandbox
+	p, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NoopAgentType, NetworkConfig{}, []ContainerConfig{contConfig}, nil)
+	assert.NoError(err)
+	defer cleanUp()
+
+	l := len(p.GetAllContainers())
+	assert.Equal(l, 1)
+
+	// persist to disk
+	err = p.storeSandbox()
+	assert.NoError(err)
+
+	loadSandboxConfigFromOldStore(ctx, p.ID())
+
+	runtimeSidPath := store.SandboxConfigurationRoot(p.ID())
+	runtimeSidPath = strings.TrimPrefix(runtimeSidPath, "file://")
+
+	p.store, err = store.NewVCSandboxStore(ctx, testSandboxID)
+	assert.Nil(err)
+
+	p.ctx = context.WithValue(ctx, oldstoreKey, true)
+	err = p.Delete()
+	assert.NoError(err)
+
+	// expect runtimeSidPath not exist, if exist, it means this case failed.
+	_, err = os.Stat(runtimeSidPath)
+	assert.Error(err)
+	assert.True(os.IsNotExist(err))
+}