ec2: Add support for VPC subnets

ec2: Add support for VPC subnets

Added the following new API calls:
- CreateSubnet
- DeleteSubnet
- Subnets
(and related types/responses)

This is the second step on the path to
support VPC networking in goamz, next
APIs for network interfaces will be
added.

Tested live on EC2, and extended the
ec2test package as needed.

Added a deleteVPCs test helpers which
waits until a VPC is no longer in use
and can be deleted, retrying as needed
when running against live EC2 servers.
This is needed to ensure live tests do
no leave stuff behind (I've run all live
tests several times in a row to make sure
it works).

https://code.launchpad.net/~dimitern/goamz/subnets-api-calls/+merge/204640

Requires: https://code.launchpad.net/~dimitern/goamz/vpc-api-calls/+merge/204514

(do not edit description out of merge proposal)

[dimitern, 2014-02-04 11:05:17]

Please take a look.

[dimitern, 2014-02-04 11:08:53]

Please take a look.

[dimitern, 2014-02-05 13:31:00]

Please take a look.

[rog, 2014-02-05 14:46:52]

LGTM with some suggestions below.

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

https://codereview.appspot.com/54690048/diff/40001/ec2/ec2test/server.go#newc...
ec2/ec2test/server.go:1105: availIPs :=
int(math.Exp2(float64(maskBits-maskOnes))) - 5
availIPs := 1<<uint(maskBits - maskOnes) - 5
?

i suppose we might consider making sure that there
are enough bits in the mask too, otherwise
we can get a negative AvailableIPCount.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go
File ec2/subnets.go (right):

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode42
ec2/subnets.go:42: // When you create each subnet, you provide the VPC ID and
the CIDR
// The vpcId and cidrBlock parameters specify the
// VPC id and CIDR block respectively - these cannot
// be changed after creation. The subnet's CIDR block can be the same as
// the VPC's CIDR block (assuming you want only a single subnet in the
// VPC), or a subset of the VPC's CIDR block. If more than one
// subnet is created in a VPC, their  CIDR blocks must not overlap. The
// smallest subnet (and VPC) that can be created uses a /28 netmask (16 IP
// addresses), and the largest uses a /16 netmask (65,536 IP
// addresses).

?

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode52
ec2/subnets.go:52: // availZone can be empty, in which case Amazon EC2 selects
one for
s/can/may/

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode71
ec2/subnets.go:71: // DeleteSubnet deletes the specified subnet. You must
terminate all
// All running instances in the subnet must have been terminated.

?

[dimitern, 2014-02-06 11:37:26]

Please take a look.

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

https://codereview.appspot.com/54690048/diff/40001/ec2/ec2test/server.go#newc...
ec2/ec2test/server.go:1105: availIPs :=
int(math.Exp2(float64(maskBits-maskOnes))) - 5
On 2014/02/05 14:46:53, rog wrote:
> availIPs := 1<<uint(maskBits - maskOnes) - 5
> ?
> 
> i suppose we might consider making sure that there
> are enough bits in the mask too, otherwise
> we can get a negative AvailableIPCount.

Done.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go
File ec2/subnets.go (right):

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode42
ec2/subnets.go:42: // When you create each subnet, you provide the VPC ID and
the CIDR
On 2014/02/05 14:46:53, rog wrote:
> // The vpcId and cidrBlock parameters specify the
> // VPC id and CIDR block respectively - these cannot
> // be changed after creation. The subnet's CIDR block can be the same as
> // the VPC's CIDR block (assuming you want only a single subnet in the
> // VPC), or a subset of the VPC's CIDR block. If more than one
> // subnet is created in a VPC, their  CIDR blocks must not overlap. The
> // smallest subnet (and VPC) that can be created uses a /28 netmask (16 IP
> // addresses), and the largest uses a /16 netmask (65,536 IP
> // addresses).
> 
> ?

Done.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode52
ec2/subnets.go:52: // availZone can be empty, in which case Amazon EC2 selects
one for
On 2014/02/05 14:46:53, rog wrote:
> s/can/may/

Done.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode71
ec2/subnets.go:71: // DeleteSubnet deletes the specified subnet. You must
terminate all
On 2014/02/05 14:46:53, rog wrote:
> // All running instances in the subnet must have been terminated.
> 
> ?

Done.

[dimitern, 2014-02-06 11:51:59]

Please take a look.

[gz, 2014-02-06 16:37:42]

LGTM.

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets.go
File ec2/subnets.go (right):

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets.go#newcode8
ec2/subnets.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
Er, I'm not sure what Gustavo thinks here, but should bump the copyright year at
least when adding a new file, if now the authorship bit (I'd just omit that as
policy). This probably applies to the other merge proposals as well.

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets_test.go
File ec2/subnets_test.go (right):

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets_test.go#newcod...
ec2/subnets_test.go:141: for a := testAttempt.Start(); a.Next(); {
Hm, this looks familiar. :)

