Returns a slice directly when generating constraints from space config.

There is a new test to ensure that a nil pointer for current space names is preserved when one is passed in and there are no implicit constraints.
ObviouslyGreen · Feb 15, 2018 · f9fe51c · f9fe51c
1 parent 5ec7cbf
commit f9fe51c
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 19 deletions.
diff --git a/controller/config.go b/controller/config.go
@@ -18,7 +18,6 @@ import (
 	"gopkg.in/macaroon-bakery.v1/bakery"
 
 	"github.com/juju/juju/cert"
-	"github.com/juju/juju/constraints"
 )
 
 const (
@@ -579,8 +578,10 @@ func (c Config) validateSpaceConfig(key, topic string) error {
 // AsSpaceConstraints checks to see whether config has spaces names populated
 // for management and/or HA (Mongo).
 // Non-empty values are merged with any input spaces and returned as a new
-// constraint.Value.
-func (c Config) AsSpaceConstraints(spaces *[]string) constraints.Value {
+// slice reference.
+// A slice pointer is used for congruence with the Spaces member in
+// constraints.Value.
+func (c Config) AsSpaceConstraints(spaces *[]string) *[]string {
 	newSpaces := set.NewStrings()
 	if spaces != nil {
 		for _, s := range *spaces {
@@ -594,12 +595,13 @@ func (c Config) AsSpaceConstraints(spaces *[]string) constraints.Value {
 		}
 	}
 
-	cons := constraints.Value{}
-	if len(newSpaces) > 0 {
-		s := newSpaces.Values()
-		cons.Spaces = &s
+	// Preserve a nil pointer if there is no change. This conveys information
+	// in constraints.Value (not set vs. deliberately set as empty).
+	if spaces == nil && len(newSpaces) == 0 {
+		return nil
 	}
-	return cons
+	ns := newSpaces.Values()
+	return &ns
 }
 
 // GenerateControllerCertAndKey makes sure that the config has a CACert and

diff --git a/controller/config_test.go b/controller/config_test.go
@@ -4,6 +4,7 @@
 package controller_test
 
 import (
+	"sort"
 	stdtesting "testing"
 	"time"
 
@@ -14,12 +15,9 @@ import (
 	"github.com/juju/utils/set"
 	gc "gopkg.in/check.v1"
 
-	"github.com/juju/cmd/cmdtesting"
 	"github.com/juju/juju/cert"
 	"github.com/juju/juju/controller"
-	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/testing"
-	"golang.org/x/tools/go/gcimporter15/testdata"
 )
 
 func Test(t *stdtesting.T) {
@@ -332,7 +330,7 @@ func (s *ConfigSuite) TestConfigManagementSpaceAsConstraint(c *gc.C) {
 		map[string]interface{}{controller.JujuHASpace: managementSpace},
 	)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(*(cfg.AsSpaceConstraints(nil).Spaces), gc.DeepEquals, []string{managementSpace})
+	c.Check(cfg.AsSpaceConstraints(nil), gc.DeepEquals, []string{managementSpace})
 }
 
 func (s *ConfigSuite) TestConfigHASpaceAsConstraint(c *gc.C) {
@@ -343,7 +341,7 @@ func (s *ConfigSuite) TestConfigHASpaceAsConstraint(c *gc.C) {
 		map[string]interface{}{controller.JujuHASpace: haSpace},
 	)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(*(cfg.AsSpaceConstraints(nil).Spaces), gc.DeepEquals, []string{haSpace})
+	c.Check(cfg.AsSpaceConstraints(nil), gc.DeepEquals, []string{haSpace})
 }
 
 func (s *ConfigSuite) TestConfigAllSpacesAsMergedConstraints(c *gc.C) {
@@ -361,10 +359,17 @@ func (s *ConfigSuite) TestConfigAllSpacesAsMergedConstraints(c *gc.C) {
 	)
 	c.Assert(err, jc.ErrorIsNil)
 
-	got := *(cfg.AsSpaceConstraints(&[]string{constraintSpace}).Spaces)
-	exp := set.NewStrings(haSpace, managementSpace, constraintSpace)
-	c.Assert(got, gc.HasLen, len(exp))
-	for _, s := range got {
-		c.Assert(exp.Contains(s), gc.Equals, true)
-	}
+	got := *cfg.AsSpaceConstraints(&[]string{constraintSpace})
+	sort.Strings(got)
+	c.Check(got, gc.DeepEquals, []string{constraintSpace, haSpace, managementSpace})
+}
+
+func (s *ConfigSuite) TestConfigNoSpacesNilSpaceConfigPreserved(c *gc.C) {
+	cfg, err := controller.NewConfig(
+		testing.ControllerTag.Id(),
+		testing.CACert,
+		map[string]interface{}{},
+	)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(cfg.AsSpaceConstraints(nil), gc.IsNil)
 }