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 kata-containers#821. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
jodh-intel · Oct 17, 2018 · 8831245 · 8831245
1 parent d00742f
commit 8831245
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 29 deletions.
diff --git a/cli/create.go b/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 {
-		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)
-		}
-	}
+	handleFactory(ctx, runtimeConfig)
 
 	disableOutput := noNeedForOutput(detach, ociSpec.Process.Terminal)
 

diff --git a/cli/create_test.go b/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)

diff --git a/cli/run_test.go b/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},
+	}
 
-	// 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())
-	assert.NotZero(e.ExitCode())
+	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)
+		}
+
+		e, ok := err.(*cli.ExitError)
+
+		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) {