Merge pull request #2115 from bergwolf/store

Fix cache factory UT

virtcontainers/factory/cache/cache_test.go

@@ -14,6 +14,7 @@ import (
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/kata-containers/runtime/virtcontainers/factory/direct"
+	"github.com/kata-containers/runtime/virtcontainers/store"
 )
 
 func TestTemplateFactory(t *testing.T) {
@@ -33,6 +34,12 @@ func TestTemplateFactory(t *testing.T) {
 
 	ctx := context.Background()
 
+	var savedStorePath = store.VCStorePrefix
+	store.VCStorePrefix = testDir
+	defer func() {
+		store.VCStorePrefix = savedStorePath
+	}()
+
 	// New
 	f := New(ctx, 2, direct.New(ctx, vmConfig))
 

virtcontainers/store/manager.go

@@ -100,25 +100,23 @@ type manager struct {
 
 var stores = &manager{stores: make(map[string]*Store)}
 
-func (m *manager) addStore(s *Store) (err error) {
+func (m *manager) addStore(s *Store) (rs *Store, err error) {
 	if s == nil {
-		return fmt.Errorf("Store can not be nil")
+		return nil, fmt.Errorf("Store can not be nil")
 	}
 
 	if s.url == "" {
-		return fmt.Errorf("Store URL can not be nil")
+		return nil, fmt.Errorf("Store URL can not be nil")
 	}
 
 	m.Lock()
 	defer m.Unlock()
 
-	if m.stores[s.url] != nil {
+	if m.stores[s.url] == nil {
-		return fmt.Errorf("Store %s already added", s.url)
+		m.stores[s.url] = s
 	}
 
-	m.stores[s.url] = s
+	return m.stores[s.url], nil
-
-	return nil
 }
 
 func (m *manager) removeStore(url string) {
@@ -165,11 +163,11 @@ func New(ctx context.Context, storeURL string) (*Store, error) {
 	s.backend = backend
 
 	// Create new backend
-	if err := s.backend.new(ctx, s.path, s.host); err != nil {
+	if err = s.backend.new(ctx, s.path, s.host); err != nil {
 		return nil, err
 	}
 
-	if err := stores.addStore(s); err != nil {
+	if s, err = stores.addStore(s); err != nil {
 		return nil, err
 	}
 

virtcontainers/store/manager_test.go

@@ -55,14 +55,15 @@ func TestManagerAddStore(t *testing.T) {
 	assert.NotNil(t, newStore, "findStore failed")
 
 	// Duplicate, should fail
-	err = stores.addStore(s)
+	ns, err := stores.addStore(s)
-	assert.NotNil(t, err, "addStore should have failed")
+	assert.Nil(t, err, "addStore should not failed")
+	assert.Equal(t, s, ns)
 
 	// Try with an empty URL
 	sEmpty, err := New(context.Background(), storeRoot)
 	assert.Nil(t, err)
 	sEmpty.url = ""
-	err = stores.addStore(sEmpty)
+	_, err = stores.addStore(sEmpty)
 	assert.NotNil(t, err, "addStore should have failed on an empty store URL")
 
 }

virtcontainers/store/vc.go

@@ -17,6 +17,9 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/types"
 )
 
+// VCStorePrefix is only used for tests to config a temp store dir
+var VCStorePrefix = ""
+
 // VCStore is a virtcontainers specific Store.
 // Virtcontainers typically needs a configuration Store for
 // storing permanent items across reboots.
@@ -45,6 +48,7 @@ func (s *VCStore) itemToStore(item Item) *Store {
 func NewVCStore(ctx context.Context, configRoot, stateRoot string) (*VCStore, error) {
 	config, err := New(ctx, configRoot)
 	if err != nil {
+		fmt.Printf("config root %s\n", configRoot)
 		return nil, err
 	}
 
@@ -243,12 +247,12 @@ func (s *VCStore) Unlock(token string) error {
 // It should look like file:///var/lib/vc/sbs/<sandboxID>/
 // Or for rootless: file://<rootlessDir>/var/lib/vc/sbs/<sandboxID>/
 func SandboxConfigurationRoot(id string) string {
-	return filesystemScheme + "://" + filepath.Join(ConfigStoragePath(), id)
+	return filesystemScheme + "://" + SandboxConfigurationRootPath(id)
 }
 
 // SandboxConfigurationRootPath returns a virtcontainers sandbox configuration root path.
 func SandboxConfigurationRootPath(id string) string {
-	return filepath.Join(ConfigStoragePath(), id)
+	return filepath.Join(VCStorePrefix, ConfigStoragePath(), id)
 }
 
 // SandboxConfigurationItemPath returns a virtcontainers sandbox configuration item path.
@@ -262,12 +266,12 @@ func SandboxConfigurationItemPath(id string, item Item) (string, error) {
 		return "", err
 	}
 
-	return filepath.Join(ConfigStoragePath(), id, itemFile), nil
+	return filepath.Join(VCStorePrefix, ConfigStoragePath(), id, itemFile), nil
 }
 
 // VCStoreUUIDPath returns a virtcontainers runtime uuid URL.
 func VCStoreUUIDPath() string {
-	return filesystemScheme + "://" + VMUUIDStoragePath
+	return filesystemScheme + "://" + filepath.Join(VCStorePrefix, VMUUIDStoragePath)
 }
 
 // SandboxRuntimeRoot returns a virtcontainers sandbox runtime root URL.
@@ -276,12 +280,12 @@ func VCStoreUUIDPath() string {
 // It should look like file:///run/vc/sbs/<sandboxID>/
 // or if rootless: file://<rootlessDir>/run/vc/sbs/<sandboxID>/
 func SandboxRuntimeRoot(id string) string {
-	return filesystemScheme + "://" + filepath.Join(RunStoragePath(), id)
+	return filesystemScheme + "://" + SandboxRuntimeRootPath(id)
 }
 
 // SandboxRuntimeRootPath returns a virtcontainers sandbox runtime root path.
 func SandboxRuntimeRootPath(id string) string {
-	return filepath.Join(RunStoragePath(), id)
+	return filepath.Join(VCStorePrefix, RunStoragePath(), id)
 }
 
 // SandboxRuntimeItemPath returns a virtcontainers sandbox runtime item path.
@@ -303,12 +307,12 @@ func SandboxRuntimeItemPath(id string, item Item) (string, error) {
 // It should look like file:///var/lib/vc/sbs/<sandboxID>/<containerID>
 // Or if rootless file://<rootlessDir>/var/lib/vc/sbs/<sandboxID>/<containerID>
 func ContainerConfigurationRoot(sandboxID, containerID string) string {
-	return filesystemScheme + "://" + filepath.Join(ConfigStoragePath(), sandboxID, containerID)
+	return filesystemScheme + "://" + ContainerConfigurationRootPath(sandboxID, containerID)
 }
 
 // ContainerConfigurationRootPath returns a virtcontainers container configuration root path.
 func ContainerConfigurationRootPath(sandboxID, containerID string) string {
-	return filepath.Join(ConfigStoragePath(), sandboxID, containerID)
+	return filepath.Join(VCStorePrefix, ConfigStoragePath(), sandboxID, containerID)
 }
 
 // ContainerRuntimeRoot returns a virtcontainers container runtime root URL.
@@ -317,12 +321,12 @@ func ContainerConfigurationRootPath(sandboxID, containerID string) string {
 // It should look like file:///run/vc/sbs/<sandboxID>/<containerID>/
 // Or for rootless file://<rootlessDir>/run/vc/sbs/<sandboxID>/<containerID>/
 func ContainerRuntimeRoot(sandboxID, containerID string) string {
-	return filesystemScheme + "://" + filepath.Join(RunStoragePath(), sandboxID, containerID)
+	return filesystemScheme + "://" + ContainerRuntimeRootPath(sandboxID, containerID)
 }
 
 // ContainerRuntimeRootPath returns a virtcontainers container runtime root path.
 func ContainerRuntimeRootPath(sandboxID, containerID string) string {
-	return filepath.Join(RunStoragePath(), sandboxID, containerID)
+	return filepath.Join(VCStorePrefix, RunStoragePath(), sandboxID, containerID)
 }
 
 // VCSandboxStoreExists returns true if a sandbox store already exists.

virtcontainers/store/vc_test.go

@@ -8,6 +8,8 @@ package store
 import (
 	"context"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -99,6 +101,17 @@ func TestStoreVCSandboxFileNegative(t *testing.T) {
 }
 
 func TestStoreVCNewVCSandboxStore(t *testing.T) {
+	testDir, _ := ioutil.TempDir("", "vmfactory-tmp-")
+	defer func() {
+		os.RemoveAll(testDir)
+	}()
+
+	var savedStorePath = VCStorePrefix
+	VCStorePrefix = testDir
+	defer func() {
+		VCStorePrefix = savedStorePath
+	}()
+
 	_, err := NewVCSandboxStore(context.Background(), testSandboxID)
 	assert.Nil(t, err)
 
@@ -107,6 +120,17 @@ func TestStoreVCNewVCSandboxStore(t *testing.T) {
 }
 
 func TestStoreVCNewVCContainerStore(t *testing.T) {
+	testDir, _ := ioutil.TempDir("", "vmfactory-tmp-")
+	defer func() {
+		os.RemoveAll(testDir)
+	}()
+
+	var savedStorePath = VCStorePrefix
+	VCStorePrefix = testDir
+	defer func() {
+		VCStorePrefix = savedStorePath
+	}()
+
 	_, err := NewVCContainerStore(context.Background(), testSandboxID, "foobar")
 	assert.Nil(t, err)