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 b2aa32833..8e0f0e86f 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 880a8df66..b839c0d91 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 114a4e3b0..4f22ed26d 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 78c200bc9..ec4daa032 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 e1b1c1b42..a4743fe4f 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 be6dd1caf..3e8d0b95e 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 bc7d22504..33274542c 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 002081a6a..a9627e157 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") }