Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

ec2: check subnet exists in vpc #664

Merged
merged 3 commits into from Mar 2, 2015

Conversation

ehazlett
Copy link
Contributor

Fixes #663

This also fixes the IP allocation eventual consistency.

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@@ -250,7 +250,7 @@ func (d *Driver) Create() error {
}

for _, s := range subnets {
if s.AvailabilityZone == regionZone {
if s.AvailabilityZone == regionZone && s.VpcId == d.VpcId {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both availability zone and VPC id can be passed as Filter.N params in the call to DescribeSubnets.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might also be reasonable to see if one of the subnets is the default for the AZ and prefer that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to both :) - i wanted to try not to do too much in our ec2 implementation as eventually we will be moving to the official sdk but i think this works better for this case. thanks!

@iffy
Copy link

iffy commented Feb 27, 2015

This fixes my problem from #663 👍

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@ehazlett
Copy link
Contributor Author

@md5 PTAL

@ehazlett
Copy link
Contributor Author

@md5 it should "fallback" safely. It will only check for the default if there is more than one and will only set it if there is one marked. Otherwise, it uses the first one it finds.

@ehazlett
Copy link
Contributor Author

@md5 i'm not sure what you mean by log the err -- if an error occurs while waiting on the IP, it's returned.

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@md5
Copy link

md5 commented Feb 27, 2015 via email

@ehazlett
Copy link
Contributor Author

Correct. In the case where one is marked as default it will use that one. Otherwise it will just use the one already set.

We need a retry and timeout function in the amz lib itself. I'll look into this for better retry and timeout support. Thanks!

@ehazlett
Copy link
Contributor Author

Ah gotcha. Good point.

@md5
Copy link

md5 commented Feb 28, 2015

LGTM

I had missed the d.SubnetId = subnets[0].SubnetId in my first pass through the diff, so I see how it still gets a value for SubnetId if there is no defaultForAz.

ehazlett added a commit that referenced this pull request Mar 2, 2015
@ehazlett ehazlett merged commit c3878a9 into docker:master Mar 2, 2015
@ehazlett ehazlett deleted the ec2-fix-subnet-check branch March 2, 2015 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EC2: Seems like same VPC isn't used for subnet and security groups
3 participants