2.2 bootstrap to subnet 1659640 #6907

Merged
merged 7 commits into from Feb 22, 2017

Conversation

Projects
None yet
6 participants
Owner

jameinel commented Feb 2, 2017

Description of change

This implements support for "subnet=" as a placement directive on AWS. This allows you to do "juju bootstrap --to subnet=A.B.C.D/24" and have Juju match that to a specific subnet to provision a machine in. It works for bootstrap, and I believe it will also work for any instance that you provision thereafter. Since Subnet in AWS also constrains you to an Availability Zone it also has that effect.

Note, this doesn't have Unit tests yet, as it was mostly meant as an exploration as to what was possible, and it turned out to be quite easy to just implement. I'd like to add tests before we would actually land this.

QA steps

Create a bunch of subnets in your AWS VPC. Bootstrap with

  juju bootstrap aws/REGION --config vpc-id=VPC --to subnet=SUBNET

You can use --debug to see some of the details about the selection process. Or just note in your EC2 dashboard what subnet the resulting instance ends up in.

SUBNET can be a Name, subnet-id, or CIDR.

I haven't tested exactly what happens with "juju deploy --to" or "juju add-unit --to", though both should use the same logic (and it essentially just piggybacks on all our existing code to handle AvailabilityZones.)

Documentation changes

This would affect what a user can specify during "juju bootstrap", and if we have explicit documentation about using Spaces in AWS, we should definitely include this information.

Bug reference

lp:#1659640

This is beautiful! Thanks!

Member

anastasiamac commented Feb 7, 2017

!!build!!!

I am really weary of code changes that have no tests added/modified.

Code by itself LGTM.
@nskaggs Considering the complexities involved, would be nice to have a set of functional tests for this...

Owner

jameinel commented Feb 7, 2017

This was mainly a prototype to have us discuss whether this is a route to go. We definitely will want direct unit tests for the parsing function and functional tests that we can use this feature in various clouds. (Ideally it would not be AWS specific, though it is today.)

babbageclunk and others added some commits Feb 9, 2017

Call AdoptResources from the migrationmaster worker
It's done in the success phase after all of the minions have reported
back.
Merge pull request #6952 from babbageclunk/mm-adopt-resources-worker
migrations: Call AdoptResources from migrationmaster worker

## Description of change
Once the migration is successful we call AdoptResources on the target controller to mark all of the resources used by the model as managed by the target controller. This prevents the source controller from incorrectly cleaning those resources up if it's destroyed.

Which resources are transferred depends on the cloud hosting the model, but it generally includes things like instances, volumes, and security groups.

## QA steps
(Should be done for all clouds, but especially ec2, gce, azure, rackspace, maas, lxd and openstack, since they handle resource tagging and cleanup when a controller is destroyed.)
* bootstrap two controllers, A and B
* add a model on A
* deploy an application the model
* migrate the model to B
* once the migration is complete and the model is running on B, destroy controller A
* the model should continue running on B

## Bug reference
Fixes https://bugs.launchpad.net/juju/+bug/1648063
Implement 'subnet' as a placement directive for AWS.
Allow searching by subnet Name or by subnet ID as well as CIDR.
Still needs unit tests, etc, but manual testing shows it to work
well.

@jameinel jameinel changed the title from RFC: 2.1 bootstrap to subnet 1659640 to 2.1 bootstrap to subnet 1659640 Feb 9, 2017

Owner

jameinel commented Feb 9, 2017

Refactored the code and added a bunch of unit tests.

@jameinel jameinel changed the base branch from 2.1 to develop Feb 9, 2017

Owner

jameinel commented Feb 9, 2017

While I think the code is sane and reasonably tested, I think its a bit late to actually land in 2.1. So I'll target 2.2 for this.

@jameinel jameinel changed the title from 2.1 bootstrap to subnet 1659640 to 2.2 bootstrap to subnet 1659640 Feb 9, 2017

Owner

nskaggs commented Feb 9, 2017

2.1.1? I'm not inherently opposed if it bridges the gap, but it seems like it's specific to AWS?

Owner

jameinel commented Feb 10, 2017

It is very specific to AWS, it will take work to implement it elsewhere, as GCE/Azure doesn't have space support, and the specific changes for MAAS are also probably going to need MAAS changes (we make a request by name-of-space, not look up the subnets ourselves).

Owner

jameinel commented Feb 22, 2017

$$merge$$

Contributor

jujubot commented Feb 22, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Feb 22, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10336

Owner

jameinel commented Feb 22, 2017

$$merge$$ there was a test that needed tweaking that I had missed.

Contributor

jujubot commented Feb 22, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 2fb7d27 into juju:develop Feb 22, 2017

@jameinel jameinel deleted the jameinel:2.1-bootstrap-subnet-1659640 branch Apr 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment