Code review comment for lp:~gz/juju-core/secgroup_vpc

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

Reviewers: mp+188958_code.launchpad.net,

Message:
Please take a look.

Description:
provider/ec2: Use security group ids in EC2 API

The additional options available with VPC security groups
comes with the limitation that they must be referred to by
id rather than by name. To support the otherwise mostly
transparent default VPC mode some new accounts use, switch
our provider infrastructure to lookup and use ids.

This adds some additional overhead in ec2 api roundtrips,
which would be avoidable in the ec2-classic case by
detecting if the account is default-vpc enabled using
DescribeAccountAttributes and continuing to just use the
names if not. For now, that is punted on that in favour
of verifying the new codepath works in all cases.

There are a couple of work items pending for this branch.

I have run and confirmed that a subset of the live tests,
including those most related to security group handling,
pass on both classic and default vpc regions. However, a
clean run of the whole live suite on trunk is eluding me.

Two small updates to goamz are required for the local live
suite to pass. These are both approved, but need to land,
and dependencies.csv updated.

https://code.launchpad.net/~gz/juju-core/secgroup_vpc/+merge/188958

(do not edit description out of merge proposal)

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

Affected files (+124, -53 lines):
   A [revision details]
   M provider/ec2/ec2.go
   M provider/ec2/live_test.go

« Back to merge proposal