Merge lp:~dimitern/juju-core/302-lp-1281577-ssh-command-extra-args into lp:~go-bot/juju-core/trunk

[Dimiter Naydenov]

cmd/juju: ssh/scp commands extra arguments; fixes

Removed the extraneous "--" that the ssh command
adds to the passed arguments, so any extra arguments
after the command will be forwarded to ssh properly.
It worked in 1.16, so this fixes a regression.
This fixes bug #1281577.

The scp command claims in its help that it supports
-- to pass extra arguments to scp, but it does not
and also does not support multiple local/remote
targets to be specified, as OpenSSH scp command does.
This fixes bug #1283412.

Also, improved a lot of ssh-related tests in both
utils/ssh/ and in cmd/juju/, adding new tests as
needed for the extra arguments.

Improved help texts for ssh, scp and add-machine
commands (included examples for the extra args
and multiple targets for scp, added ssh: example
for add-machine).

All changes are tested live:
 - bootstrapping on EC2 and running ssh or scp
 to test extra args and multiple targets;
 - manual bootstrapping
 - add-machine ssh:user@host to an existing env;
 - using juju on Windows, to test the embedded
 go.crypto ssh command.

https://codereview.appspot.com/66340045/

R=axwalk, gz

cmd/juju: ssh/scp commands extra arguments; fixes

Removed the extraneous "--" that the ssh command
adds to the passed arguments, so any extra arguments
after the command will be forwarded to ssh properly.
It worked in 1.16, so this fixes a regression.
This fixes bug #1281577.

The scp command claims in its help that it supports
-- to pass extra arguments to scp, but it does not
and also does not support multiple local/remote
targets to be specified, as OpenSSH scp command does.
This fixes bug #1283412.

Also, improved a lot of ssh-related tests in both
utils/ssh/ and in cmd/juju/, adding new tests as
needed for the extra arguments.

Improved help texts for ssh, scp and add-machine
commands (included examples for the extra args
and multiple targets for scp, added ssh: example
for add-machine).

All changes are tested live:
 - bootstrapping on EC2 and running ssh or scp
 to test extra args and multiple targets;
 - manual bootstrapping
 - add-machine ssh:user@host to an existing env;
 - using juju on Windows, to test the embedded
 go.crypto ssh command.

https://codereview.appspot.com/66340045/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+207498_code.launchpad.net,

Message:
Please take a look.

Description:
ssh: Fixed bug #1281577 - ssh command-line opts

Removed the extraneous "--" that the ssh command
adds to the passed arguments, so any extra arguments
after the command will be forwarded to ssh properly.
It worked in 1.16, so this fixes a regression.

https://code.launchpad.net/~dimitern/juju-core/302-lp-1281577-ssh-command-extra-args/+merge/207498

Requires:
https://code.launchpad.net/~dimitern/juju-core/301-lp-1282553-use-curl-not-wget-precise/+merge/207443

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/66340045/

Affected files (+3, -8 lines):
   A [revision details]
   M cmd/juju/ssh.go
   M utils/ssh/ssh_openssh.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>

Index: cmd/juju/ssh.go
=== modified file 'cmd/juju/ssh.go'
--- cmd/juju/ssh.go 2014-01-13 06:25:28 +0000
+++ cmd/juju/ssh.go 2014-02-20 16:46:11 +0000
@@ -82,15 +82,9 @@
   if err != nil {
    return err
   }
- args := c.Args
- if len(args) > 0 && args[0] == "--" {
- // utils/ssh adds "--"; we will continue to accept
- // it from the CLI for backwards compatibility.
- args = args[1:]
- }
   var options ssh.Options
   options.EnablePTY()
- cmd := ssh.Command("ubuntu@"+host, args, &options)
+ cmd := ssh.Command("ubuntu@"+host, c.Args, &options)
   cmd.Stdin = ctx.Stdin
   cmd.Stdout = ctx.Stdout
   cmd.Stderr = ctx.Stderr

Index: utils/ssh/ssh_openssh.go
=== modified file 'utils/ssh/ssh_openssh.go'
--- utils/ssh/ssh_openssh.go 2014-02-05 08:48:40 +0000
+++ utils/ssh/ssh_openssh.go 2014-02-20 16:46:11 +0000
@@ -113,7 +113,6 @@
   args := opensshOptions(options, sshKind)
   args = append(args, host)
   if len(command) > 0 {
- args = append(args, "--")
    args = append(args, command...)
   }
   bin, args := sshpassWrap("ssh", args)

[Martin Packman (gz)]

LGTM, but there are a few other questionable bits involved with the
original change in r2116 that rely on sane -- behaviour - perhaps we
should bug Andrew as well.

https://codereview.appspot.com/66340045/

[Andrew Wilkins (axwalk)]

On 2014/02/20 17:14:00, gz wrote:
> LGTM, but there are a few other questionable bits involved with the
original
> change in r2116 that rely on sane -- behaviour - perhaps we should bug
Andrew as
> well.

Which bits?

I don't recall my rationale, but it was probably something like: this is
relying on implementation details, namely that we're using OpenSSH.
We're not necessarily using OpenSSH now, so those args passed through
will break if the embedded client is used. I suppose it's not a big
deal.

https://codereview.appspot.com/66340045/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/66340045/

[Andrew Wilkins (axwalk)]

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go
File cmd/juju/scp.go (left):

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#oldcode73
cmd/juju/scp.go:73: // BUG(dfc) This will not work for IPv6 addresses
like 2001:db8::1:2:/somepath.
The BUG is still relevant, no?

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go
File cmd/juju/scp.go (right):

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode32
cmd/juju/scp.go:32: Copy 2 files from two units to the local backup/
directory, passig -v
s/passig/passing/

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode35
cmd/juju/scp.go:35: juju scp ubuntu/0:/path/file1 -- -v
ubuntu/1:/path/file2 backup/
While it may be possible to do this, is it useful to highlight it as an
example? It seems a bit pathological, and not actually useful, so I'd
just move "-- -v" to the end.

Also, we shouldn't *need* -- at all if the options are at the end,
right? It would good if you could just do:
     juju scp 0:/remote/dir /local/dir -r

We may want to explicitly state that options are not supported unless
OpenSSH is available on the system... although, Copy isn't even
implemented using go.crypto at the moment.

