I think the general approach is good, but there will be issues with the identity file that my branch has introduced.
https://codereview.appspot.com/48370043/diff/1/cloudinit/sshinit/configure.go File cloudinit/sshinit/configure.go (right):
https://codereview.appspot.com/48370043/diff/1/cloudinit/sshinit/configure.go#newcode54 cloudinit/sshinit/configure.go:54: cmd.Stdin = strings.NewReader(script) One thing I noticed was the need for a carriage return on the end. Does the script end with a new line?
https://codereview.appspot.com/48370043/diff/1/environs/config/authkeys.go File environs/config/authkeys.go (right):
https://codereview.appspot.com/48370043/diff/1/environs/config/authkeys.go#newcode40 environs/config/authkeys.go:40: var err error Wondering if this function should perhaps be moved into utils/ssh?
https://codereview.appspot.com/48370043/diff/1/environs/manual/fakessh.go File environs/manual/fakessh.go (right):
https://codereview.appspot.com/48370043/diff/1/environs/manual/fakessh.go#newcode33 environs/manual/fakessh.go:33: head >/dev/null Given the number of different places we mock out the ssh executable, wondering if we should have some standard place for things like this, perhaps "testing/ssh" ?
https://codereview.appspot.com/48370043/diff/1/environs/sshstorage/storage.go File environs/sshstorage/storage.go (right):
https://codereview.appspot.com/48370043/diff/1/environs/sshstorage/storage.go#newcode48 environs/sshstorage/storage.go:48: return ssh.Command(host, []string{"bash", "-c", command}, &options) bash or /bin/bash ??
We have both in the codebase, and I think we should be consistent with ourselves.
Personally I don't care too much which we choose, just that we have one.
https://codereview.appspot.com/48370043/diff/1/utils/ssh/ssh.go File utils/ssh/ssh.go (right):
https://codereview.appspot.com/48370043/diff/1/utils/ssh/ssh.go#newcode18 utils/ssh/ssh.go:18: passwordAuthDisabled bool hmm.. one of my branches adds an identity file option using the old way. This will need to be taken into account some how.
https://codereview.appspot.com/48370043/diff/1/utils/ssh/ssh_gocrypto.go File utils/ssh/ssh_gocrypto.go (right):
https://codereview.appspot.com/48370043/diff/1/utils/ssh/ssh_gocrypto.go#newcode39 utils/ssh/ssh_gocrypto.go:39: func (c *GoCryptoClient) Command(host string, command []string, options *Options) *Cmd { Lots of public functions missing docs.
https://codereview.appspot.com/48370043/diff/1/utils/trivial.go File utils/trivial.go (right):
https://codereview.appspot.com/48370043/diff/1/utils/trivial.go#newcode66 utils/trivial.go:66: // quotes as necessary; each argument is single-quoted. this isn't escaping slashes... is it?
https://codereview.appspot.com/48370043/
« Back to merge proposal
I think the general approach is good, but there will be issues with the
identity file that my branch has introduced.
https:/ /codereview. appspot. com/48370043/ diff/1/ cloudinit/ sshinit/ configure. go sshinit/ configure. go (right):
File cloudinit/
https:/ /codereview. appspot. com/48370043/ diff/1/ cloudinit/ sshinit/ configure. go#newcode54 sshinit/ configure. go:54: cmd.Stdin = strings. NewReader( script)
cloudinit/
One thing I noticed was the need for a carriage return on the end.
Does the script end with a new line?
https:/ /codereview. appspot. com/48370043/ diff/1/ environs/ config/ authkeys. go config/ authkeys. go (right):
File environs/
https:/ /codereview. appspot. com/48370043/ diff/1/ environs/ config/ authkeys. go#newcode40 config/ authkeys. go:40: var err error
environs/
Wondering if this function should perhaps be moved into utils/ssh?
https:/ /codereview. appspot. com/48370043/ diff/1/ environs/ manual/ fakessh. go manual/ fakessh. go (right):
File environs/
https:/ /codereview. appspot. com/48370043/ diff/1/ environs/ manual/ fakessh. go#newcode33 manual/ fakessh. go:33: head >/dev/null
environs/
Given the number of different places we mock out the ssh executable,
wondering if we should have some standard place for things like this,
perhaps "testing/ssh" ?
https:/ /codereview. appspot. com/48370043/ diff/1/ environs/ sshstorage/ storage. go sshstorage/ storage. go (right):
File environs/
https:/ /codereview. appspot. com/48370043/ diff/1/ environs/ sshstorage/ storage. go#newcode48 sshstorage/ storage. go:48: return ssh.Command(host,
environs/
[]string{"bash", "-c", command}, &options)
bash or /bin/bash ??
We have both in the codebase, and I think we should be consistent with
ourselves.
Personally I don't care too much which we choose, just that we have one.
https:/ /codereview. appspot. com/48370043/ diff/1/ utils/ssh/ ssh.go
File utils/ssh/ssh.go (right):
https:/ /codereview. appspot. com/48370043/ diff/1/ utils/ssh/ ssh.go# newcode18 ssh.go: 18: passwordAuthDis abled bool
utils/ssh/
hmm.. one of my branches adds an identity file option using the old way.
This will need to be taken into account some how.
https:/ /codereview. appspot. com/48370043/ diff/1/ utils/ssh/ ssh_gocrypto. go ssh_gocrypto. go (right):
File utils/ssh/
https:/ /codereview. appspot. com/48370043/ diff/1/ utils/ssh/ ssh_gocrypto. go#newcode39 ssh_gocrypto. go:39: func (c *GoCryptoClient) Command(host
utils/ssh/
string, command []string, options *Options) *Cmd {
Lots of public functions missing docs.
https:/ /codereview. appspot. com/48370043/ diff/1/ utils/trivial. go
File utils/trivial.go (right):
https:/ /codereview. appspot. com/48370043/ diff/1/ utils/trivial. go#newcode66 go:66: // quotes as necessary; each argument is
utils/trivial.
single-quoted.
this isn't escaping slashes... is it?
https:/ /codereview. appspot. com/48370043/