persist: remove VCStore from sandbox/apis

Remove VCStore usage from sandbox.

Signed-off-by: Wei Zhang <weizhang555@gmail.com>

virtcontainers/acrn_test.go

@@ -218,11 +218,7 @@ func TestAcrnCreateSandbox(t *testing.T) {
 		},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
+	err := globalSandboxList.addSandbox(sandbox)
-	assert.NoError(err)
-	sandbox.store = vcStore
-
-	err = globalSandboxList.addSandbox(sandbox)
 	assert.NoError(err)
 
 	defer globalSandboxList.removeSandbox(sandbox.id)

virtcontainers/container.go

@@ -968,10 +968,8 @@ func (c *Container) stop(force bool) error {
 	defer func() {
 		// Save device and drive data.
 		// TODO: can we merge this saving with setContainerState()?
-		if c.sandbox.supportNewStore() {
+		if err := c.sandbox.Save(); err != nil {
-			if err := c.sandbox.Save(); err != nil {
+			c.Logger().WithError(err).Info("save container state failed")
-				c.Logger().WithError(err).Info("save container state failed")
-			}
 		}
 	}()
 

virtcontainers/container_test.go

@@ -92,18 +92,13 @@ func TestContainerRemoveDrive(t *testing.T) {
 		config:     &SandboxConfig{},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.Nil(t, err)
-
-	sandbox.store = vcStore
-
 	container := Container{
 		sandbox: sandbox,
 		id:      "testContainer",
 	}
 
 	container.state.Fstype = ""
-	err = container.removeDrive()
+	err := container.removeDrive()
 
 	// hotplugRemoveDevice for hypervisor should not be called.
 	// test should pass without a hypervisor created for the container's sandbox.
@@ -124,8 +119,6 @@ func TestContainerRemoveDrive(t *testing.T) {
 	assert.True(t, ok)
 	err = device.Attach(devReceiver)
 	assert.Nil(t, err)
-	err = sandbox.storeSandboxDevices()
-	assert.Nil(t, err)
 
 	container.state.Fstype = "xfs"
 	container.state.BlockDeviceID = device.DeviceID()
@@ -324,16 +317,12 @@ func TestContainerAddDriveDir(t *testing.T) {
 		},
 	}
 
-	defer store.DeleteAll()
-
-	sandboxStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.Nil(err)
-	sandbox.store = sandboxStore
-
 	sandbox.newStore, err = persist.GetDriver("fs")
 	assert.NoError(err)
 	assert.NotNil(sandbox.newStore)
 
+	defer sandbox.newStore.Destroy(sandbox.id)
+
 	contID := "100"
 	container := Container{
 		sandbox: sandbox,
@@ -384,9 +373,7 @@ func TestContainerRootfsPath(t *testing.T) {
 			},
 		},
 	}
-	vcstore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
+
-	sandbox.store = vcstore
-	assert.Nil(t, err)
 	container := Container{
 		id:           "rootfstestcontainerid",
 		sandbox:      sandbox,

virtcontainers/kata_agent.go

@@ -317,13 +317,6 @@ func (k *kataAgent) init(ctx context.Context, sandbox *Sandbox, config interface
 
 	k.proxyBuiltIn = isProxyBuiltIn(sandbox.config.ProxyType)
 
-	// Fetch agent runtime info.
-	if !sandbox.supportNewStore() {
-		if err := sandbox.store.Load(store.Agent, &k.state); err != nil {
-			k.Logger().Debug("Could not retrieve anything from storage")
-		}
-	}
-
 	return disableVMShutdown, nil
 }
 
@@ -730,12 +723,6 @@ func (k *kataAgent) setProxy(sandbox *Sandbox, proxy proxy, pid int, url string)
 	k.proxy = proxy
 	k.state.ProxyPid = pid
 	k.state.URL = url
-	if sandbox != nil && !sandbox.supportNewStore() {
-		if err := sandbox.store.Store(store.Agent, k.state); err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
 
@@ -952,13 +939,6 @@ func (k *kataAgent) stopSandbox(sandbox *Sandbox) error {
 	// clean up agent state
 	k.state.ProxyPid = -1
 	k.state.URL = ""
-	if !sandbox.supportNewStore() {
-		if err := sandbox.store.Store(store.Agent, k.state); err != nil {
-			// ignore error
-			k.Logger().WithError(err).WithField("sandbox", sandbox.id).Error("failed to clean up agent state")
-		}
-	}
-
 	return nil
 }
 

virtcontainers/kata_agent_test.go

@@ -31,9 +31,9 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
 	"github.com/kata-containers/runtime/virtcontainers/device/drivers"
 	"github.com/kata-containers/runtime/virtcontainers/device/manager"
+	"github.com/kata-containers/runtime/virtcontainers/persist"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/mock"
 	vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types"
-	"github.com/kata-containers/runtime/virtcontainers/store"
 	"github.com/kata-containers/runtime/virtcontainers/types"
 )
 
@@ -714,10 +714,10 @@ func TestAgentCreateContainer(t *testing.T) {
 		hypervisor: &mockHypervisor{},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
+	newStore, err := persist.GetDriver("fs")
-	assert.Nil(err)
+	assert.NoError(err)
-
+	assert.NotNil(newStore)
-	sandbox.store = vcStore
+	sandbox.newStore = newStore
 
 	container := &Container{
 		ctx:       sandbox.ctx,
@@ -815,12 +815,7 @@ func TestKataAgentSetProxy(t *testing.T) {
 		id:  "foobar",
 	}
 
-	vcStore, err := store.NewVCSandboxStore(s.ctx, s.id)
+	err := k.setProxy(s, p, 0, "")
-	assert.Nil(err)
-
-	s.store = vcStore
-
-	err = k.setProxy(s, p, 0, "")
 	assert.Error(err)
 }
 

virtcontainers/persist.go

@@ -443,10 +443,6 @@ func (c *Container) Restore() error {
 	return nil
 }
 
-func (s *Sandbox) supportNewStore() bool {
-	return true
-}
-
 func loadSandboxConfig(id string) (*SandboxConfig, error) {
 	store, err := persist.GetDriver("fs")
 	if err != nil || store == nil {

virtcontainers/qemu_test.go

@@ -16,6 +16,7 @@ import (
 
 	govmmQemu "github.com/intel/govmm/qemu"
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
+	"github.com/kata-containers/runtime/virtcontainers/persist"
 	"github.com/kata-containers/runtime/virtcontainers/store"
 	"github.com/kata-containers/runtime/virtcontainers/types"
 	"github.com/pkg/errors"
@@ -85,18 +86,13 @@ func TestQemuCreateSandbox(t *testing.T) {
 		},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.NoError(err)
-
-	sandbox.store = vcStore
-
 	// Create the hypervisor fake binary
 	testQemuPath := filepath.Join(testDir, testHypervisor)
-	_, err = os.Create(testQemuPath)
+	_, err := os.Create(testQemuPath)
 	assert.NoError(err)
 
 	// Create parent dir path for hypervisor.json
-	parentDir := store.SandboxConfigurationRootPath(sandbox.id)
+	parentDir := store.SandboxRuntimeRootPath(sandbox.id)
 	assert.NoError(os.MkdirAll(parentDir, store.DirMode))
 
 	err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, false)
@@ -118,17 +114,13 @@ func TestQemuCreateSandboxMissingParentDirFail(t *testing.T) {
 		},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.NoError(err)
-	sandbox.store = vcStore
-
 	// Create the hypervisor fake binary
 	testQemuPath := filepath.Join(testDir, testHypervisor)
-	_, err = os.Create(testQemuPath)
+	_, err := os.Create(testQemuPath)
 	assert.NoError(err)
 
 	// Ensure parent dir path for hypervisor.json does not exist.
-	parentDir := store.SandboxConfigurationRootPath(sandbox.id)
+	parentDir := store.SandboxRuntimeRootPath(sandbox.id)
 	assert.NoError(os.RemoveAll(parentDir))
 
 	err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, false)
@@ -470,11 +462,11 @@ func createQemuSandboxConfig() (*Sandbox, error) {
 		},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
+	newStore, err := persist.GetDriver("fs")
 	if err != nil {
 		return &Sandbox{}, err
 	}
-	sandbox.store = vcStore
+	sandbox.newStore = newStore
 
 	return &sandbox, nil
 }

virtcontainers/sandbox.go

@@ -175,9 +175,7 @@ type Sandbox struct {
 	factory    Factory
 	hypervisor hypervisor
 	agent      agent
-	store      *store.VCStore
+	newStore   persistapi.PersistDriver
-	// store is used to replace VCStore step by step
-	newStore persistapi.PersistDriver
 
 	network Network
 	monitor *monitor
@@ -244,10 +242,6 @@ func (s *Sandbox) SetAnnotations(annotations map[string]string) error {
 	for k, v := range annotations {
 		s.config.Annotations[k] = v
 	}
-
-	if !s.supportNewStore() {
-		return s.store.Store(store.Configuration, *(s.config))
-	}
 	return nil
 }
 
@@ -454,12 +448,6 @@ func (s *Sandbox) getAndStoreGuestDetails() error {
 			s.seccompSupported = guestDetailRes.AgentDetails.SupportsSeccomp
 		}
 		s.state.GuestMemoryHotplugProbe = guestDetailRes.SupportMemHotplugProbe
-
-		if !s.supportNewStore() {
-			if err = s.store.Store(store.State, s.state); err != nil {
-				return err
-			}
-		}
 	}
 
 	return nil
@@ -543,15 +531,8 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 		ctx:             ctx,
 	}
 
-	vcStore, err := store.NewVCSandboxStore(ctx, s.id)
-	if err != nil {
-		return nil, err
-	}
-
-	s.store = vcStore
-
 	if s.newStore, err = persist.GetDriver("fs"); err != nil || s.newStore == nil {
-		return nil, fmt.Errorf("failed to get fs persist driver")
+		return nil, fmt.Errorf("failed to get fs persist driver: %v", err)
 	}
 
 	if err = globalSandboxList.addSandbox(s); err != nil {
@@ -562,7 +543,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 		if err != nil {
 			s.Logger().WithError(err).WithField("sandboxid", s.id).Error("Create new sandbox failed")
 			globalSandboxList.removeSandbox(s.id)
-			s.store.Delete()
+			s.newStore.Destroy(s.id)
 		}
 	}()
 
@@ -588,10 +569,6 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 	return s, nil
 }
 
-func (s *Sandbox) storeSandboxDevices() error {
-	return s.store.StoreDevices(s.devManager.GetAllDevices())
-}
-
 // storeSandbox stores a sandbox config.
 func (s *Sandbox) storeSandbox() error {
 	span, _ := s.trace("storeSandbox")
@@ -622,10 +599,6 @@ func rwLockSandbox(sandboxID string) (func() error, error) {
 	return store.Lock(sandboxID, true)
 }
 
-func supportNewStore(ctx context.Context) bool {
-	return true
-}
-
 // fetchSandbox fetches a sandbox config from a sandbox ID and returns a sandbox.
 func fetchSandbox(ctx context.Context, sandboxID string) (sandbox *Sandbox, err error) {
 	virtLog.Info("fetch sandbox")
@@ -640,23 +613,11 @@ func fetchSandbox(ctx context.Context, sandboxID string) (sandbox *Sandbox, err
 
 	var config SandboxConfig
 
-	if supportNewStore(ctx) {
+	c, err := loadSandboxConfig(sandboxID)
-		c, err := loadSandboxConfig(sandboxID)
+	if err != nil {
-		if err != nil {
+		return nil, err
-			return nil, err
-		}
-		config = *c
-	} else {
-		// We're bootstrapping
-		vcStore, err := store.NewVCSandboxStore(ctx, sandboxID)
-		if err != nil {
-			return nil, err
-		}
-
-		if err := vcStore.Load(store.Configuration, &config); err != nil {
-			return nil, err
-		}
 	}
+	config = *c
 
 	// fetchSandbox is not suppose to create new sandbox VM.
 	sandbox, err = createSandbox(ctx, config, nil)
@@ -746,13 +707,7 @@ func (s *Sandbox) Delete() error {
 
 	s.agent.cleanup(s)
 
-	if s.supportNewStore() {
+	return s.newStore.Destroy(s.id)
-		if err := s.newStore.Destroy(s.id); err != nil {
-			return err
-		}
-	}
-
-	return s.store.Delete()
 }
 
 func (s *Sandbox) startNetworkMonitor() error {
@@ -820,11 +775,6 @@ func (s *Sandbox) createNetwork() error {
 			}
 		}
 	}
-
-	// Store the network
-	if !s.supportNewStore() {
-		return s.store.Store(store.Network, s.networkNS)
-	}
 	return nil
 }
 
@@ -898,14 +848,8 @@ func (s *Sandbox) AddInterface(inf *vcTypes.Interface) (*vcTypes.Interface, erro
 
 	// Update the sandbox storage
 	s.networkNS.Endpoints = append(s.networkNS.Endpoints, endpoint)
-	if s.supportNewStore() {
+	if err := s.Save(); err != nil {
-		if err := s.Save(); err != nil {
+		return nil, err
-			return nil, err
-		}
-	} else {
-		if err := s.store.Store(store.Network, s.networkNS); err != nil {
-			return nil, err
-		}
 	}
 
 	// Add network for vm
@@ -923,14 +867,8 @@ func (s *Sandbox) RemoveInterface(inf *vcTypes.Interface) (*vcTypes.Interface, e
 			}
 			s.networkNS.Endpoints = append(s.networkNS.Endpoints[:i], s.networkNS.Endpoints[i+1:]...)
 
-			if s.supportNewStore() {
+			if err := s.Save(); err != nil {
-				if err := s.Save(); err != nil {
+				return inf, err
-					return inf, err
-				}
-			} else {
-				if err := s.store.Store(store.Network, s.networkNS); err != nil {
-					return inf, err
-				}
 			}
 
 			break
@@ -1004,12 +942,6 @@ func (s *Sandbox) startVM() (err error) {
 				return err
 			}
 		}
-
-		if !s.supportNewStore() {
-			if err := s.store.Store(store.Network, s.networkNS); err != nil {
-				return err
-			}
-		}
 	}
 
 	s.Logger().Info("VM started")
@@ -1316,9 +1248,6 @@ func (s *Sandbox) UpdateContainer(containerID string, resources specs.LinuxResou
 		return err
 	}
 
-	if err := c.storeContainer(); err != nil {
-		return err
-	}
 	if err = s.storeSandbox(); err != nil {
 		return err
 	}
@@ -1550,11 +1479,6 @@ func (s *Sandbox) setSandboxState(state types.StateString) error {
 
 	// update in-memory state
 	s.state.State = state
-
-	// update on-disk state
-	if !s.supportNewStore() {
-		return s.store.Store(store.State, s.state)
-	}
 	return nil
 }
 
@@ -1573,14 +1497,6 @@ func (s *Sandbox) getAndSetSandboxBlockIndex() (int, error) {
 	// Increment so that container gets incremented block index
 	s.state.BlockIndex++
 
-	if !s.supportNewStore() {
-		// experimental runtime use "persist.json" which doesn't need "state.json" anymore
-		// update on-disk state
-		if err := s.store.Store(store.State, s.state); err != nil {
-			return -1, err
-		}
-	}
-
 	return currentIndex, nil
 }
 
@@ -1596,14 +1512,6 @@ func (s *Sandbox) decrementSandboxBlockIndex() error {
 		}
 	}()
 
-	if !s.supportNewStore() {
-		// experimental runtime use "persist.json" which doesn't need "state.json" anymore
-		// update on-disk state
-		if err = s.store.Store(store.State, s.state); err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
 
@@ -1733,12 +1641,6 @@ func (s *Sandbox) AddDevice(info config.DeviceInfo) (api.Device, error) {
 		}
 	}()
 
-	if !s.supportNewStore() {
-		if err = s.storeSandboxDevices(); err != nil {
-			return nil, err
-		}
-	}
-
 	return b, nil
 }
 

virtcontainers/sandbox_test.go

@@ -574,11 +574,6 @@ func TestSetAnnotations(t *testing.T) {
 		},
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.NoError(err)
-
-	sandbox.store = vcStore
-
 	keyAnnotation := "annotation2"
 	valueAnnotation := "xyz"
 	newAnnotations := map[string]string{
@@ -657,10 +652,6 @@ func TestContainerStateSetFstype(t *testing.T) {
 	assert.Nil(err)
 	defer cleanUp()
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.Nil(err)
-	sandbox.store = vcStore
-
 	c := sandbox.GetContainer("100")
 	assert.NotNil(c)
 
@@ -737,14 +728,8 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) {
 		config:     &SandboxConfig{},
 	}
 
-	store, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.Nil(t, err)
-	sandbox.store = store
-
 	containers[c.id].sandbox = &sandbox
 
-	err = sandbox.storeSandboxDevices()
-	assert.Nil(t, err, "Error while store sandbox devices %s", err)
 	err = containers[c.id].attachDevices()
 	assert.Nil(t, err, "Error while attaching devices %s", err)
 
@@ -1005,7 +990,7 @@ func TestDeleteStoreWhenNewContainerFail(t *testing.T) {
 	}
 	_, err = newContainer(p, &contConfig)
 	assert.NotNil(t, err, "New container with invalid device info should fail")
-	storePath := store.ContainerConfigurationRootPath(testSandboxID, contID)
+	storePath := store.ContainerRuntimeRootPath(testSandboxID, contID)
 	_, err = os.Stat(storePath)
 	assert.NotNil(t, err, "Should delete configuration root after failed to create a container")
 }
@@ -1168,10 +1153,6 @@ func TestAttachBlockDevice(t *testing.T) {
 		ctx:        context.Background(),
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.Nil(t, err)
-	sandbox.store = vcStore
-
 	contID := "100"
 	container := Container{
 		sandbox: sandbox,
@@ -1180,18 +1161,11 @@ func TestAttachBlockDevice(t *testing.T) {
 
 	// create state file
 	path := store.ContainerRuntimeRootPath(testSandboxID, container.ID())
-	err = os.MkdirAll(path, store.DirMode)
+	err := os.MkdirAll(path, store.DirMode)
 	assert.NoError(t, err)
 
 	defer os.RemoveAll(path)
 
-	stateFilePath := filepath.Join(path, store.StateFile)
-	os.Remove(stateFilePath)
-
-	_, err = os.Create(stateFilePath)
-	assert.NoError(t, err)
-	defer os.Remove(stateFilePath)
-
 	path = "/dev/hda"
 	deviceInfo := config.DeviceInfo{
 		HostPath:      path,
@@ -1255,10 +1229,6 @@ func TestPreAddDevice(t *testing.T) {
 		ctx:        context.Background(),
 	}
 
-	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)
-	assert.Nil(t, err)
-	sandbox.store = vcStore
-
 	contID := "100"
 	container := Container{
 		sandbox:   sandbox,
@@ -1269,18 +1239,11 @@ func TestPreAddDevice(t *testing.T) {
 
 	// create state file
 	path := store.ContainerRuntimeRootPath(testSandboxID, container.ID())
-	err = os.MkdirAll(path, store.DirMode)
+	err := os.MkdirAll(path, store.DirMode)
 	assert.NoError(t, err)
 
 	defer os.RemoveAll(path)
 
-	stateFilePath := filepath.Join(path, store.StateFile)
-	os.Remove(stateFilePath)
-
-	_, err = os.Create(stateFilePath)
-	assert.NoError(t, err)
-	defer os.Remove(stateFilePath)
-
 	path = "/dev/hda"
 	deviceInfo := config.DeviceInfo{
 		HostPath:      path,