From 01b4a64be2130c21091056fdeedf28fdf32a3cc9 Mon Sep 17 00:00:00 2001 From: Wei Zhang Date: Tue, 26 Nov 2019 22:09:03 +0800 Subject: [PATCH] persist: remove VCStore from sandbox/apis Remove VCStore usage from sandbox. Signed-off-by: Wei Zhang --- virtcontainers/acrn_test.go | 6 +- virtcontainers/container.go | 6 +- virtcontainers/container_test.go | 21 +---- virtcontainers/kata_agent.go | 20 ----- virtcontainers/kata_agent_test.go | 17 ++--- virtcontainers/persist.go | 4 - virtcontainers/qemu_test.go | 22 ++---- virtcontainers/sandbox.go | 122 +++--------------------------- virtcontainers/sandbox_test.go | 43 +---------- 9 files changed, 35 insertions(+), 226 deletions(-) diff --git a/virtcontainers/acrn_test.go b/virtcontainers/acrn_test.go index a0dde20480..5c0e563b4c 100644 --- a/virtcontainers/acrn_test.go +++ b/virtcontainers/acrn_test.go @@ -218,11 +218,7 @@ func TestAcrnCreateSandbox(t *testing.T) { }, } - vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id) - assert.NoError(err) - sandbox.store = vcStore - - err = globalSandboxList.addSandbox(sandbox) + err := globalSandboxList.addSandbox(sandbox) assert.NoError(err) defer globalSandboxList.removeSandbox(sandbox.id) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index f9fe271d0a..45b94be0e3 100644 --- a/virtcontainers/container.go +++ b/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 { - c.Logger().WithError(err).Info("save container state failed") - } + if err := c.sandbox.Save(); err != nil { + c.Logger().WithError(err).Info("save container state failed") } }() diff --git a/virtcontainers/container_test.go b/virtcontainers/container_test.go index 2ace0a7266..d4fea83d5d 100644 --- a/virtcontainers/container_test.go +++ b/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, diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index a08d8325e8..c7fb9102f7 100644 --- a/virtcontainers/kata_agent.go +++ b/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 } diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 7f5e99d4e0..5dc880c745 100644 --- a/virtcontainers/kata_agent_test.go +++ b/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) - assert.Nil(err) - - sandbox.store = vcStore + newStore, err := persist.GetDriver("fs") + assert.NoError(err) + assert.NotNil(newStore) + 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) - assert.Nil(err) - - s.store = vcStore - - err = k.setProxy(s, p, 0, "") + err := k.setProxy(s, p, 0, "") assert.Error(err) } diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index c3ed4f9a0e..95cdb5df65 100644 --- a/virtcontainers/persist.go +++ b/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 { diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index fd90c5cb33..1afb744a77 100644 --- a/virtcontainers/qemu_test.go +++ b/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 } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 570dae6f97..4bf81bb0a1 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -175,9 +175,7 @@ type Sandbox struct { factory Factory hypervisor hypervisor agent agent - store *store.VCStore - // store is used to replace VCStore step by step - newStore persistapi.PersistDriver + 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) - if err != nil { - 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 - } + c, err := loadSandboxConfig(sandboxID) + if 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() { - if err := s.newStore.Destroy(s.id); err != nil { - return err - } - } - - return s.store.Delete() + return s.newStore.Destroy(s.id) } 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 { - return nil, err - } - } else { - if err := s.store.Store(store.Network, s.networkNS); err != nil { - return nil, err - } + if err := s.Save(); 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 { - return inf, err - } - } else { - if err := s.store.Store(store.Network, s.networkNS); err != nil { - return inf, err - } + if err := s.Save(); 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 } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 40cb284f4e..af8a40dda0 100644 --- a/virtcontainers/sandbox_test.go +++ b/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,