Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Consume details for an offer now includes the connection details for the host controller #7504
Conversation
mjs
approved these changes
Jun 16, 2017
LGTM with the understanding (discussed verbally) that refreshing of macaroons and keeping controller addresses up to date still need to figured out.
One idea for helping to keep controller addresses up to date was to update externalControllers using the addresses in the login response following any login to a remote controller.
| @@ -120,6 +123,13 @@ type stateShim struct { | ||
| *state.State | ||
| } | ||
| +type ExternalController state.ExternalController |
mjs
Jun 16, 2017
Contributor
As per comment in state package, maybe the ExternalController interface should be here.
| "github.com/juju/juju/environs" | ||
| "github.com/juju/juju/instance" | ||
| "github.com/juju/juju/network" | ||
| "github.com/juju/juju/state" | ||
| statestorage "github.com/juju/juju/state/storage" | ||
| coretesting "github.com/juju/juju/testing" | ||
| + "gopkg.in/macaroon.v1" |
| +) | ||
| + | ||
| +// ExternalController represents the state of a controller hosting | ||
| +// models involved in cross-model relations. |
| +type ExternalControllers interface { | ||
| + Save(_ crossmodel.ControllerInfo, modelUUIDs ...string) (ExternalController, error) | ||
| + ControllerForModel(modelUUID string) (ExternalController, error) | ||
| +} |
mjs
Jun 16, 2017
Contributor
I haven't always been good at this myself, but shouldn't this interface live where it's used not next to the concrete implementation?
wallyworld
Jun 16, 2017
Owner
discussed on irc, will leave it here for now, because state is awesome
| +} | ||
| + | ||
| +// NewExternalControllers creates an external controllers instance backed by a state. | ||
| +func NewExternalControllers(st *State) ExternalControllers { |
| + defer closer() | ||
| + | ||
| + var doc []externalControllerDoc | ||
| + err := coll.Find(bson.D{{"models", bson.D{{"$all", []string{modelUUID}}}}}).All(&doc) |
mjs
Jun 16, 2017
Contributor
It would be more logical to use $in here. bson.M would also aid readability. Something like:
coll.Find(bson.M{"models": bson.M{"$in": []string{modelUUID}}})
|
This is WIP, behind a feature flag, so macaroon stuff not fully cooked yet. This PR is more of an infrastructure one to get the api params and other building blocks in place. |
wallyworld
added some commits
Jun 15, 2017
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
wallyworld commentedJun 15, 2017
Description of change
When consuming offers from another controller, we need to record details for how to connect to that controller in the consuming model. We also need to record against the remote application on the consuming side a macaroon that proves permission to consume; this macaroon will be used when communicating with the offering model once the relation is set up.
The various params structs have been modified to cater for passing the macaroon, but it's not used yet. Also, CMR is still for single controller, so the external controller details are not used yet either. This is all ground work for enabling multi-controller CMR.
QA steps
No visible changes yet - all infrastructure for later.
bootstrap and perform a consumer/relate.