juju model-defaults supports region for get

wallyworld · wallyworld · commit 5825c4f4d1a4 · 2016-10-13T13:41:04.000+10:00
diff --git a/cmd/juju/model/defaultscommand.go b/cmd/juju/model/defaultscommand.go
@@ -4,6 +4,7 @@ package model
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"sort"
 	"strings"
@@ -38,6 +39,8 @@ By default, the model is the current model.
 Examples:
     juju model-defaults
     juju model-defaults http-proxy
+    juju model-defaults aws/us-east-1 http-proxy
+    juju model-defaults us-east-1 http-proxy
     juju model-defaults -m mymodel type
     juju model-defaults ftp-proxy=10.0.0.1:8000
     juju model-defaults aws/us-east-1 ftp-proxy=10.0.0.1:8000
@@ -214,8 +217,8 @@ func (c *defaultsCommand) Init(args []string) error {
 		// set args.
 		return c.handleSetArgs(args)
 	case len(args) == 0:
-		// This should be reset only (but possibly for a region).
-		return c.handleZeroArgs()
+		c.action = c.getDefaults
+		return nil
 	case len(args) == 1:
 		// We want to get settings for the provided key.
 		return c.handleOneArg(args[0])
@@ -386,39 +389,19 @@ func (c *defaultsCommand) parseSetKeys(args []string) error {
 	return nil
 }
 
-// handleZeroArgs determines whether we are doing a reset only and if any
-// additional arguments are valid -- or why they are not.
-func (c *defaultsCommand) handleZeroArgs() error {
-	if c.regionName != "" {
-		if c.resetKeys == nil {
-			// It doesn't make sense to specify a region unless we are resetting
-			// values here.
-			return errors.New("specifying a region when retrieving defaults is invalid")
-		}
-	}
-	// We can reset for a region. We can also reset for a controller if there
-	// is no region specified. In either case c.action remains nil. We short
-	// circuited at the begining of c.Init so we should never reach a case
-	// where there is no region or reset keys specified.
-	return nil
-}
-
 // handleOneArg handles the case where we have one positional arg after
 // processing for a region and the reset flag.
 func (c *defaultsCommand) handleOneArg(arg string) error {
 	resetSpecified := c.resetKeys != nil
 	regionSpecified := c.regionName != ""
 
 	if regionSpecified {
-		if !resetSpecified {
-			// It doesn't make sense to specify a region unless we are resetting
-			// values here.
-			return errors.New("specifying a region when retrieving defaults for a setting is invalid")
+		if resetSpecified {
+			// If a region was specified and reset was specified, we shouldn't have
+			// an extra arg. If it had an "=" in it, we should have handled it
+			// already.
+			return errors.New("cannot retrieve defaults for a region and reset attributes at the same time")
 		}
-		// If a region was specified and reset was specified, we shouldn't have
-		// an extra arg. If it had an "=" in it, we should have handled it
-		// already.
-		return errors.New("cannot retrieve defaults for a key and reset args at the same time")
 	}
 	if resetSpecified {
 		// It makes no sense to supply a positional arg that isn't a region if
@@ -501,13 +484,39 @@ func (c *defaultsCommand) getDefaults(client defaultsCommandAPI, ctx *cmd.Contex
 		return err
 	}
 
+	valueForRegion := func(region string, regions []config.RegionDefaultValue) (config.RegionDefaultValue, bool) {
+		for _, r := range regions {
+			if r.Name == region {
+				return r, true
+			}
+		}
+		return config.RegionDefaultValue{}, false
+	}
+
+	// Filter by region if necessary.
+	if c.regionName != "" {
+		for attrName, attr := range attrs {
+			if regionDefault, ok := valueForRegion(c.regionName, attr.Regions); !ok {
+				delete(attrs, attrName)
+			} else {
+				attrForRegion := attr
+				attrForRegion.Regions = []config.RegionDefaultValue{regionDefault}
+				attrs[attrName] = attrForRegion
+			}
+		}
+	}
+
 	if c.key != "" {
 		if value, ok := attrs[c.key]; ok {
 			attrs = config.ModelDefaultAttributes{
 				c.key: value,
 			}
 		} else {
-			return errors.Errorf("key %q not found in %q model defaults.", c.key, attrs["name"])
+			msg := fmt.Sprintf("there are no default model values for %q", c.key)
+			if c.regionName != "" {
+				msg += fmt.Sprintf(" in region %q", c.regionName)
+			}
+			return errors.New(msg)
 		}
 	}
 	// If c.keys is empty, write out the whole lot.
diff --git a/cmd/juju/model/defaultscommand_test.go b/cmd/juju/model/defaultscommand_test.go
@@ -74,7 +74,7 @@ func (s *DefaultsCommandSuite) TestDefaultsInit(c *gc.C) {
 		{
 			description: "test reset with valid region and extra positional arg",
 			args:        []string{"--reset", "something", "dummy-region", "weird"},
-			errorMatch:  "cannot retrieve defaults for a key and reset args at the same time",
+			errorMatch:  "cannot retrieve defaults for a region and reset attributes at the same time",
 		}, {
 			description: "test reset with valid region only",
 			args:        []string{"--reset", "foo", "dummy-region"},
@@ -151,11 +151,11 @@ func (s *DefaultsCommandSuite) TestDefaultsInit(c *gc.C) {
 		}, {
 			description: "test valid region and one arg",
 			args:        []string{"dummy-region", "one"},
-			errorMatch:  "specifying a region when retrieving defaults for a setting is invalid",
+			nilErr:      true,
 		}, {
 			description: "test valid region and no args",
 			args:        []string{"dummy-region"},
-			errorMatch:  "specifying a region when retrieving defaults is invalid",
+			nilErr:      true,
 		}, {
 			// test cloud/region
 			description: "test invalid cloud fails",
@@ -242,6 +242,9 @@ func (s *DefaultsCommandSuite) TestResetAttr(c *gc.C) {
 		"attr2": {Controller: "bar", Default: nil, Regions: []config.RegionDefaultValue{{
 			Name:  "dummy-region",
 			Value: "dummy-value",
+		}, {
+			Name:  "another-region",
+			Value: "another-value",
 		}}},
 	})
 }
@@ -267,6 +270,9 @@ func (s *DefaultsCommandSuite) TestSet(c *gc.C) {
 		"attr2": {Controller: "bar", Default: nil, Regions: []config.RegionDefaultValue{{
 			Name:  "dummy-region",
 			Value: "dummy-value",
+		}, {
+			Name:  "another-region",
+			Value: "another-value",
 		}}},
 		"special": {Controller: "extra", Default: nil, Regions: nil},
 	})
@@ -307,9 +313,10 @@ func (s *DefaultsCommandSuite) TestGetSingleValue(c *gc.C) {
 
 	output := strings.TrimSpace(testing.Stdout(context))
 	expected := "" +
-		"Attribute       Default      Controller\n" +
-		"attr2           -            bar\n" +
-		"  dummy-region  dummy-value  -"
+		"Attribute         Default        Controller\n" +
+		"attr2             -              bar\n" +
+		"  dummy-region    dummy-value    -\n" +
+		"  another-region  another-value  -"
 	c.Assert(output, gc.Equals, expected)
 }
 
@@ -319,7 +326,7 @@ func (s *DefaultsCommandSuite) TestGetSingleValueJSON(c *gc.C) {
 
 	output := strings.TrimSpace(testing.Stdout(context))
 	c.Assert(output, gc.Equals,
-		`{"attr2":{"controller":"bar","regions":[{"name":"dummy-region","value":"dummy-value"}]}}`)
+		`{"attr2":{"controller":"bar","regions":[{"name":"dummy-region","value":"dummy-value"},{"name":"another-region","value":"another-value"}]}}`)
 }
 
 func (s *DefaultsCommandSuite) TestGetAllValuesYAML(c *gc.C) {
@@ -334,7 +341,9 @@ func (s *DefaultsCommandSuite) TestGetAllValuesYAML(c *gc.C) {
 		"  controller: bar\n" +
 		"  regions:\n" +
 		"  - name: dummy-region\n" +
-		"    value: dummy-value"
+		"    value: dummy-value\n" +
+		"  - name: another-region\n" +
+		"    value: another-value"
 	c.Assert(output, gc.Equals, expected)
 }
 
@@ -343,18 +352,31 @@ func (s *DefaultsCommandSuite) TestGetAllValuesJSON(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	output := strings.TrimSpace(testing.Stdout(context))
-	expected := `{"attr":{"default":"foo"},"attr2":{"controller":"bar","regions":[{"name":"dummy-region","value":"dummy-value"}]}}`
+	expected := `{"attr":{"default":"foo"},"attr2":{"controller":"bar","regions":[{"name":"dummy-region","value":"dummy-value"},{"name":"another-region","value":"another-value"}]}}`
 	c.Assert(output, gc.Equals, expected)
 }
 
 func (s *DefaultsCommandSuite) TestGetAllValuesTabular(c *gc.C) {
 	context, err := s.run(c)
 	c.Assert(err, jc.ErrorIsNil)
 
+	output := strings.TrimSpace(testing.Stdout(context))
+	expected := "" +
+		"Attribute         Default        Controller\n" +
+		"attr              foo            -\n" +
+		"attr2             -              bar\n" +
+		"  dummy-region    dummy-value    -\n" +
+		"  another-region  another-value  -"
+	c.Assert(output, gc.Equals, expected)
+}
+
+func (s *DefaultsCommandSuite) TestGetRegionValuesTabular(c *gc.C) {
+	context, err := s.run(c, "dummy-region")
+	c.Assert(err, jc.ErrorIsNil)
+
 	output := strings.TrimSpace(testing.Stdout(context))
 	expected := "" +
 		"Attribute       Default      Controller\n" +
-		"attr            foo          -\n" +
 		"attr2           -            bar\n" +
 		"  dummy-region  dummy-value  -"
 	c.Assert(output, gc.Equals, expected)
diff --git a/cmd/juju/model/fakeenv_test.go b/cmd/juju/model/fakeenv_test.go
@@ -97,7 +97,11 @@ func (s *fakeModelDefaultEnvSuite) SetUpTest(c *gc.C) {
 				Controller: "bar",
 				Regions: []config.RegionDefaultValue{{
 					"dummy-region",
-					"dummy-value"}}},
+					"dummy-value",
+				}, {
+					"another-region",
+					"another-value",
+				}}},
 		},
 	}
 	s.fakeCloudAPI = &fakeCloudAPI{
diff --git a/featuretests/cmd_juju_model_test.go b/featuretests/cmd_juju_model_test.go
@@ -141,6 +141,19 @@ special    -        known
 `[1:])
 }
 
+func (s *cmdModelSuite) TestModelDefaultsGetRegion(c *gc.C) {
+	err := s.State.UpdateModelConfigDefaultValues(map[string]interface{}{"special": "known"}, nil, &environs.RegionSpec{"dummy", "dummy-region"})
+	c.Assert(err, jc.ErrorIsNil)
+
+	context := s.run(c, "model-defaults", "dummy-region", "special")
+	c.Assert(testing.Stdout(context), gc.Equals, `
+Attribute       Default  Controller
+special         -        -
+  dummy-region  known    -
+
+`[1:])
+}
+
 func (s *cmdModelSuite) TestModelDefaultsSet(c *gc.C) {
 	s.run(c, "model-defaults", "special=known")
 	defaults, err := s.State.ModelConfigDefaultValues()
