From 290203943c7f975101c0401f4d78f32a15a0952c Mon Sep 17 00:00:00 2001 From: bin liu Date: Wed, 4 Nov 2020 14:29:24 +0800 Subject: [PATCH 1/2] 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) From 4e3a8c0124b298dc3da495171c38aa7f31af94c6 Mon Sep 17 00:00:00 2001 From: bin liu Date: Wed, 4 Nov 2020 16:18:09 +0800 Subject: [PATCH 2/2] runtime: remove global sandbox variable Remove global sandbox variable, and save *Sandbox to hypervisor struct. For some needs, hypervisor may need to use methods from Sandbox. Signed-off-by: bin liu --- src/runtime/containerd-shim-v2/service.go | 6 ++--- src/runtime/containerd-shim-v2/utils.go | 4 ++-- src/runtime/virtcontainers/acrn.go | 8 +++++-- src/runtime/virtcontainers/acrn_test.go | 5 +---- src/runtime/virtcontainers/api.go | 17 +++++++------- src/runtime/virtcontainers/api_test.go | 2 +- src/runtime/virtcontainers/clh.go | 3 +++ src/runtime/virtcontainers/fc.go | 3 +++ src/runtime/virtcontainers/hypervisor.go | 2 ++ src/runtime/virtcontainers/implementation.go | 4 ++-- src/runtime/virtcontainers/interfaces.go | 2 +- src/runtime/virtcontainers/mock_hypervisor.go | 3 +++ src/runtime/virtcontainers/pkg/vcmock/mock.go | 7 +++--- .../virtcontainers/pkg/vcmock/mock_test.go | 22 +++++++++---------- .../virtcontainers/pkg/vcmock/types.go | 4 ++-- src/runtime/virtcontainers/qemu.go | 3 +++ src/runtime/virtcontainers/sandbox.go | 7 ++---- src/runtime/virtcontainers/sandbox_test.go | 6 ++--- .../virtcontainers/virtcontainers_test.go | 1 - 19 files changed, 59 insertions(+), 50 deletions(-) diff --git a/src/runtime/containerd-shim-v2/service.go b/src/runtime/containerd-shim-v2/service.go index 795f27e4b5..2903824779 100644 --- a/src/runtime/containerd-shim-v2/service.go +++ b/src/runtime/containerd-shim-v2/service.go @@ -312,17 +312,17 @@ func (s *service) Cleanup(ctx context.Context) (_ *taskAPI.DeleteResponse, err e switch containerType { case vc.PodSandbox: - err = cleanupContainer(ctx, s.id, s.id, path) + err = cleanupContainer(ctx, s.sandbox, s.id, path) if err != nil { return nil, err } case vc.PodContainer: - sandboxID, err := oci.SandboxID(ociSpec) + _, err := oci.SandboxID(ociSpec) if err != nil { return nil, err } - err = cleanupContainer(ctx, sandboxID, s.id, path) + err = cleanupContainer(ctx, s.sandbox, s.id, path) if err != nil { return nil, err } diff --git a/src/runtime/containerd-shim-v2/utils.go b/src/runtime/containerd-shim-v2/utils.go index 95adb93faa..3172483500 100644 --- a/src/runtime/containerd-shim-v2/utils.go +++ b/src/runtime/containerd-shim-v2/utils.go @@ -31,10 +31,10 @@ func cReap(s *service, status int, id, execid string, exitat time.Time) { } } -func cleanupContainer(ctx context.Context, sid, cid, bundlePath string) error { +func cleanupContainer(ctx context.Context, sandbox vc.VCSandbox, cid, bundlePath string) error { shimLog.WithField("service", "cleanup").WithField("container", cid).Info("Cleanup container") - err := vci.CleanupContainer(ctx, sid, cid, true) + err := vci.CleanupContainer(ctx, sandbox, cid, true) if err != nil { shimLog.WithError(err).WithField("container", cid).Warn("failed to cleanup container") return err diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index 5b6010d211..b37f65bb06 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -88,6 +88,7 @@ type Acrn struct { arch acrnArch ctx context.Context store persistapi.PersistDriver + sandbox *Sandbox } type acrnPlatformInfo struct { @@ -231,10 +232,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 := globalSandbox var err error - if _, err = sandbox.GetAndSetSandboxBlockIndex(); err != nil { + if _, err = a.sandbox.GetAndSetSandboxBlockIndex(); err != nil { return nil, err } @@ -821,3 +821,7 @@ func (a *Acrn) loadInfo() error { func (a *Acrn) isRateLimiterBuiltin() bool { return false } + +func (a *Acrn) setSandbox(sandbox *Sandbox) { + a.sandbox = sandbox +} diff --git a/src/runtime/virtcontainers/acrn_test.go b/src/runtime/virtcontainers/acrn_test.go index 27dbd088e7..eb357b0034 100644 --- a/src/runtime/virtcontainers/acrn_test.go +++ b/src/runtime/virtcontainers/acrn_test.go @@ -230,10 +230,7 @@ func TestAcrnCreateSandbox(t *testing.T) { state: types.SandboxState{BlockIndexMap: make(map[int]struct{})}, } - globalSandbox = sandbox - defer func() { - globalSandbox = nil - }() + a.sandbox = sandbox //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 0335059f4d..5c3f402108 100644 --- a/src/runtime/virtcontainers/api.go +++ b/src/runtime/virtcontainers/api.go @@ -7,6 +7,7 @@ package virtcontainers import ( "context" + "fmt" "runtime" deviceApi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/api" @@ -135,26 +136,24 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f // CleanupContainer is used by shimv2 to stop and delete a container exclusively, once there is no container // in the sandbox left, do stop the sandbox and delete it. Those serial operations will be done exclusively by // locking the sandbox. -func CleanupContainer(ctx context.Context, sandboxID, containerID string, force bool) error { +func CleanupContainer(ctx context.Context, sandbox VCSandbox, containerID string, force bool) error { span, ctx := trace(ctx, "CleanupContainer") defer span.Finish() - - if sandboxID == "" { - return vcTypes.ErrNeedSandboxID - } - if containerID == "" { return vcTypes.ErrNeedContainerID } - unlock, err := rwLockSandbox(sandboxID) + s, ok := sandbox.(*Sandbox) + if !ok { + return fmt.Errorf("not a Sandbox reference") + } + + unlock, err := rwLockSandbox(s.id) if err != nil { return err } defer unlock() - s := globalSandbox - defer s.Release() _, err = s.StopContainer(containerID, force) diff --git a/src/runtime/virtcontainers/api_test.go b/src/runtime/virtcontainers/api_test.go index 927e450c8d..f0447b3ffb 100644 --- a/src/runtime/virtcontainers/api_test.go +++ b/src/runtime/virtcontainers/api_test.go @@ -307,7 +307,7 @@ func TestCleanupContainer(t *testing.T) { } for _, c := range p.GetAllContainers() { - CleanupContainer(ctx, p.ID(), c.ID(), true) + CleanupContainer(ctx, p, c.ID(), true) } s, ok := p.(*Sandbox) diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index 30aedd4c02..6125d16795 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -1297,3 +1297,6 @@ func (clh *cloudHypervisor) vmInfo() (chclient.VmInfo, error) { func (clh *cloudHypervisor) isRateLimiterBuiltin() bool { return false } + +func (clh *cloudHypervisor) setSandbox(sandbox *Sandbox) { +} diff --git a/src/runtime/virtcontainers/fc.go b/src/runtime/virtcontainers/fc.go index 37fdbdc105..8450006904 100644 --- a/src/runtime/virtcontainers/fc.go +++ b/src/runtime/virtcontainers/fc.go @@ -1247,3 +1247,6 @@ func revertBytes(num uint64) uint64 { return 1024*revertBytes(a) + b } } + +func (fc *firecracker) setSandbox(sandbox *Sandbox) { +} diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index ee7a2b6b73..75834e8da7 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -814,4 +814,6 @@ type hypervisor interface { // check if hypervisor supports built-in rate limiter. isRateLimiterBuiltin() bool + + setSandbox(sandbox *Sandbox) } diff --git a/src/runtime/virtcontainers/implementation.go b/src/runtime/virtcontainers/implementation.go index 177797ebd2..186b59e87a 100644 --- a/src/runtime/virtcontainers/implementation.go +++ b/src/runtime/virtcontainers/implementation.go @@ -38,6 +38,6 @@ func (impl *VCImpl) CreateSandbox(ctx context.Context, sandboxConfig SandboxConf // CleanupContainer is used by shimv2 to stop and delete a container exclusively, once there is no container // in the sandbox left, do stop the sandbox and delete it. Those serial operations will be done exclusively by // locking the sandbox. -func (impl *VCImpl) CleanupContainer(ctx context.Context, sandboxID, containerID string, force bool) error { - return CleanupContainer(ctx, sandboxID, containerID, force) +func (impl *VCImpl) CleanupContainer(ctx context.Context, sandbox VCSandbox, containerID string, force bool) error { + return CleanupContainer(ctx, sandbox, containerID, force) } diff --git a/src/runtime/virtcontainers/interfaces.go b/src/runtime/virtcontainers/interfaces.go index 365c329db0..8aeed1498f 100644 --- a/src/runtime/virtcontainers/interfaces.go +++ b/src/runtime/virtcontainers/interfaces.go @@ -24,7 +24,7 @@ type VC interface { SetFactory(ctx context.Context, factory Factory) CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig) (VCSandbox, error) - CleanupContainer(ctx context.Context, sandboxID, containerID string, force bool) error + CleanupContainer(ctx context.Context, sandbox VCSandbox, containerID string, force bool) error } // VCSandbox is the Sandbox interface diff --git a/src/runtime/virtcontainers/mock_hypervisor.go b/src/runtime/virtcontainers/mock_hypervisor.go index 5dff86fcbd..d7a2480c2b 100644 --- a/src/runtime/virtcontainers/mock_hypervisor.go +++ b/src/runtime/virtcontainers/mock_hypervisor.go @@ -136,3 +136,6 @@ func (m *mockHypervisor) generateSocket(id string) (interface{}, error) { func (m *mockHypervisor) isRateLimiterBuiltin() bool { return false } + +func (m *mockHypervisor) setSandbox(sandbox *Sandbox) { +} diff --git a/src/runtime/virtcontainers/pkg/vcmock/mock.go b/src/runtime/virtcontainers/pkg/vcmock/mock.go index 5a13c76c45..0dd2b579f6 100644 --- a/src/runtime/virtcontainers/pkg/vcmock/mock.go +++ b/src/runtime/virtcontainers/pkg/vcmock/mock.go @@ -18,6 +18,7 @@ package vcmock import ( "context" "fmt" + vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers" "github.com/sirupsen/logrus" ) @@ -49,9 +50,9 @@ func (m *VCMock) CreateSandbox(ctx context.Context, sandboxConfig vc.SandboxConf return nil, fmt.Errorf("%s: %s (%+v): sandboxConfig: %v", mockErrorPrefix, getSelf(), m, sandboxConfig) } -func (m *VCMock) CleanupContainer(ctx context.Context, sandboxID, containerID string, force bool) error { +func (m *VCMock) CleanupContainer(ctx context.Context, sandbox vc.VCSandbox, containerID string, force bool) error { if m.CleanupContainerFunc != nil { - return m.CleanupContainerFunc(ctx, sandboxID, containerID, true) + return m.CleanupContainerFunc(ctx, sandbox, containerID, true) } - return fmt.Errorf("%s: %s (%+v): sandboxID: %v", mockErrorPrefix, getSelf(), m, sandboxID) + return fmt.Errorf("%s: %s (%+v): sandbox: %v", mockErrorPrefix, getSelf(), m, sandbox) } diff --git a/src/runtime/virtcontainers/pkg/vcmock/mock_test.go b/src/runtime/virtcontainers/pkg/vcmock/mock_test.go index 9043b168da..79c543b3cd 100644 --- a/src/runtime/virtcontainers/pkg/vcmock/mock_test.go +++ b/src/runtime/virtcontainers/pkg/vcmock/mock_test.go @@ -17,13 +17,13 @@ import ( ) const ( - testSandboxID = "testSandboxID" testContainerID = "testContainerID" ) var ( - loggerTriggered = 0 - factoryTriggered = 0 + loggerTriggered = 0 + factoryTriggered = 0 + testSandbox vc.VCSandbox = &Sandbox{} ) func TestVCImplementations(t *testing.T) { @@ -178,21 +178,21 @@ func TestVCMockCleanupContainer(t *testing.T) { assert.Nil(m.CleanupContainerFunc) ctx := context.Background() - err := m.CleanupContainer(ctx, testSandboxID, testContainerID, false) + err := m.CleanupContainer(ctx, testSandbox, testContainerID, false) assert.Error(err) assert.True(IsMockError(err)) - m.CleanupContainerFunc = func(ctx context.Context, sandboxID, containerID string, force bool) error { + m.CleanupContainerFunc = func(ctx context.Context, sandbox vc.VCSandbox, containerID string, force bool) error { return nil } - err = m.CleanupContainer(ctx, testSandboxID, testContainerID, false) + err = m.CleanupContainer(ctx, testSandbox, testContainerID, false) assert.NoError(err) // reset m.CleanupContainerFunc = nil - err = m.CleanupContainer(ctx, testSandboxID, testContainerID, false) + err = m.CleanupContainer(ctx, testSandbox, testContainerID, false) assert.Error(err) assert.True(IsMockError(err)) } @@ -204,21 +204,21 @@ func TestVCMockForceCleanupContainer(t *testing.T) { assert.Nil(m.CleanupContainerFunc) ctx := context.Background() - err := m.CleanupContainer(ctx, testSandboxID, testContainerID, true) + err := m.CleanupContainer(ctx, testSandbox, testContainerID, true) assert.Error(err) assert.True(IsMockError(err)) - m.CleanupContainerFunc = func(ctx context.Context, sandboxID, containerID string, force bool) error { + m.CleanupContainerFunc = func(ctx context.Context, sandbox vc.VCSandbox, containerID string, force bool) error { return nil } - err = m.CleanupContainer(ctx, testSandboxID, testContainerID, true) + err = m.CleanupContainer(ctx, testSandbox, testContainerID, true) assert.NoError(err) // reset m.CleanupContainerFunc = nil - err = m.CleanupContainer(ctx, testSandboxID, testContainerID, true) + err = m.CleanupContainer(ctx, testSandbox, testContainerID, true) assert.Error(err) assert.True(IsMockError(err)) } diff --git a/src/runtime/virtcontainers/pkg/vcmock/types.go b/src/runtime/virtcontainers/pkg/vcmock/types.go index 9caa53232a..de7f034421 100644 --- a/src/runtime/virtcontainers/pkg/vcmock/types.go +++ b/src/runtime/virtcontainers/pkg/vcmock/types.go @@ -87,6 +87,6 @@ type VCMock struct { SetLoggerFunc func(ctx context.Context, logger *logrus.Entry) SetFactoryFunc func(ctx context.Context, factory vc.Factory) - CreateSandboxFunc func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) - CleanupContainerFunc func(ctx context.Context, sandboxID, containerID string, force bool) error + CreateSandboxFunc func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) + CleanupContainerFunc func(ctx context.Context, sandbox vc.VCSandbox, containerID string, force bool) error } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 932139aa01..9d5366611b 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -2389,3 +2389,6 @@ func (q *qemu) generateSocket(id string) (interface{}, error) { func (q *qemu) isRateLimiterBuiltin() bool { return false } + +func (q *qemu) setSandbox(sandbox *Sandbox) { +} diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 811bed0205..118ece60e0 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -54,9 +54,6 @@ const ( DirMode = os.FileMode(0750) | os.ModeDir ) -// globalSandbox tracks sandbox globally -var globalSandbox *Sandbox - // SandboxStatus describes a sandbox status. type SandboxStatus struct { ID string @@ -503,12 +500,12 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor ctx: ctx, } + hypervisor.setSandbox(s) + if s.newStore, err = persist.GetDriver(); err != nil || s.newStore == nil { return nil, fmt.Errorf("failed to get fs persist driver: %v", err) } - globalSandbox = s - defer func() { if retErr != nil { s.Logger().WithError(retErr).WithField("sandboxid", s.id).Error("Create new sandbox failed") diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 110cd8e9b3..24458a2117 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -301,13 +301,11 @@ func TestSandboxSetSandboxAndContainerState(t *testing.T) { } // force state to be read from disk - p2 := globalSandbox - - if err := testCheckSandboxOnDiskState(p2, newSandboxState); err != nil { + if err := testCheckSandboxOnDiskState(p, newSandboxState); err != nil { t.Error(err) } - c2, err := p2.findContainer(contID) + c2, err := p.findContainer(contID) assert.NoError(err) if err := testCheckContainerOnDiskState(c2, newContainerState); err != nil { diff --git a/src/runtime/virtcontainers/virtcontainers_test.go b/src/runtime/virtcontainers/virtcontainers_test.go index 6960ffc106..cbec355eaf 100644 --- a/src/runtime/virtcontainers/virtcontainers_test.go +++ b/src/runtime/virtcontainers/virtcontainers_test.go @@ -58,7 +58,6 @@ var testHyperstartTtySocket = "" // cleanUp Removes any stale sandbox/container state that can affect // the next test to run. func cleanUp() { - globalSandbox = nil os.RemoveAll(fs.MockRunStoragePath()) os.RemoveAll(fs.MockRunVMStoragePath()) syscall.Unmount(getSharePath(testSandboxID), syscall.MNT_DETACH|UmountNoFollow)