Merge lp:~dimitern/goamz/subnets-api-calls into lp:goamz

[Dimiter Naydenov]

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://codereview.appspot.com/54690048/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+204640_code.launchpad.net,

Message:
Please take a look.

Description:
ec2: Add support for VPC subnets

Added the following new API calls:
- CreateSubnet
- DeleteSubnet
- DescribeSubnets
(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.

Using AWS API version 2011-12-15.

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

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)

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

Affected files (+466, -2 lines):
   A [revision details]
   M ec2/ec2test/server.go
   M ec2/responses_test.go
   A ec2/subnets.go
   A ec2/subnets_test.go

[lp:~dimitern/goamz/subnets-api-calls]

51. By Dimiter Naydenov on 2014-02-04

Minor fixes

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/54690048/

[lp:~dimitern/goamz/subnets-api-calls]

52. By Dimiter Naydenov on 2014-02-05

Merged conflicts; simplified & refactored a bit; naming fixed, API version updated

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/54690048/

[Roger Peppe (rogpeppe)]

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#newcode1105
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.

?

https://codereview.appspot.com/54690048/

[lp:~dimitern/goamz/subnets-api-calls]

53. By Dimiter Naydenov on 2014-02-06

Changes after reivew

54. By Dimiter Naydenov on 2014-02-06

Merged vpc-api-calls into subnets-api-calls.

55. By Dimiter Naydenov on 2014-02-06

Improved tests not to leave stuff around live

[Dimiter Naydenov (dimitern)]

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#newcode1105
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.

https://codereview.appspot.com/54690048/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/54690048/

[Martin Packman (gz)]

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
<email address hidden>
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#newcode141
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?

https://codereview.appspot.com/54690048/

[lp:~dimitern/goamz/subnets-api-calls]

56. By Dimiter Naydenov on 2014-02-07

Merged vpc-api-calls into subnets-api-calls.

57. By Dimiter Naydenov on 2014-02-07

Changes after reivew

[Dimiter Naydenov (dimitern)]

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
<email address hidden>
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#newcode141
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.

https://codereview.appspot.com/54690048/

[Gustavo Niemeyer (niemeyer)]

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
<email address hidden>
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
<email address hidden>
Please drop this line.

https://codereview.appspot.com/54690048/

[Dimiter Naydenov (dimitern)]

*** 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
<email address hidden>
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
<email address hidden>
On 2014/02/12 13:33:51, niemeyer wrote:
> Please drop this line.

Done.

https://codereview.appspot.com/54690048/

[Dimiter Naydenov (dimitern)]

Post-submit approval.

review: Approve

=== modified file 'ec2/ec2test/server.go'
--- ec2/ec2test/server.go	2014-02-07 12:48:07 +0000
+++ ec2/ec2test/server.go	2014-02-07 12:48:07 +0000
@@ -51,12 +51,14 @@
 	reservations         map[string]*reservation   // id -> reservation
 	groups               map[string]*securityGroup // id -> group
 	vpcs                 map[string]*vpc           // id -> vpc
+	subnets              map[string]*subnet        // id -> subnet
 	maxId                counter
 	reqId                counter
 	reservationId        counter
 	groupId              counter
 	vpcId                counter
 	dhcpOptsId           counter
+	subnetId             counter
 	initialInstanceState ec2.InstanceState
 }
 
@@ -212,6 +214,28 @@
 	return false, fmt.Errorf("unknown attribute %q", attr)
 }
 
+type subnet struct {
+	ec2.Subnet
+}
+
+func (s *subnet) matchAttr(attr, value string) (ok bool, err error) {
+	switch attr {
+	case "cidr":
+		return s.CIDRBlock == value, nil
+	case "availability-zone":
+		return s.AvailZone == value, nil
+	case "state":
+		return s.State == value, nil
+	case "subnet-id":
+		return s.Id == value, nil
+	case "vpc-id":
+		return s.VPCId == value, nil
+	case "tag", "tag-key", "tag-value", "available-ip-address-count", "defaultForAz":
+		return false, fmt.Errorf("%q filter not implemented", attr)
+	}
+	return false, fmt.Errorf("unknown attribute %q", attr)
+}
+
 var actions = map[string]func(*Server, http.ResponseWriter, *http.Request, string) interface{}{
 	"RunInstances":                  (*Server).runInstances,
 	"TerminateInstances":            (*Server).terminateInstances,
@@ -224,6 +248,9 @@
 	"CreateVpc":                     (*Server).createVpc,
 	"DeleteVpc":                     (*Server).deleteVpc,
 	"DescribeVpcs":                  (*Server).describeVpcs,
+	"CreateSubnet":                  (*Server).createSubnet,
+	"DeleteSubnet":                  (*Server).deleteSubnet,
+	"DescribeSubnets":               (*Server).describeSubnets,
 }
 
 const ownerId = "9876"
