Merge lp:~thumper/juju-core/find-ipv4-address into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Utility function to find IPv4 address.

There was no simple way to get an IPv4 address for
a particular net.Interface. So I added this.

https://codereview.appspot.com/10951043/

Utility function to find IPv4 address.

There was no simple way to get an IPv4 address for
a particular net.Interface. So I added this.

https://codereview.appspot.com/10951043/

[Tim Penhey (thumper)]

Reviewers: mp+173119_code.launchpad.net,

Message:
Please take a look.

Description:
Utility function to find IPv4 address.

There was no simple way to get an IPv4 address for
a particular net.Interface. So I added this.

https://code.launchpad.net/~thumper/juju-core/find-ipv4-address/+merge/173119

Requires:
https://code.launchpad.net/~thumper/juju-core/move-cert-gen-to-config/+merge/173117

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   A utils/network.go
   A utils/network_test.go

[John A Meinel (jameinel)]

Is this an address or a range? /24 is a range over 256 addresses, isnt it?
I may be wrong on the name, but I would have thought this would be
something more like GetIPv4AddressRange. I realize you are dropping the /24
but that is only valid if it is /32 (fully specify one ip) isnt it?

Also, is it possible to avoid forcing v4? It would be really nice if we
didnt have to do a lot of work to be compatible with v6.

Anyway, if we need to do this the helper is nice to have and it looks well
tested.

John
=:->
On Jul 5, 2013 5:07 AM, "Tim Penhey" <email address hidden> wrote:

> The proposal to merge lp:~thumper/juju-core/find-ipv4-address into
> lp:juju-core has been updated.
>
> Description changed to:
>
> Utility function to find IPv4 address.
>
> There was no simple way to get an IPv4 address for
> a particular net.Interface. So I added this.
>
> https://codereview.appspot.com/10951043/
>
>
> For more details, see:
>
> https://code.launchpad.net/~thumper/juju-core/find-ipv4-address/+merge/173119
> --
>
> https://code.launchpad.net/~thumper/juju-core/find-ipv4-address/+merge/173119
> You are subscribed to branch lp:juju-core.
>

[Ian Booth (wallyworld)]

LGTM, with the ParseCIDR() function used to break up the IP address

https://codereview.appspot.com/10951043/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/10951043/diff/1/utils/network.go
File utils/network.go (right):

https://codereview.appspot.com/10951043/diff/1/utils/network.go#newcode15
utils/network.go:15: for _, addr := range addresses {
On 2013/07/05 03:06:51, dfc wrote:
> net.ParseCIDR ?

Done.

https://codereview.appspot.com/10951043/

=== added file 'utils/network.go'
--- utils/network.go	1970-01-01 00:00:00 +0000
+++ utils/network.go	2013-07-08 02:51:26 +0000
@@ -0,0 +1,26 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package utils
+
+import (
+	"fmt"
+	"net"
+)
+
+// GetIPv4Address iterates through the addresses expecting the format from
+// func (ifi *net.Interface) Addrs() ([]net.Addr, error)
+func GetIPv4Address(addresses []net.Addr) (string, error) {
+	for _, addr := range addresses {
+		ip, _, err := net.ParseCIDR(addr.String())
+		if err != nil {
+			return "", err
+		}
+		ipv4 := ip.To4()
+		if ipv4 == nil {
+			continue
+		}
+		return ipv4.String(), nil
+	}
+	return "", fmt.Errorf("no addresses match")
+}

=== added file 'utils/network_test.go'
--- utils/network_test.go	1970-01-01 00:00:00 +0000
+++ utils/network_test.go	2013-07-08 02:51:26 +0000
@@ -0,0 +1,74 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package utils_test
+
+import (
+	"net"
+
+	gc "launchpad.net/gocheck"
+	"launchpad.net/juju-core/utils"
+)
+
+type networkSuite struct {
+}
+
+var _ = gc.Suite(&networkSuite{})
+
+type fakeAddress struct {
+	address string
+}
+
+func (fake fakeAddress) Network() string {
+	return "ignored"
+}
+
+func (fake fakeAddress) String() string {
+	return fake.address
+}
+
+func makeAddresses(values ...string) (result []net.Addr) {
+	for _, v := range values {
+		result = append(result, &fakeAddress{v})
+	}
+	return
+}
+
+func (*networkSuite) TestGetIPv4Address(c *gc.C) {
+	for _, test := range []struct {
+		addresses   []net.Addr
+		expected    string
+		errorString string
+	}{{
+		addresses: makeAddresses(
+			"complete",
+			"nonsense"),
+		errorString: "invalid CIDR address: complete",
+	}, {
+		addresses: makeAddresses(
+			"fe80::90cf:9dff:fe6e:ece/64",
+		),
+		errorString: "no addresses match",
+	}, {
+		addresses: makeAddresses(
+			"fe80::90cf:9dff:fe6e:ece/64",
+			"10.0.3.1/24",
+		),
+		expected: "10.0.3.1",
+	}, {
+		addresses: makeAddresses(
+			"10.0.3.1/24",
+			"fe80::90cf:9dff:fe6e:ece/64",
+		),
+		expected: "10.0.3.1",
+	}} {
+		ip, err := utils.GetIPv4Address(test.addresses)
+		if test.errorString == "" {
+			c.Assert(err, gc.IsNil)
+			c.Assert(ip, gc.Equals, test.expected)
+		} else {
+			c.Assert(err, gc.ErrorMatches, test.errorString)
+			c.Assert(ip, gc.Equals, "")
+		}
+	}
+}