provider/azure: create common resources once only #6826

Merged
merged 2 commits into from Jan 19, 2017

Conversation

Projects
None yet
3 participants
Member

axw commented Jan 18, 2017

Create common resources (storage account, vnet, nsg)
once only, when creating the model. For bootstrap,
we do this along with the bootstrap machine as we
already did. For non-controller models, we create
a deployment for the common resources at model
creation time. Because deployments can take a while,
and model creation is synchronous, we do the common
resource creation asynchronously and wait for its
completion before starting any machines.

There is an Environ upgrade step added that creates
a common resource deployment in non-controller
models. The upgrade step will preserve application-
specific network security rules.

Requires #6818

Fixes https://bugs.launchpad.net/juju/+bug/1656723

Member

axw commented Jan 18, 2017

Note to reviewers: please review only the second commit. The first one has its own PR (#6818)

Member

axw commented Jan 18, 2017

QA

  1. bootstrap azure with 2.0
  2. juju deploy -m default ubuntu
  3. juju run --unit ubuntu/0 'open-port 1234' && juju expose ubuntu
  4. juju add-model empty
  5. juju upgrade-juju to this branch
  6. check that port 1234 is still open in default model's resource group
  7. juju add-machine -m default; check that port 1234 is still open
  8. juju add-machine -m controller
  9. juju add-machine -m empty
+ env.mu.Lock()
+ storageAccountType := env.config.storageAccountType
+ env.mu.Unlock()
+ return env.createCommonResourceDeployment(
@wallyworld

wallyworld Jan 19, 2017

Owner

Do we need to exit early if the resource exists?
Or is the api idempotent?

@axw

axw Jan 19, 2017

Member

It will replace the existing common deployment if it exists, which is fine.

provider/azure/upgrades.go
+}
+
+func isControllerEnviron(env *azureEnviron) (bool, error) {
+ if env.envName != "controller" {
@wallyworld

wallyworld Jan 19, 2017

Owner

I realise not having this short circuit makes it slower, but is it worth the person who renames "controller" not knowing this is here. I think we have a const for controller model. Let's at least use that.

@axw

axw Jan 19, 2017

Member

Yeah, probably not worthwhile. I'll drop it.

I wonder if it's worth polling using the azure console and refresh button to ensure the common resource is finished being created before any new machine comes up. Or a unit test to that effect

axw added some commits Jan 16, 2017

provider/azure: create common resources once only
Create common resources (storage account, vnet, nsg)
once only, when creating the model. For bootstrap,
we do this along with the bootstrap machine as we
already did. For non-controller models, we create
a deployment for the common resources at model
creation time. Because deployments can take a while,
and model creation is synchronous, we do the common
resource creation asynchronously and wait for its
completion before starting any machines.
provider/azure: address review comments
- drop "controller" model name special case
- add test for retrying until common deployment
  is complete
Member

axw commented Jan 19, 2017

$$merge$$

Contributor

jujubot commented Jan 19, 2017

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

@jujubot jujubot merged commit 42c1387 into juju:develop Jan 19, 2017

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