Merge pull request #4687 from medyagh/move_command_runner

moving command runner to its own package
kubernetes · Jul 5, 2019 · 4e5f173 · 4e5f173
2 parents cc99602 + 99ac1a8
commit 4e5f173
Show file tree

Hide file tree

Showing 17 changed files with 42 additions and 35 deletions.
diff --git a/cmd/minikube/cmd/config/util.go b/cmd/minikube/cmd/config/util.go
@@ -24,8 +24,8 @@ import (
 
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/assets"
-	"k8s.io/minikube/pkg/minikube/bootstrapper"
 	"k8s.io/minikube/pkg/minikube/cluster"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/console"
 	"k8s.io/minikube/pkg/minikube/constants"
@@ -154,7 +154,7 @@ func isAddonAlreadySet(addon *assets.Addon, enable bool) error {
 	return nil
 }
 
-func enableOrDisableAddonInternal(addon *assets.Addon, cmd bootstrapper.CommandRunner, data interface{}, enable bool) error {
+func enableOrDisableAddonInternal(addon *assets.Addon, cmd command.Runner, data interface{}, enable bool) error {
 	var err error
 	// check addon status before enabling/disabling it
 	if err := isAddonAlreadySet(addon, enable); err != nil {

diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go
@@ -49,6 +49,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm"
 	"k8s.io/minikube/pkg/minikube/cluster"
+	"k8s.io/minikube/pkg/minikube/command"
 	cfg "k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/console"
 	"k8s.io/minikube/pkg/minikube/constants"
@@ -759,7 +760,7 @@ func configureRuntimes(runner cruntime.CommandRunner) cruntime.Manager {
 }
 
 // bootstrapCluster starts Kubernetes using the chosen bootstrapper
-func bootstrapCluster(bs bootstrapper.Bootstrapper, r cruntime.Manager, runner bootstrapper.CommandRunner, kc cfg.KubernetesConfig, preexisting bool, isUpgrade bool) {
+func bootstrapCluster(bs bootstrapper.Bootstrapper, r cruntime.Manager, runner command.Runner, kc cfg.KubernetesConfig, preexisting bool, isUpgrade bool) {
 	// hum. bootstrapper.Bootstrapper should probably have a Name function.
 	bsName := viper.GetString(cmdcfg.Bootstrapper)
 

diff --git a/pkg/drivers/hyperkit/driver_test.go b/pkg/drivers/hyperkit/driver_test.go
@@ -67,7 +67,7 @@ func Test_portExtraction(t *testing.T) {
 }
 
 func Test_defaultDiskSize(t *testing.T) {
-	expectedDefaultDiscSize := commonutil.CalculateDiskSizeInMB(constants.DefaultDiskSize)
+	expectedDefaultDiscSize := commonutil.CalculateSizeInMB(constants.DefaultDiskSize)
 	driver := NewDriver("", "")
 	got := driver.DiskSize
 	if got != expectedDefaultDiscSize {

diff --git a/pkg/drivers/none/none.go b/pkg/drivers/none/none.go
@@ -25,8 +25,8 @@ import (
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/util/net"
-	pkgdrivers "k8s.io/minikube/pkg/drivers" // TODO(tstromberg): Extract CommandRunner into its own package
-	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	pkgdrivers "k8s.io/minikube/pkg/drivers"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/cruntime"
 )
@@ -47,7 +47,7 @@ type Driver struct {
 	*pkgdrivers.CommonDriver
 	URL     string
 	runtime cruntime.Manager
-	exec    bootstrapper.CommandRunner
+	exec    command.Runner
 }
 
 // Config is configuration for the None driver
@@ -59,7 +59,7 @@ type Config struct {
 
 // NewDriver returns a fully configured None driver
 func NewDriver(c Config) *Driver {
-	runner := &bootstrapper.ExecRunner{}
+	runner := &command.ExecRunner{}
 	runtime, err := cruntime.New(cruntime.Config{Type: c.ContainerRuntime, Runner: runner})
 	// Libraries shouldn't panic, but there is no way for drivers to return error :(
 	if err != nil {
@@ -216,19 +216,19 @@ func (d *Driver) RunSSHCommandFromDriver() error {
 }
 
 // stopKubelet idempotently stops the kubelet
-func stopKubelet(exec bootstrapper.CommandRunner) error {
+func stopKubelet(exec command.Runner) error {
 	glog.Infof("stopping kubelet.service ...")
 	return exec.Run("sudo systemctl stop kubelet.service")
 }
 
 // restartKubelet restarts the kubelet
-func restartKubelet(exec bootstrapper.CommandRunner) error {
+func restartKubelet(exec command.Runner) error {
 	glog.Infof("restarting kubelet.service ...")
 	return exec.Run("sudo systemctl restart kubelet.service")
 }
 
 // checkKubelet returns an error if the kubelet is not running.
-func checkKubelet(exec bootstrapper.CommandRunner) error {
+func checkKubelet(exec command.Runner) error {
 	glog.Infof("checking for running kubelet ...")
 	return exec.Run("systemctl is-active --quiet service kubelet")
 }
diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/client-go/tools/clientcmd/api"
 	"k8s.io/client-go/tools/clientcmd/api/latest"
 	"k8s.io/minikube/pkg/minikube/assets"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/util"
@@ -42,7 +43,7 @@ var (
 )
 
 // SetupCerts gets the generated credentials required to talk to the APIServer.
-func SetupCerts(cmd CommandRunner, k8s config.KubernetesConfig) error {
+func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig) error {
 	localPath := constants.GetMinipath()
 	glog.Infof("Setting up certificates for IP: %s\n", k8s.NodeIP)
 

diff --git a/pkg/minikube/bootstrapper/certs_test.go b/pkg/minikube/bootstrapper/certs_test.go
@@ -21,6 +21,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/tests"
@@ -31,7 +32,7 @@ func TestSetupCerts(t *testing.T) {
 	tempDir := tests.MakeTempDir()
 	defer os.RemoveAll(tempDir)
 
-	f := NewFakeCommandRunner()
+	f := command.NewFakeCommandRunner()
 	k8s := config.KubernetesConfig{
 		APIServerName: constants.APIServerName,
 		DNSDomain:     constants.ClusterDNSDomain,

diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go
@@ -36,6 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/console"
 	"k8s.io/minikube/pkg/minikube/constants"
@@ -94,7 +95,7 @@ var SkipAdditionalPreflights = map[string][]string{}
 
 // Bootstrapper is a bootstrapper using kubeadm
 type Bootstrapper struct {
-	c bootstrapper.CommandRunner
+	c command.Runner
 }
 
 // NewKubeadmBootstrapper creates a new kubeadm.Bootstrapper
@@ -640,7 +641,7 @@ func copyConfig(cfg config.KubernetesConfig, files []assets.CopyableFile, kubead
 	return files
 }
 
-func downloadBinaries(cfg config.KubernetesConfig, c bootstrapper.CommandRunner) error {
+func downloadBinaries(cfg config.KubernetesConfig, c command.Runner) error {
 	var g errgroup.Group
 	for _, bin := range constants.GetKubeadmCachedBinaries() {
 		bin := bin

diff --git a/pkg/minikube/bootstrapper/command_runner.go → pkg/minikube/command/command_runner.go b/pkg/minikube/bootstrapper/command_runner.go → pkg/minikube/command/command_runner.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package bootstrapper
+package command
 
 import (
 	"fmt"
@@ -24,8 +24,8 @@ import (
 	"k8s.io/minikube/pkg/minikube/assets"
 )
 
-// CommandRunner represents an interface to run commands.
-type CommandRunner interface {
+// Runner represents an interface to run commands.
+type Runner interface {
 	// Run starts the specified command and waits for it to complete.
 	Run(cmd string) error
 

diff --git a/pkg/minikube/bootstrapper/exec_runner.go → pkg/minikube/command/exec_runner.go b/pkg/minikube/bootstrapper/exec_runner.go → pkg/minikube/command/exec_runner.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package bootstrapper
+package command
 
 import (
 	"bytes"

diff --git a/pkg/minikube/bootstrapper/fake_runner.go → pkg/minikube/command/fake_runner.go b/pkg/minikube/bootstrapper/fake_runner.go → pkg/minikube/command/fake_runner.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package bootstrapper
+package command
 
 import (
 	"bytes"

diff --git a/pkg/minikube/bootstrapper/ssh_runner.go → pkg/minikube/command/ssh_runner.go b/pkg/minikube/bootstrapper/ssh_runner.go → pkg/minikube/command/ssh_runner.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package bootstrapper
+package command
 
 import (
 	"bytes"

diff --git a/pkg/minikube/cruntime/cruntime.go b/pkg/minikube/cruntime/cruntime.go
@@ -25,7 +25,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/console"
 )
 
-// CommandRunner is the subset of bootstrapper.CommandRunner this package consumes
+// CommandRunner is the subset of command.Runner this package consumes
 type CommandRunner interface {
 	Run(string) error
 	CombinedOutput(string) (string, error)

diff --git a/pkg/minikube/logs/logs.go b/pkg/minikube/logs/logs.go
@@ -28,6 +28,7 @@ import (
 
 	"github.com/golang/glog"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/console"
 	"k8s.io/minikube/pkg/minikube/cruntime"
 )
@@ -54,7 +55,7 @@ var importantPods = []string{
 const lookBackwardsCount = 200
 
 // Follow follows logs from multiple files in tail(1) format
-func Follow(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner bootstrapper.CommandRunner) error {
+func Follow(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner command.Runner) error {
 	cs := []string{}
 	for _, v := range logCommands(r, bs, 0, true) {
 		cs = append(cs, v+" &")
@@ -69,7 +70,7 @@ func IsProblem(line string) bool {
 }
 
 // FindProblems finds possible root causes among the logs
-func FindProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner bootstrapper.CommandRunner) map[string][]string {
+func FindProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner command.Runner) map[string][]string {
 	pMap := map[string][]string{}
 	cmds := logCommands(r, bs, lookBackwardsCount, false)
 	for name, cmd := range cmds {
@@ -110,7 +111,7 @@ func OutputProblems(problems map[string][]string, maxLines int) {
 }
 
 // Output displays logs from multiple sources in tail(1) format
-func Output(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner bootstrapper.CommandRunner, lines int) error {
+func Output(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner command.Runner, lines int) error {
 	cmds := logCommands(r, bs, lines, false)
 
 	// These are not technically logs, but are useful to have in bug reports.

diff --git a/pkg/minikube/machine/cache_binaries.go b/pkg/minikube/machine/cache_binaries.go
@@ -28,6 +28,7 @@ import (
 	"golang.org/x/sync/errgroup"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/console"
 	"k8s.io/minikube/pkg/minikube/constants"
 )
@@ -90,7 +91,7 @@ func CacheBinary(binary, version, osName, archName string) (string, error) {
 }
 
 // CopyBinary copies previously cached binaries into the path
-func CopyBinary(cr bootstrapper.CommandRunner, binary, path string) error {
+func CopyBinary(cr command.Runner, binary, path string) error {
 	f, err := assets.NewFileAsset(path, "/usr/bin", binary, "0755")
 	if err != nil {
 		return errors.Wrap(err, "new file asset")

diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go
@@ -35,6 +35,7 @@ import (
 	"golang.org/x/sync/errgroup"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/cruntime"
@@ -84,7 +85,7 @@ func CacheImages(images []string, cacheDir string) error {
 }
 
 // LoadImages loads previously cached images into the container runtime
-func LoadImages(cmd bootstrapper.CommandRunner, images []string, cacheDir string) error {
+func LoadImages(cmd command.Runner, images []string, cacheDir string) error {
 	var g errgroup.Group
 	// Load profile cluster config from file
 	cc, err := config.Load()
@@ -194,7 +195,7 @@ func getWindowsVolumeNameCmd(d string) (string, error) {
 }
 
 // loadImageFromCache loads a single image from the cache
-func loadImageFromCache(cr bootstrapper.CommandRunner, k8s config.KubernetesConfig, src string) error {
+func loadImageFromCache(cr command.Runner, k8s config.KubernetesConfig, src string) error {
 	glog.Infof("Loading image from cache: %s", src)
 	filename := filepath.Base(src)
 	if _, err := os.Stat(src); err != nil {

diff --git a/pkg/minikube/machine/client.go b/pkg/minikube/machine/client.go
@@ -40,7 +40,7 @@ import (
 	"github.com/docker/machine/libmachine/swarm"
 	"github.com/docker/machine/libmachine/version"
 	"github.com/pkg/errors"
-	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/exit"
 	"k8s.io/minikube/pkg/minikube/registry"
@@ -145,15 +145,15 @@ func (api *LocalClient) Close() error {
 }
 
 // CommandRunner returns best available command runner for this host
-func CommandRunner(h *host.Host) (bootstrapper.CommandRunner, error) {
+func CommandRunner(h *host.Host) (command.Runner, error) {
 	if h.DriverName == constants.DriverNone {
-		return &bootstrapper.ExecRunner{}, nil
+		return &command.ExecRunner{}, nil
 	}
 	client, err := sshutil.NewSSHClient(h.Driver)
 	if err != nil {
 		return nil, errors.Wrap(err, "getting ssh client for bootstrapper")
 	}
-	return bootstrapper.NewSSHRunner(client), nil
+	return command.NewSSHRunner(client), nil
 }
 
 // Create creates the host

diff --git a/pkg/provision/buildroot.go b/pkg/provision/buildroot.go
@@ -37,7 +37,7 @@ import (
 	"github.com/docker/machine/libmachine/swarm"
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/assets"
-	"k8s.io/minikube/pkg/minikube/bootstrapper"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/sshutil"
 	"k8s.io/minikube/pkg/util"
@@ -314,7 +314,7 @@ func configureAuth(p *BuildrootProvisioner) error {
 }
 
 func copyHostCerts(authOptions auth.Options) error {
-	execRunner := &bootstrapper.ExecRunner{}
+	execRunner := &command.ExecRunner{}
 	hostCerts := map[string]string{
 		authOptions.CaCertPath:     path.Join(authOptions.StorePath, "ca.pem"),
 		authOptions.ClientCertPath: path.Join(authOptions.StorePath, "cert.pem"),
@@ -345,7 +345,7 @@ func copyRemoteCerts(authOptions auth.Options, driver drivers.Driver) error {
 	if err != nil {
 		return errors.Wrap(err, "provisioning: error getting ssh client")
 	}
-	sshRunner := bootstrapper.NewSSHRunner(sshClient)
+	sshRunner := command.NewSSHRunner(sshClient)
 	for src, dst := range remoteCerts {
 		f, err := assets.NewFileAsset(src, path.Dir(dst), filepath.Base(dst), "0640")
 		if err != nil {