At third one, should figure out how to make a helper for this.

https://codereview.appspot.com/54690048/diff/80001/ec2/vpc_test.go
File ec2/vpc_test.go (right):

https://codereview.appspot.com/54690048/diff/80001/ec2/vpc_test.go#newcode155
ec2/vpc_test.go:155: func (s *ServerTests) deleteVPCs(c *C, ids []string) {
Somewher later in the pipe this gets picked up for the existing vpc live test
too?

[dimitern, 2014-02-07 12:48:20]

Please take a look.

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets.go
File ec2/subnets.go (right):

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets.go#newcode8
ec2/subnets.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
On 2014/02/06 16:37:42, gz wrote:
> Er, I'm not sure what Gustavo thinks here, but should bump the copyright year
at
> least when adding a new file, if now the authorship bit (I'd just omit that as
> policy). This probably applies to the other merge proposals as well.

I'll change the year.

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets_test.go
File ec2/subnets_test.go (right):

https://codereview.appspot.com/54690048/diff/80001/ec2/subnets_test.go#newcod...
ec2/subnets_test.go:141: for a := testAttempt.Start(); a.Next(); {
On 2014/02/06 16:37:42, gz wrote:
> Hm, this looks familiar. :)
> 
> At third one, should figure out how to make a helper for this.

The problem is in all cases the code is similar, but it needs to call a
different "list" call and process a different slice of things. I'm not sure how
to make it unified (without generics, that is).

https://codereview.appspot.com/54690048/diff/80001/ec2/vpc_test.go
File ec2/vpc_test.go (right):

https://codereview.appspot.com/54690048/diff/80001/ec2/vpc_test.go#newcode155
ec2/vpc_test.go:155: func (s *ServerTests) deleteVPCs(c *C, ids []string) {
On 2014/02/06 16:37:42, gz wrote:
> Somewher later in the pipe this gets picked up for the existing vpc live test
> too?

No, because there's no need - TestVPCs does not create any dependent objects, so
VPC can be removed right away, but other cases where you need a VPC, a subnet
and something else, deleteVPCs is useful.

[niemeyer, 2014-02-12 13:33:50]

LGTM

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go
File ec2/subnets.go (right):

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

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode45
ec2/subnets.go:45: // (assuming you want only a single subnet in the VPC), or a
subset of
Documentation should be impersonal:

    "assuming a single subnet is wanted"

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode51
ec2/subnets.go:51: // availZone may be empty, in which case Amazon EC2 selects
one for
// If availZone is empty, an availability zone is automatically
// selected.

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode93
ec2/subnets.go:93: // Subnets describes one or more of your subnets. Both
parameters are
// Subnets returns one or more subnets. Both ...

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets_test.go
File ec2/subnets_test.go (right):

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

[dimitern, 2014-02-12 13:55:00]

*** Submitted:

ec2: Add support for VPC subnets

Added the following new API calls:
- CreateSubnet
- DeleteSubnet
- Subnets
(and related types/responses)

This is the second step on the path to
support VPC networking in goamz, next
APIs for network interfaces will be
added.

Tested live on EC2, and extended the
ec2test package as needed.

Added a deleteVPCs test helpers which
waits until a VPC is no longer in use
and can be deleted, retrying as needed
when running against live EC2 servers.
This is needed to ensure live tests do
no leave stuff behind (I've run all live
tests several times in a row to make sure
it works).

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

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go
File ec2/subnets.go (right):

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode8
ec2/subnets.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
On 2014/02/12 13:33:51, niemeyer wrote:
> Please drop this line.

Done.

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode45
ec2/subnets.go:45: // (assuming you want only a single subnet in the VPC), or a
subset of
On 2014/02/12 13:33:51, niemeyer wrote:
> Documentation should be impersonal:
> 
>     "assuming a single subnet is wanted"

Done.

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode51
ec2/subnets.go:51: // availZone may be empty, in which case Amazon EC2 selects
one for
On 2014/02/12 13:33:51, niemeyer wrote:
> // If availZone is empty, an availability zone is automatically
> // selected.

Done.

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets.go#newcode93
ec2/subnets.go:93: // Subnets describes one or more of your subnets. Both
parameters are
On 2014/02/12 13:33:51, niemeyer wrote:
> // Subnets returns one or more subnets. Both ...

Done.

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets_test.go
File ec2/subnets_test.go (right):

https://codereview.appspot.com/54690048/diff/100001/ec2/subnets_test.go#newcode8
ec2/subnets_test.go:8: // Written by Gustavo Niemeyer
<gustavo.niemeyer@canonical.com>
On 2014/02/12 13:33:51, niemeyer wrote:
> Please drop this line.

Done.