From 8831245e30bc6d0e3c2e88f465016b4a05e7bf9a Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Tue, 16 Oct 2018 11:11:31 +0100 Subject: [PATCH] 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 --- cli/create.go | 65 ++++++++++++++++++++++++++++++---------------- cli/create_test.go | 8 ++++++ cli/run_test.go | 44 ++++++++++++++++++++++++++----- 3 files changed, 88 insertions(+), 29 deletions(-) diff --git a/cli/create.go b/cli/create.go index dbbf2e445..0bbc288ca 100644 --- 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 index 930f6832f..287773e29 100644 --- 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 index 362e56389..2154f9db1 100644 --- 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 - // 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()) + // 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) + } + + 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) {