From 290203943c7f975101c0401f4d78f32a15a0952c Mon Sep 17 00:00:00 2001 From: bin liu Date: Wed, 4 Nov 2020 14:29:24 +0800 Subject: [PATCH] runtime: delete sandboxlist.go and sandboxlist_test.go Delete sandboxlist.go and sandboxlist_test.go under virtcontainers package. Fixes: #1078 Signed-off-by: bin liu --- src/runtime/virtcontainers/acrn.go | 7 +- src/runtime/virtcontainers/acrn_test.go | 8 +- src/runtime/virtcontainers/api.go | 5 +- src/runtime/virtcontainers/api_test.go | 2 +- src/runtime/virtcontainers/persist.go | 116 ------------------ src/runtime/virtcontainers/sandbox.go | 84 +------------ src/runtime/virtcontainers/sandbox_test.go | 6 +- src/runtime/virtcontainers/sandboxlist.go | 49 -------- .../virtcontainers/sandboxlist_test.go | 31 ----- .../virtcontainers/virtcontainers_test.go | 2 +- 10 files changed, 15 insertions(+), 295 deletions(-) delete mode 100644 src/runtime/virtcontainers/sandboxlist.go delete mode 100644 src/runtime/virtcontainers/sandboxlist_test.go diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index 4854ac8f2f..5b6010d211 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -231,10 +231,9 @@ func (a *Acrn) appendImage(devices []Device, imagePath string) ([]Device, error) // Get sandbox and increment the globalIndex. // This is to make sure the VM rootfs occupies // the first Index which is /dev/vda. - sandbox, err := globalSandboxList.lookupSandbox(a.id) - if sandbox == nil && err != nil { - return nil, err - } + sandbox := globalSandbox + var err error + if _, err = sandbox.GetAndSetSandboxBlockIndex(); err != nil { return nil, err } diff --git a/src/runtime/virtcontainers/acrn_test.go b/src/runtime/virtcontainers/acrn_test.go index 89a035dce9..27dbd088e7 100644 --- a/src/runtime/virtcontainers/acrn_test.go +++ b/src/runtime/virtcontainers/acrn_test.go @@ -230,10 +230,10 @@ func TestAcrnCreateSandbox(t *testing.T) { state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } - err = globalSandboxList.addSandbox(sandbox) - assert.NoError(err) - - defer globalSandboxList.removeSandbox(sandbox.id) + globalSandbox = sandbox + defer func() { + globalSandbox = nil + }() //set PID to 1 to ignore hypercall to get UUID and set a random UUID a.state.PID = 1 diff --git a/src/runtime/virtcontainers/api.go b/src/runtime/virtcontainers/api.go index 9efdff3c72..0335059f4d 100644 --- a/src/runtime/virtcontainers/api.go +++ b/src/runtime/virtcontainers/api.go @@ -153,10 +153,7 @@ func CleanupContainer(ctx context.Context, sandboxID, containerID string, force } defer unlock() - s, err := fetchSandbox(ctx, sandboxID) - if err != nil { - return err - } + s := globalSandbox defer s.Release() diff --git a/src/runtime/virtcontainers/api_test.go b/src/runtime/virtcontainers/api_test.go index 7c7fadae54..927e450c8d 100644 --- a/src/runtime/virtcontainers/api_test.go +++ b/src/runtime/virtcontainers/api_test.go @@ -316,6 +316,6 @@ func TestCleanupContainer(t *testing.T) { _, err = os.Stat(sandboxDir) if err == nil { - t.Fatal(err) + t.Fatal("sandbox dir should be deleted") } } diff --git a/src/runtime/virtcontainers/persist.go b/src/runtime/virtcontainers/persist.go index 78c200bc99..a842df4404 100644 --- a/src/runtime/virtcontainers/persist.go +++ b/src/runtime/virtcontainers/persist.go @@ -9,8 +9,6 @@ import ( "errors" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/api" - exp "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/experimental" - "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" ) @@ -428,117 +426,3 @@ func (c *Container) Restore() error { c.loadContMounts(cs) return nil } - -func loadSandboxConfig(id string) (*SandboxConfig, error) { - store, err := persist.GetDriver() - if err != nil || store == nil { - return nil, errors.New("failed to get fs persist driver") - } - - ss, _, err := store.FromDisk(id) - if err != nil { - return nil, err - } - - savedConf := ss.Config - sconfig := &SandboxConfig{ - ID: id, - HypervisorType: HypervisorType(savedConf.HypervisorType), - NetworkConfig: NetworkConfig{ - NetNSPath: savedConf.NetworkConfig.NetNSPath, - NetNsCreated: savedConf.NetworkConfig.NetNsCreated, - DisableNewNetNs: savedConf.NetworkConfig.DisableNewNetNs, - InterworkingModel: NetInterworkingModel(savedConf.NetworkConfig.InterworkingModel), - }, - - ShmSize: savedConf.ShmSize, - SharePidNs: savedConf.SharePidNs, - SystemdCgroup: savedConf.SystemdCgroup, - SandboxCgroupOnly: savedConf.SandboxCgroupOnly, - DisableGuestSeccomp: savedConf.DisableGuestSeccomp, - Cgroups: savedConf.Cgroups, - } - - for _, name := range savedConf.Experimental { - sconfig.Experimental = append(sconfig.Experimental, *exp.Get(name)) - } - - hconf := savedConf.HypervisorConfig - sconfig.HypervisorConfig = HypervisorConfig{ - NumVCPUs: hconf.NumVCPUs, - DefaultMaxVCPUs: hconf.DefaultMaxVCPUs, - MemorySize: hconf.MemorySize, - DefaultBridges: hconf.DefaultBridges, - Msize9p: hconf.Msize9p, - MemSlots: hconf.MemSlots, - MemOffset: hconf.MemOffset, - VirtioMem: hconf.VirtioMem, - VirtioFSCacheSize: hconf.VirtioFSCacheSize, - KernelPath: hconf.KernelPath, - ImagePath: hconf.ImagePath, - InitrdPath: hconf.InitrdPath, - FirmwarePath: hconf.FirmwarePath, - MachineAccelerators: hconf.MachineAccelerators, - CPUFeatures: hconf.CPUFeatures, - HypervisorPath: hconf.HypervisorPath, - HypervisorPathList: hconf.HypervisorPathList, - HypervisorCtlPath: hconf.HypervisorCtlPath, - HypervisorCtlPathList: hconf.HypervisorCtlPathList, - JailerPath: hconf.JailerPath, - JailerPathList: hconf.JailerPathList, - BlockDeviceDriver: hconf.BlockDeviceDriver, - HypervisorMachineType: hconf.HypervisorMachineType, - MemoryPath: hconf.MemoryPath, - DevicesStatePath: hconf.DevicesStatePath, - EntropySource: hconf.EntropySource, - SharedFS: hconf.SharedFS, - VirtioFSDaemon: hconf.VirtioFSDaemon, - VirtioFSDaemonList: hconf.VirtioFSDaemonList, - VirtioFSCache: hconf.VirtioFSCache, - VirtioFSExtraArgs: hconf.VirtioFSExtraArgs[:], - BlockDeviceCacheSet: hconf.BlockDeviceCacheSet, - BlockDeviceCacheDirect: hconf.BlockDeviceCacheDirect, - BlockDeviceCacheNoflush: hconf.BlockDeviceCacheNoflush, - DisableBlockDeviceUse: hconf.DisableBlockDeviceUse, - EnableIOThreads: hconf.EnableIOThreads, - Debug: hconf.Debug, - MemPrealloc: hconf.MemPrealloc, - HugePages: hconf.HugePages, - FileBackedMemRootDir: hconf.FileBackedMemRootDir, - FileBackedMemRootList: hconf.FileBackedMemRootList, - Realtime: hconf.Realtime, - Mlock: hconf.Mlock, - DisableNestingChecks: hconf.DisableNestingChecks, - DisableImageNvdimm: hconf.DisableImageNvdimm, - HotplugVFIOOnRootBus: hconf.HotplugVFIOOnRootBus, - PCIeRootPort: hconf.PCIeRootPort, - BootToBeTemplate: hconf.BootToBeTemplate, - BootFromTemplate: hconf.BootFromTemplate, - DisableVhostNet: hconf.DisableVhostNet, - EnableVhostUserStore: hconf.EnableVhostUserStore, - VhostUserStorePath: hconf.VhostUserStorePath, - VhostUserStorePathList: hconf.VhostUserStorePathList, - GuestHookPath: hconf.GuestHookPath, - VMid: hconf.VMid, - RxRateLimiterMaxRate: hconf.RxRateLimiterMaxRate, - TxRateLimiterMaxRate: hconf.TxRateLimiterMaxRate, - SGXEPCSize: hconf.SGXEPCSize, - EnableAnnotations: hconf.EnableAnnotations, - } - - sconfig.AgentConfig = KataAgentConfig{ - LongLiveConn: savedConf.KataAgentConfig.LongLiveConn, - } - - for _, contConf := range savedConf.ContainerConfigs { - sconfig.Containers = append(sconfig.Containers, ContainerConfig{ - ID: contConf.ID, - Annotations: contConf.Annotations, - Resources: contConf.Resources, - RootFs: RootFs{ - Target: contConf.RootFs, - }, - }) - } - return sconfig, nil -} diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index ff2c5729dd..811bed0205 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -38,7 +38,6 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/agent/protocols/grpc" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/annotations" vccgroups "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/cgroups" - "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/compatoci" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/cpuset" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" @@ -55,6 +54,9 @@ const ( DirMode = os.FileMode(0750) | os.ModeDir ) +// globalSandbox tracks sandbox globally +var globalSandbox *Sandbox + // SandboxStatus describes a sandbox status. type SandboxStatus struct { ID string @@ -270,7 +272,6 @@ func (s *Sandbox) GetContainer(containerID string) VCContainer { // Release closes the agent connection and removes sandbox from internal list. func (s *Sandbox) Release() error { s.Logger().Info("release sandbox") - globalSandboxList.removeSandbox(s.id) if s.monitor != nil { s.monitor.stop() } @@ -506,15 +507,11 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor return nil, fmt.Errorf("failed to get fs persist driver: %v", err) } - if err = globalSandboxList.addSandbox(s); err != nil { - s.newStore.Destroy(s.id) - return nil, err - } + globalSandbox = s defer func() { if retErr != nil { s.Logger().WithError(retErr).WithField("sandboxid", s.id).Error("Create new sandbox failed") - globalSandboxList.removeSandbox(s.id) s.newStore.Destroy(s.id) } }() @@ -636,50 +633,6 @@ func rwLockSandbox(sandboxID string) (func() error, error) { return store.Lock(sandboxID, 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") - if sandboxID == "" { - return nil, vcTypes.ErrNeedSandboxID - } - - sandbox, err = globalSandboxList.lookupSandbox(sandboxID) - if sandbox != nil && err == nil { - return sandbox, err - } - - var config SandboxConfig - - // load sandbox config fromld store. - c, err := loadSandboxConfig(sandboxID) - if err != nil { - virtLog.Warningf("failed to get sandbox config from new store: %v", err) - return nil, err - } - - config = *c - - // fetchSandbox is not suppose to create new sandbox VM. - sandbox, err = createSandbox(ctx, config, nil) - if err != nil { - return nil, fmt.Errorf("failed to create sandbox with config %+v: %v", config, err) - } - - if sandbox.config.SandboxCgroupOnly { - if err := sandbox.createCgroupManager(); err != nil { - return nil, err - } - } - - // This sandbox already exists, we don't need to recreate the containers in the guest. - // We only need to fetch the containers from storage and create the container structs. - if err := sandbox.fetchContainers(); err != nil { - return nil, err - } - - return sandbox, nil -} - // findContainer returns a container from the containers list held by the // sandbox structure, based on a container ID. func (s *Sandbox) findContainer(containerID string) (*Container, error) { @@ -741,8 +694,6 @@ func (s *Sandbox) Delete() error { } } - globalSandboxList.removeSandbox(s.id) - if s.monitor != nil { s.monitor.stop() } @@ -1145,33 +1096,6 @@ func (s *Sandbox) addContainer(c *Container) error { return nil } -// newContainers creates new containers structure and -// adds them to the sandbox. It does not create the containers -// in the guest. This should only be used when fetching a -// sandbox that already exists. -func (s *Sandbox) fetchContainers() error { - for i, contConfig := range s.config.Containers { - // Add spec from bundle path - spec, err := compatoci.GetContainerSpec(contConfig.Annotations) - if err != nil { - return err - } - contConfig.CustomSpec = &spec - s.config.Containers[i] = contConfig - - c, err := newContainer(s, &s.config.Containers[i]) - if err != nil { - return err - } - - if err := s.addContainer(c); err != nil { - return err - } - } - - return nil -} - // CreateContainer creates a new container in the sandbox // This should be called only when the sandbox is already created. // It will add new container config to sandbox.config.Containers diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index a075bbc3b5..110cd8e9b3 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -301,8 +301,7 @@ func TestSandboxSetSandboxAndContainerState(t *testing.T) { } // force state to be read from disk - p2, err := fetchSandbox(context.Background(), p.ID()) - assert.NoError(err) + p2 := globalSandbox if err := testCheckSandboxOnDiskState(p2, newSandboxState); err != nil { t.Error(err) @@ -1328,9 +1327,6 @@ func checkSandboxRemains() error { if err = checkDirNotExist(path.Join(kataHostSharedDir(), testSandboxID)); err != nil { return fmt.Errorf("%s still exists", path.Join(kataHostSharedDir(), testSandboxID)) } - if _, err = globalSandboxList.lookupSandbox(testSandboxID); err == nil { - return fmt.Errorf("globalSandboxList for %s stil exists", testSandboxID) - } return nil } diff --git a/src/runtime/virtcontainers/sandboxlist.go b/src/runtime/virtcontainers/sandboxlist.go deleted file mode 100644 index d67b4d21fd..0000000000 --- a/src/runtime/virtcontainers/sandboxlist.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) 2018 HyperHQ Inc. -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "fmt" - "sync" -) - -type sandboxList struct { - lock sync.RWMutex - sandboxes map[string]*Sandbox -} - -// globalSandboxList tracks sandboxes globally -var globalSandboxList = &sandboxList{sandboxes: make(map[string]*Sandbox)} - -func (p *sandboxList) addSandbox(sandbox *Sandbox) (err error) { - if sandbox == nil { - return nil - } - - p.lock.Lock() - defer p.lock.Unlock() - if p.sandboxes[sandbox.id] == nil { - p.sandboxes[sandbox.id] = sandbox - } else { - err = fmt.Errorf("sandbox %s exists", sandbox.id) - } - return err -} - -func (p *sandboxList) removeSandbox(id string) { - p.lock.Lock() - defer p.lock.Unlock() - delete(p.sandboxes, id) -} - -func (p *sandboxList) lookupSandbox(id string) (*Sandbox, error) { - p.lock.RLock() - defer p.lock.RUnlock() - if p.sandboxes[id] != nil { - return p.sandboxes[id], nil - } - return nil, fmt.Errorf("sandbox %s does not exist", id) -} diff --git a/src/runtime/virtcontainers/sandboxlist_test.go b/src/runtime/virtcontainers/sandboxlist_test.go deleted file mode 100644 index c320fb3eec..0000000000 --- a/src/runtime/virtcontainers/sandboxlist_test.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2018 HyperHQ Inc. -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestSandboxListOperations(t *testing.T) { - p := &Sandbox{id: "testsandboxListsandbox"} - l := &sandboxList{sandboxes: make(map[string]*Sandbox)} - err := l.addSandbox(p) - assert.Nil(t, err, "addSandbox failed") - - err = l.addSandbox(p) - assert.NotNil(t, err, "add same sandbox should fail") - - np, err := l.lookupSandbox(p.id) - assert.Nil(t, err, "lookupSandbox failed") - assert.Equal(t, np, p, "lookupSandbox returns different sandbox %v:%v", np, p) - - _, err = l.lookupSandbox("some-non-existing-sandbox-name") - assert.NotNil(t, err, "lookupSandbox for non-existing sandbox should fail") - - l.removeSandbox(p.id) -} diff --git a/src/runtime/virtcontainers/virtcontainers_test.go b/src/runtime/virtcontainers/virtcontainers_test.go index 8a3d2a9854..6960ffc106 100644 --- a/src/runtime/virtcontainers/virtcontainers_test.go +++ b/src/runtime/virtcontainers/virtcontainers_test.go @@ -58,7 +58,7 @@ var testHyperstartTtySocket = "" // cleanUp Removes any stale sandbox/container state that can affect // the next test to run. func cleanUp() { - globalSandboxList.removeSandbox(testSandboxID) + globalSandbox = nil os.RemoveAll(fs.MockRunStoragePath()) os.RemoveAll(fs.MockRunVMStoragePath()) syscall.Unmount(getSharePath(testSandboxID), syscall.MNT_DETACH|UmountNoFollow)