goamz

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

  1. vpc-api-calls
  2. Merge into trunk
Proposed by Dimiter Naydenov
Status: Merged
Merged at revision: 46
Proposed branch: lp:~dimitern/goamz/vpc-api-calls
Merge into: lp:goamz
Prerequisite: lp:~dimitern/goamz/lp-1275406-lpcalserversuite-testinstanceinfo
Diff against target: 758 lines (+526/-25)
7 files modified
ec2/ec2.go (+14/-2)
ec2/ec2_test.go (+7/-7)
ec2/ec2t_test.go (+50/-11)
ec2/ec2test/server.go (+121/-4)
ec2/responses_test.go (+42/-1)
ec2/vpc.go (+120/-0)
ec2/vpc_test.go (+172/-0)
To merge this branch: bzr merge lp:~dimitern/goamz/vpc-api-calls
Reviewer Review Type Date Requested Status
Dimiter Naydenov (community) Approve
Review via email: mp+204514@code.launchpad.net

Description of the change

ec2: Add support for AWS VPCs

Added the following new API calls:
- CreateVPC
- DeleteVPC
- VPCs
(and the associated types/responses).

Most new code is in vpc.go and vpc_test.go.

These are needed in order to support VPC-related
operations on EC2, which will come in later follow-ups.
Implementation complexity in ec2test package for the
new calls is minimal.

Some changes were needed in order to support these
and the upcoming API calls. For the new ones, AWS API
version 2013-10-15 (latest) is used, while for the
existing calls use the previous API version, as before
(2011-12-15).

Added tests, updated test doubles and tested live on
EC2.

After running the EC2 live tests numerous times, I
realized some tests are leaking security groups, so
I added a deleteGroups() tests helper that retries
to ensure all groups are deleted. Also improved the
VPC tests to include retrying as well, so the live
tests are more stable and clean up after themselves.

https://codereview.appspot.com/49930045/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Reviewers: mp+204514_code.launchpad.net,

Message:
Please take a look.

Description:
ec2: Add support for AWS VPCs

Added the following new API calls:
- CreateVpc
- DeleteVpc
- DescribeVpcs
(and the associated types/responses).

These are needed in order to support VPC-related
operations on EC2, which will come in later follow-ups.

Added tests, updated test doubles and tested live on EC2.

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

Requires:
https://code.launchpad.net/~dimitern/goamz/lp-1275406-lpcalserversuite-testinstanceinfo/+merge/204469

(do not edit description out of merge proposal)

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

Affected files (+404, -13 lines):
   A [revision details]
   M ec2/ec2.go
   M ec2/ec2_test.go
   M ec2/ec2t_test.go
   M ec2/ec2test/server.go
   M ec2/responses_test.go

Revision history for this message
Martin Packman (gz) wrote :

LGTM, a couple of comments.

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

