lp:1616098: ssh,scp,dh pick reachable address among all

[dimitern]

Uses the new SSHClient API facade v2 (from #6468), if
available, and network.ReachableHostPort() (#6454) in
code shared between the ssh, scp, and debug-hooks CLI
commands, to select a reachable address from all known
addresses of the target (machine/unit/host).

Fixes http://pad.lv/1616098.

QA steps (live tested on both MAAS 1.9 with multiple-
NIC nodes and on lxd - see below):

1. Using the tip of develop, bootstrap a lxd xenial controller.
2. juju switch controller
3. juju status 0 # note of the IP address (IP1)
4. lxc exec -- ip addr add /24 dev eth0
   (add an extra IP on machine-0, from the same subnet)
5. lxc exec -- systemctl restart jujud-machine-0.service
   (bounce the controller agent to pick up the new IP)
6. juju ssh 0 # should work
7. sudo ip route add blackhole /32
   (make the preferred public IP unreachable from your laptop)
8. juju --debug ssh 0 # should fail with 'Invalid argument'
9. git checkout dimitern/lp-1616098-ssh+scp-alladdresses
   (switch to this PR's branch and rebuild juju & jujud)
10. juju ssh 0 # should work now, using IP2
11. juju scp 0:/etc/issue . # as above
12. juju deploy ubuntu --to 0
13. juju debug-hooks ubuntu/0 # should work as well
    (small delay ~10s observed on lxd, because of 'sudo')

[dimitern]

@nskaggs Why are my PR not picked up by the pre-check job (almost every one I had to poke to get a run)?

[dimitern]

!!build!!

[nskaggs]

@dimitern I'm not sure. Ping me on IRC and I can check the logs when you open a new one to see what the reason is.

[frobware]

This is a large change (> 500 lines). Somebody else needs to bless it.

As a nit, I did find the TestSSHCommandHostAddressRetry* functions non-obvious in how/what they were testing.

[frobware]

This stuck out - why does the change require this?

[dimitern]

This adds LinkLayerDevices and sets addresses for them, to exercise apiClient.AllAddresses() returns them in addition to m.Addresses() already set above (only the public and private). Also setupModel is used in all 3 commands' test suites, so it seemed best to add it here only.

[frobware]

It's not clear what the comment means.

[dimitern]

It tries to clarify where that "0.1.2.3" came from - it was set by suite.setLinkLayerDevicesAddresses() for suite.m (machine-0). I'll rephrase it to make it clearer.

[frobware]

I did actually QA this change with the LXD provider and it worked as advertised. Subjectively, it also "feels" quicker when running juju ssh 0, for example.

[macgreagoir]

I can't bless it, but LGTM.

[bz2]

+1

[dimitern]

$$fixes-1616098$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

[jujubot]

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9543

[dimitern]

$$retry-transient-failure$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

[jujubot]

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9544

[dimitern]

$$windows-this-time$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

[jujubot]

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9545

[dimitern]

$$again$$

[jujubot]

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju