create/run: Make bundle path default to cwd

The bundle path was documented as defaulting to the current directory
but was not being set to that value if not explicitly specified.

Also moved factory creation code to a new `handleFactory()` function to
avoid cyclomatic complexity issues.

Fixes #821.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

cli/create.go

@@ -92,6 +92,36 @@ var createCLICommand = cli.Command{
 // Use a variable to allow tests to modify its value
 var getKernelParamsFunc = getKernelParams
 
+func handleFactory(ctx context.Context, runtimeConfig oci.RuntimeConfig) {
+	if !runtimeConfig.FactoryConfig.Template {
+		return
+	}
+
+	factoryConfig := vf.Config{
+		Template: true,
+		VMConfig: vc.VMConfig{
+			HypervisorType:   runtimeConfig.HypervisorType,
+			HypervisorConfig: runtimeConfig.HypervisorConfig,
+			AgentType:        runtimeConfig.AgentType,
+			AgentConfig:      runtimeConfig.AgentConfig,
+		},
+	}
+
+	kataLog.WithField("factory", factoryConfig).Info("load vm factory")
+
+	f, err := vf.NewFactory(ctx, factoryConfig, true)
+	if err != nil {
+		kataLog.WithError(err).Warn("load vm factory failed, about to create new one")
+		f, err = vf.NewFactory(ctx, factoryConfig, false)
+		if err != nil {
+			kataLog.WithError(err).Warn("create vm factory failed")
+			return
+		}
+	}
+
+	vci.SetFactory(ctx, f)
+}
+
 func create(ctx context.Context, containerID, bundlePath, console, pidFilePath string, detach, systemdCgroup bool,
 	runtimeConfig oci.RuntimeConfig) error {
 	var err error
@@ -103,6 +133,17 @@ func create(ctx context.Context, containerID, bundlePath, console, pidFilePath s
 	setExternalLoggers(ctx, kataLog)
 	span.SetTag("container", containerID)
 
+	if bundlePath == "" {
+		cwd, err := os.Getwd()
+		if err != nil {
+			return err
+		}
+
+		kataLog.WithField("directory", cwd).Debug("Defaulting bundle path to current directory")
+
+		bundlePath = cwd
+	}
+
 	// Checks the MUST and MUST NOT from OCI runtime specification
 	if bundlePath, err = validCreateParams(ctx, containerID, bundlePath); err != nil {
 		return err
@@ -118,29 +159,7 @@ func create(ctx context.Context, containerID, bundlePath, console, pidFilePath s
 		return err
 	}
 
-	if runtimeConfig.FactoryConfig.Template {
+	handleFactory(ctx, runtimeConfig)
-		factoryConfig := vf.Config{
-			Template: true,
-			VMConfig: vc.VMConfig{
-				HypervisorType:   runtimeConfig.HypervisorType,
-				HypervisorConfig: runtimeConfig.HypervisorConfig,
-				AgentType:        runtimeConfig.AgentType,
-				AgentConfig:      runtimeConfig.AgentConfig,
-			},
-		}
-		kataLog.WithField("factory", factoryConfig).Info("load vm factory")
-		f, err := vf.NewFactory(ctx, factoryConfig, true)
-		if err != nil {
-			kataLog.WithError(err).Warn("load vm factory failed, about to create new one")
-			f, err = vf.NewFactory(ctx, factoryConfig, false)
-			if err != nil {
-				kataLog.WithError(err).Warn("create vm factory failed")
-			}
-		}
-		if err == nil {
-			vci.SetFactory(ctx, f)
-		}
-	}
 
 	disableOutput := noNeedForOutput(detach, ociSpec.Process.Terminal)
 

cli/create_test.go

@@ -13,6 +13,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -561,6 +562,13 @@ func TestCreateProcessCgroupsPathSuccessful(t *testing.T) {
 	err = writeOCIConfigFile(spec, ociConfigFile)
 	assert.NoError(err)
 
+	err = create(context.Background(), testContainerID, "", testConsole, pidFilePath, false, true, runtimeConfig)
+	assert.Error(err, "bundle path not set")
+
+	re := regexp.MustCompile("config.json.*no such file or directory")
+	matches := re.FindAllStringSubmatch(err.Error(), -1)
+	assert.NotEmpty(matches)
+
 	for _, detach := range []bool{true, false} {
 		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, detach, true, runtimeConfig)
 		assert.NoError(err, "detached: %+v", detach)

cli/run_test.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"testing"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
@@ -290,13 +291,44 @@ func TestRunContainerSuccessful(t *testing.T) {
 		testingImpl.DeleteContainerFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
+	type errorTestArgs struct {
+		bundlePath string
+
+		// regex string for text of error message
+		errorRE string
+
+		// If true, expect a cli.ExitError, else expect any other type
+		// of error.
+		expectExitError bool
+	}
+
+	args := []errorTestArgs{
+		{"", "config.json: no such file or directory", false},
+		{d.bundlePath, "", true},
+	}
+
+	for i, a := range args {
+		err = run(context.Background(), d.sandbox.ID(), a.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
+		assert.Errorf(err, "test args %d (%+v)", i, a)
+
+		if a.errorRE == "" {
+			assert.Empty(err.Error())
+		} else {
+			re := regexp.MustCompile(a.errorRE)
+			matches := re.FindAllStringSubmatch(err.Error(), -1)
+			assert.NotEmpty(matches)
+		}
 
-	// should return ExitError with the message and exit code
 		e, ok := err.(*cli.ExitError)
-	assert.True(ok, "error should be a cli.ExitError: %s", err)
+
-	assert.Empty(e.Error())
+		if a.expectExitError {
+			// should return ExitError with the message and exit code
+			assert.Truef(ok, "test args %d (%+v): error should be a cli.ExitError: %s", i, a, err)
 			assert.NotZero(e.ExitCode())
+		} else {
+			assert.Falsef(ok, "test args %d (%+v): error should not be a cli.ExitError: %s", i, a, err)
+		}
+	}
 }
 
 func TestRunContainerDetachSuccessful(t *testing.T) {