https://codereview.appspot.com/49930045/diff/1/ec2/ec2.go#newcode925
ec2/ec2.go:925: func (ec2 *EC2) CreateVpc(cidrBlock string) (resp
*CreateVpcResp, err error) {
Do want to take and pass through the instanceTenancy argument here even
if we don't need it ourselves?

https://codereview.appspot.com/49930045/diff/1/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/1/ec2/ec2t_test.go#newcode631
ec2/ec2t_test.go:631: resp2, err := s.ec2.CreateVpc("1.2.0.0/18")
Is this a safe cidr to be using?

https://codereview.appspot.com/49930045/diff/1/ec2/ec2t_test.go#newcode659
ec2/ec2t_test.go:659: _, err = s.ec2.DeleteVpc(id1)
This feels race/eventual consistency prone, perhaps not an issue for
vpcs or delete calls specifically?

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

https://codereview.appspot.com/49930045/diff/1/ec2/ec2test/server.go#newcode217
ec2/ec2test/server.go:217: // Not done: tag, tag-key, tag-value.
Do we want an error not implemented type of thing rather than just not
matching?

https://codereview.appspot.com/49930045/diff/1/ec2/responses_test.go
File ec2/responses_test.go (right):

https://codereview.appspot.com/49930045/diff/1/ec2/responses_test.go#newcode594
ec2/responses_test.go:594: <CreateVpcResponse
xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
We probably don't want to mix and match schema versions in our example
output unless we really don't have a feature we need in an older schema.

https://codereview.appspot.com/49930045/

lp:~dimitern/goamz/vpc-api-calls updated
48. By Dimiter Naydenov

Changes after review

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

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

https://codereview.appspot.com/49930045/diff/1/ec2/ec2.go#newcode925
ec2/ec2.go:925: func (ec2 *EC2) CreateVpc(cidrBlock string) (resp
*CreateVpcResp, err error) {
On 2014/02/03 21:40:54, gz wrote:
> Do want to take and pass through the instanceTenancy argument here
 even if we
> don't need it ourselves?

It's not supported as an argument in API version 2011-12-15.

https://codereview.appspot.com/49930045/diff/1/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/1/ec2/ec2t_test.go#newcode631
ec2/ec2t_test.go:631: resp2, err := s.ec2.CreateVpc("1.2.0.0/18")
On 2014/02/03 21:40:54, gz wrote:
> Is this a safe cidr to be using?

Yes, it passes even live tests :)

https://codereview.appspot.com/49930045/diff/1/ec2/ec2t_test.go#newcode659
ec2/ec2t_test.go:659: _, err = s.ec2.DeleteVpc(id1)
On 2014/02/03 21:40:54, gz wrote:
> This feels race/eventual consistency prone, perhaps not an issue for
 vpcs or
> delete calls specifically?

I run them multiple times live without issues so far.

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

https://codereview.appspot.com/49930045/diff/1/ec2/ec2test/server.go#newcode217
ec2/ec2test/server.go:217: // Not done: tag, tag-key, tag-value.
On 2014/02/03 21:40:54, gz wrote:
> Do we want an error not implemented type of thing rather than just not
 matching?

It's technically not an error, but since this is just a test double, I
don't think it's a big deal to error out.

https://codereview.appspot.com/49930045/diff/1/ec2/responses_test.go
File ec2/responses_test.go (right):

https://codereview.appspot.com/49930045/diff/1/ec2/responses_test.go#newcode594
ec2/responses_test.go:594: <CreateVpcResponse
xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
On 2014/02/03 21:40:54, gz wrote:
> We probably don't want to mix and match schema versions in our example
 output
> unless we really don't have a feature we need in an older schema.

You're right. I found the API docs for version 2011-12-15 and updated
the example responses, as well as the code to conform to that (i.e. not
IsDefault and no InstanceTenancy fields anymore).

https://codereview.appspot.com/49930045/

lp:~dimitern/goamz/vpc-api-calls updated
49. By Dimiter Naydenov

Split VPC calls/tests in a separate file

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/49930045/

lp:~dimitern/goamz/vpc-api-calls updated
50. By Dimiter Naydenov

Simplified & refactored a bit; naming fixed, API version updated

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/49930045/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/49930045/

Revision history for this message
Roger Peppe (rogpeppe) wrote :
Download full text (3.5 KiB)

LGTM with some suggestions, mostly documentation related.

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

https://codereview.appspot.com/49930045/diff/40001/ec2/ec2test/server.go#newcode212
ec2/ec2test/server.go:212: return false, fmt.Errorf("tag, tag-key, and
tag-value filters not implemented")
fmt.Errorf("%s filter not implemented", attr)

?

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

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2.go#newcode122
ec2/ec2.go:122: if params["Version"] == "" {
Given that makeParams now always creates the Version
entry, why bother with this?

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2.go#newcode183
ec2/ec2.go:183: func makeParamsVPC(action string) map[string]string {
I'm not sure this really justifies itself over
just calling makeParamsForVersion(action, VPCAPIVersion)
when necessary.

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2.go#newcode187
ec2/ec2.go:187: func makeParamsForVersion(version, action string)
map[string]string {
makeParamsWithVersion(action, version string) map[string]string
?

then makeParamsWithVersion is just makeParams with an extra argument.
(i *think* that With is more appropriate than For in this case).

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

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2test/server.go#newcode1095
ec2/ec2test/server.go:1095: func collectIds(form url.Values, prefix
string) map[string]bool {
doc comment?

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go
File ec2/vpc.go (right):

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode26
ec2/vpc.go:26: // AWS API version used for VPC-related calls.
Do we actually want to export this?

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode53
ec2/vpc.go:53: // The smallest VPC you can create uses a /28 netmask (16
IP
s/you can create/that can be created/

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode57
ec2/vpc.go:57: // The supported tenancy options for instances launched
into the
// The instanceTenancy value holds the tenancy options for
// instances launched into the VPC - either DefaultTenancy
// or DedicatedTenancy.
//
// See http://goo.gl/nkwjvN for more details.

?

I think the details of the tenancy semantics can reasonably
be left to the amazon docs.

We should also document that instanceTenancy may be empty
or (my preference) just leave the description unchanged,
and lose the special case for instanceTenancy == "".

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode79
ec2/vpc.go:79: // DeleteVPC deletes the specified VPC. You must detach
or delete all
I'm not keen on the "you" phrasing (it might not be
the reader that's responsible for doing it).

I'd prefer something like this:

// DeleteVPC deletes the VPC with the specified id.
// All gateways and resources that are associated with
// the VPC must have been previously deleted,
// including instances running in the VPC, and non-default security
// groups a...

Read more...

lp:~dimitern/goamz/vpc-api-calls updated
51. By Dimiter Naydenov

Changes after review; tests improvements

52. By Dimiter Naydenov

Changed CIDRs of VPC for easier tracking of leaky tests

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Download full text (4.2 KiB)

Please take a look.

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

https://codereview.appspot.com/49930045/diff/40001/ec2/ec2test/server.go#newcode212
ec2/ec2test/server.go:212: return false, fmt.Errorf("tag, tag-key, and
tag-value filters not implemented")
On 2014/02/05 14:29:14, rog wrote:
> fmt.Errorf("%s filter not implemented", attr)

> ?

It was changed in later patch sets.

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

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2.go#newcode122
ec2/ec2.go:122: if params["Version"] == "" {
On 2014/02/05 14:29:14, rog wrote:
> Given that makeParams now always creates the Version
> entry, why bother with this?

Just to be on the safe side, but fair enough. Removed.

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2.go#newcode183
ec2/ec2.go:183: func makeParamsVPC(action string) map[string]string {
On 2014/02/05 14:29:14, rog wrote:
> I'm not sure this really justifies itself over
> just calling makeParamsForVersion(action, VPCAPIVersion)
> when necessary.

I'd prefer to leave it, because it's shorter to write - it's just a
helper anyway.

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2.go#newcode187
ec2/ec2.go:187: func makeParamsForVersion(version, action string)
map[string]string {
On 2014/02/05 14:29:14, rog wrote:
> makeParamsWithVersion(action, version string) map[string]string
> ?

> then makeParamsWithVersion is just makeParams with an extra argument.
> (i *think* that With is more appropriate than For in this case).

Done.

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

https://codereview.appspot.com/49930045/diff/80001/ec2/ec2test/server.go#newcode1095
ec2/ec2test/server.go:1095: func collectIds(form url.Values, prefix
string) map[string]bool {
On 2014/02/05 14:29:14, rog wrote:
> doc comment?

Done.

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go
File ec2/vpc.go (right):

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode26
ec2/vpc.go:26: // AWS API version used for VPC-related calls.
On 2014/02/05 14:29:14, rog wrote:
> Do we actually want to export this?

Not really. Done.

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode53
ec2/vpc.go:53: // The smallest VPC you can create uses a /28 netmask (16
IP
On 2014/02/05 14:29:14, rog wrote:
> s/you can create/that can be created/

Done.

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode57
ec2/vpc.go:57: // The supported tenancy options for instances launched
into the
On 2014/02/05 14:29:14, rog wrote:
> // The instanceTenancy value holds the tenancy options for
> // instances launched into the VPC - either DefaultTenancy
> // or DedicatedTenancy.
> //
> // See http://goo.gl/nkwjvN for more details.

> ?

> I think the details of the tenancy semantics can reasonably
> be left to the amazon docs.

> We should also document that instanceTenancy may be empty
> or (my preference) just leave the description unchanged,
> and lose the special case for instanceTenancy == "".

I'll change...

Read more...

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM with a few further suggestions below.

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go
File ec2/vpc.go (right):

https://codereview.appspot.com/49930045/diff/80001/ec2/vpc.go#newcode57
ec2/vpc.go:57: // The supported tenancy options for instances launched
into the
On 2014/02/06 11:32:22, dimitern wrote:
> On 2014/02/05 14:29:14, rog wrote:
> > // The instanceTenancy value holds the tenancy options for
> > // instances launched into the VPC - either DefaultTenancy
> > // or DedicatedTenancy.
> > //
> > // See http://goo.gl/nkwjvN for more details.
> >
> > ?
> >
> > I think the details of the tenancy semantics can reasonably
> > be left to the amazon docs.
> >
> > We should also document that instanceTenancy may be empty
> > or (my preference) just leave the description unchanged,
> > and lose the special case for instanceTenancy == "".

> I'll change the comment as suggested, but will change the if below to
set it to
> DefaultTenancy when empty, because we can't pass it as "" to AWS.

If the user passes "" to the function, they deserve any error they get -
the function doesn't document that that's allowed (it actually
explicitly documents that instanceTenancy should be either
DefaultTenancy or DedicatedTenancy)

ec2 doesn't accept "furble" either, but we don't check that, and I don't
think there's a need to.

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go#newcode656
ec2/ec2t_test.go:656: func (s *ServerTests) errorCode(err error, code
string) bool {
When I've done this function before, I've implemented it as:

func errorCode(err error) string {
     if err, _ := err.(*ec2.Error); err != nil {
         return err.Code
     }
     return ""
}

then the call looks like:

   if errorCode(err) == "InvalidGroup.NotFound" {

I think that reads a little better, and it's more
flexible.

I'd actually suggest doing that, and then a subsequent
CL could change other tests to use it (I count about 5 places).

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

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2test/server.go#newcode1096
ec2/ec2test/server.go:1096: // returns a map with the ids as keys for
easier lookup.
s/ for easier lookup././

No particular need for the rationalisation.

https://codereview.appspot.com/49930045/

Revision history for this message
Martin Packman (gz) wrote :

LGTM.

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

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2.go#newcode177
ec2/ec2.go:177: return makeParamsWithVersion(action, "2011-12-15")
I'd stick "2011-12-15" in a const too. Nice adaptation to allow version
passing.

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go#newcode627
ec2/ec2t_test.go:627: func (s *ServerTests) deleteGroups(c *C, groups
[]ec2.SecurityGroup) {
This is somewhat ick, but I like that there's enough logging going on so
failures that hit the timeout should be clear enough.

https://codereview.appspot.com/49930045/

lp:~dimitern/goamz/vpc-api-calls updated
53. By Dimiter Naydenov

Changes after reivew

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

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

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2.go#newcode177
ec2/ec2.go:177: return makeParamsWithVersion(action, "2011-12-15")
On 2014/02/06 16:25:07, gz wrote:
> I'd stick "2011-12-15" in a const too. Nice adaptation to allow
 version passing.

Done.

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go#newcode627
ec2/ec2t_test.go:627: func (s *ServerTests) deleteGroups(c *C, groups
[]ec2.SecurityGroup) {
On 2014/02/06 16:25:07, gz wrote:
> This is somewhat ick, but I like that there's enough logging going on
 so
> failures that hit the timeout should be clear enough.

Yeah, it's more resilient than just trying to delete them and not
checking the error :)

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2t_test.go#newcode656
ec2/ec2t_test.go:656: func (s *ServerTests) errorCode(err error, code
string) bool {
On 2014/02/06 14:42:08, rog wrote:
> When I've done this function before, I've implemented it as:

> func errorCode(err error) string {
> if err, _ := err.(*ec2.Error); err != nil {
> return err.Code
> }
> return ""
> }

> then the call looks like:

> if errorCode(err) == "InvalidGroup.NotFound" {

> I think that reads a little better, and it's more
> flexible.

> I'd actually suggest doing that, and then a subsequent
> CL could change other tests to use it (I count about 5 places).

Done.

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

https://codereview.appspot.com/49930045/diff/100001/ec2/ec2test/server.go#newcode1096
ec2/ec2test/server.go:1096: // returns a map with the ids as keys for
easier lookup.
On 2014/02/06 14:42:08, rog wrote:
> s/ for easier lookup././

> No particular need for the rationalisation.

Done.

https://codereview.appspot.com/49930045/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

LGTM with trivial points.

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2.go
File ec2/ec2.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2.go#newcode31
ec2/ec2.go:31: // VPC-related requests.
Moving the VPC API version here as well would make it more clear. The
constant isn't even used in vpc.go either way.

Also, is there a known reason not to use the new API version across the
board?

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2t_test.go#newcode657
ec2/ec2t_test.go:657: func (s *ServerTests) errorCode(err error) string
{
This can be a function (s is unnecessary).

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go
File ec2/vpc.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go#newcode8
ec2/vpc.go:8: // Written by Gustavo Niemeyer
<email address hidden>
Please drop this line. We should drop it from all the files.

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go#newcode19
ec2/vpc.go:19: PendingState = "pending"
The names of these constants do not look entirely sane. Although this is
inside the vpc.go file, this is living inside the ec2 package, which
means it's ec2.PendingState, and ec2.DefaultTenancy, which is a lot more
broad than "supported values for VPC state".

I suggest either fixing them to include the fact they are about VPC
(VPCPendingState, for example), or just dropping the constants until we
have a better idea.

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go#newcode24
ec2/vpc.go:24: DedicatedTenancy = "dedicated"
Same as above.

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go#newcode30
ec2/vpc.go:30: // VPC describes an Amazon Virtual Private Cloud (VPC).
s/an Amazon Virtual/a Virtual/

We might have other pre-existent instances of that in the documentation,
but we should eventually fix them as well to not refer to Amazon. First,
because every single documentation entry refers to something coming out
of Amazon, and second, because this will work with whoever implements
this API.

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc_test.go
File ec2/vpc_test.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc_test.go#newcode8
ec2/vpc_test.go:8: // Written by Gustavo Niemeyer
<email address hidden>
Please drop this line.

https://codereview.appspot.com/49930045/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Download full text (4.2 KiB)

*** Submitted:

ec2: Add support for AWS VPCs

Added the following new API calls:
- CreateVPC
- DeleteVPC
- VPCs
(and the associated types/responses).

Most new code is in vpc.go and vpc_test.go.

These are needed in order to support VPC-related
operations on EC2, which will come in later follow-ups.
Implementation complexity in ec2test package for the
new calls is minimal.

Some changes were needed in order to support these
and the upcoming API calls. For the new ones, AWS API
version 2013-10-15 (latest) is used, while for the
existing calls use the previous API version, as before
(2011-12-15).

Added tests, updated test doubles and tested live on
EC2.

After running the EC2 live tests numerous times, I
realized some tests are leaking security groups, so
I added a deleteGroups() tests helper that retries
to ensure all groups are deleted. Also improved the
VPC tests to include retrying as well, so the live
tests are more stable and clean up after themselves.

R=
CC=
https://codereview.appspot.com/49930045

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2.go
File ec2/ec2.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2.go#newcode31
ec2/ec2.go:31: // VPC-related requests.
On 2014/02/12 13:23:49, niemeyer wrote:
> Moving the VPC API version here as well would make it more clear. The
 constant
> isn't even used in vpc.go either way.

> Also, is there a known reason not to use the new API version across
the board?

I'll move it here. The reason for not using the newer API version is
that some things are changed, possibly in an incompatible way (responses
include different sub sections, more filters, certain arguments have
changed). I didn't want to make a massive change to all current API
calls to make them compatible with 2013-10-15. I could do it in a
follow-up, after the current chain have landed though.

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2t_test.go
File ec2/ec2t_test.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/ec2t_test.go#newcode657
ec2/ec2t_test.go:657: func (s *ServerTests) errorCode(err error) string
{
On 2014/02/12 13:23:49, niemeyer wrote:
> This can be a function (s is unnecessary).

Done.

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go
File ec2/vpc.go (right):

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go#newcode8
ec2/vpc.go:8: // Written by Gustavo Niemeyer
<email address hidden>
On 2014/02/12 13:23:49, niemeyer wrote:
> Please drop this line. We should drop it from all the files.

I'll remove it from all files then.

https://codereview.appspot.com/49930045/diff/120001/ec2/vpc.go#newcode19
ec2/vpc.go:19: PendingState = "pending"
On 2014/02/12 13:23:49, niemeyer wrote:
> The names of these constants do not look entirely sane. Although this
 is inside
> the vpc.go file, this is living inside the ec2 package, which means
 it's
> ec2.PendingState, and ec2.DefaultTenancy, which is a lot more broad
 than
> "supported values for VPC state".

> I suggest either fixing them to include the fact they are about VPC
> (VPCPendingState, for example), or just dropping the constants until
we have a
> better idea.

I'll drop ...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Post-submit approval.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ec2/ec2.go'
--- ec2/ec2.go 2013-10-04 17:05:49 +0000
+++ ec2/ec2.go 2014-02-07 12:35:18 +0000
@@ -27,6 +27,10 @@
2727
28const debug = false28const debug = false
2929
30// legacyAPIVersion is the AWS API version used for all but
31// VPC-related requests.
32const legacyAPIVersion = "2011-12-15"
33
30// The EC2 type encapsulates operations with a specific EC2 region.34// The EC2 type encapsulates operations with a specific EC2 region.
31type EC2 struct {35type EC2 struct {
32 aws.Auth36 aws.Auth
@@ -119,7 +123,6 @@
119var timeNow = time.Now123var timeNow = time.Now
120124
121func (ec2 *EC2) query(params map[string]string, resp interface{}) error {125func (ec2 *EC2) query(params map[string]string, resp interface{}) error {
122 params["Version"] = "2011-12-15"
123 params["Timestamp"] = timeNow().In(time.UTC).Format(time.RFC3339)126 params["Timestamp"] = timeNow().In(time.UTC).Format(time.RFC3339)
124 endpoint, err := url.Parse(ec2.Region.EC2Endpoint)127 endpoint, err := url.Parse(ec2.Region.EC2Endpoint)
125 if err != nil {128 if err != nil {
@@ -175,8 +178,17 @@
175}178}
176179
177func makeParams(action string) map[string]string {180func makeParams(action string) map[string]string {
181 return makeParamsWithVersion(action, legacyAPIVersion)
182}
183
184func makeParamsVPC(action string) map[string]string {
185 return makeParamsWithVersion(action, vpcAPIVersion)
186}
187
188func makeParamsWithVersion(action, version string) map[string]string {
178 params := make(map[string]string)189 params := make(map[string]string)
179 params["Action"] = action190 params["Action"] = action
191 params["Version"] = version
180 return params192 return params
181}193}
182194
@@ -803,7 +815,7 @@
803 return resp, nil815 return resp, nil
804}816}
805817
806// ResourceTag represents key-value metadata used to classify and organize818// Tag represents key-value metadata used to classify and organize
807// EC2 instances.819// EC2 instances.
808//820//
809// See http://goo.gl/bncl3 for more details821// See http://goo.gl/bncl3 for more details
810822
=== modified file 'ec2/ec2_test.go'
--- ec2/ec2_test.go 2013-10-04 17:05:49 +0000
+++ ec2/ec2_test.go 2014-02-07 12:35:18 +0000
@@ -96,14 +96,14 @@
96 DisableAPITermination: true,96 DisableAPITermination: true,
97 ShutdownBehavior: "terminate",97 ShutdownBehavior: "terminate",
98 PrivateIPAddress: "10.0.0.25",98 PrivateIPAddress: "10.0.0.25",
99 BlockDeviceMappings: []ec2.BlockDeviceMapping{{99 BlockDeviceMappings: []ec2.BlockDeviceMapping{{
100 DeviceName: "device-name",100 DeviceName: "device-name",
101 VirtualName: "virtual-name",101 VirtualName: "virtual-name",
102 SnapshotId: "snapshot-id",102 SnapshotId: "snapshot-id",
103 VolumeType: "volume-type",103 VolumeType: "volume-type",
104 VolumeSize: 10,104 VolumeSize: 10,
105 DeleteOnTermination: true,105 DeleteOnTermination: true,
106 IOPS: 1000,106 IOPS: 1000,
107 }},107 }},
108 }108 }
109 resp, err := s.ec2.RunInstances(&options)109 resp, err := s.ec2.RunInstances(&options)
110110
=== modified file 'ec2/ec2t_test.go'
--- ec2/ec2t_test.go 2014-02-07 12:35:18 +0000
+++ ec2/ec2t_test.go 2014-02-07 12:35:18 +0000
@@ -5,6 +5,7 @@
5 "net"5 "net"
6 "regexp"6 "regexp"
7 "sort"7 "sort"
8 "time"
89
9 "launchpad.net/goamz/aws"10 "launchpad.net/goamz/aws"
10 "launchpad.net/goamz/ec2"11 "launchpad.net/goamz/ec2"
@@ -111,7 +112,7 @@
111}112}
112113
113// AmazonServerSuite runs the ec2test server tests against a live EC2 server.114// AmazonServerSuite runs the ec2test server tests against a live EC2 server.
114// It will only be activated if the -all flag is specified.115// It will only be activated if the -amazon flag is specified.
115type AmazonServerSuite struct {116type AmazonServerSuite struct {
116 srv AmazonServer117 srv AmazonServer
117 ServerTests118 ServerTests
@@ -149,7 +150,7 @@
149func (s *ServerTests) makeTestGroup(c *C, name, descr string) ec2.SecurityGroup {150func (s *ServerTests) makeTestGroup(c *C, name, descr string) ec2.SecurityGroup {
150 // Clean it up if a previous test left it around.151 // Clean it up if a previous test left it around.
151 _, err := s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})152 _, err := s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
152 if err != nil && err.(*ec2.Error).Code != "InvalidGroup.NotFound" {153 if err != nil && s.errorCode(err) != "InvalidGroup.NotFound" {
153 c.Fatalf("delete security group: %v", err)154 c.Fatalf("delete security group: %v", err)
154 }155 }
155156
@@ -161,10 +162,8 @@
161162
162func (s *ServerTests) TestIPPerms(c *C) {163func (s *ServerTests) TestIPPerms(c *C) {
163 g0 := s.makeTestGroup(c, "goamz-test0", "ec2test group 0")164 g0 := s.makeTestGroup(c, "goamz-test0", "ec2test group 0")
164 defer s.ec2.DeleteSecurityGroup(g0)
165
166 g1 := s.makeTestGroup(c, "goamz-test1", "ec2test group 1")165 g1 := s.makeTestGroup(c, "goamz-test1", "ec2test group 1")
167 defer s.ec2.DeleteSecurityGroup(g1)166 defer s.deleteGroups(c, []ec2.SecurityGroup{g0, g1})
168167
169 resp, err := s.ec2.SecurityGroups([]ec2.SecurityGroup{g0, g1}, nil)168 resp, err := s.ec2.SecurityGroups([]ec2.SecurityGroup{g0, g1}, nil)
170 c.Assert(err, IsNil)169 c.Assert(err, IsNil)
@@ -183,7 +182,7 @@
183 SourceIPs: []string{"z127.0.0.1/24"},182 SourceIPs: []string{"z127.0.0.1/24"},
184 }})183 }})
185 c.Assert(err, NotNil)184 c.Assert(err, NotNil)
186 c.Check(err.(*ec2.Error).Code, Equals, "InvalidPermission.Malformed")185 c.Check(s.errorCode(err), Equals, "InvalidPermission.Malformed")
187186
188 // Check that AuthorizeSecurityGroup adds the correct authorizations.187 // Check that AuthorizeSecurityGroup adds the correct authorizations.
189 _, err = s.ec2.AuthorizeSecurityGroup(g0, []ec2.IPPerm{{188 _, err = s.ec2.AuthorizeSecurityGroup(g0, []ec2.IPPerm{{
@@ -230,7 +229,7 @@
230 // Check that we can't delete g1 (because g0 is using it)229 // Check that we can't delete g1 (because g0 is using it)
231 _, err = s.ec2.DeleteSecurityGroup(g1)230 _, err = s.ec2.DeleteSecurityGroup(g1)
232 c.Assert(err, NotNil)231 c.Assert(err, NotNil)
233 c.Check(err.(*ec2.Error).Code, Equals, "InvalidGroup.InUse")232 c.Check(s.errorCode(err), Equals, "InvalidGroup.InUse")
234233
235 _, err = s.ec2.RevokeSecurityGroup(g0, []ec2.IPPerm{{234 _, err = s.ec2.RevokeSecurityGroup(g0, []ec2.IPPerm{{
236 Protocol: "tcp",235 Protocol: "tcp",
@@ -301,7 +300,7 @@
301 c.Assert(err, IsNil)300 c.Assert(err, IsNil)
302301
303 _, err = s.ec2.AuthorizeSecurityGroup(ec2.SecurityGroup{Name: name}, perms[0:2])302 _, err = s.ec2.AuthorizeSecurityGroup(ec2.SecurityGroup{Name: name}, perms[0:2])
304 c.Assert(err, ErrorMatches, `.*\(InvalidPermission.Duplicate\)`)303 c.Assert(s.errorCode(err), Equals, "InvalidPermission.Duplicate")
305}304}
306305
307type filterSpec struct {306type filterSpec struct {
@@ -313,12 +312,12 @@
313 groupResp, err := s.ec2.CreateSecurityGroup(sessionName("testgroup1"), "testgroup one description")312 groupResp, err := s.ec2.CreateSecurityGroup(sessionName("testgroup1"), "testgroup one description")
314 c.Assert(err, IsNil)313 c.Assert(err, IsNil)
315 group1 := groupResp.SecurityGroup314 group1 := groupResp.SecurityGroup
316 defer s.ec2.DeleteSecurityGroup(group1)
317315
318 groupResp, err = s.ec2.CreateSecurityGroup(sessionName("testgroup2"), "testgroup two description")316 groupResp, err = s.ec2.CreateSecurityGroup(sessionName("testgroup2"), "testgroup two description")
319 c.Assert(err, IsNil)317 c.Assert(err, IsNil)
320 group2 := groupResp.SecurityGroup318 group2 := groupResp.SecurityGroup
321 defer s.ec2.DeleteSecurityGroup(group2)319
320 defer s.deleteGroups(c, []ec2.SecurityGroup{group1, group2})
322321
323 insts := make([]*ec2.Instance, 3)322 insts := make([]*ec2.Instance, 3)
324 inst, err := s.ec2.RunInstances(&ec2.RunInstances{323 inst, err := s.ec2.RunInstances(&ec2.RunInstances{
@@ -484,8 +483,8 @@
484 c.Assert(err, IsNil)483 c.Assert(err, IsNil)
485 g[i] = resp.SecurityGroup484 g[i] = resp.SecurityGroup
486 c.Logf("group %d: %v", i, g[i])485 c.Logf("group %d: %v", i, g[i])
487 defer s.ec2.DeleteSecurityGroup(g[i])
488 }486 }
487 defer s.deleteGroups(c, g)
489488
490 perms := [][]ec2.IPPerm{489 perms := [][]ec2.IPPerm{
491 {{490 {{
@@ -621,3 +620,43 @@
621 }620 }
622 }621 }
623}622}
623
624// deleteGroups ensures the given groups are deleted, by retrying
625// until a timeout or all groups cannot be found anymore.
626// This should be used to make sure tests leave no groups around.
627func (s *ServerTests) deleteGroups(c *C, groups []ec2.SecurityGroup) {
628 testAttempt := aws.AttemptStrategy{
629 Total: 2 * time.Minute,
630 Delay: 5 * time.Second,
631 }
632 for a := testAttempt.Start(); a.Next(); {
633 deleted := 0
634 c.Logf("deleting groups %v", groups)
635 for _, group := range groups {
636 _, err := s.ec2.DeleteSecurityGroup(group)
637 if err == nil || s.errorCode(err) == "InvalidGroup.NotFound" {
638 c.Logf("group %v deleted", group)
639 deleted++
640 continue
641 }
642 if err != nil {
643 c.Logf("retrying; DeleteSecurityGroup returned: %v", err)
644 }
645 }
646 if deleted == len(groups) {
647 c.Logf("all groups deleted")
648 return
649 }
650 }
651 c.Fatalf("timeout while waiting %v groups to get deleted!", groups)
652}
653
654// errorCode returns the code of the given error, assuming it's not
655// nil and it's an instance of *ec2.Error. It returns an empty string
656// otherwise.
657func (s *ServerTests) errorCode(err error) string {
658 if err, _ := err.(*ec2.Error); err != nil {
659 return err.Code
660 }
661 return ""
662}
624663
=== modified file 'ec2/ec2test/server.go'
--- ec2/ec2test/server.go 2013-12-18 14:22:35 +0000
+++ ec2/ec2test/server.go 2014-02-07 12:35:18 +0000
@@ -50,10 +50,13 @@
50 instances map[string]*Instance // id -> instance50 instances map[string]*Instance // id -> instance
51 reservations map[string]*reservation // id -> reservation51 reservations map[string]*reservation // id -> reservation
52 groups map[string]*securityGroup // id -> group52 groups map[string]*securityGroup // id -> group
53 vpcs map[string]*vpc // id -> vpc
53 maxId counter54 maxId counter
54 reqId counter55 reqId counter
55 reservationId counter56 reservationId counter
56 groupId counter57 groupId counter
58 vpcId counter
59 dhcpOptsId counter
57 initialInstanceState ec2.InstanceState60 initialInstanceState ec2.InstanceState
58}61}
5962
@@ -191,6 +194,24 @@
191 return194 return
192}195}
193196
197type vpc struct {
198 ec2.VPC
199}
200
201func (v *vpc) matchAttr(attr, value string) (ok bool, err error) {
202 switch attr {
203 case "cidr":
204 return v.CIDRBlock == value, nil
205 case "state":
206 return v.State == value, nil
207 case "vpc-id":
208 return v.Id == value, nil
209 case "tag", "tag-key", "tag-value", "dhcp-options-id", "isDefault":
210 return false, fmt.Errorf("%q filter is not implemented", attr)
211 }
212 return false, fmt.Errorf("unknown attribute %q", attr)
213}
214
194var actions = map[string]func(*Server, http.ResponseWriter, *http.Request, string) interface{}{215var actions = map[string]func(*Server, http.ResponseWriter, *http.Request, string) interface{}{
195 "RunInstances": (*Server).runInstances,216 "RunInstances": (*Server).runInstances,
196 "TerminateInstances": (*Server).terminateInstances,217 "TerminateInstances": (*Server).terminateInstances,
@@ -200,6 +221,9 @@
200 "DeleteSecurityGroup": (*Server).deleteSecurityGroup,221 "DeleteSecurityGroup": (*Server).deleteSecurityGroup,
201 "AuthorizeSecurityGroupIngress": (*Server).authorizeSecurityGroupIngress,222 "AuthorizeSecurityGroupIngress": (*Server).authorizeSecurityGroupIngress,
202 "RevokeSecurityGroupIngress": (*Server).revokeSecurityGroupIngress,223 "RevokeSecurityGroupIngress": (*Server).revokeSecurityGroupIngress,
224 "CreateVpc": (*Server).createVpc,
225 "DeleteVpc": (*Server).deleteVpc,
226 "DescribeVpcs": (*Server).describeVpcs,
203}227}
204228
205const ownerId = "9876"229const ownerId = "9876"
@@ -220,6 +244,7 @@
220 srv := &Server{244 srv := &Server{
221 instances: make(map[string]*Instance),245 instances: make(map[string]*Instance),
222 groups: make(map[string]*securityGroup),246 groups: make(map[string]*securityGroup),
247 vpcs: make(map[string]*vpc),
223 reservations: make(map[string]*reservation),248 reservations: make(map[string]*reservation),
224 initialInstanceState: Pending,249 initialInstanceState: Pending,
225 }250 }
@@ -798,9 +823,11 @@
798 }823 }
799}824}
800825
801var secGroupPat = regexp.MustCompile(`^sg-[a-z0-9]+$`)826var (
802var ipPat = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+$`)827 secGroupPat = regexp.MustCompile(`^sg-[a-z0-9]+$`)
803var ownerIdPat = regexp.MustCompile(`^[0-9]+$`)828 cidrIpPat = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/([0-9]+)$`)
829 ownerIdPat = regexp.MustCompile(`^[0-9]+$`)
830)
804831
805// parsePerms returns a slice of permKey values extracted832// parsePerms returns a slice of permKey values extracted
806// from the permission fields in req.833// from the permission fields in req.
@@ -884,7 +911,7 @@
884 }911 }
885 switch rest {912 switch rest {
886 case "CidrIp":913 case "CidrIp":
887 if !ipPat.MatchString(val) {914 if !cidrIpPat.MatchString(val) {
888 fatalf(400, "InvalidPermission.Malformed", "Invalid IP range: %q", val)915 fatalf(400, "InvalidPermission.Malformed", "Invalid IP range: %q", val)
889 }916 }
890 ec2p.SourceIPs = append(ec2p.SourceIPs, val)917 ec2p.SourceIPs = append(ec2p.SourceIPs, val)
@@ -978,6 +1005,61 @@
978 }1005 }
979}1006}
9801007
1008func (srv *Server) createVpc(w http.ResponseWriter, req *http.Request, reqId string) interface{} {
1009 cidrBlock := parseCidr(req.Form.Get("CidrBlock"))
1010 tenancy := req.Form.Get("InstanceTenancy")
1011 if tenancy == "" {
1012 tenancy = ec2.DefaultTenancy
1013 }
1014
1015 srv.mu.Lock()
1016 defer srv.mu.Unlock()
1017 v := &vpc{ec2.VPC{
1018 Id: fmt.Sprintf("vpc-%d", srv.vpcId.next()),
1019 State: ec2.AvailableState,
1020 CIDRBlock: cidrBlock,
1021 DHCPOptionsId: fmt.Sprintf("dopt-%d", srv.dhcpOptsId.next()),
1022 InstanceTenancy: tenancy,
1023 }}
1024 srv.vpcs[v.Id] = v
1025 r := &ec2.CreateVPCResp{
1026 RequestId: reqId,
1027 VPC: v.VPC,
1028 }
1029 return r
1030}
1031
1032func (srv *Server) deleteVpc(w http.ResponseWriter, req *http.Request, reqId string) interface{} {
1033 v := srv.vpc(req.Form.Get("VpcId"))
1034 srv.mu.Lock()
1035 defer srv.mu.Unlock()
1036
1037 delete(srv.vpcs, v.Id)
1038 return &ec2.SimpleResp{
1039 XMLName: xml.Name{"", "DeleteVpcResponse"},
1040 RequestId: reqId,
1041 }
1042}
1043
1044func (srv *Server) describeVpcs(w http.ResponseWriter, req *http.Request, reqId string) interface{} {
1045 srv.mu.Lock()
1046 defer srv.mu.Unlock()
1047
1048 idMap := collectIds(req.Form, "VpcId.")
1049 f := newFilter(req.Form)
1050 var resp ec2.VPCsResp
1051 resp.RequestId = reqId
1052 for _, v := range srv.vpcs {
1053 ok, err := f.ok(v)
1054 if ok && (len(idMap) == 0 || idMap[v.Id]) {
1055 resp.VPCs = append(resp.VPCs, v.VPC)
1056 } else if err != nil {
1057 fatalf(400, "InvalidParameterValue", "describe VPCs: %v", err)
1058 }
1059 }
1060 return &resp
1061}
1062
981func (r *reservation) hasRunningMachine() bool {1063func (r *reservation) hasRunningMachine() bool {
982 for _, inst := range r.instances {1064 for _, inst := range r.instances {
983 if inst.state.Code != ShuttingDown.Code && inst.state.Code != Terminated.Code {1065 if inst.state.Code != ShuttingDown.Code && inst.state.Code != Terminated.Code {
@@ -987,6 +1069,41 @@
987 return false1069 return false
988}1070}
9891071
1072func parseCidr(val string) string {
1073 if val == "" {
1074 fatalf(400, "MissingParameter", "missing cidrBlock")
1075 }
1076 if _, _, err := net.ParseCIDR(val); err != nil {
1077 fatalf(400, "InvalidParameterValue", "bad CIDR %q: %v", val, err)
1078 }
1079 return val
1080}
1081
1082func (srv *Server) vpc(id string) *vpc {
1083 if id == "" {
1084 fatalf(400, "MissingParameter", "missing vpcId")
1085 }
1086 srv.mu.Lock()
1087 defer srv.mu.Unlock()
1088 v, found := srv.vpcs[id]
1089 if !found {
1090 fatalf(400, "InvalidVpcID.NotFound", "VPC %s not found", id)
1091 }
1092 return v
1093}
1094
1095// collectIds takes all values with the given prefix from form and
1096// returns a map with the ids as keys.
1097func collectIds(form url.Values, prefix string) map[string]bool {
1098 idMap := make(map[string]bool)
1099 for name, vals := range form {
1100 if strings.HasPrefix(name, prefix) {
1101 idMap[vals[0]] = true
1102 }
1103 }
1104 return idMap
1105}
1106
990type counter int1107type counter int
9911108
992func (c *counter) next() (i int) {1109func (c *counter) next() (i int) {
9931110
=== modified file 'ec2/responses_test.go'
--- ec2/responses_test.go 2013-10-02 19:10:36 +0000
+++ ec2/responses_test.go 2014-02-07 12:35:18 +0000
@@ -584,7 +584,48 @@
584// http://goo.gl/baoUf584// http://goo.gl/baoUf
585var RebootInstancesExample = `585var RebootInstancesExample = `
586<RebootInstancesResponse xmlns="http://ec2.amazonaws.com/doc/2011-12-15/">586<RebootInstancesResponse xmlns="http://ec2.amazonaws.com/doc/2011-12-15/">
587 <requestId>59dbff89-35bd-4eac-99ed-be587EXAMPLE</requestId> 587 <requestId>59dbff89-35bd-4eac-99ed-be587EXAMPLE</requestId>
588 <return>true</return>588 <return>true</return>
589</RebootInstancesResponse>589</RebootInstancesResponse>
590`590`
591
592// http://goo.gl/nkwjv
593var CreateVpcExample = `
594<CreateVpcResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
595 <requestId>7a62c49f-347e-4fc4-9331-6e8eEXAMPLE</requestId>
596 <vpc>
597 <vpcId>vpc-1a2b3c4d</vpcId>
598 <state>pending</state>
599 <cidrBlock>10.0.0.0/16</cidrBlock>
600 <dhcpOptionsId>dopt-1a2b3c4d2</dhcpOptionsId>
601 <instanceTenancy>default</instanceTenancy>
602 <tagSet/>
603 </vpc>
604</CreateVpcResponse>
605`
606
607// http://goo.gl/bcxtbf
608var DeleteVpcExample = `
609<DeleteVpcResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
610 <requestId>7a62c49f-347e-4fc4-9331-6e8eEXAMPLE</requestId>
611 <return>true</return>
612</DeleteVpcResponse>
613`
614
615// http://goo.gl/Y5kHqG
616var DescribeVpcsExample = `
617<DescribeVpcsResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
618 <requestId>7a62c49f-347e-4fc4-9331-6e8eEXAMPLE</requestId>
619 <vpcSet>
620 <item>
621 <vpcId>vpc-1a2b3c4d</vpcId>
622 <state>available</state>
623 <cidrBlock>10.0.0.0/23</cidrBlock>
624 <dhcpOptionsId>dopt-7a8b9c2d</dhcpOptionsId>
625 <instanceTenancy>default</instanceTenancy>
626 <isDefault>false</isDefault>
627 <tagSet/>
628 </item>
629 </vpcSet>
630</DescribeVpcsResponse>
631`
591632
=== added file 'ec2/vpc.go'
--- ec2/vpc.go 1970-01-01 00:00:00 +0000
+++ ec2/vpc.go 2014-02-07 12:35:18 +0000
@@ -0,0 +1,120 @@
1//
2// goamz - Go packages to interact with the Amazon Web Services.
3//
4// https://wiki.ubuntu.com/goamz
5//
6// Copyright (c) 2014 Canonical Ltd.
7//
8// Written by Gustavo Niemeyer <gustavo.niemeyer@canonical.com>
9//
10
11package ec2
12
13import (
14 "strconv"
15)
16
17const (
18 // Supported values for VPC state.
19 PendingState = "pending"
20 AvailableState = "available"
21
22 // Supported values for InstanceTenancy.
23 DefaultTenancy = "default"
24 DedicatedTenancy = "dedicated"
25
26 // AWS API version used for VPC-related calls.
27 vpcAPIVersion = "2013-10-15"
28)
29
30// VPC describes an Amazon Virtual Private Cloud (VPC).
31//
32// See http://goo.gl/Uy6ZLL for more details.
33type VPC struct {
34 Id string `xml:"vpcId"`
35 State string `xml:"state"`
36 CIDRBlock string `xml:"cidrBlock"`
37 DHCPOptionsId string `xml:"dhcpOptionsId"`
38 Tags []Tag `xml:"tagSet>item"`
39 InstanceTenancy string `xml:"instanceTenancy"`
40 IsDefault bool `xml:"isDefault"`
41}
42
43// CreateVPCResp is the response to a CreateVPC request.
44//
45// See http://goo.gl/nkwjvN for more details.
46type CreateVPCResp struct {
47 RequestId string `xml:"requestId"`
48 VPC VPC `xml:"vpc"`
49}
50
51// CreateVPC creates a VPC with the specified CIDR block.
52//
53// The smallest VPC that can be created uses a /28 netmask (16 IP
54// addresses), and the largest uses a /16 netmask (65,536 IP
55// addresses).
56//
57// The instanceTenancy value holds the tenancy options for instances
58// launched into the VPC - either DefaultTenancy or DedicatedTenancy.
59//
60// See http://goo.gl/nkwjvN for more details.
61func (ec2 *EC2) CreateVPC(CIDRBlock, instanceTenancy string) (resp *CreateVPCResp, err error) {
62 params := makeParamsVPC("CreateVpc")
63 params["CidrBlock"] = CIDRBlock
64 if instanceTenancy == "" {
65 instanceTenancy = DefaultTenancy
66 }
67 params["InstanceTenancy"] = instanceTenancy
68 resp = &CreateVPCResp{}
69 err = ec2.query(params, resp)
70 if err != nil {
71 return nil, err
72 }
73 return resp, nil
74}
75
76// DeleteVPC deletes the VPC with the specified id. All gateways and
77// resources that are associated with the VPC must have been
78// previously deleted, including instances running in the VPC, and
79// non-default security groups and route tables associated with the
80// VPC.
81//
82// See http://goo.gl/bcxtbf for more details.
83func (ec2 *EC2) DeleteVPC(id string) (resp *SimpleResp, err error) {
84 params := makeParamsVPC("DeleteVpc")
85 params["VpcId"] = id
86 resp = &SimpleResp{}
87 err = ec2.query(params, resp)
88 if err != nil {
89 return nil, err
90 }
91 return resp, nil
92}
93
94// VPCsResp is the response to a VPCs request.
95//
96// See http://goo.gl/Y5kHqG for more details.
97type VPCsResp struct {
98 RequestId string `xml:"requestId"`
99 VPCs []VPC `xml:"vpcSet>item"`
100}
101
102// VPCs describes one or more VPCs. Both parameters are optional, and
103// if specified will limit the returned VPCs to the matching ids or
104// filtering rules.
105//
106// See http://goo.gl/Y5kHqG for more details.
107func (ec2 *EC2) VPCs(ids []string, filter *Filter) (resp *VPCsResp, err error) {
108 params := makeParamsVPC("DescribeVpcs")
109 for i, id := range ids {
110 params["VpcId."+strconv.Itoa(i+1)] = id
111 }
112 filter.addParams(params)
113
114 resp = &VPCsResp{}
115 err = ec2.query(params, resp)
116 if err != nil {
117 return nil, err
118 }
119 return resp, nil
120}
0121
=== added file 'ec2/vpc_test.go'
--- ec2/vpc_test.go 1970-01-01 00:00:00 +0000
+++ ec2/vpc_test.go 2014-02-07 12:35:18 +0000
@@ -0,0 +1,172 @@
1//
2// goamz - Go packages to interact with the Amazon Web Services.
3//
4// https://wiki.ubuntu.com/goamz
5//
6// Copyright (c) 2014 Canonical Ltd.
7//
8// Written by Gustavo Niemeyer <gustavo.niemeyer@canonical.com>
9//
10
11package ec2_test
12
13import (
14 "launchpad.net/goamz/aws"
15 "launchpad.net/goamz/ec2"
16 . "launchpad.net/gocheck"
17 "time"
18)
19
20// VPC tests with example responses
21
22func (s *S) TestCreateVPCExample(c *C) {
23 testServer.Response(200, nil, CreateVpcExample)
24
25 resp, err := s.ec2.CreateVPC("10.0.0.0/16", ec2.DefaultTenancy)
26 req := testServer.WaitRequest()
27
28 c.Assert(req.Form["Action"], DeepEquals, []string{"CreateVpc"})
29 c.Assert(req.Form["CidrBlock"], DeepEquals, []string{"10.0.0.0/16"})
30 c.Assert(req.Form["InstanceTenancy"], DeepEquals, []string{ec2.DefaultTenancy})
31
32 c.Assert(err, IsNil)
33 c.Assert(resp.RequestId, Equals, "7a62c49f-347e-4fc4-9331-6e8eEXAMPLE")
34 vpc := resp.VPC
35 c.Check(vpc.Id, Equals, "vpc-1a2b3c4d")
36 c.Check(vpc.State, Equals, ec2.PendingState)
37 c.Check(vpc.CIDRBlock, Equals, "10.0.0.0/16")
38 c.Check(vpc.DHCPOptionsId, Equals, "dopt-1a2b3c4d2")
39 c.Check(vpc.Tags, HasLen, 0)
40 c.Check(vpc.IsDefault, Equals, false)
41 c.Check(vpc.InstanceTenancy, Equals, ec2.DefaultTenancy)
42}
43
44func (s *S) TestDeleteVPCExample(c *C) {
45 testServer.Response(200, nil, DeleteVpcExample)
46
47 resp, err := s.ec2.DeleteVPC("vpc-id")
48 req := testServer.WaitRequest()
49
50 c.Assert(req.Form["Action"], DeepEquals, []string{"DeleteVpc"})
51 c.Assert(req.Form["VpcId"], DeepEquals, []string{"vpc-id"})
52
53 c.Assert(err, IsNil)
54 c.Assert(resp.RequestId, Equals, "7a62c49f-347e-4fc4-9331-6e8eEXAMPLE")
55}
56
57func (s *S) TestVPCsExample(c *C) {
58 testServer.Response(200, nil, DescribeVpcsExample)
59
60 resp, err := s.ec2.VPCs([]string{"vpc-1a2b3c4d"}, nil)
61 req := testServer.WaitRequest()
62
63 c.Assert(req.Form["Action"], DeepEquals, []string{"DescribeVpcs"})
64 c.Assert(req.Form["VpcId.1"], DeepEquals, []string{"vpc-1a2b3c4d"})
65
66 c.Assert(err, IsNil)
67 c.Assert(resp.RequestId, Equals, "7a62c49f-347e-4fc4-9331-6e8eEXAMPLE")
68 c.Assert(resp.VPCs, HasLen, 1)
69 vpc := resp.VPCs[0]
70 c.Check(vpc.Id, Equals, "vpc-1a2b3c4d")
71 c.Check(vpc.State, Equals, ec2.AvailableState)
72 c.Check(vpc.CIDRBlock, Equals, "10.0.0.0/23")
73 c.Check(vpc.DHCPOptionsId, Equals, "dopt-7a8b9c2d")
74 c.Check(vpc.Tags, HasLen, 0)
75 c.Check(vpc.IsDefault, Equals, false)
76 c.Check(vpc.InstanceTenancy, Equals, ec2.DefaultTenancy)
77}
78
79// VPC tests to run against either a local test server or live on EC2.
80
81func (s *ServerTests) TestVPCs(c *C) {
82 resp1, err := s.ec2.CreateVPC("10.0.0.0/16", "")
83 c.Assert(err, IsNil)
84 assertVPC(c, resp1.VPC, "", "10.0.0.0/16")
85 id1 := resp1.VPC.Id
86
87 resp2, err := s.ec2.CreateVPC("10.1.0.0/16", ec2.DefaultTenancy)
88 c.Assert(err, IsNil)
89 assertVPC(c, resp2.VPC, "", "10.1.0.0/16")
90 id2 := resp2.VPC.Id
91
92 // We only check for the VPCs we just created, because the user
93 // might have others in his account (when testing against the EC2
94 // servers). In some cases it takes a short while until both VPCs
95 // are created, so we need to retry a few times to make sure.
96 var list *ec2.VPCsResp
97 done := false
98 testAttempt := aws.AttemptStrategy{
99 Total: 2 * time.Minute,
100 Delay: 5 * time.Second,
101 }
102 for a := testAttempt.Start(); a.Next(); {
103 c.Logf("waiting for %v to be created", []string{id1, id2})
104 list, err = s.ec2.VPCs(nil, nil)
105 if err != nil {
106 c.Logf("retrying; VPCs returned: %v", err)
107 continue
108 }
109 found := 0
110 for _, vpc := range list.VPCs {
111 c.Logf("found VPC %v", vpc)
112 switch vpc.Id {
113 case id1:
114 assertVPC(c, vpc, id1, resp1.VPC.CIDRBlock)
115 found++
116 case id2:
117 assertVPC(c, vpc, id2, resp2.VPC.CIDRBlock)
118 found++
119 }
120 if found == 2 {
121 done = true
122 break
123 }
124 }
125 if done {
126 c.Logf("all VPCs were created")
127 break
128 }
129 }
130 if !done {
131 c.Fatalf("timeout while waiting for VPCs %v", []string{id1, id2})
132 }
133
134 list, err = s.ec2.VPCs([]string{id1}, nil)
135 c.Assert(err, IsNil)
136 c.Assert(list.VPCs, HasLen, 1)
137 assertVPC(c, list.VPCs[0], id1, resp1.VPC.CIDRBlock)
138
139 f := ec2.NewFilter()
140 f.Add("cidr", resp2.VPC.CIDRBlock)
141 list, err = s.ec2.VPCs(nil, f)
142 c.Assert(err, IsNil)
143 c.Assert(list.VPCs, HasLen, 1)
144 assertVPC(c, list.VPCs[0], id2, resp2.VPC.CIDRBlock)
145
146 _, err = s.ec2.DeleteVPC(id1)
147 c.Assert(err, IsNil)
148 _, err = s.ec2.DeleteVPC(id2)
149 c.Assert(err, IsNil)
150}
151
152func assertVPC(c *C, obtained ec2.VPC, expectId, expectCidr string) {
153 if expectId != "" {
154 c.Check(obtained.Id, Equals, expectId)
155 } else {
156 c.Check(obtained.Id, Matches, `^vpc-[0-9a-f]+$`)
157 }
158 c.Check(obtained.State, Matches, "("+ec2.AvailableState+"|"+ec2.PendingState+")")
159 if expectCidr != "" {
160 c.Check(obtained.CIDRBlock, Equals, expectCidr)
161 } else {
162 c.Check(obtained.CIDRBlock, Matches, `^\d+\.\d+\.\d+\.\d+/\d+$`)
163 }
164 c.Check(obtained.DHCPOptionsId, Matches, `^dopt-[0-9a-f]+$`)
165 c.Check(obtained.IsDefault, Equals, false)
166 c.Check(obtained.Tags, HasLen, 0)
167 c.Check(
168 obtained.InstanceTenancy,
169 Matches,
170 "("+ec2.DefaultTenancy+"|"+ec2.DedicatedTenancy+")",
171 )
172}

Subscribers

People subscribed via source and target branches