First steps to make ssh command smoother

Signed-off-by: Nathan LeClaire <nathan.leclaire@gmail.com>

commands/commands.go

@@ -357,10 +357,11 @@ var Commands = []cli.Command{
 		Action:      cmdRm,
 	},
 	{
-		Name:        "ssh",
+		Name:            "ssh",
-		Usage:       "Log into or run a command on a machine with SSH.",
+		Usage:           "Log into or run a command on a machine with SSH.",
-		Description: "Arguments are [machine-name] [command]",
+		Description:     "Arguments are [machine-name] [command]",
-		Action:      cmdSsh,
+		Action:          cmdSsh,
+		SkipFlagParsing: true,
 	},
 	{
 		Name:        "scp",

commands/ssh.go

@@ -13,7 +13,6 @@ import (
 func cmdSsh(c *cli.Context) {
 	args := c.Args()
 	name := args.First()
-	cmd := ""
 
 	if name == "" {
 		log.Fatal("Error: Please specify a machine name.")
@@ -48,36 +47,13 @@ func cmdSsh(c *cli.Context) {
 		log.Fatalf("Error: Cannot run SSH command: Host %q is not running", host.Name)
 	}
 
-	// Loop through the arguments and parse out a command which relies on
-	// flags if it exists, for instance an invocation of the form
-	// `docker-machine ssh dev -- df -h` would mandate this, otherwise we
-	// will accidentally trigger the codegangsta/cli help text because it
-	// thinks we are trying to specify codegangsta flags.
-	//
-	// TODO: I thought codegangsta/cli supported the flag parsing
-	// terminator manually, which would mitigate the need for this kind of
-	// hack.  We should investigate.
-	for i, arg := range args {
-		if arg == "--" {
-			cmd = strings.Join(args[i+1:], " ")
-			break
-		}
-	}
-
-	// It is possible that the user has specified an appended command which
-	// does not rely on the flag parsing terminator, such as
-	// `docker-machine ssh dev ls`, so this block accounts for that case.
-	if len(cmd) == 0 {
-		cmd = strings.Join(args[1:], " ")
-	}
-
 	if len(c.Args()) == 1 {
 		err := host.CreateSSHShell()
 		if err != nil {
 			log.Fatal(err)
 		}
 	} else {
-		output, err := host.RunSSHCommand(cmd)
+		output, err := host.RunSSHCommand(strings.Join(c.Args().Tail(), " "))
 		if err != nil {
 			log.Fatal(err)
 		}

docs/reference/ssh.md

@@ -46,12 +46,10 @@ Mem:       1023556       183136       840420            0        30920
 Swap:      1212036            0      1212036
 ```
 
-If the command you are appending has flags, e.g. `df -h`, you can use the flag
+Commands with flags will work as well:
-parsing terminator (`--`) to avoid confusing the `docker-machine` client, which
-will otherwise interpret them as flags you intended to pass to it:
 
 ```
-$ docker-machine ssh dev -- df -h
+$ docker-machine ssh dev df -h
 Filesystem                Size      Used Available Use% Mounted on
 rootfs                  899.6M     85.9M    813.7M  10% /
 tmpfs                   899.6M     85.9M    813.7M  10% /
@@ -62,6 +60,17 @@ cgroup                  499.8M         0    499.8M   0% /sys/fs/cgroup
 /mnt/sda1/var/lib/docker/aufs
 ```
 
+If you are using the "external" SSH type as detailed in the next section, you
+can include additional arguments to pass through to the `ssh` binary in the
+generated command (unless they conflict with any of the default arguments for
+the command generated by Docker Machine).  For instance, the following command
+will forward port 8080 from the `default` machine to `localhost` on your host
+computer:
+
+```
+$ docker-machine ssh default -L 8080:localhost:8080
+```
+
 ## Different types of SSH
 
 When Docker Machine is invoked, it will check to see if you have the venerable

ssh/client.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"strings"
 
 	"github.com/docker/docker/pkg/term"
 	"github.com/docker/machine/log"
@@ -256,9 +257,7 @@ func NewExternalClient(sshBinaryPath, user, host string, port int, auth *Auth) (
 		BinaryPath: sshBinaryPath,
 	}
 
-	// Base args take care of settings some options for us, e.g. don't use
+	args := append(baseSSHArgs, fmt.Sprintf("%s@%s", user, host))
-	// the authorized hosts file.
-	args := baseSSHArgs
 
 	// Specify which private keys to use to authorize the SSH request.
 	for _, privateKeyPath := range auth.Keys {
@@ -268,16 +267,15 @@ func NewExternalClient(sshBinaryPath, user, host string, port int, auth *Auth) (
 	// Set which port to use for SSH.
 	args = append(args, "-p", fmt.Sprintf("%d", port))
 
-	// Set the user and hostname, e.g. ubuntu@12.34.56.78
-	args = append(args, fmt.Sprintf("%s@%s", user, host))
-
 	client.BaseArgs = args
 
 	return client, nil
 }
 
 func (client ExternalClient) Output(command string) (string, error) {
-	args := append(client.BaseArgs, command)
+	// TODO: Ugh, gross hack.  Replace with all instances using variadic
+	// syntax
+	args := append(client.BaseArgs, strings.Split(command, " ")...)
 
 	cmd := exec.Command(client.BinaryPath, args...)
 	log.Debug(cmd)

test/integration/core/ssh-backends.bats

@@ -10,7 +10,7 @@ load ${BASE_TEST_DIR}/helpers.bash
 }
 
 @test "$DRIVER: test external ssh backend" {
-  run machine ssh $NAME -- df -h
+  run machine ssh $NAME df -h
   [[ "$status" -eq 0 ]]
 }
 
@@ -20,7 +20,7 @@ load ${BASE_TEST_DIR}/helpers.bash
 }
 
 @test "$DRIVER: test native ssh backend" {
-  run machine --native-ssh ssh $NAME -- df -h
+  run machine --native-ssh ssh $NAME df -h
   [[ "$status" -eq 0  ]]
 }
 
@@ -28,3 +28,18 @@ load ${BASE_TEST_DIR}/helpers.bash
   run machine --native-ssh ssh $NAME echo foo
   [[ "$output" == "foo"  ]]
 }
+
+@test "$DRIVER: ensure that ssh extra arguments work" {
+  # don't run this test if we can't use external SSH
+  which ssh
+  if [[ $? -ne 0 ]]; then
+    skip
+  fi
+
+  # this will not fare well if -C doesn't get interpreted as "use compression"
+  # like intended
+  run machine ssh $NAME -C echo foo
+
+  [[ "$status" -eq 0 ]]
+  [[ "$output" == "foo" ]]
+}

test/integration/helpers.bash

@@ -26,3 +26,6 @@ function require_env () {
         exit 1
     fi
 }
+
+# Make sure these aren't set while tests run (can cause confusing behavior)
+unset DOCKER_HOST DOCKER_TLS_VERIFY DOCKER_CERT_DIR