Allow for Multi-AZ Mongo Cluster in a VPC

[alexdebrie]

This pull requests allows for the creation of a Multi-AZ Mongo Cluster within a VPC. Previously, you could only spin up a Multi-AZ cluster in EC2-Classic.

Boto's VPC module doesn't have the most elegant API. For example, you can't use a VPC id to grab all subnets for that particular VPC. You need to grab all subnets for your account, then you can check the vpc_id on each subnet.

I tried to limit the damage and stay close to Tyr's style, but I'm open to suggestions. I tested both a cluster within a VPC and without, and they both worked.

[citruspi]

So while I can definitely appreciate the flexibility that dynamically populating the list of subnets brings us, I have a slight fear of this coming back to bite us. So a couple notes:

• we only use the availability zones c, d, and e
• a VPC can have multiple subnets using the same availability zone

So we would run into an issue if a new subnet was created. The new subnet might just be created for testing something or to isolate some instances in a separate subnet, but all of a sudden instances spun up with MongoCluster would automatically be placed inside the subnet.

If an experimental subnet were created for zone a or b, then what happened would be pretty apparent in the logs - you'd see an instance with a name like p-monolith-mongo-use1a-01.app.hudl.com. But what if the new subnet was created in zones c, d, or e? You'd see what looks like an expected instance name and might not realize what had happened if

• you don't spot the different subnet ID in the logs when the instance is being provisioned
• you don't spot the private IP address being placed in a new CIDR block

So basically, I'm worried about someone creating a new subnet which may not be intended for general use, MongoCluster automatically electing to place nodes there, and no one realizing what's happened.

It's not as flexible, but I think I might just lean towards hard coding it and then updating the list when we create a new subnet which is intended for use. Instead, or in addition, you could have an argument subnet_ids in addition to vpc_id so that developers can customize the subnets being used if they need to. If the value is None, use the hardcoded values, if not, use the list provided.

[jamiegs]

I agree, we'd need to be able to pass in a list of subnets

[alexdebrie]

Latest commit removes the vpc_id option and instead allows a user to pass in a comma-separated list of subnet IDs in the subnet_ids argument. Let me know if either of you would like any changes.

[citruspi]

Since we're no longer querying Amazon VPC for the subnets associated with a VPC, this import is no longer necessary.

[alexdebrie]

Fixed to have subnet_ids be a list and removed boto. Worked on a quick test run.

[citruspi]

Just a thought. The length of a list will never be negative, so the only possible case where

len(self.subnet_ids) < 1

is True is when the list is empty. It might be better to write it as

if len(self.subnet_ids) == 0

so you don't have to take the time to think about all the cases where it would be True.

[citruspi]

Aside from my comment on

len(self.subnet_ids) < 1

this looks good to me. 👍