@@ -245,6 +272,7 @@
 		instances:            make(map[string]*Instance),
 		groups:               make(map[string]*securityGroup),
 		vpcs:                 make(map[string]*vpc),
+		subnets:              make(map[string]*subnet),
 		reservations:         make(map[string]*reservation),
 		initialInstanceState: Pending,
 	}
@@ -1060,6 +1088,70 @@
 	return &resp
 }
 
+func (srv *Server) createSubnet(w http.ResponseWriter, req *http.Request, reqId string) interface{} {
+	v := srv.vpc(req.Form.Get("VpcId"))
+	cidrBlock := parseCidr(req.Form.Get("CidrBlock"))
+	availZone := req.Form.Get("AvailabilityZone")
+	if availZone == "" {
+		// Assign one automatically as AWS does.
+		availZone = "us-east-1b"
+	}
+	// calculate the available IP addresses, removing the first 4 and
+	// the last, which are reserved by AWS. Since we already checked
+	// the CIDR is valid, we don't check the error here.
+	_, ipnet, _ := net.ParseCIDR(cidrBlock)
+	maskOnes, maskBits := ipnet.Mask.Size()
+	availIPs := 1<<uint(maskBits-maskOnes) - 5
+
+	srv.mu.Lock()
+	defer srv.mu.Unlock()
+	s := &subnet{ec2.Subnet{
+		Id:               fmt.Sprintf("subnet-%d", srv.subnetId.next()),
+		VPCId:            v.Id,
+		State:            ec2.AvailableState,
+		CIDRBlock:        cidrBlock,
+		AvailZone:        availZone,
+		AvailableIPCount: availIPs,
+	}}
+	srv.subnets[s.Id] = s
+	r := &ec2.CreateSubnetResp{
+		RequestId: reqId,
+		Subnet:    s.Subnet,
+	}
+	return r
+}
+
+func (srv *Server) deleteSubnet(w http.ResponseWriter, req *http.Request, reqId string) interface{} {
+	s := srv.subnet(req.Form.Get("SubnetId"))
+	srv.mu.Lock()
+	defer srv.mu.Unlock()
+
+	delete(srv.subnets, s.Id)
+	return &ec2.SimpleResp{
+		XMLName:   xml.Name{"", "DeleteSubnetResponse"},
+		RequestId: reqId,
+	}
+}
+
+func (srv *Server) describeSubnets(w http.ResponseWriter, req *http.Request, reqId string) interface{} {
+	srv.mu.Lock()
+	defer srv.mu.Unlock()
+
+	idMap := collectIds(req.Form, "SubnetId.")
+	f := newFilter(req.Form)
+	var resp ec2.SubnetsResp
+	resp.RequestId = reqId
+	for _, s := range srv.subnets {
+		ok, err := f.ok(s)
+		if ok && (len(idMap) == 0 || idMap[s.Id]) {
+			resp.Subnets = append(resp.Subnets, s.Subnet)
+		} else if err != nil {
+			fatalf(400, "InvalidParameterValue", "describe subnets: %v", err)
+		}
+	}
+	return &resp
+}
+
 func (r *reservation) hasRunningMachine() bool {
 	for _, inst := range r.instances {
 		if inst.state.Code != ShuttingDown.Code && inst.state.Code != Terminated.Code {
@@ -1092,6 +1184,19 @@
 	return v
 }
 
+func (srv *Server) subnet(id string) *subnet {
+	if id == "" {
+		fatalf(400, "MissingParameter", "missing subnetId")
+	}
+	srv.mu.Lock()
+	defer srv.mu.Unlock()
+	s, found := srv.subnets[id]
+	if !found {
+		fatalf(400, "InvalidSubnetID.NotFound", "subnet %s not found", id)
+	}
+	return s
+}
+
 // collectIds takes all values with the given prefix from form and
 // returns a map with the ids as keys.
 func collectIds(form url.Values, prefix string) map[string]bool {
 
=== modified file 'ec2/responses_test.go'
--- ec2/responses_test.go	2014-02-07 12:48:07 +0000
+++ ec2/responses_test.go	2014-02-07 12:48:07 +0000
@@ -629,3 +629,58 @@
   </vpcSet>
 </DescribeVpcsResponse>
 `
+
+// http://goo.gl/wLPhf
+var CreateSubnetExample = `
+<CreateSubnetResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
+  <requestId>7a62c49f-347e-4fc4-9331-6e8eEXAMPLE</requestId>
+  <subnet>
+    <subnetId>subnet-9d4a7b6c</subnetId>
+    <state>pending</state>
+    <vpcId>vpc-1a2b3c4d</vpcId>
+    <cidrBlock>10.0.1.0/24</cidrBlock>
+    <availableIpAddressCount>251</availableIpAddressCount>
+    <availabilityZone>us-east-1a</availabilityZone>
+    <tagSet/>
+  </subnet>
+</CreateSubnetResponse>
+`
+
+// http://goo.gl/KmhcBM
+var DeleteSubnetExample = `
+<DeleteSubnetResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
+   <requestId>7a62c49f-347e-4fc4-9331-6e8eEXAMPLE</requestId>
+   <return>true</return>
+</DeleteSubnetResponse>
+`
+
+// http://goo.gl/NTKQVI
+var DescribeSubnetsExample = `
+<DescribeSubnetsResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
+  <requestId>7a62c49f-347e-4fc4-9331-6e8eEXAMPLE</requestId>
+  <subnetSet>
+    <item>
+      <subnetId>subnet-9d4a7b6c</subnetId>
+      <state>available</state>
+      <vpcId>vpc-1a2b3c4d</vpcId>
+      <cidrBlock>10.0.1.0/24</cidrBlock>
+      <availableIpAddressCount>251</availableIpAddressCount>
+      <availabilityZone>us-east-1a</availabilityZone>
+      <defaultForAz>false</defaultForAz>
+      <mapPublicIpOnLaunch>false</mapPublicIpOnLaunch>
+      <tagSet/>
+    </item>
+    <item>
+      <subnetId>subnet-6e7f829e</subnetId>
+      <state>available</state>
+      <vpcId>vpc-1a2b3c4d</vpcId>
+      <cidrBlock>10.0.0.0/24</cidrBlock>
+      <availableIpAddressCount>251</availableIpAddressCount>
+      <availabilityZone>us-east-1a</availabilityZone>
+      <defaultForAz>false</defaultForAz>
+      <mapPublicIpOnLaunch>false</mapPublicIpOnLaunch>
+      <tagSet/>
+    </item>
+  </subnetSet>
+</DescribeSubnetsResponse>
+`
 
=== added file 'ec2/subnets.go'
--- ec2/subnets.go	1970-01-01 00:00:00 +0000
+++ ec2/subnets.go	2014-02-07 12:48:07 +0000
@@ -0,0 +1,111 @@
+//
+// goamz - Go packages to interact with the Amazon Web Services.
+//
+//   https://wiki.ubuntu.com/goamz
+//
+// Copyright (c) 2014 Canonical Ltd.
+//
+// Written by Gustavo Niemeyer <[e-mail]>
+//
+
+package ec2
+
+import (
+	"strconv"
+)
+
+// Subnet describes an Amazon VPC subnet.
+//
+// See http://goo.gl/CdkvO2 for more details.
+type Subnet struct {
+	Id                  string `xml:"subnetId"`
+	State               string `xml:"state"`
+	VPCId               string `xml:"vpcId"`
+	CIDRBlock           string `xml:"cidrBlock"`
+	AvailableIPCount    int    `xml:"availableIpAddressCount"`
+	AvailZone           string `xml:"availabilityZone"`
+	DefaultForAZ        bool   `xml:"defaultForAz"`
+	MapPublicIPOnLaunch bool   `xml:"mapPublicIpOnLaunch"`
+	Tags                []Tag  `xml:"tagSet>item"`
+}
+
+// CreateSubnetResp is the response to a CreateSubnet request.
+//
+// See http://goo.gl/wLPhfI for more details.
+type CreateSubnetResp struct {
+	RequestId string `xml:"requestId"`
+	Subnet    Subnet `xml:"subnet"`
+}
+
+// CreateSubnet creates a subnet in an existing VPC.
+//
+// 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).
+//
+// availZone may be empty, in which case Amazon EC2 selects one for
+// you (recommended).
+//
+// See http://goo.gl/wLPhfI for more details.
+func (ec2 *EC2) CreateSubnet(vpcId, cidrBlock, availZone string) (resp *CreateSubnetResp, err error) {
+	params := makeParamsVPC("CreateSubnet")
+	params["VpcId"] = vpcId
+	params["CidrBlock"] = cidrBlock
+	if availZone != "" {
+		params["AvailabilityZone"] = availZone
+	}
+	resp = &CreateSubnetResp{}
+	err = ec2.query(params, resp)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
+// DeleteSubnet deletes the specified subnet. All running instances in
+// the subnet must have been terminated.
+//
+// See http://goo.gl/KmhcBM for more details.
+func (ec2 *EC2) DeleteSubnet(id string) (resp *SimpleResp, err error) {
+	params := makeParamsVPC("DeleteSubnet")
+	params["SubnetId"] = id
+	resp = &SimpleResp{}
+	err = ec2.query(params, resp)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
+// SubnetsResp is the response to a Subnets request.
+//
+// See http://goo.gl/NTKQVI for more details.
+type SubnetsResp struct {
+	RequestId string   `xml:"requestId"`
+	Subnets   []Subnet `xml:"subnetSet>item"`
+}
+
+// Subnets describes one or more of your subnets. Both parameters are
+// optional, and if specified will limit the returned subnets to the
+// matching ids or filtering rules.
+//
+// See http://goo.gl/NTKQVI for more details.
+func (ec2 *EC2) Subnets(ids []string, filter *Filter) (resp *SubnetsResp, err error) {
+	params := makeParamsVPC("DescribeSubnets")
+	for i, id := range ids {
+		params["SubnetId."+strconv.Itoa(i+1)] = id
+	}
+	filter.addParams(params)
+
+	resp = &SubnetsResp{}
+	err = ec2.query(params, resp)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
 
=== added file 'ec2/subnets_test.go'
--- ec2/subnets_test.go	1970-01-01 00:00:00 +0000
+++ ec2/subnets_test.go	2014-02-07 12:48:07 +0000
@@ -0,0 +1,212 @@
+//
+// goamz - Go packages to interact with the Amazon Web Services.
+//
+//   https://wiki.ubuntu.com/goamz
+//
+// Copyright (c) 2014 Canonical Ltd.
+//
+// Written by Gustavo Niemeyer <[e-mail]>
+//
+
+package ec2_test
+
+import (
+	"launchpad.net/goamz/aws"
+	"launchpad.net/goamz/ec2"
+	. "launchpad.net/gocheck"
+	"time"
+)
+
+// Subnet tests with example responses
+
+func (s *S) TestCreateSubnetExample(c *C) {
+	testServer.Response(200, nil, CreateSubnetExample)
+
+	resp, err := s.ec2.CreateSubnet("vpc-1a2b3c4d", "10.0.1.0/24", "us-east-1a")
+	req := testServer.WaitRequest()
+
+	c.Assert(req.Form["Action"], DeepEquals, []string{"CreateSubnet"})
+	c.Assert(req.Form["VpcId"], DeepEquals, []string{"vpc-1a2b3c4d"})
+	c.Assert(req.Form["CidrBlock"], DeepEquals, []string{"10.0.1.0/24"})
+	c.Assert(req.Form["AvailabilityZone"], DeepEquals, []string{"us-east-1a"})
+
+	c.Assert(err, IsNil)
+	c.Assert(resp.RequestId, Equals, "7a62c49f-347e-4fc4-9331-6e8eEXAMPLE")
+	subnet := resp.Subnet
+	c.Check(subnet.Id, Equals, "subnet-9d4a7b6c")
+	c.Check(subnet.State, Equals, "pending")
+	c.Check(subnet.VPCId, Equals, "vpc-1a2b3c4d")
+	c.Check(subnet.CIDRBlock, Equals, "10.0.1.0/24")
+	c.Check(subnet.AvailableIPCount, Equals, 251)
+	c.Check(subnet.AvailZone, Equals, "us-east-1a")
+	c.Check(subnet.Tags, HasLen, 0)
+}
+
+func (s *S) TestDeleteSubnetExample(c *C) {
+	testServer.Response(200, nil, DeleteSubnetExample)
+
+	resp, err := s.ec2.DeleteSubnet("subnet-id")
+	req := testServer.WaitRequest()
+
+	c.Assert(req.Form["Action"], DeepEquals, []string{"DeleteSubnet"})
+	c.Assert(req.Form["SubnetId"], DeepEquals, []string{"subnet-id"})
+
+	c.Assert(err, IsNil)
+	c.Assert(resp.RequestId, Equals, "7a62c49f-347e-4fc4-9331-6e8eEXAMPLE")
+}
+
+func (s *S) TestSubnetsExample(c *C) {
+	testServer.Response(200, nil, DescribeSubnetsExample)
+
+	ids := []string{"subnet-9d4a7b6c", "subnet-6e7f829e"}
+	resp, err := s.ec2.Subnets(ids, nil)
+	req := testServer.WaitRequest()
+
+	c.Assert(req.Form["Action"], DeepEquals, []string{"DescribeSubnets"})
+	c.Assert(req.Form["SubnetId.1"], DeepEquals, []string{ids[0]})
+	c.Assert(req.Form["SubnetId.2"], DeepEquals, []string{ids[1]})
+
+	c.Assert(err, IsNil)
+	c.Assert(resp.RequestId, Equals, "7a62c49f-347e-4fc4-9331-6e8eEXAMPLE")
+	c.Check(resp.Subnets, HasLen, 2)
+	subnet := resp.Subnets[0]
+	c.Check(subnet.Id, Equals, "subnet-9d4a7b6c")
+	c.Check(subnet.State, Equals, ec2.AvailableState)
+	c.Check(subnet.VPCId, Equals, "vpc-1a2b3c4d")
+	c.Check(subnet.CIDRBlock, Equals, "10.0.1.0/24")
+	c.Check(subnet.AvailableIPCount, Equals, 251)
+	c.Check(subnet.AvailZone, Equals, "us-east-1a")
+	c.Check(subnet.DefaultForAZ, Equals, false)
+	c.Check(subnet.MapPublicIPOnLaunch, Equals, false)
+	c.Check(subnet.Tags, HasLen, 0)
+	subnet = resp.Subnets[1]
+	c.Check(subnet.Id, Equals, "subnet-6e7f829e")
+	c.Check(subnet.State, Equals, ec2.AvailableState)
+	c.Check(subnet.VPCId, Equals, "vpc-1a2b3c4d")
+	c.Check(subnet.CIDRBlock, Equals, "10.0.0.0/24")
+	c.Check(subnet.AvailableIPCount, Equals, 251)
+	c.Check(subnet.AvailZone, Equals, "us-east-1a")
+	c.Check(subnet.DefaultForAZ, Equals, false)
+	c.Check(subnet.MapPublicIPOnLaunch, Equals, false)
+	c.Check(subnet.Tags, HasLen, 0)
+}
+
+// Subnet tests run against either a local test server or live on EC2.
+
+func (s *ServerTests) TestSubnets(c *C) {
+	resp, err := s.ec2.CreateVPC("10.2.0.0/16", "")
+	c.Assert(err, IsNil)
+	vpcId := resp.VPC.Id
+	defer s.deleteVPCs(c, []string{vpcId})
+
+	// It might take some time for the VPC to get created, so we need
+	// to retry a few times when running against the EC2 servers.
+	var resp1 *ec2.CreateSubnetResp
+	testAttempt := aws.AttemptStrategy{
+		Total: 2 * time.Minute,
+		Delay: 5 * time.Second,
+	}
+	done := false
+	for a := testAttempt.Start(); a.Next(); {
+		resp1, err = s.ec2.CreateSubnet(vpcId, "10.2.1.0/24", "")
+		if s.errorCode(err) == "InvalidVpcID.NotFound" {
+			c.Logf("VPC %v not created yet; retrying", vpcId)
+			continue
+		}
+		if err != nil {
+			c.Logf("retrying; CreateSubnet returned: %v", err)
+			continue
+		}
+		done = true
+		break
+	}
+	if !done {
+		c.Fatalf("timeout while waiting for VPC and subnet")
+	}
+	assertSubnet(c, resp1.Subnet, "", vpcId, "10.2.1.0/24")
+	id1 := resp1.Subnet.Id
+
+	resp2, err := s.ec2.CreateSubnet(vpcId, "10.2.2.0/24", "")
+	c.Assert(err, IsNil)
+	assertSubnet(c, resp2.Subnet, "", vpcId, "10.2.2.0/24")
+	id2 := resp2.Subnet.Id
+
+	// We only check for the subnets we just created, because the user
+	// might have others in his account (when testing against the EC2
+	// servers). In some cases it takes a short while until both
+	// subnets are created, so we need to retry a few times to make
+	// sure.
+	var list *ec2.SubnetsResp
+	done = false
+	for a := testAttempt.Start(); a.Next(); {
+		c.Logf("waiting for %v to be created", []string{id1, id2})
+		list, err = s.ec2.Subnets(nil, nil)
+		if err != nil {
+			c.Logf("retrying; Subnets returned: %v", err)
+			continue
+		}
+		found := 0
+		for _, subnet := range list.Subnets {
+			c.Logf("found subnet %v", subnet)
+			switch subnet.Id {
+			case id1:
+				assertSubnet(c, subnet, id1, vpcId, resp1.Subnet.CIDRBlock)
+				found++
+			case id2:
+				assertSubnet(c, subnet, id2, vpcId, resp2.Subnet.CIDRBlock)
+				found++
+			}
+			if found == 2 {
+				done = true
+				break
+			}
+		}
+		if done {
+			c.Logf("all subnets were created")
+			break
+		}
+	}
+	if !done {
+		c.Fatalf("timeout while waiting for subnets %v", []string{id1, id2})
+	}
+
+	list, err = s.ec2.Subnets([]string{id1}, nil)
+	c.Assert(err, IsNil)
+	c.Assert(list.Subnets, HasLen, 1)
+	assertSubnet(c, list.Subnets[0], id1, vpcId, resp1.Subnet.CIDRBlock)
+
+	f := ec2.NewFilter()
+	f.Add("cidr", resp2.Subnet.CIDRBlock)
+	list, err = s.ec2.Subnets(nil, f)
+	c.Assert(err, IsNil)
+	c.Assert(list.Subnets, HasLen, 1)
+	assertSubnet(c, list.Subnets[0], id2, vpcId, resp2.Subnet.CIDRBlock)
+
+	_, err = s.ec2.DeleteSubnet(id1)
+	c.Assert(err, IsNil)
+	_, err = s.ec2.DeleteSubnet(id2)
+	c.Assert(err, IsNil)
+}
+
+func assertSubnet(c *C, obtained ec2.Subnet, expectId, expectVpcId, expectCidr string) {
+	if expectId != "" {
+		c.Check(obtained.Id, Equals, expectId)
+	} else {
+		c.Check(obtained.Id, Matches, `^subnet-[0-9a-f]+$`)
+	}
+	c.Check(obtained.State, Matches, "("+ec2.AvailableState+"|"+ec2.PendingState+")")
+	if expectVpcId != "" {
+		c.Check(obtained.VPCId, Equals, expectVpcId)
+	} else {
+		c.Check(obtained.VPCId, Matches, `^vpc-[0-9a-f]+$`)
+	}
+	if expectCidr != "" {
+		c.Check(obtained.CIDRBlock, Equals, expectCidr)
+	} else {
+		c.Check(obtained.CIDRBlock, Matches, `^\d+\.\d+\.\d+\.\d+/\d+$`)
+	}
+	c.Check(obtained.AvailZone, Not(Equals), "")
+	c.Check(obtained.AvailableIPCount, Not(Equals), 0)
+	c.Check(obtained.DefaultForAZ, Equals, false)
+	c.Check(obtained.MapPublicIPOnLaunch, Equals, false)
+}
 
=== modified file 'ec2/vpc_test.go'
--- ec2/vpc_test.go	2014-02-07 12:48:07 +0000
+++ ec2/vpc_test.go	2014-02-07 12:48:07 +0000
@@ -149,6 +149,36 @@
 	c.Assert(err, IsNil)
 }
 
+// deleteVPCs ensures the given VPCs are deleted, by retrying until a
+// timeout or all VPC cannot be found anymore.  This should be used to
+// make sure tests leave no VPCs around.
+func (s *ServerTests) deleteVPCs(c *C, ids []string) {
+	testAttempt := aws.AttemptStrategy{
+		Total: 2 * time.Minute,
+		Delay: 5 * time.Second,
+	}
+	for a := testAttempt.Start(); a.Next(); {
+		deleted := 0
+		c.Logf("deleting VPCs %v", ids)
+		for _, id := range ids {
+			_, err := s.ec2.DeleteVPC(id)
+			if err == nil || s.errorCode(err) == "InvalidVpcID.NotFound" {
+				c.Logf("VPC %s deleted", id)
+				deleted++
+				continue
+			}
+			if err != nil {
+				c.Logf("retrying; DeleteVPC returned: %v", err)
+			}
+		}
+		if deleted == len(ids) {
+			c.Logf("all VPCs deleted")
+			return
+		}
+	}
+	c.Fatalf("timeout while waiting %v VPCs to get deleted!", ids)
+}
+
 func assertVPC(c *C, obtained ec2.VPC, expectId, expectCidr string) {
 	if expectId != "" {
 		c.Check(obtained.Id, Equals, expectId)