ec2: Added (Un)AssignPrivateIPAddresses APIs

ec2: Added (Un)AssignPrivateIPAddresses APIs

Added two new API calls:
- AssignPrivateIPAddresses
- UnassignPrivateIPAddresses

They allow adding or removing secondary private
IP addresses to a network interface of an instance.

This is the final part of the VPC-related goamz
changes needed to support initial container
networking in juju-core.

Added a live-only test for the two new APIs and
a couple of example response tests.

Removed ec2.*Status constants, as suggested in
a prereq review, which I omitted previously.

https://code.launchpad.net/~dimitern/goamz/vpc-private-ips/+merge/205203

Requires: https://code.launchpad.net/~dimitern/goamz/vpc-instance-addons/+merge/205148

(do not edit description out of merge proposal)

[dimitern, 2014-02-06 16:04:44]

Please take a look.

[rog, 2014-02-06 18:36:03]

LGTM with some suggestions below.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go
File ec2/privateips.go (right):

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode6
ec2/privateips.go:6: // Copyright (c) 2011 Canonical Ltd.
I guess files created later should probably have a more current copyright date.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode19
ec2/privateips.go:19: // secondary IP addresses can be given explicitly, or a
number of
s/number/count/

?
"a number of" is almost exactly the same as "one of more"

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode20
ec2/privateips.go:20: // secondary IP addresses can be given, to be
automatically assigned
s/, //

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode21
ec2/privateips.go:21: // within the subnet's CIDR block range. The number of
secondary IP
s/number/total count/

or

s/number/total number/

?

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode25
ec2/privateips.go:25: // Either ipAddresses or secondaryIPsCount can be given
but not both.
Perhaps be more specific here:

// If secondaryIPsCount is greater than zero, ipAddresses
// is ignored.

except that as ipAddresses is first in the arguments, I'd
probably choose:

// If ipAddresses is non-empty, secondaryIPsCount is ignored.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode27
ec2/privateips.go:27: // allowReassignment is optional and indicates whether to
allow an IP
What does it mean to have an "optional" bool?

I think you just want something like this:

// allowReassignment specifies whether to allow reassignment
// of addresses currently assigned to a different
// network interface.

[dimitern, 2014-02-07 13:18:51]

Please take a look.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go
File ec2/privateips.go (right):

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode6
ec2/privateips.go:6: // Copyright (c) 2011 Canonical Ltd.
On 2014/02/06 18:36:03, rog wrote:
> I guess files created later should probably have a more current copyright
date.

Done.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode19
ec2/privateips.go:19: // secondary IP addresses can be given explicitly, or a
number of
On 2014/02/06 18:36:03, rog wrote:
> s/number/count/
> 
> ?
> "a number of" is almost exactly the same as "one of more"

Done.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode20
ec2/privateips.go:20: // secondary IP addresses can be given, to be
automatically assigned
On 2014/02/06 18:36:03, rog wrote:
> s/, //

Done.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode21
ec2/privateips.go:21: // within the subnet's CIDR block range. The number of
secondary IP
On 2014/02/06 18:36:03, rog wrote:
> s/number/total count/
> 
> or
> 
> s/number/total number/
> 
> ?

Done.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode25
ec2/privateips.go:25: // Either ipAddresses or secondaryIPsCount can be given
but not both.
On 2014/02/06 18:36:03, rog wrote:
> Perhaps be more specific here:
> 
> // If secondaryIPsCount is greater than zero, ipAddresses
> // is ignored.
> 
> except that as ipAddresses is first in the arguments, I'd
> probably choose:
> 
> // If ipAddresses is non-empty, secondaryIPsCount is ignored.

Done, and added that both mutually exclusive.

https://codereview.appspot.com/54210047/diff/1/ec2/privateips.go#newcode27
ec2/privateips.go:27: // allowReassignment is optional and indicates whether to
allow an IP
On 2014/02/06 18:36:03, rog wrote:
> What does it mean to have an "optional" bool?
> 
> I think you just want something like this:
> 
> // allowReassignment specifies whether to allow reassignment
> // of addresses currently assigned to a different
> // network interface.

Done.

[niemeyer, 2014-02-12 16:30:30]

LGTM

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go
File ec2/privateips.go (right):

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go#newcode1
ec2/privateips.go:1: //
FWIW, it doesn't feel like the functionality here needs a file on its own.

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go#newcode8
ec2/privateips.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
Please drop this line.

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go#newcode26
ec2/privateips.go:26: // mutually exclusive).
Suggested wording for the whole block of documentation above:

// AssignPrivateIPAddresses assigns secondary IP addresses to the network
// interface interfaceId.
//
// If ipCount is non-zero, ipAddresses must be nil, and the specified
// number of secondary IP addresses will be allocated within the subnet's
// CIDR block range.

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips_test.go
File ec2/privateips_test.go (right):

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips_test.go#new...
ec2/privateips_test.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
Please drop this line.

[dimitern, 2014-02-12 17:31:30]

*** Submitted:

ec2: Added (Un)AssignPrivateIPAddresses APIs

Added two new API calls:
- AssignPrivateIPAddresses
- UnassignPrivateIPAddresses

They allow adding or removing secondary private
IP addresses to a network interface of an instance.

This is the final part of the VPC-related goamz
changes needed to support initial container
networking in juju-core.

Added a live-only test for the two new APIs and
a couple of example response tests.

Removed ec2.*Status constants, as suggested in
a prereq review, which I omitted previously.

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

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go
File ec2/privateips.go (right):

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go#newcode1
ec2/privateips.go:1: //
On 2014/02/12 16:30:30, niemeyer wrote:
> FWIW, it doesn't feel like the functionality here needs a file on its own.

I followed the same approach as for the other CLs, it seems cleaner IMO. If you
feel strongly against it, I can move all these into networkinterfaces.go.

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go#newcode8
ec2/privateips.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
On 2014/02/12 16:30:30, niemeyer wrote:
> Please drop this line.

Done.

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips.go#newcode26
ec2/privateips.go:26: // mutually exclusive).
On 2014/02/12 16:30:30, niemeyer wrote:
> Suggested wording for the whole block of documentation above:
> 
> // AssignPrivateIPAddresses assigns secondary IP addresses to the network
> // interface interfaceId.
> //
> // If ipCount is non-zero, ipAddresses must be nil, and the specified
> // number of secondary IP addresses will be allocated within the subnet's
> // CIDR block range.
> 

Done.

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips_test.go
File ec2/privateips_test.go (right):

https://codereview.appspot.com/54210047/diff/20001/ec2/privateips_test.go#new...
ec2/privateips_test.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
On 2014/02/12 16:30:30, niemeyer wrote:
> Please drop this line.

Done.