From d75fe956852133cc2b418188f552a8f90c751a1b Mon Sep 17 00:00:00 2001 From: bin Date: Thu, 8 Apr 2021 20:17:52 +0800 Subject: [PATCH] virtcontainers: replace newStore by store in Sandbox struct The property name make newcomers confused when reading code. Since in Kata Containers 2.0 there will only be one type of store, so it's safe to replace it by `store` simply. Fixes: #1660 Signed-off-by: bin --- src/runtime/virtcontainers/api_test.go | 8 ++++---- src/runtime/virtcontainers/container_test.go | 6 +++--- src/runtime/virtcontainers/kata_agent_test.go | 6 +++--- src/runtime/virtcontainers/persist.go | 6 +++--- src/runtime/virtcontainers/persist_test.go | 6 +++--- src/runtime/virtcontainers/qemu_test.go | 16 +++++++-------- src/runtime/virtcontainers/sandbox.go | 20 +++++++++---------- src/runtime/virtcontainers/sandbox_test.go | 2 +- 8 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/runtime/virtcontainers/api_test.go b/src/runtime/virtcontainers/api_test.go index b2aa32833f..8e0f0e86f2 100644 --- a/src/runtime/virtcontainers/api_test.go +++ b/src/runtime/virtcontainers/api_test.go @@ -144,7 +144,7 @@ func TestCreateSandboxNoopAgentSuccessful(t *testing.T) { assert.True(ok) assert.NotNil(s) - sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) + sandboxDir := filepath.Join(s.store.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) } @@ -175,7 +175,7 @@ func TestCreateSandboxKataAgentSuccessful(t *testing.T) { s, ok := p.(*Sandbox) assert.True(ok) - sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) + sandboxDir := filepath.Join(s.store.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) } @@ -224,7 +224,7 @@ func createAndStartSandbox(ctx context.Context, config SandboxConfig) (sandbox V if !ok { return nil, "", fmt.Errorf("Could not get Sandbox") } - sandboxDir = filepath.Join(s.newStore.RunStoragePath(), sandbox.ID()) + sandboxDir = filepath.Join(s.store.RunStoragePath(), sandbox.ID()) _, err = os.Stat(sandboxDir) if err != nil { return nil, "", err @@ -285,7 +285,7 @@ func TestCleanupContainer(t *testing.T) { s, ok := p.(*Sandbox) assert.True(ok) - sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) + sandboxDir := filepath.Join(s.store.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) if err == nil { diff --git a/src/runtime/virtcontainers/container_test.go b/src/runtime/virtcontainers/container_test.go index 880a8df66e..b839c0d911 100644 --- a/src/runtime/virtcontainers/container_test.go +++ b/src/runtime/virtcontainers/container_test.go @@ -316,11 +316,11 @@ func TestContainerAddDriveDir(t *testing.T) { }, } - sandbox.newStore, err = persist.GetDriver() + sandbox.store, err = persist.GetDriver() assert.NoError(err) - assert.NotNil(sandbox.newStore) + assert.NotNil(sandbox.store) - defer sandbox.newStore.Destroy(sandbox.id) + defer sandbox.store.Destroy(sandbox.id) contID := "100" container := Container{ diff --git a/src/runtime/virtcontainers/kata_agent_test.go b/src/runtime/virtcontainers/kata_agent_test.go index 114a4e3b0f..4f22ed26d0 100644 --- a/src/runtime/virtcontainers/kata_agent_test.go +++ b/src/runtime/virtcontainers/kata_agent_test.go @@ -831,10 +831,10 @@ func TestAgentCreateContainer(t *testing.T) { hypervisor: &mockHypervisor{}, } - newStore, err := persist.GetDriver() + store, err := persist.GetDriver() assert.NoError(err) - assert.NotNil(newStore) - sandbox.newStore = newStore + assert.NotNil(store) + sandbox.store = store container := &Container{ ctx: sandbox.ctx, diff --git a/src/runtime/virtcontainers/persist.go b/src/runtime/virtcontainers/persist.go index 78c200bc99..ec4daa0325 100644 --- a/src/runtime/virtcontainers/persist.go +++ b/src/runtime/virtcontainers/persist.go @@ -287,7 +287,7 @@ func (s *Sandbox) Save() error { s.dumpNetwork(&ss) s.dumpConfig(&ss) - if err := s.newStore.ToDisk(ss, cs); err != nil { + if err := s.store.ToDisk(ss, cs); err != nil { return err } @@ -397,7 +397,7 @@ func (s *Sandbox) loadNetwork(netInfo persistapi.NetworkInfo) { // Restore will restore sandbox data from persist file on disk func (s *Sandbox) Restore() error { - ss, _, err := s.newStore.FromDisk(s.id) + ss, _, err := s.store.FromDisk(s.id) if err != nil { return err } @@ -412,7 +412,7 @@ func (s *Sandbox) Restore() error { // Restore will restore container data from persist file on disk func (c *Container) Restore() error { - _, css, err := c.sandbox.newStore.FromDisk(c.sandbox.id) + _, css, err := c.sandbox.store.FromDisk(c.sandbox.id) if err != nil { return err } diff --git a/src/runtime/virtcontainers/persist_test.go b/src/runtime/virtcontainers/persist_test.go index e1b1c1b421..a4743fe4f5 100644 --- a/src/runtime/virtcontainers/persist_test.go +++ b/src/runtime/virtcontainers/persist_test.go @@ -38,9 +38,9 @@ func TestSandboxRestore(t *testing.T) { state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } - sandbox.newStore, err = persist.GetDriver() + sandbox.store, err = persist.GetDriver() assert.NoError(err) - assert.NotNil(sandbox.newStore) + assert.NotNil(sandbox.store) // if we don't call Save(), we can get nothing from disk err = sandbox.Restore() @@ -67,7 +67,7 @@ func TestSandboxRestore(t *testing.T) { // empty the sandbox sandbox.state = types.SandboxState{} - if sandbox.newStore, err = persist.GetDriver(); err != nil || sandbox.newStore == nil { + if sandbox.store, err = persist.GetDriver(); err != nil || sandbox.store == nil { t.Fatal("failed to get persist driver") } diff --git a/src/runtime/virtcontainers/qemu_test.go b/src/runtime/virtcontainers/qemu_test.go index be6dd1caf3..3e8d0b95e5 100644 --- a/src/runtime/virtcontainers/qemu_test.go +++ b/src/runtime/virtcontainers/qemu_test.go @@ -194,7 +194,7 @@ func TestQemuKnobs(t *testing.T) { assert.NoError(err) q := &qemu{ - store: sandbox.newStore, + store: sandbox.store, } err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig) assert.NoError(err) @@ -462,7 +462,7 @@ func TestQemuFileBackedMem(t *testing.T) { assert.NoError(err) q := &qemu{ - store: sandbox.newStore, + store: sandbox.store, } sandbox.config.HypervisorConfig.SharedFS = config.VirtioFS err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig) @@ -477,7 +477,7 @@ func TestQemuFileBackedMem(t *testing.T) { assert.NoError(err) q = &qemu{ - store: sandbox.newStore, + store: sandbox.store, } sandbox.config.HypervisorConfig.BootToBeTemplate = true sandbox.config.HypervisorConfig.SharedFS = config.VirtioFS @@ -493,7 +493,7 @@ func TestQemuFileBackedMem(t *testing.T) { assert.NoError(err) q = &qemu{ - store: sandbox.newStore, + store: sandbox.store, } sandbox.config.HypervisorConfig.FileBackedMemRootDir = "/tmp/xyzabc" err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig) @@ -507,7 +507,7 @@ func TestQemuFileBackedMem(t *testing.T) { assert.NoError(err) q = &qemu{ - store: sandbox.newStore, + store: sandbox.store, } sandbox.config.HypervisorConfig.EnableVhostUserStore = true sandbox.config.HypervisorConfig.HugePages = true @@ -520,7 +520,7 @@ func TestQemuFileBackedMem(t *testing.T) { assert.NoError(err) q = &qemu{ - store: sandbox.newStore, + store: sandbox.store, } sandbox.config.HypervisorConfig.EnableVhostUserStore = true sandbox.config.HypervisorConfig.HugePages = false @@ -541,11 +541,11 @@ func createQemuSandboxConfig() (*Sandbox, error) { }, } - newStore, err := persist.GetDriver() + store, err := persist.GetDriver() if err != nil { return &Sandbox{}, err } - sandbox.newStore = newStore + sandbox.store = store return &sandbox, nil } diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index bc7d225042..33274542ca 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -168,7 +168,7 @@ type Sandbox struct { factory Factory hypervisor hypervisor agent agent - newStore persistapi.PersistDriver + store persistapi.PersistDriver network Network monitor *monitor @@ -282,7 +282,7 @@ func (s *Sandbox) GetContainer(containerID string) VCContainer { return nil } -// Release closes the agent connection and removes sandbox from internal list. +// Release closes the agent connection. func (s *Sandbox) Release(ctx context.Context) error { s.Logger().Info("release sandbox") if s.monitor != nil { @@ -293,7 +293,6 @@ func (s *Sandbox) Release(ctx context.Context) error { } // Status gets the status of the sandbox -// TODO: update container status properly, see kata-containers/runtime#253 func (s *Sandbox) Status() SandboxStatus { var contStatusList []ContainerStatus for _, c := range s.containers { @@ -491,8 +490,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor } // create agent instance - newAagentFunc := getNewAgentFunc(ctx) - agent := newAagentFunc() + agent := getNewAgentFunc(ctx)() hypervisor, err := newHypervisor(sandboxConfig.HypervisorType) if err != nil { @@ -518,14 +516,14 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor hypervisor.setSandbox(s) - if s.newStore, err = persist.GetDriver(); err != nil || s.newStore == nil { + if s.store, err = persist.GetDriver(); err != nil || s.store == nil { return nil, fmt.Errorf("failed to get fs persist driver: %v", err) } defer func() { if retErr != nil { - s.Logger().WithError(retErr).WithField("sandboxid", s.id).Error("Create new sandbox failed") - s.newStore.Destroy(s.id) + s.Logger().WithError(retErr).Error("Create new sandbox failed") + s.store.Destroy(s.id) } }() @@ -543,7 +541,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor s.Logger().WithError(err).Debug("restore sandbox failed") } - // new store doesn't require hypervisor to be stored immediately + // store doesn't require hypervisor to be stored immediately if err = s.hypervisor.createSandbox(ctx, s.id, s.networkNS, &sandboxConfig.HypervisorConfig); err != nil { return nil, err } @@ -708,7 +706,7 @@ func (s *Sandbox) Delete(ctx context.Context) error { s.agent.cleanup(ctx, s) - return s.newStore.Destroy(s.id) + return s.store.Destroy(s.id) } func (s *Sandbox) startNetworkMonitor(ctx context.Context) error { @@ -2283,7 +2281,7 @@ func fetchSandbox(ctx context.Context, sandboxID string) (sandbox *Sandbox, err // load sandbox config fromld store. c, err := loadSandboxConfig(sandboxID) if err != nil { - virtLog.WithError(err).Warning("failed to get sandbox config from new store") + virtLog.WithError(err).Warning("failed to get sandbox config from store") return nil, err } diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 002081a6a8..a9627e1574 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -1009,7 +1009,7 @@ func TestDeleteStoreWhenNewContainerFail(t *testing.T) { } _, err = newContainer(context.Background(), p, &contConfig) assert.NotNil(t, err, "New container with invalid device info should fail") - storePath := filepath.Join(p.newStore.RunStoragePath(), testSandboxID, contID) + storePath := filepath.Join(p.store.RunStoragePath(), testSandboxID, contID) _, err = os.Stat(storePath) assert.NotNil(t, err, "Should delete configuration root after failed to create a container") }