juju · jujubot · Aug 25, 2016 · Aug 20, 2016 · Aug 23, 2016 · Aug 24, 2016
diff --git a/apiserver/common/modelmanagerinterface.go b/apiserver/common/modelmanagerinterface.go
@@ -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)

diff --git a/apiserver/modelmanager/modelinfo_test.go b/apiserver/modelmanager/modelinfo_test.go
@@ -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 {

diff --git a/apiserver/modelmanager/modelmanager.go b/apiserver/modelmanager/modelmanager.go
@@ -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) {

diff --git a/environs/cloudspec.go b/environs/cloudspec.go
@@ -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
+}
diff --git a/environs/config/source.go b/environs/config/source.go
@@ -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"

diff --git a/state/backups/restore_test.go b/state/backups/restore_test.go
@@ -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)

diff --git a/state/initialize_test.go b/state/initialize_test.go
@@ -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"])
+	}
+}
diff --git a/state/modelconfig.go b/state/modelconfig.go
@@ -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...)
 }