Cleanup command runner (kubernetes-sigs#647)

We had a very elaborate scheme for testing command execution,
involving calling back in to the test program, but we weren't really
using it, much simpler to just use a mock.

Also avoid hyphenated package naming, which isn't traditional
kubernetes / go, and confuses IDEs / static analysis tools.  Also
clean up some name stuttering.

pkg/cmd-runner/BUILD.bazel → pkg/cmdrunner/BUILD.bazel

@@ -3,7 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 go_library(
     name = "go_default_library",
     srcs = ["cmd_runner.go"],
-    importpath = "sigs.k8s.io/cluster-api/pkg/cmd-runner",
+    importpath = "sigs.k8s.io/cluster-api/pkg/cmdrunner",
     visibility = ["//visibility:public"],
 )
 

pkg/cmd-runner/cmd_runner.go → pkg/cmdrunner/cmd_runner.go

@@ -14,24 +14,24 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cmd_runner
+package cmdrunner
 
 import (
 	"os/exec"
 )
 
-type CmdRunner interface {
+type Runner interface {
 	CombinedOutput(cmd string, args ...string) (output string, err error)
 }
 
-type realCmdRunner struct {
+type realRunner struct {
 }
 
-func New() *realCmdRunner {
-	return &realCmdRunner{}
+func New() *realRunner {
+	return &realRunner{}
 }
 
-func (runner *realCmdRunner) CombinedOutput(cmd string, args ...string) (string, error) {
+func (runner *realRunner) CombinedOutput(cmd string, args ...string) (string, error) {
 	output, err := exec.Command(cmd, args...).CombinedOutput()
 	return string(output), err
 }

pkg/cmd-runner/cmd_runner_test.go → pkg/cmdrunner/cmd_runner_test.go

@@ -14,17 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cmd_runner_test
+package cmdrunner
 
 import (
 	"io/ioutil"
 	"os"
-	"sigs.k8s.io/cluster-api/pkg/cmd-runner"
 	"testing"
 )
 
 func TestInvalidCommand(t *testing.T) {
-	runner := cmd_runner.New()
+	runner := New()
 	output, err := runner.CombinedOutput("asdf")
 	if err == nil {
 		t.Errorf("invalid error: expected 'nil', got '%v'", err)
@@ -36,7 +35,7 @@ func TestInvalidCommand(t *testing.T) {
 
 func TestValidCommandErrors(t *testing.T) {
 	skipIfCommandNotPresent(t, "ls")
-	runner := cmd_runner.New()
+	runner := New()
 	_, err := runner.CombinedOutput("ls /invalid/path")
 	if err == nil {
 		t.Errorf("invalid error: expected 'nil', got '%v'", err)
@@ -51,7 +50,7 @@ func TestValidCommandSucceeds(t *testing.T) {
 		t.FailNow()
 	}
 	defer os.RemoveAll(dir)
-	runner := cmd_runner.New()
+	runner := New()
 	output, err := runner.CombinedOutput("ls", "-al", dir)
 	if err != nil {
 		t.Errorf("invalid error: expected 'nil', got '%v'", err)
@@ -64,7 +63,7 @@ func TestValidCommandSucceeds(t *testing.T) {
 func TestCombinedOutputShouldIncludeStdOutAndErr(t *testing.T) {
 	skipIfCommandNotPresent(t, "echo")
 	skipIfCommandNotPresent(t, "sh")
-	runner := cmd_runner.New()
+	runner := New()
 	output, err := runner.CombinedOutput("sh", "-c", "echo \"stdout\" && (>&2 echo \"stderr\")")
 	if err != nil {
 		t.Errorf("invalid error: expected 'nil', got '%v'", err)
@@ -76,7 +75,7 @@ func TestCombinedOutputShouldIncludeStdOutAndErr(t *testing.T) {
 }
 
 func skipIfCommandNotPresent(t *testing.T, cmd string) {
-	runner := cmd_runner.New()
+	runner := New()
 	_, err := runner.CombinedOutput("ls")
 	if err != nil {
 		t.Skipf("unable to run test, 'ls' reults in error: %v", err)

pkg/kubeadm/BUILD.bazel

@@ -5,12 +5,12 @@ go_library(
     srcs = ["kubeadm.go"],
     importpath = "sigs.k8s.io/cluster-api/pkg/kubeadm",
     visibility = ["//visibility:public"],
-    deps = ["//pkg/cmd-runner:go_default_library"],
+    deps = ["//pkg/cmdrunner:go_default_library"],
 )
 
 go_test(
     name = "go_default_test",
     srcs = ["kubeadm_test.go"],
     embed = [":go_default_library"],
-    deps = ["//pkg/test-cmd-runner:go_default_library"],
+    deps = ["//pkg/testcmdrunner:go_default_library"],
 )

pkg/kubeadm/kubeadm.go

@@ -20,13 +20,13 @@ import (
 	"strings"
 	"time"
 
-	"sigs.k8s.io/cluster-api/pkg/cmd-runner"
+	"sigs.k8s.io/cluster-api/pkg/cmdrunner"
 )
 
 // The purpose of Kubeadm and this file is to provide a unit tested wrapper around the 'kubeadm' exec command. Higher
 // level, application specific functionality built on top of kubeadm should be be in another location.
 type Kubeadm struct {
-	runner cmd_runner.CmdRunner
+	runner cmdrunner.Runner
 }
 
 // see https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-token/ for an explanation of the parameters
@@ -41,14 +41,14 @@ type TokenCreateParams struct {
 	Usages           []string
 }
 
-func NewWithCmdRunner(runner cmd_runner.CmdRunner) *Kubeadm {
+func NewWithRunner(runner cmdrunner.Runner) *Kubeadm {
 	return &Kubeadm{
 		runner: runner,
 	}
 }
 
 func New() *Kubeadm {
-	return NewWithCmdRunner(cmd_runner.New())
+	return NewWithRunner(cmdrunner.New())
 }
 
 // TokenCreate execs `kubeadm token create` with the appropriate flags added by interpreting the params argument. The output of

pkg/kubeadm/kubeadm_test.go

@@ -17,28 +17,21 @@ limitations under the License.
 package kubeadm_test
 
 import (
-	"fmt"
-	"os"
+	"errors"
 	"strings"
 	"testing"
 	"time"
 
 	"sigs.k8s.io/cluster-api/pkg/kubeadm"
-	"sigs.k8s.io/cluster-api/pkg/test-cmd-runner"
+	"sigs.k8s.io/cluster-api/pkg/testcmdrunner"
 )
 
 var (
 	joinCommand   = "kubeadm join --token a53d73.21029eb48b9002d0 35.199.169.93:443 --discovery-token-ca-cert-hash sha256:3bb9ee3aa3cee9898b85523e8dd6921752e07e3053453084c8d26d40d929a132"
-	errorExitCode = -1
+	errorExitCode = errors.New("kubeadm error")
 	errorMessage  = "error message"
 )
 
-func init() {
-	test_cmd_runner.RegisterCallback(echoCallback)
-	test_cmd_runner.RegisterCallback(errorCallback)
-	test_cmd_runner.RegisterCallback(tokenCallback)
-}
-
 func TestTokenCreateParameters(t *testing.T) {
 	var tests = []struct {
 		name   string
@@ -61,7 +54,7 @@ func TestTokenCreateParameters(t *testing.T) {
 		{"all", nil, "kubeadm token create --config /my/config --description my description --groups bootstrappers --help --print-join-command --ttl 1h1m1s --usages authentication",
 			kubeadm.TokenCreateParams{Config: "/my/config", Description: "my description", Groups: []string{"bootstrappers"}, Help: true, PrintJoinCommand: true, Ttl: toDuration(1, 1, 1), Usages: []string{"authentication"}}},
 	}
-	kadm := kubeadm.NewWithCmdRunner(test_cmd_runner.NewTestRunnerFailOnErr(t, echoCallback))
+	kadm := kubeadm.NewWithRunner(testcmdrunner.NewOrDie(t, echoCallback))
 	for _, tst := range tests {
 		output, err := kadm.TokenCreate(tst.params)
 		if err != tst.err {
@@ -74,7 +67,7 @@ func TestTokenCreateParameters(t *testing.T) {
 }
 
 func TestTokenCreateReturnsUnmodifiedOutput(t *testing.T) {
-	kadm := kubeadm.NewWithCmdRunner(test_cmd_runner.NewTestRunnerFailOnErr(t, tokenCallback))
+	kadm := kubeadm.NewWithRunner(testcmdrunner.NewOrDie(t, tokenCallback))
 	output, err := kadm.TokenCreate(kubeadm.TokenCreateParams{})
 	if err != nil {
 		t.Errorf("unexpected error: wanted nil")
@@ -85,7 +78,7 @@ func TestTokenCreateReturnsUnmodifiedOutput(t *testing.T) {
 }
 
 func TestNonZeroExitCodeResultsInError(t *testing.T) {
-	kadm := kubeadm.NewWithCmdRunner(test_cmd_runner.NewTestRunnerFailOnErr(t, errorCallback))
+	kadm := kubeadm.NewWithRunner(testcmdrunner.NewOrDie(t, errorCallback))
 	output, err := kadm.TokenCreate(kubeadm.TokenCreateParams{})
 	if err == nil {
 		t.Errorf("expected error: got nil")
@@ -95,24 +88,17 @@ func TestNonZeroExitCodeResultsInError(t *testing.T) {
 	}
 }
 
-func echoCallback(cmd string, args ...string) int {
+func echoCallback(cmd string, args ...string) (string, error) {
 	fullCmd := append([]string{cmd}, args...)
-	fmt.Print(strings.Join(fullCmd, " "))
-	return 0
-}
-
-func tokenCallback(cmd string, args ...string) int {
-	fmt.Print(joinCommand)
-	return 0
+	return strings.Join(fullCmd, " "), nil
 }
 
-func errorCallback(cmd string, args ...string) int {
-	fmt.Fprintf(os.Stderr, errorMessage)
-	return errorExitCode
+func tokenCallback(cmd string, args ...string) (string, error) {
+	return joinCommand, nil
 }
 
-func TestMain(m *testing.M) {
-	test_cmd_runner.TestMain(m)
+func errorCallback(cmd string, args ...string) (string, error) {
+	return errorMessage, errorExitCode
 }
 
 func toDuration(hour int, minute int, second int) time.Duration {