Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Take account of provider specific config when creating models and bootstrapping #6079
Conversation
axw
reviewed
Aug 24, 2016
| @@ -372,14 +373,15 @@ func (c *bootstrapCommand) Run(ctx *cmd.Context) (resultErr error) { | ||
| return errors.Trace(err) | ||
| } | ||
| + provider, err := environs.Provider(cloud.Type) | ||
| + if err != nil { | ||
| + logger.Infof(err.Error()) |
axw
reviewed
Aug 24, 2016
| + } | ||
| + } | ||
| + fields := schema.FieldMap(ps.ProviderConfig(), ps.ProviderConfigDefaults()) | ||
| + if coercedAtts, err := fields.Coerce(providerAttrs, nil); err != nil { |
axw
reviewed
Aug 24, 2016
| @@ -42,3 +46,15 @@ func (c ConfigValues) AllAttrs() map[string]interface{} { | ||
| } | ||
| return result | ||
| } | ||
| + | ||
| +// ConfigSource instances provide information on config attributes |
axw
Aug 24, 2016
Member
These names are a bit off. Suggestions:
ConfigSource -> ConfigSchemaSource
ProviderConfig -> ConfigSchema
ProviderConfigDefaults -> ConfigDefaults
axw
reviewed
Aug 24, 2016
| + return nil, errors.Trace(err) | ||
| + } | ||
| + fields := schema.FieldMap(providerDefaults.ProviderConfig(), providerDefaults.ProviderConfigDefaults()) | ||
| + if coercedAtts, err := fields.Coerce(defaults, nil); err != nil { |
|
Please add TODO(wallyworld) to have environs.EnvironProvider embed config.ConfigSource (sic), making it required, and drop the interface assertions. |
|
LGTM with a few niggles |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Generating tarball failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
wallyworld commentedAug 24, 2016
•
Edited 1 time
-
wallyworld
Aug 24, 2016
Fixes: https://bugs.launchpad.net/juju-core/+bug/1615552
Providers can define their own bespoke config in the environprovider. This is separate to the generic environs/config applicable to all providers. We need to account for this config when bootstrapping (apply the config to the model) and also when figuring out model config source ("default", "controller" etc).
Add code to bootstrap and also model config handling in state. A new state policy method is required to allow state to access the provider.
QA
Hack LXD provider to have config that doesn't do anything.
Bootstrap LXD, using --config to set one of the hacked values
$ juju model-config
$ juju model-defaults show the expected values
$ juju set-model-config foo=bar
(where foo is one of the provider attrs, no warning printed like we used to see)
$ juju model-config
shows the correct values
$ juju unset-model-config foo
resets the value to the provider value