juju-core

Merge lp:~axwalk/juju-core/utils-ssh-stat-default-identities into lp:~go-bot/juju-core/trunk

  1. utils-ssh-stat-default-identities
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2304
Proposed branch: lp:~axwalk/juju-core/utils-ssh-stat-default-identities
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 41 lines (+11/-3)
2 files modified
utils/ssh/ssh_openssh.go (+3/-1)
utils/ssh/ssh_test.go (+8/-2)
To merge this branch: bzr merge lp:~axwalk/juju-core/utils-ssh-stat-default-identities
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+204832@code.launchpad.net

Commit message

utils/ssh: only use existing default identities

"juju ssh" and bootstrap and so on are emitting
warnings from ssh if any of the default identities
don't exist. This is fixed by only specifying the
default identities that exist.

https://codereview.appspot.com/52230047/

Description of the change

utils/ssh: only use existing default identities

"juju ssh" and bootstrap and so on are emitting
warnings from ssh if any of the default identities
don't exist. This is fixed by only specifying the
default identities that exist.

https://codereview.appspot.com/52230047/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+204832_code.launchpad.net,

Message:
Please take a look.

Description:
utils/ssh: only use existing default identities

"juju ssh" and bootstrap and so on are emitting
warnings from ssh if any of the default identities
don't exist. This is fixed by only specifying the
default identities that exist.

https://code.launchpad.net/~axwalk/juju-core/utils-ssh-stat-default-identities/+merge/204832

(do not edit description out of merge proposal)

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

Affected files (+13, -3 lines):
   A [revision details]
   M utils/ssh/ssh_openssh.go
   M utils/ssh/ssh_test.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: tarmac-20140204193125-k8xmn7d6lr3zswnw
+New revision: <email address hidden>

Index: utils/ssh/ssh_openssh.go
=== modified file 'utils/ssh/ssh_openssh.go'
--- utils/ssh/ssh_openssh.go 2014-02-04 03:12:33 +0000
+++ utils/ssh/ssh_openssh.go 2014-02-05 08:48:40 +0000
@@ -88,7 +88,9 @@
      logger.Warningf("failed to normalize path %q: %v", identity, err)
      continue
     }
- identities = append(identities, path)
+ if _, err := os.Stat(path); err == nil {
+ identities = append(identities, path)
+ }
    }
   }
   for _, identity := range identities {

Index: utils/ssh/ssh_test.go
=== modified file 'utils/ssh/ssh_test.go'
--- utils/ssh/ssh_test.go 2014-02-04 03:12:33 +0000
+++ utils/ssh/ssh_test.go 2014-02-05 08:48:40 +0000
@@ -151,14 +151,20 @@

  func (s *SSHCommandSuite) TestCommandDefaultIdentities(c *gc.C) {
   var opts ssh.Options
- s.PatchValue(ssh.DefaultIdentities, []string{"def1", "def2"})
+ tempdir := c.MkDir()
+ def1 := filepath.Join(tempdir, "def1")
+ def2 := filepath.Join(tempdir, "def2")
+ 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",
   )
   // If identities are specified, then the defaults are must added.
+ // Only the defaults that exist on disk will be added.
+ err := ioutil.WriteFile(def2, nil, 0644)
+ 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 def1 -i def2 localhost -- echo 123",
+ s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i
x -i y -i "+def2+" localhost -- echo 123",
   )
  }

Revision history for this message
Tim Penhey (thumper) wrote :

On 2014/02/05 08:53:20, axw wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/52230047/

Revision history for this message
Tim Penhey (thumper) wrote :

On 2014/02/05 08:53:20, axw wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/52230047/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (26.4 KiB)

