Merge pull request #8290 from sharifelgamal/native-ssh

respect native-ssh param properly

cmd/minikube/cmd/ssh.go

@@ -19,7 +19,6 @@ package cmd
 import (
 	"os"
 
-	"github.com/docker/machine/libmachine/ssh"
 	"github.com/spf13/cobra"
 
 	"k8s.io/minikube/pkg/minikube/config"
@@ -58,13 +57,7 @@ var sshCmd = &cobra.Command{
 			}
 		}
 
-		if nativeSSHClient {
-			ssh.SetDefaultClient(ssh.Native)
-		} else {
-			ssh.SetDefaultClient(ssh.External)
-		}
-
-		err = machine.CreateSSHShell(co.API, *co.Config, *n, args)
+		err = machine.CreateSSHShell(co.API, *co.Config, *n, args, nativeSSHClient)
 		if err != nil {
 			// This is typically due to a non-zero exit code, so no need for flourish.
 			out.ErrLn("ssh: %v", err)

cmd/minikube/cmd/start.go

@@ -246,12 +246,6 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *
 		cc.MinikubeISO = url
 	}
 
-	if viper.GetBool(nativeSSH) {
-		ssh.SetDefaultClient(ssh.Native)
-	} else {
-		ssh.SetDefaultClient(ssh.External)
-	}
-
 	var existingAddons map[string]bool
 	if viper.GetBool(installAddons) {
 		existingAddons = map[string]bool{}
@@ -265,6 +259,12 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *
 		return node.Starter{}, err
 	}
 
+	if viper.GetBool(nativeSSH) {
+		ssh.SetDefaultClient(ssh.Native)
+	} else {
+		ssh.SetDefaultClient(ssh.External)
+	}
+
 	return node.Starter{
 		Runner:         mRunner,
 		PreExists:      preExists,

pkg/minikube/machine/cluster_test.go

@@ -454,7 +454,7 @@ func TestCreateSSHShell(t *testing.T) {
 	cc.Name = viper.GetString("profile")
 
 	cliArgs := []string{"exit"}
-	if err := CreateSSHShell(api, cc, config.Node{Name: "minikube"}, cliArgs); err != nil {
+	if err := CreateSSHShell(api, cc, config.Node{Name: "minikube"}, cliArgs, true); err != nil {
 		t.Fatalf("Error running ssh command: %v", err)
 	}
 

pkg/minikube/machine/ssh.go

@@ -18,14 +18,15 @@ package machine
 
 import (
 	"github.com/docker/machine/libmachine"
+	"github.com/docker/machine/libmachine/ssh"
 	"github.com/docker/machine/libmachine/state"
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/driver"
 )
 
 // CreateSSHShell creates a new SSH shell / client
-func CreateSSHShell(api libmachine.API, cc config.ClusterConfig, n config.Node, args []string) error {
+func CreateSSHShell(api libmachine.API, cc config.ClusterConfig, n config.Node, args []string, native bool) error {
 	machineName := driver.MachineName(cc, n)
 	host, err := LoadHost(api, machineName)
 	if err != nil {
@@ -42,6 +43,13 @@ func CreateSSHShell(api libmachine.API, cc config.ClusterConfig, n config.Node,
 	}
 
 	client, err := host.CreateSSHClient()
+
+	if native {
+		ssh.SetDefaultClient(ssh.Native)
+	} else {
+		ssh.SetDefaultClient(ssh.External)
+	}
+
 	if err != nil {
 		return errors.Wrap(err, "Creating ssh client")
 	}

test/integration/functional_test.go

@@ -837,7 +837,7 @@ func validateSSHCmd(ctx context.Context, t *testing.T, profile string) {
 	mctx, cancel := context.WithTimeout(ctx, Minutes(1))
 	defer cancel()
 
-	want := "hello\n"
+	want := "hello"
 
 	rr, err := Run(t, exec.CommandContext(mctx, Target(), "-p", profile, "ssh", "echo hello"))
 	if mctx.Err() == context.DeadlineExceeded {
@@ -846,15 +846,16 @@ func validateSSHCmd(ctx context.Context, t *testing.T, profile string) {
 	if err != nil {
 		t.Errorf("failed to run an ssh command. args %q : %v", rr.Command(), err)
 	}
-	if rr.Stdout.String() != want {
+	// trailing whitespace differs between native and external SSH clients, so let's trim it and call it a day
+	if strings.TrimSpace(rr.Stdout.String()) != want {
 		t.Errorf("expected minikube ssh command output to be -%q- but got *%q*. args %q", want, rr.Stdout.String(), rr.Command())
 	}
 
 	// testing hostname as well because testing something like "minikube ssh echo" could be confusing
 	// because it  is not clear if echo was run inside minikube on the powershell
 	// so better to test something inside minikube, that is meaningful per profile
 	// in this case /etc/hostname is same as the profile name
-	want = profile + "\n"
+	want = profile
 	rr, err = Run(t, exec.CommandContext(mctx, Target(), "-p", profile, "ssh", "cat /etc/hostname"))
 	if mctx.Err() == context.DeadlineExceeded {
 		t.Errorf("failed to run command by deadline. exceeded timeout : %s", rr.Command())
@@ -863,7 +864,8 @@ func validateSSHCmd(ctx context.Context, t *testing.T, profile string) {
 	if err != nil {
 		t.Errorf("failed to run an ssh command. args %q : %v", rr.Command(), err)
 	}
-	if rr.Stdout.String() != want {
+	// trailing whitespace differs between native and external SSH clients, so let's trim it and call it a day
+	if strings.TrimSpace(rr.Stdout.String()) != want {
 		t.Errorf("expected minikube ssh command output to be -%q- but got *%q*. args %q", want, rr.Stdout.String(), rr.Command())
 	}
 }