pass regionSpec for regionInheritedConfig in model-cfg #6069

Merged
merged 6 commits into from Aug 25, 2016
@@ -11,6 +11,7 @@ import (
"github.com/juju/juju/apiserver/metricsender"
"github.com/juju/juju/controller"
"github.com/juju/juju/core/description"
+ "github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/state"
"github.com/juju/juju/status"
@@ -32,7 +33,7 @@ type ModelManagerBackend interface {
IsControllerAdmin(user names.UserTag) (bool, error)
NewModel(state.ModelArgs) (Model, ModelManagerBackend, error)
- ComposeNewModelConfig(modelAttr map[string]interface{}) (map[string]interface{}, error)
+ ComposeNewModelConfig(modelAttr map[string]interface{}, regionSpec *environs.RegionSpec) (map[string]interface{}, error)
ControllerModel() (Model, error)
ControllerConfig() (controller.Config, error)
ForModel(tag names.ModelTag) (ModelManagerBackend, error)
@@ -299,7 +299,7 @@ func (st *mockState) ControllerTag() names.ControllerTag {
return names.NewControllerTag(st.controllerModel.tag.Id())
}
-func (st *mockState) ComposeNewModelConfig(modelAttr map[string]interface{}) (map[string]interface{}, error) {
+func (st *mockState) ComposeNewModelConfig(modelAttr map[string]interface{}, regionSpec *environs.RegionSpec) (map[string]interface{}, error) {
st.MethodCall(st, "ComposeNewModelConfig")
attr := make(map[string]interface{})
for attrName, val := range modelAttr {
@@ -153,9 +153,12 @@ func (mm *ModelManagerAPI) newModelConfig(
if err != nil {
return nil, errors.Trace(err)
}
- if joint, err = mm.state.ComposeNewModelConfig(joint); err != nil {
+
+ regionSpec := &environs.RegionSpec{Cloud: cloudSpec.Name, Region: cloudSpec.Region}
+ if joint, err = mm.state.ComposeNewModelConfig(joint, regionSpec); err != nil {
return nil, errors.Trace(err)
}
+
creator := modelmanager.ModelConfigCreator{
Provider: environs.Provider,
FindTools: func(n version.Number) (tools.List, error) {
View
@@ -73,3 +73,15 @@ func MakeCloudSpec(cloud jujucloud.Cloud, cloudName, cloudRegionName string, cre
}
return cloudSpec, nil
}
+
+// RegionSpec contains the information needed to lookup specific region
+// configuration. This is for use in calling
+// state/modelconfig.(ComposeNewModelConfig) so there is no need to serialize
+// it.
+type RegionSpec struct {
+ // Cloud is the name of the cloud.
+ Cloud string
+
+ // Region is the name of the cloud region.
+ Region string
+}
@@ -19,6 +19,11 @@ const (
// come from those associated with the controller.
JujuControllerSource = "controller"
+ // JujuRegionSource is used to label model config attributes that come from
+ // those associated with the region where the model is
+ // running.
+ JujuRegionSource = "region"
+
// JujuModelConfigSource is used to label model config attributes that
// have been explicitly set by the user.
JujuModelConfigSource = "model"
@@ -262,7 +262,7 @@ func (r *RestoreSuite) TestNewConnection(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
defer server.DestroyWithLog()
- st := statetesting.Initialize(c, names.NewLocalUserTag("test-admin"), nil, nil, nil)
+ st := statetesting.Initialize(c, names.NewLocalUserTag("test-admin"), nil, nil, nil, nil)
c.Assert(st.Close(), jc.ErrorIsNil)
r.PatchValue(&mongoDefaultDialOpts, mongotest.DialOpts)
View
@@ -12,6 +12,7 @@ import (
"github.com/juju/juju/cloud"
"github.com/juju/juju/constraints"
"github.com/juju/juju/controller"
+ "github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/mongo/mongotest"
"github.com/juju/juju/state"
@@ -97,7 +98,7 @@ func (s *InitializeSuite) TestInitialize(c *gc.C) {
Owner: owner,
Config: cfg,
CloudName: "dummy",
- CloudRegion: "some-region",
+ CloudRegion: "dummy-region",
CloudCredential: userPassCredentialTag,
StorageProviderRegistry: storage.StaticProviderRegistry{},
},
@@ -107,7 +108,7 @@ func (s *InitializeSuite) TestInitialize(c *gc.C) {
AuthTypes: []cloud.AuthType{
cloud.EmptyAuthType, cloud.UserPassAuthType,
},
- Regions: []cloud.Region{{Name: "some-region"}},
+ Regions: []cloud.Region{{Name: "dummy-region"}},
},
CloudCredentials: cloudCredentialsIn,
MongoInfo: statetesting.NewMongoInfo(),
@@ -135,7 +136,7 @@ func (s *InitializeSuite) TestInitialize(c *gc.C) {
model, err := s.State.Model()
c.Assert(err, jc.ErrorIsNil)
c.Assert(model.Tag(), gc.Equals, modelTag)
- c.Assert(model.CloudRegion(), gc.Equals, "some-region")
+ c.Assert(model.CloudRegion(), gc.Equals, "dummy-region")
// Check that the owner has been created.
c.Assert(model.Owner(), gc.Equals, owner)
// Check that the owner can be retrieved by the tag.
@@ -331,6 +332,7 @@ func (s *InitializeSuite) testBadModelConfig(c *gc.C, update map[string]interfac
ControllerConfig: controllerCfg,
ControllerModelArgs: state.ModelArgs{
CloudName: "dummy",
+ CloudRegion: "dummy-region",
Owner: owner,
Config: bad,
StorageProviderRegistry: storage.StaticProviderRegistry{},
@@ -339,6 +341,7 @@ func (s *InitializeSuite) testBadModelConfig(c *gc.C, update map[string]interfac
Cloud: cloud.Cloud{
Type: "dummy",
AuthTypes: []cloud.AuthType{cloud.EmptyAuthType},
+ Regions: []cloud.Region{{Name: "dummy-region"}},
},
MongoInfo: statetesting.NewMongoInfo(),
MongoDialOpts: mongotest.DialOpts(),
@@ -456,3 +459,112 @@ func (s *InitializeSuite) TestInitializeWithCloudRegionConfig(c *gc.C) {
regionInheritedConfigIn[k])
}
}
+
+func (s *InitializeSuite) TestInitializeWithCloudRegionMisses(c *gc.C) {
+ cfg := testing.ModelConfig(c)
+ uuid := cfg.UUID()
+ controllerInheritedConfigIn := map[string]interface{}{
+ "no-proxy": "local",
+ }
+ // Phony region-config
+ regionInheritedConfigIn := cloud.RegionConfig{
+ "a-region": cloud.Attrs{
+ "no-proxy": "a-value",
+ },
+ "b-region": cloud.Attrs{
+ "no-proxy": "b-value",
+ },
+ }
+ owner := names.NewLocalUserTag("initialize-admin")
+ controllerCfg := testing.FakeControllerConfig()
+ controllerCfg["controller-uuid"] = uuid
+
+ st, err := state.Initialize(state.InitializeParams{
+ ControllerConfig: controllerCfg,
+ ControllerModelArgs: state.ModelArgs{
+ CloudName: "dummy",
+ Owner: owner,
+ Config: cfg,
+ StorageProviderRegistry: storage.StaticProviderRegistry{},
+ },
+ CloudName: "dummy",
+ Cloud: cloud.Cloud{
+ Type: "dummy",
+ AuthTypes: []cloud.AuthType{cloud.EmptyAuthType},
+ RegionConfig: regionInheritedConfigIn, // Init with phony region-config
+ },
+ ControllerInheritedConfig: controllerInheritedConfigIn,
+ MongoInfo: statetesting.NewMongoInfo(),
+ MongoDialOpts: mongotest.DialOpts(),
+ })
+ c.Assert(err, jc.ErrorIsNil)
+ c.Assert(st, gc.NotNil)
+ modelTag := st.ModelTag()
+ c.Assert(modelTag.Id(), gc.Equals, uuid)
+ err = st.Close()
+ c.Assert(err, jc.ErrorIsNil)
+
+ s.openState(c, modelTag)
+
+ var attrs map[string]interface{}
+ rspec := &environs.RegionSpec{Cloud: "dummy", Region: "c-region"}
+ got, err := s.State.ComposeNewModelConfig(attrs, rspec)
+ c.Check(err, jc.ErrorIsNil)
+ c.Assert(got["no-proxy"], gc.Equals, "local")
+}
+
+func (s *InitializeSuite) TestInitializeWithCloudRegionHits(c *gc.C) {
+ cfg := testing.ModelConfig(c)
+ uuid := cfg.UUID()
+
+ controllerInheritedConfigIn := map[string]interface{}{
+ "no-proxy": "local",
+ }
+ // Phony region-config
+ regionInheritedConfigIn := cloud.RegionConfig{
+ "a-region": cloud.Attrs{
+ "no-proxy": "a-value",
+ },
+ "b-region": cloud.Attrs{
+ "no-proxy": "b-value",
+ },
+ }
+ owner := names.NewLocalUserTag("initialize-admin")
+ controllerCfg := testing.FakeControllerConfig()
+ controllerCfg["controller-uuid"] = uuid
+
+ st, err := state.Initialize(state.InitializeParams{
+ ControllerConfig: controllerCfg,
+ ControllerModelArgs: state.ModelArgs{
+ CloudName: "dummy",
+ Owner: owner,
+ Config: cfg,
+ StorageProviderRegistry: storage.StaticProviderRegistry{},
+ },
+ CloudName: "dummy",
+ Cloud: cloud.Cloud{
+ Type: "dummy",
+ AuthTypes: []cloud.AuthType{cloud.EmptyAuthType},
+ RegionConfig: regionInheritedConfigIn, // Init with phony region-config
+ },
+ ControllerInheritedConfig: controllerInheritedConfigIn,
+ MongoInfo: statetesting.NewMongoInfo(),
+ MongoDialOpts: mongotest.DialOpts(),
+ })
+ c.Assert(err, jc.ErrorIsNil)
+ c.Assert(st, gc.NotNil)
+ modelTag := st.ModelTag()
+ c.Assert(modelTag.Id(), gc.Equals, uuid)
+ err = st.Close()
+ c.Assert(err, jc.ErrorIsNil)
+
+ s.openState(c, modelTag)
+
+ var attrs map[string]interface{}
+ for r := range regionInheritedConfigIn {
+ rspec := &environs.RegionSpec{Cloud: "dummy", Region: r}
+ got, err := s.State.ComposeNewModelConfig(attrs, rspec)
+ c.Check(err, jc.ErrorIsNil)
+ c.Assert(got["no-proxy"], gc.Equals, regionInheritedConfigIn[r]["no-proxy"])
+ }
+}
View
@@ -8,6 +8,7 @@ import (
"github.com/juju/schema"
"github.com/juju/juju/controller"
+ "github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
)
@@ -50,7 +51,11 @@ func checkModelConfig(cfg *config.Config) error {
// inheritedConfigAttributes returns the merged collection of inherited config
// values used as model defaults when adding models or unsetting values.
func (st *State) inheritedConfigAttributes() (map[string]interface{}, error) {
- configSources := modelConfigSources(st)
+ rspec, err := st.regionSpec()
+ if err != nil {
+ return nil, errors.Trace(err)
+ }
+ configSources := modelConfigSources(st, rspec)
values := make(attrValues)
for _, src := range configSources {
cfg, err := src.sourceFunc()
@@ -77,7 +82,11 @@ func (st *State) modelConfigValues(modelCfg attrValues) (config.ConfigValues, er
// Read all of the current inherited config values so
// we can dynamically reflect the origin of the model config.
- configSources := modelConfigSources(st)
+ rspec, err := st.regionSpec()
+ if err != nil {
+ return nil, errors.Trace(err)
+ }
+ configSources := modelConfigSources(st, rspec)
sourceNames := make([]string, 0, len(configSources))
sourceAttrs := make([]attrValues, 0, len(configSources))
for _, src := range configSources {
@@ -157,6 +166,7 @@ func (st *State) ModelConfigValues() (config.ConfigValues, error) {
// ModelConfigDefaultValues returns the default config values to be used
// when creating a new model, and the origin of those values.
+// TODO(ro) Use this for model defaults.
func (st *State) ModelConfigDefaultValues() (config.ConfigValues, error) {
return st.modelConfigValues(nil)
}
@@ -282,13 +292,12 @@ type modelConfigSource struct {
// sources, in hierarchical order. Starting from the first source,
// config is retrieved and each subsequent source adds to the
// overall config values, later values override earlier ones.
-func modelConfigSources(st *State) []modelConfigSource {
+func modelConfigSources(st *State, regionSpec *environs.RegionSpec) []modelConfigSource {
return []modelConfigSource{
{config.JujuDefaultSource, st.defaultInheritedConfig},
{config.JujuControllerSource, st.controllerInheritedConfig},
- {config.JujuControllerSource, st.regionInheritedConfig},
+ {config.JujuRegionSource, st.regionInheritedConfig(regionSpec)},
}
-
}
const (
@@ -332,27 +341,44 @@ func (st *State) controllerInheritedConfig() (attrValues, error) {
// regionInheritedConfig returns the configuration attributes for the region in
// the cloud where the model is targeted.
-func (st *State) regionInheritedConfig() (attrValues, error) {
+func (st *State) regionInheritedConfig(regionSpec *environs.RegionSpec) func() (attrValues, error) {
+ if regionSpec == nil {
+ return func() (attrValues, error) {
+ return nil, errors.New(
+ "no environs.RegionSpec provided")
+ }
+ }
+ if regionSpec.Region == "" {
+ // It is expected that not all clouds have regions. So return not found
+ // if there is not a region here.
+ return func() (attrValues, error) {
+ return nil, errors.NotFoundf("region")
+ }
+ }
+ return func() (attrValues, error) {
+ settings, err := readSettings(st,
+ globalSettingsC,
+ regionSettingsGlobalKey(regionSpec.Cloud, regionSpec.Region),
+ )
+ if err != nil {
+ return nil, errors.Trace(err)
+ }
+ return settings.Map(), nil
+ }
+}
+
+// regionSpec returns a suitable environs.RegionSpec for use in
+// regionInheritedConfig.
+func (st *State) regionSpec() (*environs.RegionSpec, error) {
model, err := st.Model()
if err != nil {
return nil, errors.Trace(err)
}
- settings, err := readSettings(st,
- globalSettingsC,
- regionSettingsGlobalKey(
- model.Cloud(),
- model.CloudRegion()),
- )
- if err != nil {
- // Some clouds have no region so we need to handle an empty region by
- // returning and handling an error or returning nil.
- // TODO(ro): http://reviews.vapour.ws/r/5454/#comment29388 check for
- // model.CloudRegion() == "" above and return a NotFound error there,
- // saving searching for non-existent settings with a partially
- // constructed key
- return nil, errors.Trace(err)
+ rspec := &environs.RegionSpec{
+ Cloud: model.Cloud(),
+ Region: model.CloudRegion(),
}
- return settings.Map(), nil
+ return rspec, nil
}
// composeModelConfigAttributes returns a set of model config settings composed from known
@@ -386,7 +412,7 @@ func composeModelConfigAttributes(
// ComposeNewModelConfig returns a complete map of config attributes suitable for
// creating a new model, by combining user specified values with system defaults.
-func (st *State) ComposeNewModelConfig(modelAttr map[string]interface{}) (map[string]interface{}, error) {
- configSources := modelConfigSources(st)
+func (st *State) ComposeNewModelConfig(modelAttr map[string]interface{}, regionSpec *environs.RegionSpec) (map[string]interface{}, error) {
+ configSources := modelConfigSources(st, regionSpec)
return composeModelConfigAttributes(modelAttr, configSources...)
}
Oops, something went wrong.