Remove State.GetModel()

[mjs]

Description of change

These changes remove the GetModel method from state.State. This gets rid of the last remaining problematic way for state.Model instances to be created. Model instances can now only be created using State.Model() which means the State a Model references is now guaranteed to be connected to the same model UUID as the Model. This paves the way for moving more functionality from State onto Model and is a prerequisite for upcoming feature work.

A follow-up PR will clean up the internals of state.Model, taking advantage of the fact that we know a Model's State is now the correct one.

This change has had many positive follow-on effects. More uses of the evil State.ForModel() were removed and the code and tests for some apiserver facades is now significantly cleaner. Some minor inefficiencies have also been dealt with.

QA steps

The areas at risk of breakage as a result of this change are model/controller creation and destruction, and model migrations, so exercise those:

$ juju bootstrap lxd source
# wait...
$ juju bootstrap lxd target
# wait...
$ juju switch source
$ juju add-model foo
$ juju deploy ubuntu
# wait...

# Check for significant errors
$ juju debug-log -m source:foo--replay --no-tail -l ERROR
$ juju debug-log -m source:controller --replay --no-tail -l ERROR

# Migrate
$ juju migrate foo target

# Wait for model to appear on target
$ watch juju status -m target:foo

# Add another unit
$ juju add-unit ubuntu
# wait...

# Check for significant errors
$ juju debug-log -m source:controller --replay --no-tail -l ERROR
$ juju debug-log -m target:controller --replay --no-tail -l ERROR
$ juju debug-log -m target:foo --replay --no-tail -l ERROR

# Destroy controllers
$ juju destroy-controller -y --destroy-all-models source
$ juju destroy-controller -y --destroy-all-models target

Documentation changes

N.A. - internal only

Bug reference

N.A.

[wallyworld]

So much lovely red deletions

[wallyworld]

At this point, err could be not found. Will release() be valid then? Don't we want to check that err == nil?

[mjs]

Good point. Will fix.

[babbageclunk]

Great! V. minor nits.

[babbageclunk]

Maybe release this earlier? I guess it's just a test.

[mjs]

I guess checkEnvironmentMatches doesn't call anything on the model which relies on the DB so calling release here is safe enough... probably not worth it though.

[babbageclunk]

can't this just be return m, release, nil?

[mjs]

Nope. The release func returned by GetModel is func () bool but the Pool interface simplifies that to func()

[babbageclunk]

Same here.

[mjs]

Same as above

[mjs]

$$merge$$

[jujubot]

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