Code review comment for lp:~axwalk/juju-core/lp1303735-fix-address-logic

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider.go
File provider/openstack/provider.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider.go#oldcode419
provider/openstack/provider.go:419: NetworkName: network,
On 2014/04/10 11:23:27, gz wrote:
> FIX: You've lost the recording of NetworkName - we really do want this
when it's
> available.

Good catch! Thanks, done.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go
File provider/openstack/provider_test.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go#oldcode63
provider/openstack/provider_test.go:63: private: []nova.IPAddress{{4,
"127.0.0.4"}, {4, "8.8.4.4"}},
On 2014/04/10 11:23:27, gz wrote:
> And this is the real world example (mostly, not sure why I used a 127.
address
> not a 10. address) we expect to work... I'm not sure about your
changes to this
> test? The code should still pick the 8. address if a non-public one is
first,
> due to the private address range matching?

I changed it because 127.0.0.4 is machine-local, and hence will never
match. Likewise for the other test changes.

https://codereview.appspot.com/85590044/

« Back to merge proposal