From f56d70ccd65a45a0cab5cf4b23b54219850695b8 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Sun, 22 Dec 2019 19:41:56 -0800 Subject: [PATCH 1/7] vc: UT should set VCStorePrefix Otherwise we fail to run it with non-root user with errors like: `mkdir /var/lib/vc/uuid: permission denied` Fixes: #2370 Signed-off-by: Peng Tao --- virtcontainers/virtcontainers_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index 516face355..fe96644143 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -64,12 +64,14 @@ func cleanUp() { store.DeleteAll() os.RemoveAll(testDir) os.MkdirAll(testDir, store.DirMode) + store.VCStorePrefix = "" setup() } func setup() { os.Mkdir(filepath.Join(testDir, testBundle), store.DirMode) + store.VCStorePrefix = testDir for _, filename := range []string{testQemuKernelPath, testQemuInitrdPath, testQemuImagePath, testQemuPath} { _, err := os.Create(filename) From 3deb24e5de6369263fa8e353a16e934baf6a412a Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 25 Dec 2019 00:46:01 -0800 Subject: [PATCH 2/7] cli: flush coverage report in defer function Do not flush it atexit(), where the test report file might be already closed and it causes go test failure like: PASS testing: can't write /tmp/go-build146132196/b001/testlog.txt: close /tmp/go-build146132196/b001/testlog.txt: file already closed FAIL github.com/kata-containers/runtime/cli 4.256s Signed-off-by: Peng Tao --- cli/main_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/main_test.go b/cli/main_test.go index bcb613a7be..796d426479 100644 --- a/cli/main_test.go +++ b/cli/main_test.go @@ -160,7 +160,9 @@ func TestMain(m *testing.M) { // Make sure we have the opportunity to flush the coverage report to disk when // terminating the process. - atexit(cover.FlushProfiles) + defer func() { + cover.FlushProfiles() + }() // If the test binary name is kata-runtime.coverage, we've are being asked to // run the coverage-instrumented kata-runtime. From 4c35d0911a28a7b80ff780d5477fb72a0d7e7b29 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 25 Dec 2019 00:56:15 -0800 Subject: [PATCH 3/7] vc: set store RunVMStoragePath for ut Otherwise we fail ut on failures like: === RUN TestCloudHypervisorCleanupVM --- FAIL: TestCloudHypervisorCleanupVM (0.00s) clh_test.go:191: cloudHypervisor.cleanupVM() expected error != open /run/vc/vm: permission denied clh_test.go:200: Unexpected error = stat /run/vc/vm/cleanVMID: permission denied === RUN TestClhCreateSandbox --- PASS: TestClhCreateSandbox (0.00s) === RUN TestClooudHypervisorStartSandbox time="2019-12-25T00:48:47-08:00" level=error msg="trace called before context set" source=virtcontainers subsystem=cloudHypervisor type=bug --- FAIL: TestClooudHypervisorStartSandbox (0.00s) Error Trace: clh_test.go:266 Error: Received unexpected error: mkdir /run/vc/vm: permission denied Signed-off-by: Peng Tao --- virtcontainers/virtcontainers_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index fe96644143..e9af85826d 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -57,21 +57,28 @@ var testVirtiofsdPath = "" var testHyperstartCtlSocket = "" var testHyperstartTtySocket = "" +var savedRunVMStoragePathFunc func() string + // cleanUp Removes any stale sandbox/container state that can affect // the next test to run. func cleanUp() { globalSandboxList.removeSandbox(testSandboxID) store.DeleteAll() os.RemoveAll(testDir) - os.MkdirAll(testDir, store.DirMode) store.VCStorePrefix = "" + store.RunVMStoragePath = savedRunVMStoragePathFunc setup() } func setup() { - os.Mkdir(filepath.Join(testDir, testBundle), store.DirMode) store.VCStorePrefix = testDir + savedRunVMStoragePathFunc = store.RunVMStoragePath + store.RunVMStoragePath = func() string { + return filepath.Join("testDir", "vm") + } + os.MkdirAll(store.RunVMStoragePath(), store.DirMode) + os.MkdirAll(filepath.Join(testDir, testBundle), store.DirMode) for _, filename := range []string{testQemuKernelPath, testQemuInitrdPath, testQemuImagePath, testQemuPath} { _, err := os.Create(filename) From 9bf0d67fdd76038097e185278488080d1a64b555 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 25 Dec 2019 01:47:07 -0800 Subject: [PATCH 4/7] ut: direct factory needs to set VCStorePrefix Otherwise it fails with permission errors. Signed-off-by: Peng Tao --- virtcontainers/factory/direct/direct_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/virtcontainers/factory/direct/direct_test.go b/virtcontainers/factory/direct/direct_test.go index cca51cea97..5ca7df2986 100644 --- a/virtcontainers/factory/direct/direct_test.go +++ b/virtcontainers/factory/direct/direct_test.go @@ -8,17 +8,26 @@ package direct import ( "context" "io/ioutil" + "os" "testing" "github.com/stretchr/testify/assert" vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/store" ) func TestTemplateFactory(t *testing.T) { assert := assert.New(t) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") + testDir, err := ioutil.TempDir("", "vmfactory-tmp-") + assert.Nil(err) + store.VCStorePrefix = testDir + defer func() { + os.RemoveAll(testDir) + store.VCStorePrefix = "" + }() + hyperConfig := vc.HypervisorConfig{ KernelPath: testDir, ImagePath: testDir, From e5b04a5bf256e22bf5fbe6a6168643b7ac974b38 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 25 Dec 2019 01:51:41 -0800 Subject: [PATCH 5/7] ut: fs test should set RunStoragePath Otherwise it failes with permission errors. Signed-off-by: Peng Tao --- virtcontainers/persist/fs/fs_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/virtcontainers/persist/fs/fs_test.go b/virtcontainers/persist/fs/fs_test.go index 1d618a7f43..4b5d853f5e 100644 --- a/virtcontainers/persist/fs/fs_test.go +++ b/virtcontainers/persist/fs/fs_test.go @@ -7,6 +7,7 @@ package fs import ( "fmt" + "io/ioutil" "os" "testing" @@ -32,6 +33,13 @@ func TestFsLock(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, fs) + testDir, err := ioutil.TempDir("", "fs-tmp-") + assert.Nil(t, err) + TestSetRunStoragePath(testDir) + defer func() { + os.RemoveAll(testDir) + }() + fs.sandboxState.SandboxContainer = "test-fs-driver" sandboxDir, err := fs.sandboxDir() assert.Nil(t, err) @@ -51,6 +59,13 @@ func TestFsDriver(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, fs) + testDir, err := ioutil.TempDir("", "fs-tmp-") + assert.Nil(t, err) + TestSetRunStoragePath(testDir) + defer func() { + os.RemoveAll(testDir) + }() + ss := persistapi.SandboxState{} cs := make(map[string]persistapi.ContainerState) // missing sandbox container id From 5617120649e916a636250b0f69ad7cd9e06fdae3 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 25 Dec 2019 01:56:13 -0800 Subject: [PATCH 6/7] nsenter: skip ut on non-root nsenter needs root privilege to run. Signed-off-by: Peng Tao --- virtcontainers/pkg/nsenter/nsenter_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/virtcontainers/pkg/nsenter/nsenter_test.go b/virtcontainers/pkg/nsenter/nsenter_test.go index b9c38f2d94..6d1e615857 100644 --- a/virtcontainers/pkg/nsenter/nsenter_test.go +++ b/virtcontainers/pkg/nsenter/nsenter_test.go @@ -18,10 +18,14 @@ import ( "github.com/stretchr/testify/assert" "golang.org/x/sys/unix" + + ktu "github.com/kata-containers/runtime/pkg/katatestutils" ) const testPID = 12345 +var tu = ktu.NewTestConstraint(true) + func TestGetNSPathFromPID(t *testing.T) { for nsType := range CloneFlagsTable { expectedPath := fmt.Sprintf("/proc/%d/ns/%s", testPID, nsType) @@ -165,6 +169,9 @@ func TestNsEnterEmptyNamespaceListSuccess(t *testing.T) { } func TestNsEnterSuccessful(t *testing.T) { + if tu.NotValid(ktu.NeedRoot()) { + t.Skip(ktu.TestDisabledNeedRoot) + } nsList := supportedNamespaces() sleepDuration := 60 From 3ed472dc8d690674be5a19a64b0a7d79eb3abb14 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 25 Dec 2019 02:07:52 -0800 Subject: [PATCH 7/7] store: UT tmp path should be random Otherwise we might end up using the previously created store instead. Signed-off-by: Peng Tao --- virtcontainers/store/filesystem_backend_test.go | 6 +++++- virtcontainers/store/manager_test.go | 7 +++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/virtcontainers/store/filesystem_backend_test.go b/virtcontainers/store/filesystem_backend_test.go index 28589ae57f..2b1f9e5dc7 100644 --- a/virtcontainers/store/filesystem_backend_test.go +++ b/virtcontainers/store/filesystem_backend_test.go @@ -20,7 +20,11 @@ type TestNoopStructure struct { Field2 string } -var rootPath = "/tmp/root1/" +var rootPath = func() string { + dir, _ := ioutil.TempDir("", "") + return dir +}() + var expectedFilesystemData = "{\"Field1\":\"value1\",\"Field2\":\"value2\"}" func TestStoreFilesystemStore(t *testing.T) { diff --git a/virtcontainers/store/manager_test.go b/virtcontainers/store/manager_test.go index 00c80f2652..713cb30bd3 100644 --- a/virtcontainers/store/manager_test.go +++ b/virtcontainers/store/manager_test.go @@ -23,14 +23,17 @@ var sandboxDirState = "" var sandboxDirLock = "" var sandboxFileState = "" var sandboxFileLock = "" -var storeRoot = "file:///tmp/root1/" +var storeRoot, storeRootDir = func() (string, string) { + dir, _ := ioutil.TempDir("", "") + return "file://" + dir, dir +}() func TestNewStore(t *testing.T) { s, err := New(context.Background(), storeRoot) assert.Nil(t, err) assert.Equal(t, s.scheme, "file") assert.Equal(t, s.host, "") - assert.Equal(t, s.path, "/tmp/root1/") + assert.Equal(t, s.path, storeRootDir) } func TestDeleteStore(t *testing.T) {