The attempt to merge lp:~axwalk/juju-core/utils-ssh-stat-default-identities into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.015s
ok launchpad.net/juju-core/agent 0.634s
ok launchpad.net/juju-core/agent/tools 0.201s
ok launchpad.net/juju-core/bzr 6.547s
ok launchpad.net/juju-core/cert 3.653s
ok launchpad.net/juju-core/charm 0.591s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.030s
ok launchpad.net/juju-core/cloudinit/sshinit 1.069s
ok launchpad.net/juju-core/cmd 0.200s
ok launchpad.net/juju-core/cmd/charm-admin 0.859s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 237.508s
ok launchpad.net/juju-core/cmd/jujud 59.971s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 14.200s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/constraints 0.027s
ok launchpad.net/juju-core/container 0.043s
ok launchpad.net/juju-core/container/factory 0.050s
ok launchpad.net/juju-core/container/kvm 0.310s
ok launchpad.net/juju-core/container/kvm/mock 0.054s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.273s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.356s
ok launchpad.net/juju-core/environs 3.347s
ok launchpad.net/juju-core/environs/bootstrap 4.434s
ok launchpad.net/juju-core/environs/cloudinit 0.613s
ok launchpad.net/juju-core/environs/config 2.451s
ok launchpad.net/juju-core/environs/configstore 0.039s
ok launchpad.net/juju-core/environs/filestorage 0.031s
ok launchpad.net/juju-core/environs/httpstorage 0.964s
ok launchpad.net/juju-core/environs/imagemetadata 0.606s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.059s
ok launchpad.net/juju-core/environs/jujutest 0.217s
ok launchpad.net/juju-core/environs/manual 9.902s
ok launchpad.net/juju-core/environs/simplestreams 0.328s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.258s
ok launchpad.net/juju-core/environs/storage 1.133s
ok launchpad.net/juju-core/environs/sync 33.737s
ok launchpad.net/juju-core/environs/testing 0.173s
ok launchpad.net/juju-core/environs/tools 6.839s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.015s
ok launchpad.net/juju-core/instance 0.024s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 24.935s
ok launchpad.net/juju-core/juju/osenv 0.019s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.016s
ok launchpad.net/juju-core/log/syslog 0.021s
ok launchpad.net/...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'utils/ssh/ssh_openssh.go'
2--- utils/ssh/ssh_openssh.go 2014-02-04 03:12:33 +0000
3+++ utils/ssh/ssh_openssh.go 2014-02-05 08:51:23 +0000
4@@ -88,7 +88,9 @@
5 logger.Warningf("failed to normalize path %q: %v", identity, err)
6 continue
7 }
8- identities = append(identities, path)
9+ if _, err := os.Stat(path); err == nil {
10+ identities = append(identities, path)
11+ }
12 }
13 }
14 for _, identity := range identities {
15
16=== modified file 'utils/ssh/ssh_test.go'
17--- utils/ssh/ssh_test.go 2014-02-04 03:12:33 +0000
18+++ utils/ssh/ssh_test.go 2014-02-05 08:51:23 +0000
19@@ -151,14 +151,20 @@
20
21 func (s *SSHCommandSuite) TestCommandDefaultIdentities(c *gc.C) {
22 var opts ssh.Options
23- s.PatchValue(ssh.DefaultIdentities, []string{"def1", "def2"})
24+ tempdir := c.MkDir()
25+ def1 := filepath.Join(tempdir, "def1")
26+ def2 := filepath.Join(tempdir, "def2")
27+ s.PatchValue(ssh.DefaultIdentities, []string{def1, def2})
28 // If no identities are specified, then the defaults aren't added.
29 s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
30 s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no localhost -- echo 123",
31 )
32 // If identities are specified, then the defaults are must added.
33+ // Only the defaults that exist on disk will be added.
34+ err := ioutil.WriteFile(def2, nil, 0644)
35+ c.Assert(err, gc.IsNil)
36 opts.SetIdentities("x", "y")
37 s.assertCommandArgs(c, s.commandOptions([]string{"echo", "123"}, &opts),
38- s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y -i def1 -i def2 localhost -- echo 123",
39+ s.fakessh+" -o StrictHostKeyChecking no -o PasswordAuthentication no -i x -i y -i "+def2+" localhost -- echo 123",
40 )
41 }

Subscribers

People subscribed via source and target branches

to status/vote changes: