ec2: Add IP addresses to Instance struct

ec2: Add IP addresses to Instance struct

Expose IPAddress and PrivateIPAddress on Instance and add
sample values to ec2test server.

R=rogpeppe, niemeyer
https://codereview.appspot.com/12768044/

https://code.launchpad.net/~gz/goamz/add_instance_addresses/+merge/179733

(do not edit description out of merge proposal)

[gz, 2013-08-12 15:39:35]

Please take a look.

[rog, 2013-08-15 09:48:46]

On 2013/08/12 15:39:35, gz wrote:
> Please take a look.

LGTM

[niemeyer, 2013-08-15 14:18:43]

LGTM

[niemeyer, 2013-08-15 14:19:46]

I've added you to the goamz team, Martin.

[gz, 2013-08-15 15:36:18]

Please take a look.

[gz, 2013-08-15 15:40:12]

Please take a look.

[gz, 2013-08-15 15:43:55]

Thanks for the reviews! We also needed to add support to the test server (should
probably have been a seperate branch), which is now in this proposal as well.

Possibly the code is now being overly fancy about generating test ips, but this
should be all we need for support in juju-core now.

(Now needed but not marked, prereq branch lp:~gz/goamz/example_dns also up for
review seperately.)

[niemeyer, 2013-08-15 15:58:43]

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go#newco...
ec2/ec2test/server.go:549: fmt.Sscanf(inst.id, "i-%d", &id)
Is this better than randomly assigning the last three octets? Random assignment
would introduce the remote chance of clash, but avoid the certain chance of
clashing every 256 entries.

[rog, 2013-08-15 16:04:44]

LGTM modulo the below suggestions.

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go#newco...
ec2/ec2test/server.go:494: id:          fmt.Sprintf("i-%d", srv.maxId.next()),
You could consider adding a "seq" field instead of id
and defining:

func (inst *Instance) id() string {
    return fmt.Sprintf("i-%d", inst.seq)
}

and using it throughout.

That would seem easy and worth doing, rather than parsing the i-number.

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go#newco...
ec2/ec2test/server.go:549: fmt.Sscanf(inst.id, "i-%d", &id)
ugh. see above for an alternative.

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go#newco...
ec2/ec2test/server.go:549: fmt.Sscanf(inst.id, "i-%d", &id)
On 2013/08/15 15:58:43, niemeyer wrote:
> Is this better than randomly assigning the last three octets? Random
assignment
> would introduce the remote chance of clash, but avoid the certain chance of
> clashing every 256 entries.

I think it's nice to have a deterministic link between id and notional IP
address. If we really think we might have huge numbers of test instances, then
we could do: 

var b [4]byte
binary.LittleEndian.PutUint32(b, uint32(inst.id))

IPAddress: fmt.Sprintf("8.%d.%d.%d", b[1], b[2], b[3])

[gz, 2013-08-15 16:30:30]

Please take a look.

[gz, 2013-08-15 16:31:16]

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/12768044/diff/9003/ec2/ec2test/server.go#newco...
ec2/ec2test/server.go:494: id:          fmt.Sprintf("i-%d", srv.maxId.next()),
On 2013/08/15 16:04:44, rog wrote:
> You could consider adding a "seq" field instead of id
> and defining:
> 
> func (inst *Instance) id() string {
>     return fmt.Sprintf("i-%d", inst.seq)
> }
> 
> and using it throughout.
> 
> That would seem easy and worth doing, rather than parsing the i-number.

Done, and TODO removed.

[niemeyer, 2013-08-15 16:39:47]

LGTM

[rog, 2013-08-15 17:29:37]

still LGTM, thanks.

[gz, 2013-08-20 12:01:16]

*** Submitted:

ec2: Add IP addresses to Instance struct

Expose IPAddress and PrivateIPAddress on Instance and add
sample values to ec2test server.

R=rogpeppe, niemeyer
https://codereview.appspot.com/12768044/

R=rog, niemeyer
CC=
https://codereview.appspot.com/12768044