From f4877914ec40760a5b2a6f070ccab3ec7a7f44fb Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Thu, 14 May 2020 19:56:49 +0200 Subject: [PATCH] Removed string command parameter, it's not used anywhere except for a test (#90) Signed-off-by: Andreas Neumann --- pkg/test/harness.go | 2 +- pkg/test/step.go | 2 +- pkg/test/utils/kubernetes.go | 20 ++---- pkg/test/utils/kubernetes_integration_test.go | 10 +-- pkg/test/utils/kubernetes_test.go | 64 +++++-------------- 5 files changed, 29 insertions(+), 69 deletions(-) diff --git a/pkg/test/harness.go b/pkg/test/harness.go index 975b9ed4..6cc9acf7 100644 --- a/pkg/test/harness.go +++ b/pkg/test/harness.go @@ -371,7 +371,7 @@ func (h *Harness) Setup() { h.fatal(fmt.Errorf("fatal error installing manifests: %v", err)) } } - bgs, errs := testutils.RunCommands(h.GetLogger(), "default", "", h.TestSuite.Commands, "") + bgs, errs := testutils.RunCommands(h.GetLogger(), "default", h.TestSuite.Commands, "") // assign any background processes first for cleanup in case of any errors h.bgProcesses = append(h.bgProcesses, bgs...) if len(errs) > 0 { diff --git a/pkg/test/step.go b/pkg/test/step.go index 24dffa63..524cd839 100644 --- a/pkg/test/step.go +++ b/pkg/test/step.go @@ -379,7 +379,7 @@ func (s *Step) Run(namespace string) []error { command.Background = false } } - if _, errors := testutils.RunCommands(s.Logger, namespace, "", s.Step.Commands, s.Dir); errors != nil { + if _, errors := testutils.RunCommands(s.Logger, namespace, s.Step.Commands, s.Dir); errors != nil { testErrors = append(testErrors, errors...) } } diff --git a/pkg/test/utils/kubernetes.go b/pkg/test/utils/kubernetes.go index 2222ad29..9459c12e 100644 --- a/pkg/test/utils/kubernetes.go +++ b/pkg/test/utils/kubernetes.go @@ -910,7 +910,7 @@ func StartTestEnvironment(KubeAPIServerFlags []string) (env TestEnvironment, err // GetArgs parses a command line string into its arguments and appends a namespace if it is not already set. // provides OS expansion of defined ENV VARs inside args to commands. The expansion is limited to what is defined on the OS // and not variables defined for kuttl tests -func GetArgs(ctx context.Context, command string, cmd harness.Command, namespace string, env map[string]string) (*exec.Cmd, error) { +func GetArgs(ctx context.Context, cmd harness.Command, namespace string, env map[string]string) (*exec.Cmd, error) { argSlice := []string{} c := os.ExpandEnv(cmd.Command) @@ -922,14 +922,6 @@ func GetArgs(ctx context.Context, command string, cmd harness.Command, namespace return nil, err } - if command != "" && argSplit[0] != command { - command = os.Expand(command, func(s string) string { - return env[s] - }) - - argSlice = append(argSlice, os.ExpandEnv(command)) - } - argSlice = append(argSlice, argSplit...) if cmd.Namespaced { @@ -955,7 +947,7 @@ func GetArgs(ctx context.Context, command string, cmd harness.Command, namespace // RunCommand runs a command with args. // args gets split on spaces (respecting quoted strings). // if the command is run in the background a reference to the process is returned for later cleanup -func RunCommand(ctx context.Context, namespace string, command string, cmd harness.Command, cwd string, stdout io.Writer, stderr io.Writer) (*exec.Cmd, error) { +func RunCommand(ctx context.Context, namespace string, cmd harness.Command, cwd string, stdout io.Writer, stderr io.Writer) (*exec.Cmd, error) { actualDir, err := os.Getwd() if err != nil { return nil, err @@ -966,7 +958,7 @@ func RunCommand(ctx context.Context, namespace string, command string, cmd harne kudoENV["KUBECONFIG"] = fmt.Sprintf("%s/kubeconfig", actualDir) kudoENV["PATH"] = fmt.Sprintf("%s/bin/:%s", actualDir, os.Getenv("PATH")) - builtCmd, err := GetArgs(ctx, command, cmd, namespace, kudoENV) + builtCmd, err := GetArgs(ctx, cmd, namespace, kudoENV) if err != nil { return nil, err } @@ -1003,7 +995,7 @@ func RunCommand(ctx context.Context, namespace string, command string, cmd harne // RunCommands runs a set of commands, returning any errors. // If `command` is set, then `command` will be the command that is invoked (if a command specifies it already, it will not be prepended again). // commands running in the background are returned -func RunCommands(logger Logger, namespace string, command string, commands []harness.Command, workdir string) ([]*exec.Cmd, []error) { +func RunCommands(logger Logger, namespace string, commands []harness.Command, workdir string) ([]*exec.Cmd, []error) { errs := []error{} bgs := []*exec.Cmd{} @@ -1012,9 +1004,9 @@ func RunCommands(logger Logger, namespace string, command string, commands []har } for _, cmd := range commands { - logger.Logf("running command: %s %q", command, cmd.Command) + logger.Logf("running command: %s", cmd) - bg, err := RunCommand(context.TODO(), namespace, command, cmd, workdir, logger, logger) + bg, err := RunCommand(context.TODO(), namespace, cmd, workdir, logger, logger) if err != nil { errs = append(errs, err) } diff --git a/pkg/test/utils/kubernetes_integration_test.go b/pkg/test/utils/kubernetes_integration_test.go index 7b2e1974..a88f77c4 100644 --- a/pkg/test/utils/kubernetes_integration_test.go +++ b/pkg/test/utils/kubernetes_integration_test.go @@ -120,7 +120,7 @@ func TestRunCommand(t *testing.T) { } // assert foreground cmd returns nil - cmd, err := RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr) + cmd, err := RunCommand(context.TODO(), "", hcmd, "", stdout, stderr) assert.NoError(t, err) assert.Nil(t, cmd) // foreground processes should have stdout @@ -130,7 +130,7 @@ func TestRunCommand(t *testing.T) { stdout = &bytes.Buffer{} // assert background cmd returns process - cmd, err = RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr) + cmd, err = RunCommand(context.TODO(), "", hcmd, "", stdout, stderr) assert.NoError(t, err) assert.NotNil(t, cmd) // no stdout for background processes @@ -146,12 +146,12 @@ func TestRunCommandIgnoreErrors(t *testing.T) { } // assert foreground cmd returns nil - cmd, err := RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr) + cmd, err := RunCommand(context.TODO(), "", hcmd, "", stdout, stderr) assert.NoError(t, err) assert.Nil(t, cmd) hcmd.IgnoreFailure = false - cmd, err = RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr) + cmd, err = RunCommand(context.TODO(), "", hcmd, "", stdout, stderr) assert.Error(t, err) assert.Nil(t, cmd) @@ -160,7 +160,7 @@ func TestRunCommandIgnoreErrors(t *testing.T) { Command: "bad-command", IgnoreFailure: true, } - cmd, err = RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr) + cmd, err = RunCommand(context.TODO(), "", hcmd, "", stdout, stderr) assert.Error(t, err) assert.Nil(t, cmd) } diff --git a/pkg/test/utils/kubernetes_test.go b/pkg/test/utils/kubernetes_test.go index 74b576ba..0c68b72a 100644 --- a/pkg/test/utils/kubernetes_test.go +++ b/pkg/test/utils/kubernetes_test.go @@ -257,7 +257,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace long, combined already set at end is not modified", namespace: "default", - args: "kuttl test --namespace=test-canary", + args: "kubectl kuttl test --namespace=test-canary", expected: []string{ "kubectl", "kuttl", "test", "--namespace=test-canary", }, @@ -265,7 +265,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace long already set at end is not modified", namespace: "default", - args: "kuttl test --namespace test-canary", + args: "kubectl kuttl test --namespace test-canary", expected: []string{ "kubectl", "kuttl", "test", "--namespace", "test-canary", }, @@ -273,7 +273,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace short, combined already set at end is not modified", namespace: "default", - args: "kuttl test -n=test-canary", + args: "kubectl kuttl test -n=test-canary", expected: []string{ "kubectl", "kuttl", "test", "-n=test-canary", }, @@ -281,47 +281,15 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace short already set at end is not modified", namespace: "default", - args: "kuttl test -n test-canary", + args: "kubectl kuttl test -n test-canary", expected: []string{ "kubectl", "kuttl", "test", "-n", "test-canary", }, }, - { - testName: "namespace long, combined already set at beginning is not modified", - namespace: "default", - args: "--namespace=test-canary kuttl test", - expected: []string{ - "kubectl", "--namespace=test-canary", "kuttl", "test", - }, - }, - { - testName: "namespace long already set at beginning is not modified", - namespace: "default", - args: "--namespace test-canary kuttl test", - expected: []string{ - "kubectl", "--namespace", "test-canary", "kuttl", "test", - }, - }, - { - testName: "namespace short, combined already set at beginning is not modified", - namespace: "default", - args: "-n=test-canary kuttl test", - expected: []string{ - "kubectl", "-n=test-canary", "kuttl", "test", - }, - }, - { - testName: "namespace short already set at beginning is not modified", - namespace: "default", - args: "-n test-canary kuttl test", - expected: []string{ - "kubectl", "-n", "test-canary", "kuttl", "test", - }, - }, { testName: "namespace long, combined already set in middle is not modified", namespace: "default", - args: "kuttl --namespace=test-canary test", + args: "kubectl kuttl --namespace=test-canary test", expected: []string{ "kubectl", "kuttl", "--namespace=test-canary", "test", }, @@ -329,7 +297,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace long already set in middle is not modified", namespace: "default", - args: "kuttl --namespace test-canary test", + args: "kubectl kuttl --namespace test-canary test", expected: []string{ "kubectl", "kuttl", "--namespace", "test-canary", "test", }, @@ -337,7 +305,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace short, combined already set in middle is not modified", namespace: "default", - args: "kuttl -n=test-canary test", + args: "kubectl kuttl -n=test-canary test", expected: []string{ "kubectl", "kuttl", "-n=test-canary", "test", }, @@ -345,7 +313,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace short already set in middle is not modified", namespace: "default", - args: "kuttl -n test-canary test", + args: "kubectl kuttl -n test-canary test", expected: []string{ "kubectl", "kuttl", "-n", "test-canary", "test", }, @@ -353,7 +321,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "namespace not set is appended", namespace: "default", - args: "kuttl test", + args: "kubectl kuttl test", expected: []string{ "kubectl", "kuttl", "test", "--namespace", "default", }, @@ -361,7 +329,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "unknown arguments do not break parsing with namespace is not set", namespace: "default", - args: "kuttl test --config kuttl-test.yaml", + args: "kubectl kuttl test --config kuttl-test.yaml", expected: []string{ "kubectl", "kuttl", "test", "--config", "kuttl-test.yaml", "--namespace", "default", }, @@ -369,7 +337,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "unknown arguments do not break parsing if namespace is set at beginning", namespace: "default", - args: "--namespace=test-canary kuttl test --config kuttl-test.yaml", + args: "kubectl --namespace=test-canary kuttl test --config kuttl-test.yaml", expected: []string{ "kubectl", "--namespace=test-canary", "kuttl", "test", "--config", "kuttl-test.yaml", }, @@ -377,7 +345,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "unknown arguments do not break parsing if namespace is set at middle", namespace: "default", - args: "kuttl --namespace=test-canary test --config kuttl-test.yaml", + args: "kubectl kuttl --namespace=test-canary test --config kuttl-test.yaml", expected: []string{ "kubectl", "kuttl", "--namespace=test-canary", "test", "--config", "kuttl-test.yaml", }, @@ -385,7 +353,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "unknown arguments do not break parsing if namespace is set at end", namespace: "default", - args: "kuttl test --config kuttl-test.yaml --namespace=test-canary", + args: "kubectl kuttl test --config kuttl-test.yaml --namespace=test-canary", expected: []string{ "kubectl", "kuttl", "test", "--config", "kuttl-test.yaml", "--namespace=test-canary", }, @@ -393,7 +361,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "quotes are respected when parsing", namespace: "default", - args: "kuttl \"test quoted\"", + args: "kubectl kuttl \"test quoted\"", expected: []string{ "kubectl", "kuttl", "test quoted", "--namespace", "default", }, @@ -401,7 +369,7 @@ func TestGetKubectlArgs(t *testing.T) { { testName: "os ENV are expanded", namespace: "default", - args: "kuttl $TEST_FOO ${TEST_FOO}", + args: "kubectl kuttl $TEST_FOO ${TEST_FOO}", env: map[string]string{"TEST_FOO": "test"}, expected: []string{ "kubectl", "kuttl", "test", "test", "--namespace", "default", @@ -430,7 +398,7 @@ func TestGetKubectlArgs(t *testing.T) { } }() } - cmd, err := GetArgs(context.TODO(), "kubectl", harness.Command{ + cmd, err := GetArgs(context.TODO(), harness.Command{ Command: test.args, Namespaced: true, }, test.namespace, nil)