Frankly, I think it's a bad idea to rely on implementation details like
this (which is not your fault of course). It would be better if we just
implemented a subset of the scp options and passed them through. Then
we're more easily able to implement those for the go.crypto client.

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode91
cmd/juju/scp.go:91: if strings.HasPrefix(arg, "-") {
Actually, one of the more common usages of pass-through args that I've
seen so far is "-o", "option". This won't work, since the option value
will end up in the wrong place.

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh.go
File utils/ssh/ssh.go (right):

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh.go#newcode72
utils/ssh/ssh.go:72: // Copy copies file(s) between the local host a
remote host(s).
s/a /and /
(or just "local and remote")

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh.go#newcode74
utils/ssh/ssh.go:74: // any extra arguments are specified in extraArgs,
they are passed
Client is implementation agnostic. Talking about scp here is wrong.

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh_openssh.go
File utils/ssh/ssh_openssh.go (right):

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh_openssh.go#newcode17
utils/ssh/ssh_openssh.go:17: var opensshCommonOptions =
map[string][]string{"-o": []string{"StrictHostKeyChecking no"}}
What's the point of this change? To group options? Why? Seems to be
complicating things for no practical benefit.

https://codereview.appspot.com/66340045/

[Adam Collard (adam-collard)]

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode32
cmd/juju/scp.go:32: Copy 2 files from two units to the local backup/
directory, passig -v
s/passig/passing/

It doesn't look like this comment got addressed.

http://bazaar.launchpad.net/~go-bot/juju-core/trunk/view/head:/cmd/juju/scp.go#L32

Shows it still says "passig"

review: Needs Fixing

[Dimiter Naydenov (dimitern)]

I'm about to propose a follow-up with the last review's suggestions, as the branch landed before that.

[Dimiter Naydenov (dimitern)]

Thanks for the review!
Find below my answers, and please review the follow-up which implements
the suggestions, here: https://codereview.appspot.com/67820048

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go
File cmd/juju/scp.go (left):

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#oldcode73
cmd/juju/scp.go:73: // BUG(dfc) This will not work for IPv6 addresses
like 2001:db8::1:2:/somepath.
On 2014/02/24 04:29:36, axw wrote:
> The BUG is still relevant, no?

I thought I fixed it, but I forgot. According to
http://marc.info/?l=openssh-unix-dev&m=112975365929888 we need to add
`\[`+host+`\]:` so that IPv6 addresses will be properly separated from
the path. Fixed and added a test for it.

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go
File cmd/juju/scp.go (right):

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode32
cmd/juju/scp.go:32: Copy 2 files from two units to the local backup/
directory, passig -v
On 2014/02/24 04:29:36, axw wrote:
> s/passig/passing/

Done.

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode35
cmd/juju/scp.go:35: juju scp ubuntu/0:/path/file1 -- -v
ubuntu/1:/path/file2 backup/
On 2014/02/24 04:29:36, axw wrote:
> While it may be possible to do this, is it useful to highlight it as
an example?
> It seems a bit pathological, and not actually useful, so I'd just move
"-- -v"
> to the end.

> Also, we shouldn't *need* -- at all if the options are at the end,
right? It
> would good if you could just do:
> juju scp 0:/remote/dir /local/dir -r

Good point, I've changed the implementation, help text, and tests to
accept extra arguments at the end only and -- is no longer needed or
allowed.

> We may want to explicitly state that options are not supported unless
OpenSSH is
> available on the system... although, Copy isn't even implemented using
go.crypto
> at the moment.

Mentioning OpenSSH is useful, added.

Also improved the error message given by go.crypto's Copy, because:
$ juju scp xx yy
ERROR Copy is not implemented

> Frankly, I think it's a bad idea to rely on implementation details
like this
> (which is not your fault of course). It would be better if we just
implemented a
> subset of the scp options and passed them through. Then we're more
easily able
> to implement those for the go.crypto client.

Yeah, that's definitely a future improvement worth doing, but not now.

https://codereview.appspot.com/66340045/diff/20001/cmd/juju/scp.go#newcode91
cmd/juju/scp.go:91: if strings.HasPrefix(arg, "-") {
On 2014/02/24 04:29:36, axw wrote:
> Actually, one of the more common usages of pass-through args that I've
seen so
> far is "-o", "option". This won't work, since the option value will
end up in
> the wrong place.

It works actually, added -o SomeOption in the extra args tests.

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh.go
File utils/ssh/ssh.go (right):

https://codereview.appspot.com/66340045/diff/20001/utils/ssh/ssh.go#newcode72
utils/ssh/ssh.go:72: // Copy copies file(s) between the local host a
remote host(s).
On 2014/02/24 04:29:36, axw wrote:
> s/a /and /
> (or just "local and remote"...

=== modified file 'cmd/juju/addmachine.go'
--- cmd/juju/addmachine.go	2013-12-20 09:25:57 +0000
+++ cmd/juju/addmachine.go	2014-02-22 15:58:57 +0000
@@ -48,6 +48,7 @@
    juju add-machine lxc                  (starts a new machine with an lxc container)
    juju add-machine lxc:4                (starts a new lxc container on machine 4)
    juju add-machine --constraints mem=8G (starts a machine with at least 8GB RAM)
+   juju add-machine ssh:user@10.10.0.3   (manually provisions a machine with ssh)
 
 See Also:
    juju help constraints

=== modified file 'cmd/juju/debughooks_test.go'
--- cmd/juju/debughooks_test.go	2013-12-03 14:19:47 +0000
+++ cmd/juju/debughooks_test.go	2014-02-22 15:58:57 +0000
@@ -29,10 +29,10 @@
 	stderr string
 }{{
 	args:   []string{"mysql/0"},
-	result: regexp.QuoteMeta(debugHooksArgs + "[e-mail] -- sudo /bin/bash -c 'F=$(mktemp); echo IyEvYmluL2Jhc2gKKAojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnIGxvY2tmaWxlLgpmbG9jayAtbiA4IHx8IChlY2hvICJGYWlsZWQgdG8gYWNxdWlyZSAvdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzOiB1bml0IGlzIGFscmVhZHkgYmVpbmcgZGVidWdnZWQiIDI+JjE7IGV4aXQgMSkKKAojIENsb3NlIHRoZSBpbmhlcml0ZWQgbG9jayBGRCwgb3IgdG11eCB3aWxsIGtlZXAgaXQgb3Blbi4KZXhlYyA4PiYtCgojIFdyaXRlIG91dCB0aGUgZGVidWctaG9va3MgYXJncy4KZWNobyAiZTMwSyIgfCBiYXNlNjQgLWQgPiAvdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzCgojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnLWV4aXQgbG9ja2ZpbGUuCmZsb2NrIC1uIDkgfHwgZXhpdCAxCgojIFdhaXQgZm9yIHRtdXggdG8gYmUgaW5zdGFsbGVkLgp3aGlsZSBbICEgLWYgL3Vzci9iaW4vdG11eCBdOyBkbwogICAgc2xlZXAgMQpkb25lCgppZiBbICEgLWYgfi8udG11eC5jb25mIF07IHRoZW4KICAgICAgICBpZiBbIC1mIC91c3Ivc2hhcmUvYnlvYnUvcHJvZmlsZXMvdG11eCBdOyB0aGVuCiAgICAgICAgICAgICAgICAjIFVzZSBieW9idS90bXV4IHByb2ZpbGUgZm9yIGZhbWlsaWFyIGtleWJpbmRpbmdzIGFuZCBicmFuZGluZwogICAgICAgICAgICAgICAgZWNobyAic291cmNlLWZpbGUgL3Vzci9zaGFyZS9ieW9idS9wcm9maWxlcy90bXV4IiA+IH4vLnRtdXguY29uZgogICAgICAgIGVsc2UKICAgICAgICAgICAgICAgICMgT3RoZXJ3aXNlLCB1c2UgdGhlIGxlZ2FjeSBqdWp1L3RtdXggY29uZmlndXJhdGlvbgogICAgICAgICAgICAgICAgY2F0ID4gfi8udG11eC5jb25mIDw8RU5ECiAgICAgICAgICAgICAgICAKIyBTdGF0dXMgYmFyCnNldC1vcHRpb24gLWcgc3RhdHVzLWJnIGJsYWNrCnNldC1vcHRpb24gLWcgc3RhdHVzLWZnIHdoaXRlCgpzZXQtd2luZG93LW9wdGlvbiAtZyB3aW5kb3ctc3RhdHVzLWN1cnJlbnQtYmcgcmVkCnNldC13aW5kb3ctb3B0aW9uIC1nIHdpbmRvdy1zdGF0dXMtY3VycmVudC1hdHRyIGJyaWdodAoKc2V0LW9wdGlvbiAtZyBzdGF0dXMtcmlnaHQgJycKCiMgUGFuZXMKc2V0LW9wdGlvbiAtZyBwYW5lLWJvcmRlci1mZyB3aGl0ZQpzZXQtb3B0aW9uIC1nIHBhbmUtYWN0aXZlLWJvcmRlci1mZyB3aGl0ZQoKIyBNb25pdG9yIGFjdGl2aXR5IG9uIHdpbmRvd3MKc2V0LXdpbmRvdy1vcHRpb24gLWcgbW9uaXRvci1hY3Rpdml0eSBvbgoKIyBTY3JlZW4gYmluZGluZ3MsIHNpbmNlIHBlb3BsZSBhcmUgbW9yZSBmYW1pbGlhciB3aXRoIHRoYXQuCnNldC1vcHRpb24gLWcgcHJlZml4IEMtYQpiaW5kIEMtYSBsYXN0LXdpbmRvdwpiaW5kIGEgc2VuZC1rZXkgQy1hCgpiaW5kIHwgc3BsaXQtd2luZG93IC1oCmJpbmQgLSBzcGxpdC13aW5kb3cgLXYKCiMgRml4IENUUkwtUEdVUC9QR0RPV04gZm9yIHZpbQpzZXQtd2luZG93LW9wdGlvbiAtZyB4dGVybS1rZXlzIG9uCgojIFByZXZlbnQgRVNDIGtleSBmcm9tIGFkZGluZyBkZWxheSBhbmQgYnJlYWtpbmcgVmltJ3MgRVNDID4gYXJyb3cga2V5CnNldC1vcHRpb24gLXMgZXNjYXBlLXRpbWUgMAoKRU5ECiAgICAgICAgZmkKZmkKCigKICAgICMgQ2xvc2UgdGhlIGluaGVyaXRlZCBsb2NrIEZELCBvciB0bXV4IHdpbGwga2VlcCBpdCBvcGVuLgogICAgZXhlYyA5PiYtCiAgICBleGVjIHRtdXggbmV3LXNlc3Npb24gLXMgbXlzcWwvMAopCikgOT4vdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzLWV4aXQKKSA4Pi90bXAvanVqdS11bml0LW15c3FsLTAtZGVidWctaG9va3MKZXhpdCAkPwo= | base64 -d > $F; . $F'\n"),
+	result: regexp.QuoteMeta(debugHooksArgs + "[e-mail] sudo /bin/bash -c 'F=$(mktemp); echo IyEvYmluL2Jhc2gKKAojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnIGxvY2tmaWxlLgpmbG9jayAtbiA4IHx8IChlY2hvICJGYWlsZWQgdG8gYWNxdWlyZSAvdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzOiB1bml0IGlzIGFscmVhZHkgYmVpbmcgZGVidWdnZWQiIDI+JjE7IGV4aXQgMSkKKAojIENsb3NlIHRoZSBpbmhlcml0ZWQgbG9jayBGRCwgb3IgdG11eCB3aWxsIGtlZXAgaXQgb3Blbi4KZXhlYyA4PiYtCgojIFdyaXRlIG91dCB0aGUgZGVidWctaG9va3MgYXJncy4KZWNobyAiZTMwSyIgfCBiYXNlNjQgLWQgPiAvdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzCgojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnLWV4aXQgbG9ja2ZpbGUuCmZsb2NrIC1uIDkgfHwgZXhpdCAxCgojIFdhaXQgZm9yIHRtdXggdG8gYmUgaW5zdGFsbGVkLgp3aGlsZSBbICEgLWYgL3Vzci9iaW4vdG11eCBdOyBkbwogICAgc2xlZXAgMQpkb25lCgppZiBbICEgLWYgfi8udG11eC5jb25mIF07IHRoZW4KICAgICAgICBpZiBbIC1mIC91c3Ivc2hhcmUvYnlvYnUvcHJvZmlsZXMvdG11eCBdOyB0aGVuCiAgICAgICAgICAgICAgICAjIFVzZSBieW9idS90bXV4IHByb2ZpbGUgZm9yIGZhbWlsaWFyIGtleWJpbmRpbmdzIGFuZCBicmFuZGluZwogICAgICAgICAgICAgICAgZWNobyAic291cmNlLWZpbGUgL3Vzci9zaGFyZS9ieW9idS9wcm9maWxlcy90bXV4IiA+IH4vLnRtdXguY29uZgogICAgICAgIGVsc2UKICAgICAgICAgICAgICAgICMgT3RoZXJ3aXNlLCB1c2UgdGhlIGxlZ2FjeSBqdWp1L3RtdXggY29uZmlndXJhdGlvbgogICAgICAgICAgICAgICAgY2F0ID4gfi8udG11eC5jb25mIDw8RU5ECiAgICAgICAgICAgICAgICAKIyBTdGF0dXMgYmFyCnNldC1vcHRpb24gLWcgc3RhdHVzLWJnIGJsYWNrCnNldC1vcHRpb24gLWcgc3RhdHVzLWZnIHdoaXRlCgpzZXQtd2luZG93LW9wdGlvbiAtZyB3aW5kb3ctc3RhdHVzLWN1cnJlbnQtYmcgcmVkCnNldC13aW5kb3ctb3B0aW9uIC1nIHdpbmRvdy1zdGF0dXMtY3VycmVudC1hdHRyIGJyaWdodAoKc2V0LW9wdGlvbiAtZyBzdGF0dXMtcmlnaHQgJycKCiMgUGFuZXMKc2V0LW9wdGlvbiAtZyBwYW5lLWJvcmRlci1mZyB3aGl0ZQpzZXQtb3B0aW9uIC1nIHBhbmUtYWN0aXZlLWJvcmRlci1mZyB3aGl0ZQoKIyBNb25pdG9yIGFjdGl2aXR5IG9uIHdpbmRvd3MKc2V0LXdpbmRvdy1vcHRpb24gLWcgbW9uaXRvci1hY3Rpdml0eSBvbgoKIyBTY3JlZW4gYmluZGluZ3MsIHNpbmNlIHBlb3BsZSBhcmUgbW9yZSBmYW1pbGlhciB3aXRoIHRoYXQuCnNldC1vcHRpb24gLWcgcHJlZml4IEMtYQpiaW5kIEMtYSBsYXN0LXdpbmRvdwpiaW5kIGEgc2VuZC1rZXkgQy1hCgpiaW5kIHwgc3BsaXQtd2luZG93IC1oCmJpbmQgLSBzcGxpdC13aW5kb3cgLXYKCiMgRml4IENUUkwtUEdVUC9QR0RPV04gZm9yIHZpbQpzZXQtd2luZG93LW9wdGlvbiAtZyB4dGVybS1rZXlzIG9uCgojIFByZXZlbnQgRVNDIGtleSBmcm9tIGFkZGluZyBkZWxheSBhbmQgYnJlYWtpbmcgVmltJ3MgRVNDID4gYXJyb3cga2V5CnNldC1vcHRpb24gLXMgZXNjYXBlLXRpbWUgMAoKRU5ECiAgICAgICAgZmkKZmkKCigKICAgICMgQ2xvc2UgdGhlIGluaGVyaXRlZCBsb2NrIEZELCBvciB0bXV4IHdpbGwga2VlcCBpdCBvcGVuLgogICAgZXhlYyA5PiYtCiAgICBleGVjIHRtdXggbmV3LXNlc3Npb24gLXMgbXlzcWwvMAopCikgOT4vdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzLWV4aXQKKSA4Pi90bXAvanVqdS11bml0LW15c3FsLTAtZGVidWctaG9va3MKZXhpdCAkPwo= | base64 -d > $F; . $F'\n"),
 }, {
 	args:   []string{"mongodb/1"},
-	result: regexp.QuoteMeta(debugHooksArgs + "[e-mail] -- sudo /bin/bash -c 'F=$(mktemp); echo IyEvYmluL2Jhc2gKKAojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnIGxvY2tmaWxlLgpmbG9jayAtbiA4IHx8IChlY2hvICJGYWlsZWQgdG8gYWNxdWlyZSAvdG1wL2p1anUtdW5pdC1tb25nb2RiLTEtZGVidWctaG9va3M6IHVuaXQgaXMgYWxyZWFkeSBiZWluZyBkZWJ1Z2dlZCIgMj4mMTsgZXhpdCAxKQooCiMgQ2xvc2UgdGhlIGluaGVyaXRlZCBsb2NrIEZELCBvciB0bXV4IHdpbGwga2VlcCBpdCBvcGVuLgpleGVjIDg+Ji0KCiMgV3JpdGUgb3V0IHRoZSBkZWJ1Zy1ob29rcyBhcmdzLgplY2hvICJlMzBLIiB8IGJhc2U2NCAtZCA+IC90bXAvanVqdS11bml0LW1vbmdvZGItMS1kZWJ1Zy1ob29rcwoKIyBMb2NrIHRoZSBqdWp1LTx1bml0Pi1kZWJ1Zy1leGl0IGxvY2tmaWxlLgpmbG9jayAtbiA5IHx8IGV4aXQgMQoKIyBXYWl0IGZvciB0bXV4IHRvIGJlIGluc3RhbGxlZC4Kd2hpbGUgWyAhIC1mIC91c3IvYmluL3RtdXggXTsgZG8KICAgIHNsZWVwIDEKZG9uZQoKaWYgWyAhIC1mIH4vLnRtdXguY29uZiBdOyB0aGVuCiAgICAgICAgaWYgWyAtZiAvdXNyL3NoYXJlL2J5b2J1L3Byb2ZpbGVzL3RtdXggXTsgdGhlbgogICAgICAgICAgICAgICAgIyBVc2UgYnlvYnUvdG11eCBwcm9maWxlIGZvciBmYW1pbGlhciBrZXliaW5kaW5ncyBhbmQgYnJhbmRpbmcKICAgICAgICAgICAgICAgIGVjaG8gInNvdXJjZS1maWxlIC91c3Ivc2hhcmUvYnlvYnUvcHJvZmlsZXMvdG11eCIgPiB+Ly50bXV4LmNvbmYKICAgICAgICBlbHNlCiAgICAgICAgICAgICAgICAjIE90aGVyd2lzZSwgdXNlIHRoZSBsZWdhY3kganVqdS90bXV4IGNvbmZpZ3VyYXRpb24KICAgICAgICAgICAgICAgIGNhdCA+IH4vLnRtdXguY29uZiA8PEVORAogICAgICAgICAgICAgICAgCiMgU3RhdHVzIGJhcgpzZXQtb3B0aW9uIC1nIHN0YXR1cy1iZyBibGFjawpzZXQtb3B0aW9uIC1nIHN0YXR1cy1mZyB3aGl0ZQoKc2V0LXdpbmRvdy1vcHRpb24gLWcgd2luZG93LXN0YXR1cy1jdXJyZW50LWJnIHJlZApzZXQtd2luZG93LW9wdGlvbiAtZyB3aW5kb3ctc3RhdHVzLWN1cnJlbnQtYXR0ciBicmlnaHQKCnNldC1vcHRpb24gLWcgc3RhdHVzLXJpZ2h0ICcnCgojIFBhbmVzCnNldC1vcHRpb24gLWcgcGFuZS1ib3JkZXItZmcgd2hpdGUKc2V0LW9wdGlvbiAtZyBwYW5lLWFjdGl2ZS1ib3JkZXItZmcgd2hpdGUKCiMgTW9uaXRvciBhY3Rpdml0eSBvbiB3aW5kb3dzCnNldC13aW5kb3ctb3B0aW9uIC1nIG1vbml0b3ItYWN0aXZpdHkgb24KCiMgU2NyZWVuIGJpbmRpbmdzLCBzaW5jZSBwZW9wbGUgYXJlIG1vcmUgZmFtaWxpYXIgd2l0aCB0aGF0LgpzZXQtb3B0aW9uIC1nIHByZWZpeCBDLWEKYmluZCBDLWEgbGFzdC13aW5kb3cKYmluZCBhIHNlbmQta2V5IEMtYQoKYmluZCB8IHNwbGl0LXdpbmRvdyAtaApiaW5kIC0gc3BsaXQtd2luZG93IC12CgojIEZpeCBDVFJMLVBHVVAvUEdET1dOIGZvciB2aW0Kc2V0LXdpbmRvdy1vcHRpb24gLWcgeHRlcm0ta2V5cyBvbgoKIyBQcmV2ZW50IEVTQyBrZXkgZnJvbSBhZGRpbmcgZGVsYXkgYW5kIGJyZWFraW5nIFZpbSdzIEVTQyA+IGFycm93IGtleQpzZXQtb3B0aW9uIC1zIGVzY2FwZS10aW1lIDAKCkVORAogICAgICAgIGZpCmZpCgooCiAgICAjIENsb3NlIHRoZSBpbmhlcml0ZWQgbG9jayBGRCwgb3IgdG11eCB3aWxsIGtlZXAgaXQgb3Blbi4KICAgIGV4ZWMgOT4mLQogICAgZXhlYyB0bXV4IG5ldy1zZXNzaW9uIC1zIG1vbmdvZGIvMQopCikgOT4vdG1wL2p1anUtdW5pdC1tb25nb2RiLTEtZGVidWctaG9va3MtZXhpdAopIDg+L3RtcC9qdWp1LXVuaXQtbW9uZ29kYi0xLWRlYnVnLWhvb2tzCmV4aXQgJD8K | base64 -d > $F; . $F'\n"),
+	result: regexp.QuoteMeta(debugHooksArgs + "[e-mail] sudo /bin/bash -c 'F=$(mktemp); echo IyEvYmluL2Jhc2gKKAojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnIGxvY2tmaWxlLgpmbG9jayAtbiA4IHx8IChlY2hvICJGYWlsZWQgdG8gYWNxdWlyZSAvdG1wL2p1anUtdW5pdC1tb25nb2RiLTEtZGVidWctaG9va3M6IHVuaXQgaXMgYWxyZWFkeSBiZWluZyBkZWJ1Z2dlZCIgMj4mMTsgZXhpdCAxKQooCiMgQ2xvc2UgdGhlIGluaGVyaXRlZCBsb2NrIEZELCBvciB0bXV4IHdpbGwga2VlcCBpdCBvcGVuLgpleGVjIDg+Ji0KCiMgV3JpdGUgb3V0IHRoZSBkZWJ1Zy1ob29rcyBhcmdzLgplY2hvICJlMzBLIiB8IGJhc2U2NCAtZCA+IC90bXAvanVqdS11bml0LW1vbmdvZGItMS1kZWJ1Zy1ob29rcwoKIyBMb2NrIHRoZSBqdWp1LTx1bml0Pi1kZWJ1Zy1leGl0IGxvY2tmaWxlLgpmbG9jayAtbiA5IHx8IGV4aXQgMQoKIyBXYWl0IGZvciB0bXV4IHRvIGJlIGluc3RhbGxlZC4Kd2hpbGUgWyAhIC1mIC91c3IvYmluL3RtdXggXTsgZG8KICAgIHNsZWVwIDEKZG9uZQoKaWYgWyAhIC1mIH4vLnRtdXguY29uZiBdOyB0aGVuCiAgICAgICAgaWYgWyAtZiAvdXNyL3NoYXJlL2J5b2J1L3Byb2ZpbGVzL3RtdXggXTsgdGhlbgogICAgICAgICAgICAgICAgIyBVc2UgYnlvYnUvdG11eCBwcm9maWxlIGZvciBmYW1pbGlhciBrZXliaW5kaW5ncyBhbmQgYnJhbmRpbmcKICAgICAgICAgICAgICAgIGVjaG8gInNvdXJjZS1maWxlIC91c3Ivc2hhcmUvYnlvYnUvcHJvZmlsZXMvdG11eCIgPiB+Ly50bXV4LmNvbmYKICAgICAgICBlbHNlCiAgICAgICAgICAgICAgICAjIE90aGVyd2lzZSwgdXNlIHRoZSBsZWdhY3kganVqdS90bXV4IGNvbmZpZ3VyYXRpb24KICAgICAgICAgICAgICAgIGNhdCA+IH4vLnRtdXguY29uZiA8PEVORAogICAgICAgICAgICAgICAgCiMgU3RhdHVzIGJhcgpzZXQtb3B0aW9uIC1nIHN0YXR1cy1iZyBibGFjawpzZXQtb3B0aW9uIC1nIHN0YXR1cy1mZyB3aGl0ZQoKc2V0LXdpbmRvdy1vcHRpb24gLWcgd2luZG93LXN0YXR1cy1jdXJyZW50LWJnIHJlZApzZXQtd2luZG93LW9wdGlvbiAtZyB3aW5kb3ctc3RhdHVzLWN1cnJlbnQtYXR0ciBicmlnaHQKCnNldC1vcHRpb24gLWcgc3RhdHVzLXJpZ2h0ICcnCgojIFBhbmVzCnNldC1vcHRpb24gLWcgcGFuZS1ib3JkZXItZmcgd2hpdGUKc2V0LW9wdGlvbiAtZyBwYW5lLWFjdGl2ZS1ib3JkZXItZmcgd2hpdGUKCiMgTW9uaXRvciBhY3Rpdml0eSBvbiB3aW5kb3dzCnNldC13aW5kb3ctb3B0aW9uIC1nIG1vbml0b3ItYWN0aXZpdHkgb24KCiMgU2NyZWVuIGJpbmRpbmdzLCBzaW5jZSBwZW9wbGUgYXJlIG1vcmUgZmFtaWxpYXIgd2l0aCB0aGF0LgpzZXQtb3B0aW9uIC1nIHByZWZpeCBDLWEKYmluZCBDLWEgbGFzdC13aW5kb3cKYmluZCBhIHNlbmQta2V5IEMtYQoKYmluZCB8IHNwbGl0LXdpbmRvdyAtaApiaW5kIC0gc3BsaXQtd2luZG93IC12CgojIEZpeCBDVFJMLVBHVVAvUEdET1dOIGZvciB2aW0Kc2V0LXdpbmRvdy1vcHRpb24gLWcgeHRlcm0ta2V5cyBvbgoKIyBQcmV2ZW50IEVTQyBrZXkgZnJvbSBhZGRpbmcgZGVsYXkgYW5kIGJyZWFraW5nIFZpbSdzIEVTQyA+IGFycm93IGtleQpzZXQtb3B0aW9uIC1zIGVzY2FwZS10aW1lIDAKCkVORAogICAgICAgIGZpCmZpCgooCiAgICAjIENsb3NlIHRoZSBpbmhlcml0ZWQgbG9jayBGRCwgb3IgdG11eCB3aWxsIGtlZXAgaXQgb3Blbi4KICAgIGV4ZWMgOT4mLQogICAgZXhlYyB0bXV4IG5ldy1zZXNzaW9uIC1zIG1vbmdvZGIvMQopCikgOT4vdG1wL2p1anUtdW5pdC1tb25nb2RiLTEtZGVidWctaG9va3MtZXhpdAopIDg+L3RtcC9qdWp1LXVuaXQtbW9uZ29kYi0xLWRlYnVnLWhvb2tzCmV4aXQgJD8K | base64 -d > $F; . $F'\n"),
 }, {
 	info:   `"*" is a valid hook name: it means hook everything`,
 	args:   []string{"mysql/0", "*"},

=== modified file 'cmd/juju/scp.go'
--- cmd/juju/scp.go	2014-01-14 02:53:16 +0000
+++ cmd/juju/scp.go	2014-02-22 15:58:57 +0000
@@ -17,17 +17,23 @@
 }
 
 const scpDoc = `
-Lauch an scp command to copy files. <to> and <from> are either local
-file paths or remote locations of the form <target>:<path>, where
-<target> can be either a machine id as listed by "juju status" in the
+Launch an scp command to copy files. Each argument <file1> ... <file2>
+is either local file path or remote locations of the form <target>:<path>,
+where <target> can be either a machine id as listed by "juju status" in the
 "machines" section or a unit name as listed in the "services" section.
+Any extra arguments to scp can be passed after --, in any place.
 
-Examples
+Examples:
 
 Copy a single file from machine 2 to the local machine:
 
     juju scp 2:/var/log/syslog .
 
+Copy 2 files from two units to the local backup/ directory, passig -v
+to scp as an extra argument:
+
+    juju scp ubuntu/0:/path/file1 -- -v ubuntu/1:/path/file2 backup/
+
 Recursively copy the directory /var/log/mongodb/ on the first mongodb
 server to the local directory remote-logs:
 
@@ -41,7 +47,7 @@
 func (c *SCPCommand) Info() *cmd.Info {
 	return &cmd.Info{
 		Name:    "scp",
-		Args:    "[-- scp-option...] <from> <to>",
+		Args:    "[-- scp-option...] <file1> ... <file2> [-- scp-option...]",
 		Purpose: "launch a scp command to copy files to/from remote machine(s)",
 		Doc:     scpDoc,
 	}
@@ -67,17 +73,33 @@
 	}
 	defer c.apiClient.Close()
 
-	// translate arguments in the form 0:/somepath or service/0:/somepath into
-	// ubuntu@machine:/somepath so they can be presented to scp.
-	for i := range c.Args {
-		// BUG(dfc) This will not work for IPv6 addresses like 2001:db8::1:2:/somepath.
-		if v := strings.SplitN(c.Args[i], ":", 2); len(v) > 1 {
+	// Parse all arguments, translating those in the form 0:/somepath
+	// or service/0:/somepath into ubuntu@machine:/somepath so they
+	// can be given to scp as targets (source(s) and destination(s)),
+	// and passing any others that look like extra arguments (starting
+	// with "-") verbatim to scp.
+	var targets, extraArgs []string
+	for _, arg := range c.Args {
+		if v := strings.SplitN(arg, ":", 2); len(v) > 1 {
 			host, err := c.hostFromTarget(v[0])
 			if err != nil {
 				return err
 			}
-			c.Args[i] = "ubuntu@" + host + ":" + v[1]
+			targets = append(targets, "ubuntu@"+host+":"+v[1])
+			continue
+		}
+		if strings.HasPrefix(arg, "-") {
+			// Allow -- to be specified last, in which case
+			// we need to strip it.
+			arg = strings.TrimSpace(strings.TrimPrefix(arg, "--"))
+			if arg != "" {
+				// Extra argument(s).
+				extraArgs = append(extraArgs, arg)
+			}
+		} else {
+			// Local path
+			targets = append(targets, arg)
 		}
 	}
-	return ssh.Copy(c.Args[0], c.Args[1], nil)
+	return ssh.Copy(targets, extraArgs, nil)
 }

=== modified file 'cmd/juju/scp_test.go'
--- cmd/juju/scp_test.go	2014-01-13 06:25:28 +0000
+++ cmd/juju/scp_test.go	2014-02-22 15:58:57 +0000
@@ -13,7 +13,6 @@
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/charm"
-	"launchpad.net/juju-core/cmd"
 	coretesting "launchpad.net/juju-core/testing"
 )
 
@@ -24,28 +23,54 @@
 }
 
 var scpTests = []struct {
+	about  string
 	args   []string
 	result string
 }{
 	{
+		"scp from machine 0 to current dir",
 		[]string{"0:foo", "."},
 		commonArgs + "[e-mail]:foo .\n",
 	},
 	{
+		"scp from machine 0 to current dir with extra args before",
+		[]string{"-- -rv", "0:foo", "."},
+		commonArgs + "-rv [e-mail]:foo .\n",
+	},
+	{
+		"scp from machine 0 to current dir with extra args after",
+		[]string{"0:foo", ".", "-- -rv"},
+		commonArgs + "-rv [e-mail]:foo .\n",
+	},
+	{
+		"scp from current dir to machine 0",
 		[]string{"foo", "0:"},
 		commonArgs + "foo [e-mail]:\n",
 	},
 	{
+		"scp from current dir to machine 0 with extra args in the middle",
+		[]string{"foo", "-- -r -v", "0:"},
+		commonArgs + "-r -v foo [e-mail]:\n",
+	},
+	{
+		"scp from machine 0 to unit mysql/0",
 		[]string{"0:foo", "mysql/0:/foo"},
 		commonArgs + "[e-mail]:foo [e-mail]:/foo\n",
 	},
 	{
-		[]string{"a", "b", "mysql/0"},
-		commonArgs + "a b\n",
-	},
-	{
-		[]string{"mongodb/1:foo", "mongodb/0:"},
-		commonArgs + "[e-mail]:foo [e-mail]:\n",
+		"scp from machine 0 to unit mysql/0 and extra args",
+		[]string{"-- -q", "0:foo", "mysql/0:/foo"},
+		commonArgs + "-q [e-mail]:foo [e-mail]:/foo\n",
+	},
+	{
+		"scp two local files to unit mysql/0",
+		[]string{"file1", "file2", "mysql/0:/foo/"},
+		commonArgs + "file1 file2 [e-mail]:/foo/\n",
+	},
+	{
+		"scp from unit mongodb/1 to unit mongodb/0 and multiple extra args",
+		[]string{"-- -r", "mongodb/1:foo", "-- -v", "mongodb/0:", "-- -q -l5"},
+		commonArgs + "-r -v -q -l5 [e-mail]:foo [e-mail]:\n",
 	},
 }
 
@@ -66,11 +91,15 @@
 	s.addUnit(srv, m[1], c)
 	s.addUnit(srv, m[2], c)
 
-	for _, t := range scpTests {
-		c.Logf("testing juju scp %s", t.args)
+	for i, t := range scpTests {
+		c.Logf("test %d: %s -> %s\n", i, t.about, t.args)
 		ctx := coretesting.Context(c)
-		code := cmd.Main(&SCPCommand{}, ctx, t.args)
-		c.Check(code, gc.Equals, 0)
+		scpcmd := &SCPCommand{}
+
+		err := scpcmd.Init(t.args)
+		c.Check(err, gc.IsNil)
+		err = scpcmd.Run(ctx)
+		c.Check(err, gc.IsNil)
 		// we suppress stdout from scp
 		c.Check(ctx.Stderr.(*bytes.Buffer).String(), gc.Equals, "")
 		c.Check(ctx.Stdout.(*bytes.Buffer).String(), gc.Equals, "")

=== modified file 'cmd/juju/ssh.go'
--- cmd/juju/ssh.go	2014-01-13 06:25:28 +0000
+++ cmd/juju/ssh.go	2014-02-22 15:58:57 +0000
@@ -39,15 +39,23 @@
 "machines" section or a unit name as listed in the "services" section.
 Any extra parameters are passsed as extra parameters to the ssh command.
 
-Examples
+Examples:
 
 Connect to machine 0:
 
     juju ssh 0
 
+Connect to machine 1 and run 'uname -a':
+
+    juju ssh 1 uname -a
+
 Connect to the first mysql unit:
 
     juju ssh mysql/0
+
+Connect to the first mysql unit and run 'ls -la /var/log/juju':
+
+    juju ssh mysql/0 ls -la /var/log/juju
 `
 
 func (c *SSHCommand) Info() *cmd.Info {
@@ -82,15 +90,9 @@
 	if err != nil {
 		return err
 	}
-	args := c.Args
-	if len(args) > 0 && args[0] == "--" {
-		// utils/ssh adds "--"; we will continue to accept
-		// it from the CLI for backwards compatibility.
-		args = args[1:]
-	}
 	var options ssh.Options
 	options.EnablePTY()
-	cmd := ssh.Command("ubuntu@"+host, args, &options)
+	cmd := ssh.Command("ubuntu@"+host, c.Args, &options)
 	cmd.Stdin = ctx.Stdin
 	cmd.Stdout = ctx.Stdout
 	cmd.Stderr = ctx.Stderr

=== modified file 'cmd/juju/ssh_test.go'
--- cmd/juju/ssh_test.go	2014-02-18 17:08:55 +0000
+++ cmd/juju/ssh_test.go	2014-02-22 15:58:57 +0000
@@ -54,39 +54,33 @@
 
 const (
 	commonArgs = `-o StrictHostKeyChecking no -o PasswordAuthentication no `
-	sshArgs    = commonArgs + `-t `
+	sshArgs    = commonArgs + `-t -t `
 )
 
 var sshTests = []struct {
+	about  string
 	args   []string
 	result string
 }{
 	{
+		"connect to machine 0",
 		[]string{"ssh", "0"},
 		sshArgs + "[e-mail]\n",
 	},
-	// juju ssh 0 'uname -a'
-	{
-		[]string{"ssh", "0", "uname -a"},
-		sshArgs + "[e-mail] -- uname -a\n",
-	},
-	// juju ssh 0 -- uname -a
-	{
-		[]string{"ssh", "0", "--", "uname", "-a"},
-		sshArgs + "[e-mail] -- uname -a\n",
-	},
-	// juju ssh 0 uname -a
-	{
+	{
+		"connect to machine 0 and pass extra arguments",
 		[]string{"ssh", "0", "uname", "-a"},
-		sshArgs + "[e-mail] -- uname -a\n",
+		sshArgs + "[e-mail] uname -a\n",
 	},
 	{
+		"connect to unit mysql/0",
 		[]string{"ssh", "mysql/0"},
 		sshArgs + "[e-mail]\n",
 	},
 	{
-		[]string{"ssh", "mongodb/1"},
-		sshArgs + "[e-mail]\n",
+		"connect to unit mongodb/1 and pass extra arguments",
+		[]string{"ssh", "mongodb/1", "ls", "/"},
+		sshArgs + "[e-mail] ls /\n",
 	},
 }
 
@@ -107,8 +101,8 @@
 	s.addUnit(srv, m[1], c)
 	s.addUnit(srv, m[2], c)
 
-	for _, t := range sshTests {
-		c.Logf("testing juju ssh %s", t.args)
+	for i, t := range sshTests {
+		c.Logf("test %d: %s -> %s\n", i, t.about, t.args)
 		ctx := coretesting.Context(c)
 		jujucmd := cmd.NewSuperCommand(cmd.SuperCommandParams{})
 		jujucmd.Register(&SSHCommand{})

=== modified file 'utils/ssh/run_test.go'
--- utils/ssh/run_test.go	2014-02-18 17:08:55 +0000
+++ utils/ssh/run_test.go	2014-02-22 15:58:57 +0000
@@ -48,7 +48,7 @@
 	c.Assert(response.Code, gc.Equals, 0)
 	c.Assert(string(response.Stdout), gc.Equals, "sudo apt-get update\nsudo apt-get upgrade\n")
 	c.Assert(string(response.Stderr), gc.Equals,
-		"-o StrictHostKeyChecking no -o PasswordAuthentication no hostname -- /bin/bash -s\n")
+		"-o StrictHostKeyChecking no -o PasswordAuthentication no hostname /bin/bash -s\n")
 }
 
 func (s *ExecuteSSHCommandSuite) TestIdentityFile(c *gc.C) {

=== modified file 'utils/ssh/ssh.go'
--- utils/ssh/ssh.go	2014-02-07 11:06:43 +0000
+++ utils/ssh/ssh.go	2014-02-22 15:58:57 +0000
@@ -5,7 +5,6 @@
 // key management, and so on. All SSH-based command executions in
 // Juju should use the Command/ScpCommand functions in this package.
 //
-// TODO(axw) fallback to go.crypto/ssh if no native client is available.
 package ssh
 
 import (
@@ -70,10 +69,11 @@
 	// Host is specified in the format [user@]host.
 	Command(host string, command []string, options *Options) *Cmd
 
-	// Copy copies a file between the local host and
-	// target host. Paths are specified in the scp format,
-	// [[user@]host:]path.
-	Copy(source, dest string, options *Options) error
+	// Copy copies file(s) between the local host a remote host(s).
+	// Paths are specified in the scp format, [[user@]host:]path. If
+	// any extra arguments are specified in extraArgs, they are passed
+	// to scp verbatim.
+	Copy(targets, extraArgs []string, options *Options) error
 }
 
 // Cmd represents a command to be (or being) executed
@@ -203,6 +203,10 @@
 // an embedded client based on go.crypto/ssh.
 var DefaultClient Client
 
+// chosenClient holds the type of SSH client created for
+// DefaultClient, so that we can log it in Command or Copy.
+var chosenClient string
+
 func init() {
 	initDefaultClient()
 }
@@ -210,17 +214,21 @@
 func initDefaultClient() {
 	if client, err := NewOpenSSHClient(); err == nil {
 		DefaultClient = client
+		chosenClient = "OpenSSH"
 	} else if client, err := NewGoCryptoClient(); err == nil {
 		DefaultClient = client
+		chosenClient = "go.crypto (embedded)"
 	}
 }
 
 // Command is a short-cut for DefaultClient.Command.
 func Command(host string, command []string, options *Options) *Cmd {
+	logger.Debugf("using %s ssh client", chosenClient)
 	return DefaultClient.Command(host, command, options)
 }
 
 // Copy is a short-cut for DefaultClient.Copy.
-func Copy(source, dest string, options *Options) error {
-	return DefaultClient.Copy(source, dest, options)
+func Copy(targets, extraArgs []string, options *Options) error {
+	logger.Debugf("using %s ssh client", chosenClient)
+	return DefaultClient.Copy(targets, extraArgs, options)
 }

=== modified file 'utils/ssh/ssh_gocrypto.go'
--- utils/ssh/ssh_gocrypto.go	2014-02-14 04:58:10 +0000
+++ utils/ssh/ssh_gocrypto.go	2014-02-22 15:58:57 +0000
@@ -50,6 +50,7 @@
 			port = options.port
 		}
 	}
+	logger.Debugf(`running (equivalent of): ssh "%s@%s" -p %d '%s'`, user, host, port, shellCommand)
 	return &Cmd{impl: &goCryptoCommand{
 		signers: signers,
 		user:    user,
@@ -61,7 +62,7 @@
 // Copy implements Client.Copy.
 //
 // Copy is currently unimplemented, and will always return an error.
-func (c *GoCryptoClient) Copy(source, dest string, options *Options) error {
+func (c *GoCryptoClient) Copy(targets, extraArgs []string, options *Options) error {
 	return fmt.Errorf("Copy is not implemented")
 }
 

=== modified file 'utils/ssh/ssh_gocrypto_test.go'
--- utils/ssh/ssh_gocrypto_test.go	2014-02-14 04:58:10 +0000
+++ utils/ssh/ssh_gocrypto_test.go	2014-02-22 15:58:57 +0000
@@ -145,6 +145,6 @@
 func (s *SSHGoCryptoCommandSuite) TestCopy(c *gc.C) {
 	client, err := ssh.NewGoCryptoClient()
 	c.Assert(err, gc.IsNil)
-	err = client.Copy("0.1.2.3:b", c.MkDir(), nil)
+	err = client.Copy([]string{"0.1.2.3:b", c.MkDir()}, nil, nil)
 	c.Assert(err, gc.ErrorMatches, "Copy is not implemented")
 }

=== modified file 'utils/ssh/ssh_openssh.go'
--- utils/ssh/ssh_openssh.go	2014-02-05 08:48:40 +0000
+++ utils/ssh/ssh_openssh.go	2014-02-22 15:58:57 +0000
@@ -14,7 +14,7 @@
 	"launchpad.net/juju-core/utils"
 )
 
-var opensshCommonOptions = []string{"-o", "StrictHostKeyChecking no"}
+var opensshCommonOptions = map[string][]string{"-o": []string{"StrictHostKeyChecking no"}}
 
 // default identities will not be attempted if
 // -i is specified and they are not explcitly
@@ -63,16 +63,19 @@
 	return &c, nil
 }
 
-func opensshOptions(options *Options, commandKind opensshCommandKind) []string {
-	args := append([]string{}, opensshCommonOptions...)
+func opensshOptions(options *Options, commandKind opensshCommandKind) map[string][]string {
+	args := make(map[string][]string)
+	for k, v := range opensshCommonOptions {
+		args[k] = v
+	}
 	if options == nil {
 		options = &Options{}
 	}
 	if !options.passwordAuthAllowed {
-		args = append(args, "-o", "PasswordAuthentication no")
+		args["-o"] = append(args["-o"], "PasswordAuthentication no")
 	}
 	if options.allocatePTY {
-		args = append(args, "-t")
+		args["-t"] = []string{}
 	}
 	identities := append([]string{}, options.identities...)
 	if pk := PrivateKeyFiles(); len(pk) > 0 {
@@ -94,45 +97,76 @@
 		}
 	}
 	for _, identity := range identities {
-		args = append(args, "-i", identity)
+		args["-i"] = append(args["-i"], identity)
 	}
 	if options.port != 0 {
+		port := fmt.Sprint(options.port)
 		if commandKind == scpKind {
 			// scp uses -P instead of -p (-p means preserve).
-			args = append(args, "-P")
+			args["-P"] = []string{port}
 		} else {
-			args = append(args, "-p")
+			args["-p"] = []string{port}
 		}
-		args = append(args, fmt.Sprint(options.port))
 	}
 	return args
 }
 
+func expandArgs(args map[string][]string, quote bool) []string {
+	var list []string
+	for opt, vals := range args {
+		if len(vals) == 0 {
+			list = append(list, opt)
+			if opt == "-t" {
+				// In order to force a PTY to be allocated, we need to
+				// pass -t twice.
+				list = append(list, opt)
+			}
+		}
+		for _, val := range vals {
+			list = append(list, opt)
+			if quote {
+				val = fmt.Sprintf("%q", val)
+			}
+			list = append(list, val)
+		}
+	}
+	return list
+}
+
 // Command implements Client.Command.
 func (c *OpenSSHClient) Command(host string, command []string, options *Options) *Cmd {
-	args := opensshOptions(options, sshKind)
+	opts := opensshOptions(options, sshKind)
+	args := expandArgs(opts, false)
 	args = append(args, host)
 	if len(command) > 0 {
-		args = append(args, "--")
 		args = append(args, command...)
 	}
 	bin, args := sshpassWrap("ssh", args)
+	optsList := strings.Join(expandArgs(opts, true), " ")
+	fullCommand := strings.Join(command, " ")
+	logger.Debugf("running: %s %s %q '%s'", bin, optsList, host, fullCommand)
 	return &Cmd{impl: &opensshCmd{exec.Command(bin, args...)}}
 }
 
 // Copy implements Client.Copy.
-func (c *OpenSSHClient) Copy(source, dest string, userOptions *Options) error {
+func (c *OpenSSHClient) Copy(targets, extraArgs []string, userOptions *Options) error {
 	var options Options
 	if userOptions != nil {
 		options = *userOptions
 		options.allocatePTY = false // doesn't make sense for scp
 	}
-	args := opensshOptions(&options, scpKind)
-	args = append(args, source, dest)
+	opts := opensshOptions(&options, scpKind)
+	args := expandArgs(opts, false)
+	args = append(args, extraArgs...)
+	args = append(args, targets...)
 	bin, args := sshpassWrap("scp", args)
 	cmd := exec.Command(bin, args...)
 	var stderr bytes.Buffer
 	cmd.Stderr = &stderr
+	allOpts := append(expandArgs(opts, true), extraArgs...)
+	optsList := strings.Join(allOpts, " ")
+	targetList := `"` + strings.Join(targets, `" "`) + `"`
+	logger.Debugf("running: %s %s %s", bin, optsList, targetList)
 	if err := cmd.Run(); err != nil {
 		stderr := strings.TrimSpace(stderr.String())
 		if len(stderr) > 0 {

=== modified file 'utils/ssh/ssh_test.go'
--- utils/ssh/ssh_test.go	2014-02-18 17:08:55 +0000
+++ utils/ssh/ssh_test.go	2014-02-22 15:58:57 +0000
@@ -70,24 +70,24 @@
 	fakesshpass := filepath.Join(s.testbin, "sshpass")
 	err := ioutil.WriteFile(fakesshpass, []byte(echoCommandScript), 0755)
 	s.assertCommandArgs(c, s.command("echo", "123"),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost echo 123",
 	)
 	// Now set $SSHPASS.
 	s.PatchEnvironment("SSHPASS", "anyoldthing")
 	s.assertCommandArgs(c, s.command("echo", "123"),
-		fakesshpass+" -e ssh -o StrictHostKeyChecking no -o PasswordAuthentication no localhost -- echo 123",
+		fakesshpass+" -e ssh -o StrictHostKeyChecking no -o PasswordAuthentication no localhost echo 123",
 	)
 	// Finally, remove sshpass from $PATH.
 	err = os.Remove(fakesshpass)
 	c.Assert(err, gc.IsNil)
 	s.assertCommandArgs(c, s.command("echo", "123"),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost echo 123",
 	)
 }
 
 func (s *SSHCommandSuite) TestCommand(c *gc.C) {
 	s.assertCommandArgs(c, s.command("echo", "123"),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost echo 123",
 	)
 }
 
@@ -95,7 +95,7 @@
 	var opts ssh.Options
 	opts.EnablePTY()
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -t localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -t -t localhost echo 123",
 	)
 }
 
@@ -103,7 +103,7 @@
 	var opts ssh.Options
 	opts.AllowPasswordAuthentication()
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no localhost echo 123",
 	)
 }
 
@@ -111,7 +111,7 @@
 	var opts ssh.Options
 	opts.SetIdentities("x", "y")
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y localhost echo 123",
 	)
 }
 
@@ -119,7 +119,7 @@
 	var opts ssh.Options
 	opts.SetPort(2022)
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -p 2022 localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -p 2022 localhost echo 123",
 	)
 }
 
@@ -129,12 +129,19 @@
 	opts.AllowPasswordAuthentication()
 	opts.SetIdentities("x", "y")
 	opts.SetPort(2022)
-	err := s.client.Copy("/tmp/blah", "[e-mail]:baz", &opts)
+	err := s.client.Copy([]string{"/tmp/blah", "[e-mail]:baz"}, nil, &opts)
 	c.Assert(err, gc.IsNil)
 	out, err := ioutil.ReadFile(s.fakescp + ".args")
 	c.Assert(err, gc.IsNil)
 	// EnablePTY has no effect for Copy
 	c.Assert(string(out), gc.Equals, s.fakescp+" -o StrictHostKeyChecking no -i x -i y -P 2022 /tmp/blah [e-mail]:baz\n")
+
+	// Try passing extra args
+	err = s.client.Copy([]string{"/tmp/blah", "[e-mail]:baz"}, []string{"-r", "-v"}, &opts)
+	c.Assert(err, gc.IsNil)
+	out, err = ioutil.ReadFile(s.fakescp + ".args")
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(out), gc.Equals, s.fakescp+" -o StrictHostKeyChecking no -i x -i y -P 2022 -r -v /tmp/blah [e-mail]:baz\n")
 }
 
 func (s *SSHCommandSuite) TestCommandClientKeys(c *gc.C) {
@@ -146,7 +153,7 @@
 	var opts ssh.Options
 	opts.SetIdentities("x", "y")
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y -i "+ck+" localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y -i "+ck+" localhost echo 123",
 	)
 }
 
@@ -167,7 +174,7 @@
 	s.PatchValue(ssh.DefaultIdentities, []string{def1, def2})
 	// If no identities are specified, then the defaults aren't added.
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost echo 123",
 	)
 	// If identities are specified, then the defaults are must added.
 	// Only the defaults that exist on disk will be added.
@@ -175,6 +182,6 @@
 	c.Assert(err, gc.IsNil)
 	opts.SetIdentities("x", "y")
 	s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
-		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y -i "+def2+" localhost -- echo 123",
+		s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y -i "+def2+" localhost echo 123",
 	)
 }