From 658f77979c7396376d526614ef57e442bdb67bed Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 30 Jan 2020 14:41:33 +0000 Subject: [PATCH 01/12] rootless: move pkg/rootless to virtcontainers rootless is used in katautils, cli and virtcontainers. It makes more sense if it's part of virtcontainer, this way virtcontainers won't depend on other runtime subpackages Signed-off-by: Julio Montes --- cli/main.go | 2 +- cli/main_test.go | 2 +- pkg/katautils/network.go | 2 +- pkg/katautils/oci.go | 2 +- pkg/katautils/oci_test.go | 2 +- virtcontainers/acrn.go | 2 +- virtcontainers/api_test.go | 2 +- virtcontainers/cgroups.go | 2 +- virtcontainers/container.go | 2 +- virtcontainers/kata_agent.go | 2 +- virtcontainers/kata_agent_test.go | 2 +- virtcontainers/network.go | 2 +- virtcontainers/persist/fs/fs.go | 2 +- {pkg => virtcontainers/pkg}/rootless/rootless.go | 0 {pkg => virtcontainers/pkg}/rootless/rootless_test.go | 0 virtcontainers/sandbox.go | 2 +- virtcontainers/store/filesystem_backend.go | 2 +- 17 files changed, 15 insertions(+), 15 deletions(-) rename {pkg => virtcontainers/pkg}/rootless/rootless.go (100%) rename {pkg => virtcontainers/pkg}/rootless/rootless_test.go (100%) diff --git a/cli/main.go b/cli/main.go index d263af02a9..d2ca4835e9 100644 --- a/cli/main.go +++ b/cli/main.go @@ -18,12 +18,12 @@ import ( "syscall" "github.com/kata-containers/runtime/pkg/katautils" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/pkg/signals" vc "github.com/kata-containers/runtime/virtcontainers" exp "github.com/kata-containers/runtime/virtcontainers/experimental" vf "github.com/kata-containers/runtime/virtcontainers/factory" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" specs "github.com/opencontainers/runtime-spec/specs-go" opentracing "github.com/opentracing/opentracing-go" "github.com/sirupsen/logrus" diff --git a/cli/main_test.go b/cli/main_test.go index e6b92c334d..911159faec 100644 --- a/cli/main_test.go +++ b/cli/main_test.go @@ -24,10 +24,10 @@ import ( "github.com/dlespiau/covertool/pkg/cover" ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/kata-containers/runtime/pkg/katautils" - "github.com/kata-containers/runtime/pkg/rootless" vc "github.com/kata-containers/runtime/virtcontainers" "github.com/kata-containers/runtime/virtcontainers/pkg/compatoci" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" "github.com/kata-containers/runtime/virtcontainers/types" specs "github.com/opencontainers/runtime-spec/specs-go" diff --git a/pkg/katautils/network.go b/pkg/katautils/network.go index b01d13cee0..714fb46142 100644 --- a/pkg/katautils/network.go +++ b/pkg/katautils/network.go @@ -16,8 +16,8 @@ import ( "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" - "github.com/kata-containers/runtime/pkg/rootless" vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "golang.org/x/sys/unix" ) diff --git a/pkg/katautils/oci.go b/pkg/katautils/oci.go index ad0eaf2e0b..6de8101e94 100644 --- a/pkg/katautils/oci.go +++ b/pkg/katautils/oci.go @@ -13,7 +13,7 @@ import ( "path/filepath" "strings" - "github.com/kata-containers/runtime/pkg/rootless" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" ) const ctrsMappingDirMode = os.FileMode(0750) diff --git a/pkg/katautils/oci_test.go b/pkg/katautils/oci_test.go index f6588e7cce..a5e67a1408 100644 --- a/pkg/katautils/oci_test.go +++ b/pkg/katautils/oci_test.go @@ -13,7 +13,7 @@ import ( "path/filepath" "testing" - "github.com/kata-containers/runtime/pkg/rootless" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/stretchr/testify/assert" ) diff --git a/virtcontainers/acrn.go b/virtcontainers/acrn.go index 1b1c7f7df4..099de8e253 100644 --- a/virtcontainers/acrn.go +++ b/virtcontainers/acrn.go @@ -21,11 +21,11 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/persist" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" "github.com/kata-containers/runtime/virtcontainers/persist/fs" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 999999696e..7461ef130a 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -16,11 +16,11 @@ import ( "testing" ktu "github.com/kata-containers/runtime/pkg/katatestutils" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" "github.com/kata-containers/runtime/virtcontainers/pkg/mock" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/types" specs "github.com/opencontainers/runtime-spec/specs-go" diff --git a/virtcontainers/cgroups.go b/virtcontainers/cgroups.go index 3aa71db9d0..7c6cf78be1 100644 --- a/virtcontainers/cgroups.go +++ b/virtcontainers/cgroups.go @@ -15,7 +15,7 @@ import ( "strings" "github.com/containerd/cgroups" - "github.com/kata-containers/runtime/pkg/rootless" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" libcontcgroups "github.com/opencontainers/runc/libcontainer/cgroups" libcontcgroupsfs "github.com/opencontainers/runc/libcontainer/cgroups/fs" libcontcgroupssystemd "github.com/opencontainers/runc/libcontainer/cgroups/systemd" diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 4690e8bc02..2a9f2269ee 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -26,9 +26,9 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/unix" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/manager" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/store" ) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index bf8e6b8f27..fa3f339871 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -22,12 +22,12 @@ import ( aTypes "github.com/kata-containers/agent/pkg/types" kataclient "github.com/kata-containers/agent/protocols/client" "github.com/kata-containers/agent/protocols/grpc" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" "github.com/kata-containers/runtime/virtcontainers/persist/fs" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" ns "github.com/kata-containers/runtime/virtcontainers/pkg/nsenter" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/store" diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index dcd6db82c2..27534220f0 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -26,13 +26,13 @@ import ( aTypes "github.com/kata-containers/agent/pkg/types" pb "github.com/kata-containers/agent/protocols/grpc" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/device/api" "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" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/types" ) diff --git a/virtcontainers/network.go b/virtcontainers/network.go index c6c199b83b..d70c536043 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -25,7 +25,7 @@ import ( "github.com/vishvananda/netns" "golang.org/x/sys/unix" - "github.com/kata-containers/runtime/pkg/rootless" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/utils" diff --git a/virtcontainers/persist/fs/fs.go b/virtcontainers/persist/fs/fs.go index 33acebecd6..34f30d519d 100644 --- a/virtcontainers/persist/fs/fs.go +++ b/virtcontainers/persist/fs/fs.go @@ -14,8 +14,8 @@ import ( "path/filepath" "syscall" - "github.com/kata-containers/runtime/pkg/rootless" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/sirupsen/logrus" ) diff --git a/pkg/rootless/rootless.go b/virtcontainers/pkg/rootless/rootless.go similarity index 100% rename from pkg/rootless/rootless.go rename to virtcontainers/pkg/rootless/rootless.go diff --git a/pkg/rootless/rootless_test.go b/virtcontainers/pkg/rootless/rootless_test.go similarity index 100% rename from pkg/rootless/rootless_test.go rename to virtcontainers/pkg/rootless/rootless_test.go diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 19caccc15c..9f5f831b04 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -26,7 +26,6 @@ import ( "github.com/vishvananda/netlink" "github.com/kata-containers/agent/protocols/grpc" - "github.com/kata-containers/runtime/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/device/api" "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/device/drivers" @@ -36,6 +35,7 @@ import ( persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" "github.com/kata-containers/runtime/virtcontainers/pkg/compatoci" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/store" "github.com/kata-containers/runtime/virtcontainers/types" diff --git a/virtcontainers/store/filesystem_backend.go b/virtcontainers/store/filesystem_backend.go index b74699f666..124d339ce2 100644 --- a/virtcontainers/store/filesystem_backend.go +++ b/virtcontainers/store/filesystem_backend.go @@ -14,7 +14,7 @@ import ( "path/filepath" "syscall" - "github.com/kata-containers/runtime/pkg/rootless" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" opentracing "github.com/opentracing/opentracing-go" "github.com/sirupsen/logrus" From 6be74811dcf6f7a4f9bcc9048c926358bb9afb51 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 30 Jan 2020 19:56:58 +0000 Subject: [PATCH 02/12] virtcontainers: remove getVMPath method from agent `agent.getVMPath()` is an almost useless method that can be easily replaced with `filepath.Join()` Signed-off-by: Julio Montes --- virtcontainers/agent.go | 3 --- virtcontainers/kata_agent.go | 5 ----- virtcontainers/kata_agent_test.go | 5 ----- virtcontainers/noop_agent.go | 7 +------ virtcontainers/noop_agent_test.go | 7 ------- virtcontainers/vm.go | 4 ++-- 6 files changed, 3 insertions(+), 28 deletions(-) diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 32163dbf99..4d350b6c6b 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -224,9 +224,6 @@ type agent interface { // configureFromGrpc will update agent settings based on provided arguments which from Grpc configureFromGrpc(h hypervisor, id string, builtin bool, config interface{}) error - // getVMPath will return the agent vm socket's directory path - getVMPath(id string) string - // getSharePath will return the agent 9pfs share mount path getSharePath(id string) string diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index fa3f339871..2071e79d9d 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -24,7 +24,6 @@ import ( "github.com/kata-containers/agent/protocols/grpc" "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" ns "github.com/kata-containers/runtime/virtcontainers/pkg/nsenter" "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" @@ -216,10 +215,6 @@ func (k *kataAgent) Logger() *logrus.Entry { return virtLog.WithField("subsystem", "kata_agent") } -func (k *kataAgent) getVMPath(id string) string { - return filepath.Join(fs.RunVMStoragePath(), id) -} - func (k *kataAgent) getSharePath(id string) string { return filepath.Join(kataHostSharedDir(), id) } diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 27534220f0..e0d5d0d69d 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -620,11 +620,6 @@ func TestAgentPathAPI(t *testing.T) { k2 := &kataAgent{} id := "foobar" - // getVMPath - path1 := k1.getVMPath(id) - path2 := k2.getVMPath(id) - assert.Equal(path1, path2) - // getSharePath path1 = k1.getSharePath(id) path2 = k2.getSharePath(id) diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 914dbb719f..a63d717af7 100644 --- a/virtcontainers/noop_agent.go +++ b/virtcontainers/noop_agent.go @@ -185,12 +185,7 @@ func (n *noopAgent) configureFromGrpc(h hypervisor, id string, builtin bool, con return nil } -// getVMPath is the Noop agent vm path getter. It does nothing. -func (n *noopAgent) getVMPath(id string) string { - return "" -} - -// getVMPath is the Noop agent share path getter. It does nothing. +// getSharePath is the Noop agent share path getter. It does nothing. func (n *noopAgent) getSharePath(id string) string { return "" } diff --git a/virtcontainers/noop_agent_test.go b/virtcontainers/noop_agent_test.go index 109dd270bf..4d4fd2b463 100644 --- a/virtcontainers/noop_agent_test.go +++ b/virtcontainers/noop_agent_test.go @@ -156,13 +156,6 @@ func TestNoopAgentConfigure(t *testing.T) { assert.NoError(err) } -func TestNoopAgentGetVMPath(t *testing.T) { - n := &noopAgent{} - path := n.getVMPath("") - assert := assert.New(t) - assert.Empty(path) -} - func TestNoopAgentGetSharePath(t *testing.T) { n := &noopAgent{} path := n.getSharePath("") diff --git a/virtcontainers/vm.go b/virtcontainers/vm.go index e76273d03f..812d02082b 100644 --- a/virtcontainers/vm.go +++ b/virtcontainers/vm.go @@ -412,9 +412,9 @@ func (v *VM) assignSandbox(s *Sandbox) error { // - link 9pfs share path from sandbox dir (/run/kata-containers/shared/sandboxes/sbid/) to vm dir (/run/vc/vm/vmid/shared/) vmSharePath := buildVMSharePath(v.id) - vmSockDir := v.agent.getVMPath(v.id) + vmSockDir := filepath.Join(v.store.RunVMStoragePath(), v.id) sbSharePath := s.agent.getSharePath(s.id) - sbSockDir := s.agent.getVMPath(s.id) + sbSockDir := filepath.Join(v.store.RunVMStoragePath(), s.id) v.logger().WithFields(logrus.Fields{ "vmSharePath": vmSharePath, From 768db1bdc4e98172985a5f4d6dcbebce77032cbd Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 19:26:55 +0000 Subject: [PATCH 03/12] virtcontainers/persist: update API and interface Update persist FS API and interface to support rootless and mock filesystem implementations. `RunStoragePath` and `RunVMStoragePath` are part of FS object and may change their path depending on the driver (rootless/mock/fs) Signed-off-by: Julio Montes --- virtcontainers/persist/api/interface.go | 8 +++ virtcontainers/persist/fs/fs.go | 68 ++++++++----------------- virtcontainers/persist/fs/fs_test.go | 20 ++------ 3 files changed, 33 insertions(+), 63 deletions(-) diff --git a/virtcontainers/persist/api/interface.go b/virtcontainers/persist/api/interface.go index ea26dfbc3f..ef349d1c28 100644 --- a/virtcontainers/persist/api/interface.go +++ b/virtcontainers/persist/api/interface.go @@ -25,4 +25,12 @@ type PersistDriver interface { // Don't use them too much unless you have no other choice! @weizhang555 GlobalWrite(relativePath string, data []byte) error GlobalRead(relativePath string) ([]byte, error) + + // RunStoragePath is the sandbox runtime directory. + // It will contain one state.json and one lock file for each created sandbox. + RunStoragePath() string + + // RunVMStoragePath is the vm directory. + // It will contain all guest vm sockets and shared mountpoints. + RunVMStoragePath() string } diff --git a/virtcontainers/persist/fs/fs.go b/virtcontainers/persist/fs/fs.go index 34f30d519d..8582e552d5 100644 --- a/virtcontainers/persist/fs/fs.go +++ b/virtcontainers/persist/fs/fs.go @@ -15,7 +15,6 @@ import ( "syscall" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/sirupsen/logrus" ) @@ -40,45 +39,12 @@ const sandboxPathSuffix = "sbs" // vmPathSuffix is the suffix used for guest VMs. const vmPathSuffix = "vm" -var StorageRootPath = func() string { - path := filepath.Join("/run", storagePathSuffix) - if rootless.IsRootless() { - return filepath.Join(rootless.GetRootlessDir(), path) - } - return path -} - -// RunStoragePath is the sandbox runtime directory. -// It will contain one state.json and one lock file for each created sandbox. -var RunStoragePath = func() string { - return filepath.Join(StorageRootPath(), sandboxPathSuffix) -} - -// RunVMStoragePath is the vm directory. -// It will contain all guest vm sockets and shared mountpoints. -// The function is declared this way for mocking in unit tests -var RunVMStoragePath = func() string { - return filepath.Join(StorageRootPath(), vmPathSuffix) -} - -// TestSetRunStoragePath set RunStoragePath to path -// this function is only used for testing purpose -func TestSetRunStoragePath(path string) { - RunStoragePath = func() string { - return path - } -} - -func TestSetStorageRootPath(path string) { - StorageRootPath = func() string { - return path - } -} - // FS storage driver implementation type FS struct { - sandboxState *persistapi.SandboxState - containerState map[string]persistapi.ContainerState + sandboxState *persistapi.SandboxState + containerState map[string]persistapi.ContainerState + storageRootPath string + driverName string } var fsLog = logrus.WithField("source", "virtcontainers/persist/fs") @@ -87,24 +53,22 @@ var fsLog = logrus.WithField("source", "virtcontainers/persist/fs") func (fs *FS) Logger() *logrus.Entry { return fsLog.WithFields(logrus.Fields{ "subsystem": "persist", + "driver": fs.driverName, }) } -// Name returns driver name -func Name() string { - return "fs" -} - // Init FS persist driver and return abstract PersistDriver func Init() (persistapi.PersistDriver, error) { return &FS{ - sandboxState: &persistapi.SandboxState{}, - containerState: make(map[string]persistapi.ContainerState), + sandboxState: &persistapi.SandboxState{}, + containerState: make(map[string]persistapi.ContainerState), + storageRootPath: filepath.Join("/run", storagePathSuffix), + driverName: "fs", }, nil } func (fs *FS) sandboxDir(sandboxID string) (string, error) { - return filepath.Join(RunStoragePath(), sandboxID), nil + return filepath.Join(fs.RunStoragePath(), sandboxID), nil } // ToDisk sandboxState and containerState to disk @@ -314,7 +278,7 @@ func (fs *FS) Lock(sandboxID string, exclusive bool) (func() error, error) { } func (fs *FS) GlobalWrite(relativePath string, data []byte) error { - path := filepath.Join(StorageRootPath(), relativePath) + path := filepath.Join(fs.storageRootPath, relativePath) path, err := filepath.Abs(filepath.Clean(path)) if err != nil { return fmt.Errorf("failed to find abs path for %q: %v", relativePath, err) @@ -347,7 +311,7 @@ func (fs *FS) GlobalWrite(relativePath string, data []byte) error { } func (fs *FS) GlobalRead(relativePath string) ([]byte, error) { - path := filepath.Join(StorageRootPath(), relativePath) + path := filepath.Join(fs.storageRootPath, relativePath) path, err := filepath.Abs(filepath.Clean(path)) if err != nil { return nil, fmt.Errorf("failed to find abs path for %q: %v", relativePath, err) @@ -367,3 +331,11 @@ func (fs *FS) GlobalRead(relativePath string) ([]byte, error) { } return data, nil } + +func (fs *FS) RunStoragePath() string { + return filepath.Join(fs.storageRootPath, sandboxPathSuffix) +} + +func (fs *FS) RunVMStoragePath() string { + return filepath.Join(fs.storageRootPath, vmPathSuffix) +} diff --git a/virtcontainers/persist/fs/fs_test.go b/virtcontainers/persist/fs/fs_test.go index 431baa9a04..ef45c7e6b6 100644 --- a/virtcontainers/persist/fs/fs_test.go +++ b/virtcontainers/persist/fs/fs_test.go @@ -7,9 +7,7 @@ package fs import ( "fmt" - "io/ioutil" "os" - "path/filepath" "testing" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" @@ -17,29 +15,21 @@ import ( ) func getFsDriver() (*FS, error) { - driver, err := Init() + driver, err := MockFSInit() if err != nil { return nil, fmt.Errorf("failed to init fs driver") } - fs, ok := driver.(*FS) + fs, ok := driver.(*MockFS) if !ok { - return nil, fmt.Errorf("failed to convert driver to *FS") + return nil, fmt.Errorf("failed to convert driver to *MockFS") } - return fs, nil + return fs.FS, nil } func initTestDir() func() { - testDir, _ := ioutil.TempDir("", "vc-tmp-") - // allow the tests to run without affecting the host system. - rootSave := StorageRootPath() - TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - os.MkdirAll(testDir, dirMode) - return func() { - TestSetStorageRootPath(rootSave) - os.RemoveAll(testDir) + os.RemoveAll(MockStorageRootPath()) } } From ea8fb96c3e93697fbd50379d07273bcf03e9f992 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 19:32:01 +0000 Subject: [PATCH 04/12] virtcontainers/persist: introduce rootless fs driver Rootless fs driver inherits from FS and may overwrite its methods. All files and directories created by this driver are under a path accessible for the current user, typically this path is defined by the environment variable `XDG_RUNTIME_DIR`, if this variable is not defined, the default path `/run/user/$UID` is used instead, where $UID is the current user ID. fixes #2416 Signed-off-by: Julio Montes --- virtcontainers/persist/fs/rootlessfs.go | 48 +++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 virtcontainers/persist/fs/rootlessfs.go diff --git a/virtcontainers/persist/fs/rootlessfs.go b/virtcontainers/persist/fs/rootlessfs.go new file mode 100644 index 0000000000..19fec2b9bc --- /dev/null +++ b/virtcontainers/persist/fs/rootlessfs.go @@ -0,0 +1,48 @@ +// Copyright (c) 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package fs + +import ( + "fmt" + "os" + "path/filepath" + + persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" +) + +// default xdg runtime directory just in case XDG_RUNTIME_DIR is not set +var defaultXdgRuntimeDir = fmt.Sprintf("/run/user/%d", os.Getuid()) + +type RootlessFS struct { + // inherit from FS. Overwrite if needed. + *FS +} + +func RootlessInit() (persistapi.PersistDriver, error) { + driver, err := Init() + if err != nil { + return nil, fmt.Errorf("Could not create Rootless FS driver: %v", err) + } + + fsDriver, ok := driver.(*FS) + if !ok { + return nil, fmt.Errorf("Could not create Rootless FS driver") + } + + // XDG_RUNTIME_DIR defines the base directory relative to + // which user-specific non-essential runtime files are stored. + rootlessDir := os.Getenv("XDG_RUNTIME_DIR") + if rootlessDir == "" { + rootlessDir = defaultXdgRuntimeDir + fsLog.WithField("default-runtime-dir", defaultXdgRuntimeDir). + Warnf("XDG_RUNTIME_DIR variable is not set. Using default runtime directory") + } + + fsDriver.storageRootPath = filepath.Join(rootlessDir, fsDriver.storageRootPath) + fsDriver.driverName = "rootlessfs" + + return &RootlessFS{fsDriver}, nil +} From dd2762fdadeea6c0e12d71e33ee591f702ffcd8f Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 19:42:47 +0000 Subject: [PATCH 05/12] virtcontainers/persist: introduce mock fs driver Mock FS driver can be used in unit testing to allow Mock fs driver inherits from FS and may overwrite its methods. All files and directories created by this driver are under a path accessible for all users, this path is created under the system temporal directory. Signed-off-by: Julio Montes --- virtcontainers/persist/fs/mockfs.go | 52 +++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 virtcontainers/persist/fs/mockfs.go diff --git a/virtcontainers/persist/fs/mockfs.go b/virtcontainers/persist/fs/mockfs.go new file mode 100644 index 0000000000..bbbc27d160 --- /dev/null +++ b/virtcontainers/persist/fs/mockfs.go @@ -0,0 +1,52 @@ +// Copyright (c) 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package fs + +import ( + "fmt" + "os" + "path/filepath" + + persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" +) + +type MockFS struct { + // inherit from FS. Overwrite if needed. + *FS +} + +func MockStorageRootPath() string { + return filepath.Join(os.TempDir(), "vc", "mockfs") +} + +func MockRunStoragePath() string { + return filepath.Join(MockStorageRootPath(), sandboxPathSuffix) +} + +func MockRunVMStoragePath() string { + return filepath.Join(MockStorageRootPath(), vmPathSuffix) +} + +func MockStorageDestroy() { + os.RemoveAll(MockStorageRootPath()) +} + +func MockFSInit() (persistapi.PersistDriver, error) { + driver, err := Init() + if err != nil { + return nil, fmt.Errorf("Could not create Mock FS driver: %v", err) + } + + fsDriver, ok := driver.(*FS) + if !ok { + return nil, fmt.Errorf("Could not create Mock FS driver") + } + + fsDriver.storageRootPath = MockStorageRootPath() + fsDriver.driverName = "mockfs" + + return &MockFS{fsDriver}, nil +} From 71f48a33645975c96dcb50fe13c0dc04a789c2e1 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 20:13:14 +0000 Subject: [PATCH 06/12] virtcontainers/persist: update `GetDriver` to support rootless fs GetDriver returns new PersistDriver according to current needs, a mock fs driver is returned when mockTesting is enabled, a rootless fs is returned when rootless is detected, otherwise a fs driver is used. Signed-off-by: Julio Montes --- virtcontainers/persist/manager.go | 44 ++++++++++++++++++++++++-- virtcontainers/persist/manager_test.go | 41 ++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/virtcontainers/persist/manager.go b/virtcontainers/persist/manager.go index 7e46b36bce..228b835953 100644 --- a/virtcontainers/persist/manager.go +++ b/virtcontainers/persist/manager.go @@ -1,4 +1,5 @@ // Copyright (c) 2019 Huawei Corporation +// Copyright (c) 2020 Intel Corporation // // SPDX-License-Identifier: Apache-2.0 // @@ -9,12 +10,18 @@ import ( "fmt" exp "github.com/kata-containers/runtime/virtcontainers/experimental" - "github.com/kata-containers/runtime/virtcontainers/persist/api" + persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" "github.com/kata-containers/runtime/virtcontainers/persist/fs" + "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" ) type initFunc (func() (persistapi.PersistDriver, error)) +const ( + RootFSName = "fs" + RootlessFSName = "rootlessfs" +) + var ( // NewStoreFeature is an experimental feature NewStoreFeature = exp.Feature{ @@ -25,16 +32,22 @@ var ( expErr error supportedDrivers = map[string]initFunc{ - "fs": fs.Init, + RootFSName: fs.Init, + RootlessFSName: fs.RootlessInit, } + mockTesting = false ) func init() { expErr = exp.Register(NewStoreFeature) } +func EnableMockTesting() { + mockTesting = true +} + // GetDriver returns new PersistDriver according to driver name -func GetDriver(name string) (persistapi.PersistDriver, error) { +func GetDriverByName(name string) (persistapi.PersistDriver, error) { if expErr != nil { return nil, expErr } @@ -45,3 +58,28 @@ func GetDriver(name string) (persistapi.PersistDriver, error) { return nil, fmt.Errorf("failed to get storage driver %q", name) } + +// GetDriver returns new PersistDriver according to current needs. +// For example, a rootless FS driver is returned if the process is running +// as unprivileged process. +func GetDriver() (persistapi.PersistDriver, error) { + if expErr != nil { + return nil, expErr + } + + if mockTesting { + return fs.MockFSInit() + } + + if rootless.IsRootless() { + if f, ok := supportedDrivers[RootlessFSName]; ok { + return f() + } + } + + if f, ok := supportedDrivers[RootFSName]; ok { + return f() + } + + return nil, fmt.Errorf("Could not find a FS driver") +} diff --git a/virtcontainers/persist/manager_test.go b/virtcontainers/persist/manager_test.go index f0d5b0383e..bd74b36657 100644 --- a/virtcontainers/persist/manager_test.go +++ b/virtcontainers/persist/manager_test.go @@ -1,4 +1,5 @@ // Copyright (c) 2019 Huawei Corporation +// Copyright (c) 2020 Intel Corporation // // SPDX-License-Identifier: Apache-2.0 // @@ -6,17 +7,51 @@ package persist import ( + "os" "testing" + persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" + "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/stretchr/testify/assert" ) -func TestGetDriver(t *testing.T) { - nonexist, err := GetDriver("non-exist") +func TestGetDriverByName(t *testing.T) { + nonexist, err := GetDriverByName("non-exist") assert.NotNil(t, err) assert.Nil(t, nonexist) - fsDriver, err := GetDriver("fs") + fsDriver, err := GetDriverByName("fs") assert.Nil(t, err) assert.NotNil(t, fsDriver) } + +func TestGetDriver(t *testing.T) { + assert := assert.New(t) + orgMockTesting := mockTesting + defer func() { + mockTesting = orgMockTesting + }() + + mockTesting = false + + fsd, err := GetDriver() + assert.NoError(err) + + var expectedFS persistapi.PersistDriver + if os.Getuid() != 0 { + expectedFS, err = fs.RootlessInit() + } else { + expectedFS, err = fs.Init() + } + + assert.NoError(err) + assert.Equal(expectedFS, fsd) + + // Testing mock driver + mockTesting = true + fsd, err = GetDriver() + assert.NoError(err) + expectedFS, err = fs.MockFSInit() + assert.NoError(err) + assert.Equal(expectedFS, fsd) +} From 4b9ab557c8fad2942b473e88f35395913e509722 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 20:39:34 +0000 Subject: [PATCH 07/12] virtcontainers/factory: support new persist API Fix factory implementation and unit tests to support the new persist API Signed-off-by: Julio Montes --- virtcontainers/factory/cache/cache_test.go | 21 +------ virtcontainers/factory/direct/direct_test.go | 24 +------- virtcontainers/factory/factory_test.go | 59 ++++--------------- .../factory/template/template_test.go | 13 +--- 4 files changed, 19 insertions(+), 98 deletions(-) diff --git a/virtcontainers/factory/cache/cache_test.go b/virtcontainers/factory/cache/cache_test.go index 8489fe42d9..0048d77e56 100644 --- a/virtcontainers/factory/cache/cache_test.go +++ b/virtcontainers/factory/cache/cache_test.go @@ -7,9 +7,6 @@ package cache import ( "context" - "io/ioutil" - "os" - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -19,18 +16,11 @@ import ( "github.com/kata-containers/runtime/virtcontainers/persist/fs" ) -var rootPathSave = fs.StorageRootPath() - func TestTemplateFactory(t *testing.T) { assert := assert.New(t) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() + testDir := fs.MockStorageRootPath() + defer fs.MockStorageDestroy() hyperConfig := vc.HypervisorConfig{ KernelPath: testDir, @@ -45,13 +35,6 @@ func TestTemplateFactory(t *testing.T) { ctx := context.Background() - runPathSave := fs.RunStoragePath() - fs.TestSetRunStoragePath(filepath.Join(testDir, "vc", "run")) - // allow the tests to run without affecting the host system. - defer func() { - fs.TestSetRunStoragePath(runPathSave) - }() - // New f := New(ctx, 2, direct.New(ctx, vmConfig)) diff --git a/virtcontainers/factory/direct/direct_test.go b/virtcontainers/factory/direct/direct_test.go index 20eec0eb89..6f02b53204 100644 --- a/virtcontainers/factory/direct/direct_test.go +++ b/virtcontainers/factory/direct/direct_test.go @@ -7,9 +7,6 @@ package direct import ( "context" - "io/ioutil" - "os" - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -18,28 +15,11 @@ import ( "github.com/kata-containers/runtime/virtcontainers/persist/fs" ) -var rootPathSave = fs.StorageRootPath() - func TestTemplateFactory(t *testing.T) { assert := assert.New(t) - testDir, err := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() - - assert.Nil(err) - - runPathSave := fs.RunStoragePath() - fs.TestSetRunStoragePath(filepath.Join(testDir, "vc", "run")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetRunStoragePath(runPathSave) - }() + testDir := fs.MockStorageRootPath() + defer fs.MockStorageDestroy() hyperConfig := vc.HypervisorConfig{ KernelPath: testDir, diff --git a/virtcontainers/factory/factory_test.go b/virtcontainers/factory/factory_test.go index 91e0a2c32f..ab42423e38 100644 --- a/virtcontainers/factory/factory_test.go +++ b/virtcontainers/factory/factory_test.go @@ -7,9 +7,7 @@ package factory import ( "context" - "io/ioutil" "os" - "path/filepath" "testing" vc "github.com/kata-containers/runtime/virtcontainers" @@ -22,8 +20,6 @@ import ( const testDisabledAsNonRoot = "Test disabled as requires root privileges" -var rootPathSave = fs.StorageRootPath() - func TestNewFactory(t *testing.T) { var config Config @@ -44,17 +40,10 @@ func TestNewFactory(t *testing.T) { _, err = NewFactory(ctx, config, false) assert.Error(err) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() - + defer fs.MockStorageDestroy() config.VMConfig.HypervisorConfig = vc.HypervisorConfig{ - KernelPath: testDir, - ImagePath: testDir, + KernelPath: fs.MockStorageRootPath(), + ImagePath: fs.MockStorageRootPath(), } // direct @@ -71,7 +60,7 @@ func TestNewFactory(t *testing.T) { } config.Template = true - config.TemplatePath = testDir + config.TemplatePath = fs.MockStorageRootPath() f, err = NewFactory(ctx, config, false) assert.Nil(err) f.CloseFactory(ctx) @@ -119,19 +108,12 @@ func TestFactorySetLogger(t *testing.T) { func TestVMConfigValid(t *testing.T) { assert := assert.New(t) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() - + defer fs.MockStorageDestroy() config := vc.VMConfig{ HypervisorType: vc.MockHypervisor, HypervisorConfig: vc.HypervisorConfig{ - KernelPath: testDir, - ImagePath: testDir, + KernelPath: fs.MockStorageRootPath(), + ImagePath: fs.MockStorageRootPath(), }, } @@ -174,13 +156,8 @@ func TestCheckVMConfig(t *testing.T) { err = checkVMConfig(config1, config2) assert.Nil(err) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() + testDir := fs.MockStorageRootPath() + defer fs.MockStorageDestroy() config1.HypervisorConfig = vc.HypervisorConfig{ KernelPath: testDir, @@ -200,13 +177,8 @@ func TestCheckVMConfig(t *testing.T) { func TestFactoryGetVM(t *testing.T) { assert := assert.New(t) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() + testDir := fs.MockStorageRootPath() + defer fs.MockStorageDestroy() hyperConfig := vc.HypervisorConfig{ KernelPath: testDir, @@ -366,13 +338,8 @@ func TestDeepCompare(t *testing.T) { AgentType: vc.NoopAgentType, ProxyType: vc.NoopProxyType, } - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() + testDir := fs.MockStorageRootPath() + defer fs.MockStorageDestroy() config.VMConfig.HypervisorConfig = vc.HypervisorConfig{ KernelPath: testDir, diff --git a/virtcontainers/factory/template/template_test.go b/virtcontainers/factory/template/template_test.go index 150d81e2b6..d1c24f75ef 100644 --- a/virtcontainers/factory/template/template_test.go +++ b/virtcontainers/factory/template/template_test.go @@ -7,9 +7,7 @@ package template import ( "context" - "io/ioutil" "os" - "path/filepath" "testing" "time" @@ -21,8 +19,6 @@ import ( const testDisabledAsNonRoot = "Test disabled as requires root privileges" -var rootPathSave = fs.StorageRootPath() - func TestTemplateFactory(t *testing.T) { if os.Geteuid() != 0 { t.Skip(testDisabledAsNonRoot) @@ -32,13 +28,8 @@ func TestTemplateFactory(t *testing.T) { templateWaitForAgent = 1 * time.Microsecond - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - os.RemoveAll(testDir) - fs.TestSetStorageRootPath(rootPathSave) - }() + testDir := fs.MockStorageRootPath() + defer fs.MockStorageDestroy() hyperConfig := vc.HypervisorConfig{ KernelPath: testDir, From 00307a70eeae5a7cc8c21ff9cc502c38fdf75188 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 20:41:56 +0000 Subject: [PATCH 08/12] virtcontainers/sandbox: support new persist API Fix sandbox implementation and unit tests to support the new persist API Signed-off-by: Julio Montes --- virtcontainers/persist.go | 2 +- virtcontainers/persist_test.go | 2 +- virtcontainers/sandbox.go | 6 +++--- virtcontainers/sandbox_test.go | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index ceff379600..bf4d4bd09a 100644 --- a/virtcontainers/persist.go +++ b/virtcontainers/persist.go @@ -451,7 +451,7 @@ func (c *Container) Restore() error { } func loadSandboxConfig(id string) (*SandboxConfig, error) { - store, err := persist.GetDriver("fs") + store, err := persist.GetDriver() if err != nil || store == nil { return nil, errors.New("failed to get fs persist driver") } diff --git a/virtcontainers/persist_test.go b/virtcontainers/persist_test.go index 03b6252f41..b7b0184bf8 100644 --- a/virtcontainers/persist_test.go +++ b/virtcontainers/persist_test.go @@ -37,7 +37,7 @@ func TestSandboxRestore(t *testing.T) { config: &sconfig, } - sandbox.newStore, err = persist.GetDriver("fs") + sandbox.newStore, err = persist.GetDriver() assert.NoError(err) assert.NotNil(sandbox.newStore) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 9f5f831b04..04259bc808 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -538,7 +538,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor ctx: ctx, } - if s.newStore, err = persist.GetDriver("fs"); err != nil || s.newStore == nil { + if s.newStore, err = persist.GetDriver(); err != nil || s.newStore == nil { return nil, fmt.Errorf("failed to get fs persist driver: %v", err) } @@ -622,7 +622,7 @@ func (s *Sandbox) storeSandbox() error { } func rLockSandbox(sandboxID string) (func() error, error) { - store, err := persist.GetDriver("fs") + store, err := persist.GetDriver() if err != nil { return nil, fmt.Errorf("failed to get fs persist driver: %v", err) } @@ -631,7 +631,7 @@ func rLockSandbox(sandboxID string) (func() error, error) { } func rwLockSandbox(sandboxID string) (func() error, error) { - store, err := persist.GetDriver("fs") + store, err := persist.GetDriver() if err != nil { return nil, fmt.Errorf("failed to get fs persist driver: %v", err) } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index f47ad857a0..58d8874cdc 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -990,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 := filepath.Join(fs.RunStoragePath(), testSandboxID, contID) + storePath := filepath.Join(p.newStore.RunStoragePath(), testSandboxID, contID) _, err = os.Stat(storePath) assert.NotNil(t, err, "Should delete configuration root after failed to create a container") } @@ -1160,7 +1160,7 @@ func TestAttachBlockDevice(t *testing.T) { } // create state file - path := filepath.Join(fs.RunStoragePath(), testSandboxID, container.ID()) + path := filepath.Join(fs.MockRunStoragePath(), testSandboxID, container.ID()) err := os.MkdirAll(path, DirMode) assert.NoError(t, err) @@ -1238,7 +1238,7 @@ func TestPreAddDevice(t *testing.T) { container.state.State = types.StateReady // create state file - path := filepath.Join(fs.RunStoragePath(), testSandboxID, container.ID()) + path := filepath.Join(fs.MockRunStoragePath(), testSandboxID, container.ID()) err := os.MkdirAll(path, DirMode) assert.NoError(t, err) From 9585bc929ae4245a045abaccfeb0be448e44e492 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 20:48:13 +0000 Subject: [PATCH 09/12] virtcontainers/hypervisors: support new persist API Fix hypervisor implementations and unit tests to support the new persist API Signed-off-by: Julio Montes --- virtcontainers/acrn.go | 35 +++++----------------- virtcontainers/acrn_arch_base_test.go | 2 +- virtcontainers/acrn_test.go | 20 +++++++++---- virtcontainers/clh.go | 18 +++++------ virtcontainers/clh_test.go | 43 ++++++++++++++++----------- virtcontainers/hypervisor.go | 23 ++++++++++---- virtcontainers/hypervisor_test.go | 10 ++----- virtcontainers/qemu.go | 21 ++++++------- virtcontainers/qemu_arch_base_test.go | 2 +- virtcontainers/qemu_test.go | 42 ++++++++++++++++++-------- virtcontainers/vm.go | 13 ++++---- 11 files changed, 123 insertions(+), 106 deletions(-) diff --git a/virtcontainers/acrn.go b/virtcontainers/acrn.go index 099de8e253..761eda0379 100644 --- a/virtcontainers/acrn.go +++ b/virtcontainers/acrn.go @@ -22,10 +22,7 @@ import ( "github.com/sirupsen/logrus" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/persist" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" - "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -39,17 +36,6 @@ const ( uuidFile = "uuid.json" ) -// VMUUIDStoragePath is the uuid directory. -// It will contain all uuid info used by guest vm. -var VMUUIDStoragePath = func() string { - path := filepath.Join(fs.StorageRootPath(), UUIDPathSuffix) - if rootless.IsRootless() { - return filepath.Join(rootless.GetRootlessDir(), path) - } - return path - -} - // ACRN currently supports only known UUIDs for security // reasons (FuSa). When launching VM, only these pre-defined // UUID should be used else VM launch will fail. The main @@ -101,6 +87,7 @@ type Acrn struct { info AcrnInfo arch acrnArch ctx context.Context + store persistapi.PersistDriver } type acrnPlatformInfo struct { @@ -328,7 +315,7 @@ func (a *Acrn) setup(id string, hypervisorConfig *HypervisorConfig) error { // The path might already exist, but in case of VM templating, // we have to create it since the sandbox has not created it yet. - if err = os.MkdirAll(filepath.Join(fs.RunStoragePath(), id), DirMode); err != nil { + if err = os.MkdirAll(filepath.Join(a.store.RunStoragePath(), id), DirMode); err != nil { return err } @@ -444,7 +431,7 @@ func (a *Acrn) startSandbox(timeoutSecs int) error { a.Logger().WithField("default-kernel-parameters", formatted).Debug() } - vmPath := filepath.Join(fs.RunVMStoragePath(), a.id) + vmPath := filepath.Join(a.store.RunVMStoragePath(), a.id) err := os.MkdirAll(vmPath, DirMode) if err != nil { return err @@ -658,7 +645,7 @@ func (a *Acrn) getSandboxConsole(id string) (string, error) { span, _ := a.trace("getSandboxConsole") defer span.Finish() - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, acrnConsoleSocket) + return utils.BuildSocketPath(a.store.RunVMStoragePath(), id, acrnConsoleSocket) } func (a *Acrn) saveSandbox() error { @@ -734,7 +721,7 @@ func (a *Acrn) check() error { } func (a *Acrn) generateSocket(id string, useVsock bool) (interface{}, error) { - return generateVMSocket(id, useVsock) + return generateVMSocket(id, useVsock, a.store.RunVMStoragePath()) } // GetACRNUUIDBytes returns UUID bytes that is used for VM creation @@ -797,10 +784,6 @@ func (a *Acrn) GetMaxSupportedACRNVM() (uint8, error) { } func (a *Acrn) storeInfo() error { - store, err := persist.GetDriver("fs") - if err != nil { - return err - } relPath := filepath.Join(UUIDPathSuffix, uuidFile) jsonOut, err := json.Marshal(a.info) @@ -808,7 +791,7 @@ func (a *Acrn) storeInfo() error { return fmt.Errorf("Could not marshal data: %s", err) } - if err := store.GlobalWrite(relPath, jsonOut); err != nil { + if err := a.store.GlobalWrite(relPath, jsonOut); err != nil { return fmt.Errorf("failed to write uuid to file: %v", err) } @@ -816,13 +799,9 @@ func (a *Acrn) storeInfo() error { } func (a *Acrn) loadInfo() error { - store, err := persist.GetDriver("fs") - if err != nil { - return err - } relPath := filepath.Join(UUIDPathSuffix, uuidFile) - data, err := store.GlobalRead(relPath) + data, err := a.store.GlobalRead(relPath) if err != nil { return fmt.Errorf("failed to read uuid from file: %v", err) } diff --git a/virtcontainers/acrn_arch_base_test.go b/virtcontainers/acrn_arch_base_test.go index 9c9c887596..f29487ab01 100644 --- a/virtcontainers/acrn_arch_base_test.go +++ b/virtcontainers/acrn_arch_base_test.go @@ -107,7 +107,7 @@ func TestAcrnArchBaseAppendConsoles(t *testing.T) { assert := assert.New(t) acrnArchBase := newAcrnArchBase() - path := filepath.Join(filepath.Join(fs.RunStoragePath(), sandboxID), consoleSocket) + path := filepath.Join(filepath.Join(fs.MockRunStoragePath(), sandboxID), consoleSocket) expectedOut := []Device{ ConsoleDevice{ diff --git a/virtcontainers/acrn_test.go b/virtcontainers/acrn_test.go index 52cba5521a..32e61c7253 100644 --- a/virtcontainers/acrn_test.go +++ b/virtcontainers/acrn_test.go @@ -12,7 +12,7 @@ import ( "testing" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" + "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/stretchr/testify/assert" ) @@ -194,11 +194,16 @@ func TestAcrnUpdateBlockDeviceInvalidIdx(t *testing.T) { func TestAcrnGetSandboxConsole(t *testing.T) { assert := assert.New(t) + + store, err := persist.GetDriver() + assert.NoError(err) + a := &Acrn{ - ctx: context.Background(), + ctx: context.Background(), + store: store, } sandboxID := "testSandboxID" - expected := filepath.Join(fs.RunVMStoragePath(), sandboxID, consoleSocket) + expected := filepath.Join(a.store.RunVMStoragePath(), sandboxID, consoleSocket) result, err := a.getSandboxConsole(sandboxID) assert.NoError(err) @@ -208,7 +213,12 @@ func TestAcrnGetSandboxConsole(t *testing.T) { func TestAcrnCreateSandbox(t *testing.T) { assert := assert.New(t) acrnConfig := newAcrnConfig() - a := &Acrn{} + store, err := persist.GetDriver() + assert.NoError(err) + + a := &Acrn{ + store: store, + } sandbox := &Sandbox{ ctx: context.Background(), @@ -218,7 +228,7 @@ func TestAcrnCreateSandbox(t *testing.T) { }, } - err := globalSandboxList.addSandbox(sandbox) + err = globalSandboxList.addSandbox(sandbox) assert.NoError(err) defer globalSandboxList.removeSandbox(sandbox.id) diff --git a/virtcontainers/clh.go b/virtcontainers/clh.go index f292f511c4..e2dbda1a43 100644 --- a/virtcontainers/clh.go +++ b/virtcontainers/clh.go @@ -21,7 +21,6 @@ import ( "time" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" chclient "github.com/kata-containers/runtime/virtcontainers/pkg/cloud-hypervisor/client" opentracing "github.com/opentracing/opentracing-go" "github.com/pkg/errors" @@ -111,6 +110,7 @@ type cloudHypervisor struct { vmconfig chclient.VmConfig cmdOutput bytes.Buffer virtiofsd Virtiofsd + store persistapi.PersistDriver } var clhKernelParams = []Param{ @@ -303,7 +303,7 @@ func (clh *cloudHypervisor) startSandbox(timeout int) error { clh.Logger().WithField("function", "startSandbox").Info("starting Sandbox") - vmPath := filepath.Join(fs.RunVMStoragePath(), clh.id) + vmPath := filepath.Join(clh.store.RunVMStoragePath(), clh.id) err := os.MkdirAll(vmPath, DirMode) if err != nil { return err @@ -604,23 +604,23 @@ func (clh *cloudHypervisor) generateSocket(id string, useVsock bool) (interface{ } func (clh *cloudHypervisor) virtioFsSocketPath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, virtioFsSocket) + return utils.BuildSocketPath(clh.store.RunVMStoragePath(), id, virtioFsSocket) } func (clh *cloudHypervisor) vsockSocketPath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, clhSocket) + return utils.BuildSocketPath(clh.store.RunVMStoragePath(), id, clhSocket) } func (clh *cloudHypervisor) serialPath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, clhSerial) + return utils.BuildSocketPath(clh.store.RunVMStoragePath(), id, clhSerial) } func (clh *cloudHypervisor) apiSocketPath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, clhAPISocket) + return utils.BuildSocketPath(clh.store.RunVMStoragePath(), id, clhAPISocket) } func (clh *cloudHypervisor) logFilePath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, clhLogFile) + return utils.BuildSocketPath(clh.store.RunVMStoragePath(), id, clhLogFile) } func (clh *cloudHypervisor) waitVMM(timeout uint) error { @@ -999,7 +999,7 @@ func (clh *cloudHypervisor) cleanupVM(force bool) error { } // cleanup vm path - dir := filepath.Join(fs.RunVMStoragePath(), clh.id) + dir := filepath.Join(clh.store.RunVMStoragePath(), clh.id) // If it's a symlink, remove both dir and the target. link, err := filepath.EvalSymlinks(dir) @@ -1028,7 +1028,7 @@ func (clh *cloudHypervisor) cleanupVM(force bool) error { } if clh.config.VMid != "" { - dir = filepath.Join(fs.RunStoragePath(), clh.config.VMid) + dir = filepath.Join(clh.store.RunStoragePath(), clh.config.VMid) if err := os.RemoveAll(dir); err != nil { if !force { return err diff --git a/virtcontainers/clh_test.go b/virtcontainers/clh_test.go index 0e96186f01..2299a65c62 100644 --- a/virtcontainers/clh_test.go +++ b/virtcontainers/clh_test.go @@ -13,7 +13,7 @@ import ( "testing" "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" + "github.com/kata-containers/runtime/virtcontainers/persist" chclient "github.com/kata-containers/runtime/virtcontainers/pkg/cloud-hypervisor/client" "github.com/pkg/errors" "github.com/stretchr/testify/assert" @@ -172,33 +172,32 @@ func TestCloudHypervisorBootVM(t *testing.T) { } func TestCloudHypervisorCleanupVM(t *testing.T) { - clh := &cloudHypervisor{} + assert := assert.New(t) + store, err := persist.GetDriver() + assert.NoError(err, "persist.GetDriver() unexpected error") - if err := clh.cleanupVM(true); err == nil { - t.Errorf("cloudHypervisor.cleanupVM() expected error != %v", err) + clh := &cloudHypervisor{ + store: store, } + err = clh.cleanupVM(true) + assert.Error(err, "persist.GetDriver() expected error") + clh.id = "cleanVMID" - if err := clh.cleanupVM(true); err != nil { - t.Errorf("cloudHypervisor.cleanupVM() expected error != %v", err) - } + err = clh.cleanupVM(true) + assert.NoError(err, "persist.GetDriver() unexpected error") - dir := filepath.Join(fs.RunVMStoragePath(), clh.id) + dir := filepath.Join(clh.store.RunVMStoragePath(), clh.id) os.MkdirAll(dir, os.ModePerm) - if err := clh.cleanupVM(false); err != nil { - t.Errorf("cloudHypervisor.cleanupVM() expected error != %v", err) - } - _, err := os.Stat(dir) + err = clh.cleanupVM(false) + assert.NoError(err, "persist.GetDriver() unexpected error") - if err == nil { - t.Errorf("dir should not exist %s", dir) - } + _, err = os.Stat(dir) + assert.Error(err, "dir should not exist %s", dir) - if !os.IsNotExist(err) { - t.Errorf("Unexpected error = %v", err) - } + assert.True(os.IsNotExist(err), "persist.GetDriver() unexpected error") } func TestClhCreateSandbox(t *testing.T) { @@ -207,8 +206,12 @@ func TestClhCreateSandbox(t *testing.T) { clhConfig, err := newClhConfig() assert.NoError(err) + store, err := persist.GetDriver() + assert.NoError(err) + clh := &cloudHypervisor{ config: clhConfig, + store: store, } sandbox := &Sandbox{ @@ -229,10 +232,14 @@ func TestClooudHypervisorStartSandbox(t *testing.T) { clhConfig, err := newClhConfig() assert.NoError(err) + store, err := persist.GetDriver() + assert.NoError(err) + clh := &cloudHypervisor{ config: clhConfig, APIClient: &clhClientMock{}, virtiofsd: &virtiofsdMock{}, + store: store, } err = clh.startSandbox(10) diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index c0b657eda8..8b6a6cb8ac 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -16,8 +16,8 @@ import ( "strings" "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/persist" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" ) @@ -193,15 +193,26 @@ func (hType *HypervisorType) String() string { // newHypervisor returns an hypervisor from and hypervisor type. func newHypervisor(hType HypervisorType) (hypervisor, error) { + store, err := persist.GetDriver() + if err != nil { + return nil, err + } + switch hType { case QemuHypervisor: - return &qemu{}, nil + return &qemu{ + store: store, + }, nil case FirecrackerHypervisor: return &firecracker{}, nil case AcrnHypervisor: - return &Acrn{}, nil + return &Acrn{ + store: store, + }, nil case ClhHypervisor: - return &cloudHypervisor{}, nil + return &cloudHypervisor{ + store: store, + }, nil case MockHypervisor: return &mockHypervisor{}, nil default: @@ -713,7 +724,7 @@ func getHypervisorPid(h hypervisor) int { return pids[0] } -func generateVMSocket(id string, useVsock bool) (interface{}, error) { +func generateVMSocket(id string, useVsock bool, vmStogarePath string) (interface{}, error) { if useVsock { vhostFd, contextID, err := utils.FindContextID() if err != nil { @@ -727,7 +738,7 @@ func generateVMSocket(id string, useVsock bool) (interface{}, error) { }, nil } - path, err := utils.BuildSocketPath(filepath.Join(fs.RunVMStoragePath(), id), defaultSocketName) + path, err := utils.BuildSocketPath(filepath.Join(vmStogarePath, id), defaultSocketName) if err != nil { return nil, err } diff --git a/virtcontainers/hypervisor_test.go b/virtcontainers/hypervisor_test.go index 7cc97f2bf6..6f91287aa1 100644 --- a/virtcontainers/hypervisor_test.go +++ b/virtcontainers/hypervisor_test.go @@ -72,12 +72,6 @@ func testNewHypervisorFromHypervisorType(t *testing.T, hypervisorType Hypervisor assert.Exactly(hy, expected) } -func TestNewHypervisorFromQemuHypervisorType(t *testing.T) { - hypervisorType := QemuHypervisor - expectedHypervisor := &qemu{} - testNewHypervisorFromHypervisorType(t, hypervisorType, expectedHypervisor) -} - func TestNewHypervisorFromMockHypervisorType(t *testing.T) { hypervisorType := MockHypervisor expectedHypervisor := &mockHypervisor{} @@ -441,7 +435,7 @@ func genericTestRunningOnVMM(t *testing.T, data []testNestedVMMData) { func TestGenerateVMSocket(t *testing.T) { assert := assert.New(t) - s, err := generateVMSocket("a", false) + s, err := generateVMSocket("a", false, "") assert.NoError(err) socket, ok := s.(types.Socket) assert.True(ok) @@ -453,7 +447,7 @@ func TestGenerateVMSocket(t *testing.T) { if tc.NotValid(ktu.NeedRoot()) { t.Skip(testDisabledAsNonRoot) } - s, err = generateVMSocket("a", true) + s, err = generateVMSocket("a", true, "") assert.NoError(err) vsock, ok := s.(types.VSock) assert.True(ok) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 197ab203bb..002b208294 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -32,7 +32,6 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -97,6 +96,8 @@ type qemu struct { nvdimmCount int stopped bool + + store persistapi.PersistDriver } const ( @@ -271,7 +272,7 @@ func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig) error { // The path might already exist, but in case of VM templating, // we have to create it since the sandbox has not created it yet. - if err = os.MkdirAll(filepath.Join(fs.RunStoragePath(), id), DirMode); err != nil { + if err = os.MkdirAll(filepath.Join(q.store.RunStoragePath(), id), DirMode); err != nil { return err } } @@ -326,7 +327,7 @@ func (q *qemu) memoryTopology() (govmmQemu.Memory, error) { } func (q *qemu) qmpSocketPath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, qmpSocket) + return utils.BuildSocketPath(q.store.RunVMStoragePath(), id, qmpSocket) } func (q *qemu) getQemuMachine() (govmmQemu.Machine, error) { @@ -570,7 +571,7 @@ func (q *qemu) createSandbox(ctx context.Context, id string, networkNS NetworkNa VGA: "none", GlobalParam: "kvm-pit.lost_tick_policy=discard", Bios: firmwarePath, - PidFile: filepath.Join(fs.RunVMStoragePath(), q.id, "pid"), + PidFile: filepath.Join(q.store.RunVMStoragePath(), q.id, "pid"), } if ioThread != nil { @@ -599,7 +600,7 @@ func (q *qemu) createSandbox(ctx context.Context, id string, networkNS NetworkNa } func (q *qemu) vhostFSSocketPath(id string) (string, error) { - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, vhostFSSocket) + return utils.BuildSocketPath(q.store.RunVMStoragePath(), id, vhostFSSocket) } func (q *qemu) virtiofsdArgs(fd uintptr) []string { @@ -753,7 +754,7 @@ func (q *qemu) startSandbox(timeout int) error { q.fds = []*os.File{} }() - vmPath := filepath.Join(fs.RunVMStoragePath(), q.id) + vmPath := filepath.Join(q.store.RunVMStoragePath(), q.id) err := os.MkdirAll(vmPath, DirMode) if err != nil { return err @@ -930,7 +931,7 @@ func (q *qemu) stopSandbox() error { func (q *qemu) cleanupVM() error { // cleanup vm path - dir := filepath.Join(fs.RunVMStoragePath(), q.id) + dir := filepath.Join(q.store.RunVMStoragePath(), q.id) // If it's a symlink, remove both dir and the target. // This can happen when vm template links a sandbox to a vm. @@ -951,7 +952,7 @@ func (q *qemu) cleanupVM() error { } if q.config.VMid != "" { - dir = filepath.Join(fs.RunStoragePath(), q.config.VMid) + dir = filepath.Join(q.store.RunStoragePath(), q.config.VMid) if err := os.RemoveAll(dir); err != nil { q.Logger().WithError(err).WithField("path", dir).Warnf("failed to remove vm path") } @@ -1651,7 +1652,7 @@ func (q *qemu) getSandboxConsole(id string) (string, error) { span, _ := q.trace("getSandboxConsole") defer span.Finish() - return utils.BuildSocketPath(fs.RunVMStoragePath(), id, consoleSocket) + return utils.BuildSocketPath(q.store.RunVMStoragePath(), id, consoleSocket) } func (q *qemu) saveSandbox() error { @@ -2135,5 +2136,5 @@ func (q *qemu) check() error { } func (q *qemu) generateSocket(id string, useVsock bool) (interface{}, error) { - return generateVMSocket(id, useVsock) + return generateVMSocket(id, useVsock, q.store.RunVMStoragePath()) } diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index 7a91363b3b..e95dd85ed9 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -260,7 +260,7 @@ func TestQemuArchBaseAppendConsoles(t *testing.T) { assert := assert.New(t) qemuArchBase := newQemuArchBase() - path := filepath.Join(filepath.Join(fs.RunStoragePath(), sandboxID), consoleSocket) + path := filepath.Join(filepath.Join(fs.MockRunStoragePath(), sandboxID), consoleSocket) expectedOut := []govmmQemu.Device{ govmmQemu.SerialDevice{ diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index c4772f6ee3..5c6eec7576 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -17,7 +17,6 @@ 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/persist/fs" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/pkg/errors" "github.com/stretchr/testify/assert" @@ -75,9 +74,13 @@ func TestQemuKernelParameters(t *testing.T) { func TestQemuCreateSandbox(t *testing.T) { qemuConfig := newQemuConfig() - q := &qemu{} assert := assert.New(t) + store, err := persist.GetDriver() + assert.NoError(err) + q := &qemu{ + store: store, + } sandbox := &Sandbox{ ctx: context.Background(), id: "testSandbox", @@ -88,11 +91,11 @@ func TestQemuCreateSandbox(t *testing.T) { // 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 := filepath.Join(fs.RunStoragePath(), sandbox.id) + parentDir := filepath.Join(q.store.RunStoragePath(), sandbox.id) assert.NoError(os.MkdirAll(parentDir, DirMode)) err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, false) @@ -103,9 +106,13 @@ func TestQemuCreateSandbox(t *testing.T) { func TestQemuCreateSandboxMissingParentDirFail(t *testing.T) { qemuConfig := newQemuConfig() - q := &qemu{} assert := assert.New(t) + store, err := persist.GetDriver() + assert.NoError(err) + q := &qemu{ + store: store, + } sandbox := &Sandbox{ ctx: context.Background(), id: "testSandbox", @@ -116,11 +123,11 @@ func TestQemuCreateSandboxMissingParentDirFail(t *testing.T) { // 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 := filepath.Join(fs.RunStoragePath(), sandbox.id) + parentDir := filepath.Join(q.store.RunStoragePath(), sandbox.id) assert.NoError(os.RemoveAll(parentDir)) err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, false) @@ -276,11 +283,14 @@ func TestQemuAddDeviceKataVSOCK(t *testing.T) { func TestQemuGetSandboxConsole(t *testing.T) { assert := assert.New(t) + store, err := persist.GetDriver() + assert.NoError(err) q := &qemu{ - ctx: context.Background(), + ctx: context.Background(), + store: store, } sandboxID := "testSandboxID" - expected := filepath.Join(fs.RunVMStoragePath(), sandboxID, consoleSocket) + expected := filepath.Join(q.store.RunVMStoragePath(), sandboxID, consoleSocket) result, err := q.getSandboxConsole(sandboxID) assert.NoError(err) @@ -415,7 +425,9 @@ func TestQemuFileBackedMem(t *testing.T) { sandbox, err := createQemuSandboxConfig() assert.NoError(err) - q := &qemu{} + q := &qemu{ + store: sandbox.newStore, + } sandbox.config.HypervisorConfig.SharedFS = config.VirtioFS err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, false) assert.NoError(err) @@ -428,7 +440,9 @@ func TestQemuFileBackedMem(t *testing.T) { sandbox, err = createQemuSandboxConfig() assert.NoError(err) - q = &qemu{} + q = &qemu{ + store: sandbox.newStore, + } sandbox.config.HypervisorConfig.BootToBeTemplate = true sandbox.config.HypervisorConfig.SharedFS = config.VirtioFS sandbox.config.HypervisorConfig.MemoryPath = fallbackFileBackedMemDir @@ -442,7 +456,9 @@ func TestQemuFileBackedMem(t *testing.T) { sandbox, err = createQemuSandboxConfig() assert.NoError(err) - q = &qemu{} + q = &qemu{ + store: sandbox.newStore, + } sandbox.config.HypervisorConfig.FileBackedMemRootDir = "/tmp/xyzabc" err = q.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, false) assert.NoError(err) @@ -462,7 +478,7 @@ func createQemuSandboxConfig() (*Sandbox, error) { }, } - newStore, err := persist.GetDriver("fs") + newStore, err := persist.GetDriver() if err != nil { return &Sandbox{}, err } diff --git a/virtcontainers/vm.go b/virtcontainers/vm.go index 812d02082b..cfed806914 100644 --- a/virtcontainers/vm.go +++ b/virtcontainers/vm.go @@ -16,7 +16,6 @@ import ( pb "github.com/kata-containers/runtime/protocols/cache" "github.com/kata-containers/runtime/virtcontainers/persist" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/sirupsen/logrus" ) @@ -159,7 +158,7 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { virtLog.WithField("vm", id).WithField("config", config).Info("create new vm") - store, err := persist.GetDriver("fs") + store, err := persist.GetDriver() if err != nil { return nil, err } @@ -178,7 +177,7 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { // 2. setup agent agent := newAgent(config.AgentType) - vmSharePath := buildVMSharePath(id) + vmSharePath := buildVMSharePath(id, store.RunVMStoragePath()) err = agent.configure(hypervisor, id, vmSharePath, isProxyBuiltIn(config.ProxyType), config.AgentConfig) if err != nil { return nil, err @@ -243,7 +242,7 @@ func NewVMFromGrpc(ctx context.Context, v *pb.GrpcVM, config VMConfig) (*VM, err return nil, err } - store, err := persist.GetDriver("fs") + store, err := persist.GetDriver() if err != nil { return nil, err } @@ -284,8 +283,8 @@ func NewVMFromGrpc(ctx context.Context, v *pb.GrpcVM, config VMConfig) (*VM, err }, nil } -func buildVMSharePath(id string) string { - return filepath.Join(fs.RunVMStoragePath(), id, "shared") +func buildVMSharePath(id string, vmStoragePath string) string { + return filepath.Join(vmStoragePath, id, "shared") } func (v *VM) logger() logrus.FieldLogger { @@ -411,7 +410,7 @@ func (v *VM) assignSandbox(s *Sandbox) error { // - link vm socket from sandbox dir (/run/vc/vm/sbid/) to vm dir (/run/vc/vm/vmid/) // - link 9pfs share path from sandbox dir (/run/kata-containers/shared/sandboxes/sbid/) to vm dir (/run/vc/vm/vmid/shared/) - vmSharePath := buildVMSharePath(v.id) + vmSharePath := buildVMSharePath(v.id, v.store.RunVMStoragePath()) vmSockDir := filepath.Join(v.store.RunVMStoragePath(), v.id) sbSharePath := s.agent.getSharePath(s.id) sbSockDir := filepath.Join(v.store.RunVMStoragePath(), s.id) From 11bd456a89de5beb948c06306e1873bebf3dd905 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 20:52:51 +0000 Subject: [PATCH 10/12] virtcontainers: support new persist API Fix API, container and kata implementations and unit tests to support the new persist API Signed-off-by: Julio Montes --- virtcontainers/api.go | 9 ++- virtcontainers/api_test.go | 98 +++++++++++++++++---------- virtcontainers/container_test.go | 2 +- virtcontainers/kata_agent_test.go | 6 +- virtcontainers/kata_proxy_test.go | 4 +- virtcontainers/proxy.go | 8 ++- virtcontainers/proxy_test.go | 6 +- virtcontainers/virtcontainers_test.go | 20 ++---- 8 files changed, 88 insertions(+), 65 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index 1024d28e44..f54214c925 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -13,7 +13,7 @@ import ( deviceApi "github.com/kata-containers/runtime/virtcontainers/device/api" deviceConfig "github.com/kata-containers/runtime/virtcontainers/device/config" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" + "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/pkg/compatoci" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/types" @@ -308,9 +308,12 @@ func ListSandbox(ctx context.Context) ([]SandboxStatus, error) { span, ctx := trace(ctx, "ListSandbox") defer span.Finish() - sbsdir := fs.RunStoragePath() + store, err := persist.GetDriver() + if err != nil { + return []SandboxStatus{}, err + } - dir, err := os.Open(sbsdir) + dir, err := os.Open(store.RunStoragePath()) if err != nil { if os.IsNotExist(err) { // No sandbox directory is not an error diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 7461ef130a..6d7061614b 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -17,7 +17,6 @@ import ( ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/kata-containers/runtime/virtcontainers/persist" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" "github.com/kata-containers/runtime/virtcontainers/pkg/mock" "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" @@ -75,7 +74,7 @@ func newBasicTestCmd() types.Cmd { } func rmSandboxDir(sid string) error { - store, err := persist.GetDriver("fs") + store, err := persist.GetDriver() if err != nil { return fmt.Errorf("failed to get fs persist driver: %v", err) } @@ -154,7 +153,9 @@ func TestCreateSandboxNoopAgentSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) } @@ -191,7 +192,9 @@ func TestCreateSandboxKataAgentSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) } @@ -218,7 +221,9 @@ func TestDeleteSandboxNoopAgentSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -263,7 +268,9 @@ func TestDeleteSandboxKataAgentSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -279,9 +286,6 @@ func TestDeleteSandboxFailing(t *testing.T) { defer cleanUp() assert := assert.New(t) - sandboxDir := filepath.Join(fs.RunStoragePath(), testSandboxID) - os.Remove(sandboxDir) - p, err := DeleteSandbox(context.Background(), testSandboxID) assert.Error(err) assert.Nil(p) @@ -343,9 +347,6 @@ func TestStartSandboxFailing(t *testing.T) { defer cleanUp() assert := assert.New(t) - sandboxDir := filepath.Join(fs.RunStoragePath(), testSandboxID) - os.Remove(sandboxDir) - p, err := StartSandbox(context.Background(), testSandboxID) assert.Error(err) assert.Nil(p) @@ -410,9 +411,6 @@ func TestStopSandboxKataAgentSuccessful(t *testing.T) { func TestStopSandboxFailing(t *testing.T) { defer cleanUp() - sandboxDir := filepath.Join(fs.RunStoragePath(), testSandboxID) - os.Remove(sandboxDir) - p, err := StopSandbox(context.Background(), testSandboxID, false) assert.Error(t, err) assert.Nil(t, p) @@ -428,7 +426,9 @@ func TestRunSandboxNoopAgentSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) } @@ -466,13 +466,13 @@ func TestRunSandboxKataAgentSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) - _, err = os.Stat(sandboxDir) - assert.NoError(err) - pImpl, ok := p.(*Sandbox) assert.True(ok) + sandboxDir := filepath.Join(pImpl.newStore.RunStoragePath(), p.ID()) + _, err = os.Stat(sandboxDir) + assert.NoError(err) + err = bindUnmountAllRootfs(ctx, testDir, pImpl) assert.NoError(err) } @@ -693,7 +693,9 @@ func TestCreateContainerSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -724,7 +726,9 @@ func TestCreateContainerFailingNoSandbox(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.Error(err) @@ -747,7 +751,9 @@ func TestDeleteContainerSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -791,7 +797,9 @@ func TestDeleteContainerFailingNoContainer(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -848,7 +856,9 @@ func TestStartContainerFailingNoContainer(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -869,7 +879,9 @@ func TestStartContainerFailingSandboxNotStarted(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -949,7 +961,9 @@ func TestStopContainerFailingNoContainer(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -1053,7 +1067,9 @@ func TestEnterContainerFailingNoContainer(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -1106,7 +1122,9 @@ func TestStatusContainerSuccessful(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + pImpl, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(pImpl.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -1123,9 +1141,6 @@ func TestStatusContainerSuccessful(t *testing.T) { status, err := StatusContainer(ctx, p.ID(), contID) assert.NoError(err) - pImpl, ok := p.(*Sandbox) - assert.True(ok) - cImpl, ok := c.(*Container) assert.True(ok) @@ -1149,7 +1164,9 @@ func TestStatusContainerStateReady(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -1212,7 +1229,9 @@ func TestStatusContainerStateRunning(t *testing.T) { assert.NoError(err) assert.NotNil(p) - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) assert.NoError(err) @@ -1424,7 +1443,11 @@ func createAndStartSandbox(ctx context.Context, config SandboxConfig) (sandbox V return nil, "", err } - sandboxDir = filepath.Join(fs.RunStoragePath(), sandbox.ID()) + s, ok := sandbox.(*Sandbox) + if !ok { + return nil, "", fmt.Errorf("Could not get Sandbox") + } + sandboxDir = filepath.Join(s.newStore.RunStoragePath(), sandbox.ID()) _, err = os.Stat(sandboxDir) if err != nil { return nil, "", err @@ -1682,6 +1705,7 @@ func TestNetworkOperation(t *testing.T) { func TestCleanupContainer(t *testing.T) { config := newTestSandboxConfigNoop() + assert := assert.New(t) ctx := context.Background() @@ -1709,7 +1733,9 @@ func TestCleanupContainer(t *testing.T) { CleanupContainer(ctx, p.ID(), c.ID(), true) } - sandboxDir := filepath.Join(fs.RunStoragePath(), p.ID()) + s, ok := p.(*Sandbox) + assert.True(ok) + sandboxDir := filepath.Join(s.newStore.RunStoragePath(), p.ID()) _, err = os.Stat(sandboxDir) if err == nil { diff --git a/virtcontainers/container_test.go b/virtcontainers/container_test.go index 9b003b66eb..b11e83aff3 100644 --- a/virtcontainers/container_test.go +++ b/virtcontainers/container_test.go @@ -316,7 +316,7 @@ func TestContainerAddDriveDir(t *testing.T) { }, } - sandbox.newStore, err = persist.GetDriver("fs") + sandbox.newStore, err = persist.GetDriver() assert.NoError(err) assert.NotNil(sandbox.newStore) diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index e0d5d0d69d..6d40286659 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -621,8 +621,8 @@ func TestAgentPathAPI(t *testing.T) { id := "foobar" // getSharePath - path1 = k1.getSharePath(id) - path2 = k2.getSharePath(id) + path1 := k1.getSharePath(id) + path2 := k2.getSharePath(id) assert.Equal(path1, path2) } @@ -710,7 +710,7 @@ func TestAgentCreateContainer(t *testing.T) { hypervisor: &mockHypervisor{}, } - newStore, err := persist.GetDriver("fs") + newStore, err := persist.GetDriver() assert.NoError(err) assert.NotNil(newStore) sandbox.newStore = newStore diff --git a/virtcontainers/kata_proxy_test.go b/virtcontainers/kata_proxy_test.go index d14fc7bcb5..b4a889b49e 100644 --- a/virtcontainers/kata_proxy_test.go +++ b/virtcontainers/kata_proxy_test.go @@ -5,9 +5,7 @@ package virtcontainers -import ( - "testing" -) +import "testing" func TestKataProxyStart(t *testing.T) { agent := &kataAgent{} diff --git a/virtcontainers/proxy.go b/virtcontainers/proxy.go index 86de050d92..4614eaf44d 100644 --- a/virtcontainers/proxy.go +++ b/virtcontainers/proxy.go @@ -14,7 +14,7 @@ import ( "strings" kataclient "github.com/kata-containers/agent/protocols/client" - "github.com/kata-containers/runtime/virtcontainers/persist/fs" + "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/sirupsen/logrus" ) @@ -146,7 +146,11 @@ func validateProxyConfig(proxyConfig ProxyConfig) error { func defaultProxyURL(id, socketType string) (string, error) { switch socketType { case SocketTypeUNIX: - socketPath := filepath.Join(filepath.Join(fs.RunStoragePath(), id), "proxy.sock") + store, err := persist.GetDriver() + if err != nil { + return "", err + } + socketPath := filepath.Join(filepath.Join(store.RunStoragePath(), id), "proxy.sock") return fmt.Sprintf("unix://%s", socketPath), nil case SocketTypeVSOCK: // TODO Build the VSOCK default URL diff --git a/virtcontainers/proxy_test.go b/virtcontainers/proxy_test.go index af251154c9..b1899d2ca2 100644 --- a/virtcontainers/proxy_test.go +++ b/virtcontainers/proxy_test.go @@ -173,7 +173,7 @@ func testDefaultProxyURL(expectedURL string, socketType string, sandboxID string } func TestDefaultProxyURLUnix(t *testing.T) { - path := filepath.Join(filepath.Join(fs.RunStoragePath(), sandboxID), "proxy.sock") + path := filepath.Join(filepath.Join(fs.MockRunStoragePath(), sandboxID), "proxy.sock") socketPath := fmt.Sprintf("unix://%s", path) assert.NoError(t, testDefaultProxyURL(socketPath, SocketTypeUNIX, sandboxID)) } @@ -183,7 +183,7 @@ func TestDefaultProxyURLVSock(t *testing.T) { } func TestDefaultProxyURLUnknown(t *testing.T) { - path := filepath.Join(filepath.Join(fs.RunStoragePath(), sandboxID), "proxy.sock") + path := filepath.Join(filepath.Join(fs.MockRunStoragePath(), sandboxID), "proxy.sock") socketPath := fmt.Sprintf("unix://%s", path) assert.Error(t, testDefaultProxyURL(socketPath, "foobar", sandboxID)) } @@ -204,7 +204,7 @@ func testProxyStart(t *testing.T, agent agent, proxy proxy) { } invalidPath := filepath.Join(tmpdir, "enoent") - expectedSocketPath := filepath.Join(filepath.Join(fs.RunStoragePath(), testSandboxID), "proxy.sock") + expectedSocketPath := filepath.Join(filepath.Join(fs.MockRunStoragePath(), testSandboxID), "proxy.sock") expectedURI := fmt.Sprintf("unix://%s", expectedSocketPath) data := []testData{ diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index f7a17bdf34..383310fbc7 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -15,6 +15,7 @@ import ( "path/filepath" "testing" + "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/persist/fs" "github.com/kata-containers/runtime/virtcontainers/store" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -56,8 +57,8 @@ var testHyperstartTtySocket = "" // the next test to run. func cleanUp() { globalSandboxList.removeSandbox(testSandboxID) - os.RemoveAll(fs.RunStoragePath()) - os.RemoveAll(fs.RunVMStoragePath()) + os.RemoveAll(fs.MockRunStoragePath()) + os.RemoveAll(fs.MockRunVMStoragePath()) os.RemoveAll(testDir) os.MkdirAll(testDir, DirMode) @@ -109,6 +110,8 @@ func setupClh() { func TestMain(m *testing.M) { var err error + persist.EnableMockTesting() + flag.Parse() logger := logrus.NewEntry(logrus.New()) @@ -161,19 +164,8 @@ func TestMain(m *testing.M) { setupClh() - // allow the tests to run without affecting the host system. - runPathSave := fs.RunStoragePath() - rootPathSave := fs.StorageRootPath() - fs.TestSetRunStoragePath(filepath.Join(testDir, "vc", "run")) - fs.TestSetStorageRootPath(filepath.Join(testDir, "vc")) - - defer func() { - fs.TestSetRunStoragePath(runPathSave) - fs.TestSetStorageRootPath(rootPathSave) - }() - // set now that configStoragePath has been overridden. - sandboxDirState = filepath.Join(fs.RunStoragePath(), testSandboxID) + sandboxDirState = filepath.Join(fs.MockRunStoragePath(), testSandboxID) testHyperstartCtlSocket = filepath.Join(testDir, "test_hyper.sock") testHyperstartTtySocket = filepath.Join(testDir, "test_tty.sock") From c36c667b10585a5ffa459cbf717067118470f2cc Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 31 Jan 2020 21:26:54 +0000 Subject: [PATCH 11/12] cli: implement --rootless option By default virtcontainer auto-detects if the current process is running rootless or not, but this behavior can change from commandline with the --rootless option fixes #2417 Signed-off-by: Julio Montes --- cli/main.go | 18 +++++++++++ cli/utils.go | 12 ++++++++ virtcontainers/pkg/rootless/rootless.go | 21 +++++++------ virtcontainers/pkg/rootless/rootless_test.go | 32 +++++++++++++++++++- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/cli/main.go b/cli/main.go index d2ca4835e9..948a0c4c4a 100644 --- a/cli/main.go +++ b/cli/main.go @@ -102,6 +102,11 @@ var runtimeFlags = []cli.Flag{ Value: defaultRootDirectory, Usage: "root directory for storage of container state (this should be located in tmpfs)", }, + cli.StringFlag{ + Name: "rootless", + Value: "auto", + Usage: "ignore cgroup permission errors ('true', 'false', or 'auto')", + }, cli.BoolFlag{ Name: showConfigPathsOption, Usage: "show config file paths that will be checked for (in order)", @@ -266,6 +271,19 @@ func beforeSubcommands(c *cli.Context) error { return nil } + r, err := parseBoolOrAuto(c.GlobalString("rootless")) + if err != nil { + return err + } + // If flag is true/false, assign the rootless flag. + // vc will not perform any auto-detection in that case. + // In case flag is nil or auto, vc detects if the runtime is running as rootless. + if r != nil { + rootless.SetRootless(*r) + } + // Support --systed-cgroup + // Issue: https://github.com/kata-containers/runtime/issues/2428 + ignoreConfigLogs := false var traceRootSpan string diff --git a/cli/utils.go b/cli/utils.go index cc996ca3a1..e4298162aa 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -1,3 +1,4 @@ +// Copyright (c) 2014 Docker, Inc. // Copyright (c) 2017 Intel Corporation // // SPDX-License-Identifier: Apache-2.0 @@ -8,6 +9,7 @@ package main import ( "fmt" "os" + "strconv" "strings" "github.com/kata-containers/runtime/pkg/katautils" @@ -131,3 +133,13 @@ func genericGetCPUDetails() (vendor, model string, err error) { return vendor, model, nil } + +// from runC +// parseBoolOrAuto returns (nil, nil) if s is empty or "auto" +func parseBoolOrAuto(s string) (*bool, error) { + if s == "" || strings.ToLower(s) == "auto" { + return nil, nil + } + b, err := strconv.ParseBool(s) + return &b, err +} diff --git a/virtcontainers/pkg/rootless/rootless.go b/virtcontainers/pkg/rootless/rootless.go index c26e3b87f9..5a10ccadb2 100644 --- a/virtcontainers/pkg/rootless/rootless.go +++ b/virtcontainers/pkg/rootless/rootless.go @@ -36,12 +36,9 @@ import ( ) var ( - // initRootless states whether the isRootless variable - // has been set yet - initRootless bool - // isRootless states whether execution is rootless or not - isRootless bool + // If nil, rootless is auto-detected + isRootless *bool // lock for the initRootless and isRootless variables rLock sync.Mutex @@ -58,6 +55,10 @@ var ( IsRootless = isRootlessFunc ) +func SetRootless(rootless bool) { + isRootless = &rootless +} + // SetLogger sets up a logger for the rootless pkg func SetLogger(ctx context.Context, logger *logrus.Entry) { fields := rootlessLog.Data @@ -68,9 +69,9 @@ func SetLogger(ctx context.Context, logger *logrus.Entry) { func isRootlessFunc() bool { rLock.Lock() defer rLock.Unlock() - if !initRootless { - initRootless = true - isRootless = true + // auto-detect if nil + if isRootless == nil { + SetRootless(true) // --rootless and --systemd-cgroup options must honoured // but with the current implementation this is not possible // https://github.com/kata-containers/runtime/issues/2412 @@ -80,9 +81,9 @@ func isRootlessFunc() bool { if system.RunningInUserNS() { return true } - isRootless = false + SetRootless(false) } - return isRootless + return *isRootless } // GetRootlessDir returns the path to the location for rootless diff --git a/virtcontainers/pkg/rootless/rootless_test.go b/virtcontainers/pkg/rootless/rootless_test.go index 46f387d7eb..0933f645a8 100644 --- a/virtcontainers/pkg/rootless/rootless_test.go +++ b/virtcontainers/pkg/rootless/rootless_test.go @@ -1,6 +1,36 @@ -// Copyright (c) 2019 Intel Corporation +// Copyright (c) 2020 Intel Corporation // // SPDX-License-Identifier: Apache-2.0 // package rootless + +import ( + "os" + "testing" + + "github.com/opencontainers/runc/libcontainer/system" + "github.com/stretchr/testify/assert" +) + +func TestIsRootless(t *testing.T) { + assert := assert.New(t) + isRootless = nil + + var rootless bool + if os.Getuid() != 0 { + rootless = true + } else { + rootless = system.RunningInUserNS() + } + + assert.Equal(rootless, isRootlessFunc()) + + SetRootless(true) + assert.True(isRootlessFunc()) + + SetRootless(false) + assert.False(isRootlessFunc()) + + isRootless = nil +} From a45cf62e752cc1e6a12359f046c4c2ab1ca24a5f Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 4 Feb 2020 16:41:17 +0000 Subject: [PATCH 12/12] virtcontainers/pkg/rootless: fix comment on exported var Fix comment on exported var `IsRootless` should be of the form `IsRootless ...` (golint) Signed-off-by: Julio Montes --- virtcontainers/pkg/rootless/rootless.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtcontainers/pkg/rootless/rootless.go b/virtcontainers/pkg/rootless/rootless.go index 5a10ccadb2..507549fa2a 100644 --- a/virtcontainers/pkg/rootless/rootless.go +++ b/virtcontainers/pkg/rootless/rootless.go @@ -65,7 +65,7 @@ func SetLogger(ctx context.Context, logger *logrus.Entry) { rootlessLog = logger.WithFields(fields) } -// IsRootless states whether kata is being ran with root or not +// isRootlessFunc states whether kata is being ran with root or not func isRootlessFunc() bool { rLock.Lock() defer rLock.Unlock()