Merge pull request #1334 from jongwu/factory

Factory: Fix fake return value issue on creating template

cli/factory.go

@@ -185,6 +185,7 @@ var initFactoryCommand = cli.Command{
 					HypervisorConfig: runtimeConfig.HypervisorConfig,
 					AgentType:        runtimeConfig.AgentType,
 					AgentConfig:      runtimeConfig.AgentConfig,
+					ProxyType:        runtimeConfig.ProxyType,
 				},
 			}
 			kataLog.WithField("factory", factoryConfig).Info("create vm factory")

cli/factory_test.go

@@ -17,6 +17,8 @@ import (
 	vc "github.com/kata-containers/runtime/virtcontainers"
 )
 
+const testDisabledAsNonRoot = "Test disabled as requires root privileges"
+
 func TestFactoryCLIFunctionNoRuntimeConfig(t *testing.T) {
 	assert := assert.New(t)
 
@@ -63,9 +65,14 @@ func TestFactoryCLIFunctionInit(t *testing.T) {
 	assert.Nil(err)
 
 	// With template
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledAsNonRoot)
+	}
+
 	runtimeConfig.FactoryConfig.Template = true
 	runtimeConfig.HypervisorType = vc.MockHypervisor
 	runtimeConfig.AgentType = vc.NoopAgentType
+	runtimeConfig.ProxyType = vc.NoopProxyType
 	ctx.App.Metadata["runtimeConfig"] = runtimeConfig
 	fn, ok = initFactoryCommand.Action.(func(context *cli.Context) error)
 	assert.True(ok)

virtcontainers/factory/factory.go

@@ -67,7 +67,10 @@ func NewFactory(ctx context.Context, config Config, fetchOnly bool) (vc.Factory,
 				return nil, err
 			}
 		} else {
-			b = template.New(ctx, config.VMConfig)
+			b, err = template.New(ctx, config.VMConfig)
+			if err != nil {
+				return nil, err
+			}
 		}
 	} else if config.VMCache && config.Cache == 0 {
 		b, err = grpccache.New(ctx, config.VMCacheEndpoint)

virtcontainers/factory/factory_test.go

@@ -8,6 +8,7 @@ package factory
 import (
 	"context"
 	"io/ioutil"
+	"os"
 	"testing"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
@@ -17,6 +18,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const testDisabledAsNonRoot = "Test disabled as requires root privileges"
+
 func TestNewFactory(t *testing.T) {
 	var config Config
 
@@ -53,6 +56,10 @@ func TestNewFactory(t *testing.T) {
 	f.CloseFactory(ctx)
 
 	// template
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledAsNonRoot)
+	}
+
 	config.Template = true
 	f, err = NewFactory(ctx, config, false)
 	assert.Nil(err)
@@ -187,6 +194,10 @@ func TestFactoryGetVM(t *testing.T) {
 	ctx := context.Background()
 
 	// direct factory
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledAsNonRoot)
+	}
+
 	f, err := NewFactory(ctx, Config{VMConfig: vmConfig}, false)
 	assert.Nil(err)
 

virtcontainers/factory/template/template.go

@@ -15,7 +15,6 @@ import (
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/kata-containers/runtime/virtcontainers/factory/base"
-	"github.com/kata-containers/runtime/virtcontainers/factory/direct"
 	"github.com/kata-containers/runtime/virtcontainers/store"
 )
 
@@ -42,14 +41,13 @@ func Fetch(config vc.VMConfig) (base.FactoryBase, error) {
 }
 
 // New creates a new VM template factory.
-func New(ctx context.Context, config vc.VMConfig) base.FactoryBase {
+func New(ctx context.Context, config vc.VMConfig) (base.FactoryBase, error) {
 	statePath := store.RunVMStoragePath + "/template"
 	t := &template{statePath, config}
 
 	err := t.prepareTemplateFiles()
 	if err != nil {
-		// fallback to direct factory if template is not supported.
+		return nil, err
-		return direct.New(ctx, config)
 	}
 	defer func() {
 		if err != nil {
@@ -59,11 +57,10 @@ func New(ctx context.Context, config vc.VMConfig) base.FactoryBase {
 
 	err = t.createTemplateVM(ctx)
 	if err != nil {
-		// fallback to direct factory if template is not supported.
+		return nil, err
-		return direct.New(ctx, config)
 	}
 
-	return t
+	return t, nil
 }
 
 // Config returns template factory's configuration.
@@ -116,7 +113,9 @@ func (t *template) createTemplateVM(ctx context.Context) error {
 	config.HypervisorConfig.MemoryPath = t.statePath + "/memory"
 	config.HypervisorConfig.DevicesStatePath = t.statePath + "/state"
 	// template vm uses builtin proxy
-	config.ProxyType = templateProxyType
+	if config.ProxyType != "noopProxy" {
+		config.ProxyType = templateProxyType
+	}
 
 	vm, err := vc.NewVM(ctx, config)
 	if err != nil {

virtcontainers/factory/template/template_test.go

@@ -17,7 +17,13 @@ import (
 	vc "github.com/kata-containers/runtime/virtcontainers"
 )
 
+const testDisabledAsNonRoot = "Test disabled as requires root privileges"
+
 func TestTemplateFactory(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip(testDisabledAsNonRoot)
+	}
+
 	assert := assert.New(t)
 
 	templateWaitForAgent = 1 * time.Microsecond
@@ -40,7 +46,8 @@ func TestTemplateFactory(t *testing.T) {
 	ctx := context.Background()
 
 	// New
-	f := New(ctx, vmConfig)
+	f, err := New(ctx, vmConfig)
+	assert.Nil(err)
 
 	// Config
 	assert.Equal(f.Config(), vmConfig)
@@ -74,7 +81,7 @@ func TestTemplateFactory(t *testing.T) {
 	assert.Nil(err)
 
 	err = tt.createTemplateVM(ctx)
-	assert.Error(err)
+	assert.Nil(err)
 
 	templateProxyType = vc.NoopProxyType
 	vm, err = tt.GetBaseVM(ctx, vmConfig)