Add a --model-default option to bootstrap. #6360

Merged
merged 2 commits into from Oct 4, 2016

Conversation

Projects
None yet
3 participants
Owner

howbazaar commented Sep 30, 2016

This addresses http://pad.lv/1628999 to allow specifying model-defaults on the command line during bootstrap. Previously, only config values specified in the cloud definition would be inherited by the new models in the controller.

Owner

howbazaar commented Sep 30, 2016

I'd like to talk to @wallyworld in particular about this change, and some QA steps coming later.

LGTM after ensuring we don't allow invalid default config to be specified

cmd/juju/commands/bootstrap.go
@@ -174,6 +175,7 @@ func (c *bootstrapCommand) SetFlags(f *gnuflag.FlagSet) {
f.StringVar(&c.AgentVersionParam, "agent-version", "", "Version of tools to use for Juju agents")
f.StringVar(&c.CredentialName, "credential", "", "Credentials to use when bootstrapping")
f.Var(&c.config, "config", "Specify a controller configuration file, or one or more configuration\n options\n (--config config.yaml [--config key=value ...])")
+ f.Var(&c.modelDefaults, "model-default", "Specify a configuration file, or one or more configuration\n options to be set for all models\n (--config config.yaml [--config key=value ...])")
@wallyworld

wallyworld Oct 4, 2016

Owner

... to be set for all models, unless otherwise specified.

@@ -536,6 +552,11 @@ func (c *bootstrapCommand) Run(ctx *cmd.Context) (resultErr error) {
}
inheritedControllerAttrs[k] = v
}
+ // Model defaults are added to the inherited controller attributes.
+ // Any command line set model defaults override what is in the cloud config.
+ for k, v := range modelDefaultConfigAttrs {
@wallyworld

wallyworld Oct 4, 2016

Owner

We should check and error if they try and specify a bootstrap or controller only attribute in the model defaults. Maybe easiest to use a switch{} inside a for{} similar to above
(and a test)

Owner

howbazaar commented Oct 4, 2016

@wallyworld updated, please approve if good.

Owner

howbazaar commented Oct 4, 2016

$$merge$$

Contributor

jujubot commented Oct 4, 2016

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

@jujubot jujubot merged commit 49e875a into juju:master Oct 4, 2016

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