Merge lp:~thumper/juju-core/fix-local-bootstrap-json into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Fix local provider bootstrap.

The environment configuration was being serialized
through JSON, which was coercing the integer values
to float64.

The validation schema checks were strictly looking for
integers. This has now been changed to ForceInt.

Test added.

https://codereview.appspot.com/14006043/

Fix local provider bootstrap.

The environment configuration was being serialized
through JSON, which was coercing the integer values
to float64.

The validation schema checks were strictly looking for
integers. This has now been changed to ForceInt.

Test added.

https://codereview.appspot.com/14006043/

[Tim Penhey (thumper)]

Reviewers: mp+187955_code.launchpad.net,

Message:
Please take a look.

Description:
Fix local provider bootstrap.

The environment configuration was being serialized
through JSON, which was coercing the integer values
to float64.

The validation schema checks were strictly looking for
integers. This has now been changed to ForceInt.

Test added.

https://code.launchpad.net/~thumper/juju-core/fix-local-bootstrap/+merge/187955

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/14006043/

Affected files (+20, -4 lines):
   A [revision details]
   M provider/local/config.go
   M provider/local/config_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20130926184157-5d305cr4rb895lnt
+New revision: <email address hidden>

Index: provider/local/config.go
=== modified file 'provider/local/config.go'
--- provider/local/config.go 2013-07-17 02:40:31 +0000
+++ provider/local/config.go 2013-09-26 23:34:07 +0000
@@ -21,8 +21,8 @@
   configFields = schema.Fields{
    "root-dir": schema.String(),
    "bootstrap-ip": schema.String(),
- "storage-port": schema.Int(),
- "shared-storage-port": schema.Int(),
+ "storage-port": schema.ForceInt(),
+ "shared-storage-port": schema.ForceInt(),
   }
   // The port defaults below are not entirely arbitrary. Local user web
   // frameworks often use 8000 or 8080, so I didn't want to use either of
@@ -102,11 +102,11 @@
  }

  func (c *environConfig) storagePort() int {
- return int(c.attrs["storage-port"].(int64))
+ return c.attrs["storage-port"].(int)
  }

  func (c *environConfig) sharedStoragePort() int {
- return int(c.attrs["shared-storage-port"].(int64))
+ return c.attrs["shared-storage-port"].(int)
  }

  func (c *environConfig) storageAddr() string {

Index: provider/local/config_test.go
=== modified file 'provider/local/config_test.go'
--- provider/local/config_test.go 2013-09-12 14:16:48 +0000
+++ provider/local/config_test.go 2013-09-26 23:34:58 +0000
@@ -89,6 +89,20 @@
   c.Assert(unknownAttrs["root-dir"], gc.Equals, expectedRootDir)
  }

+func (s *configSuite) TestValidateConfigWithFloatPort(c *gc.C) {
+ // When the config values get serialized through JSON, the integers
+ // get coerced to float64 values. The parsing needs to handle this.
+ values := minimalConfigValues()
+ values["storage-port"] = float64(8040)
+ testConfig, err := config.New(config.NoDefaults, values)
+ c.Assert(err, gc.IsNil)
+
+ valid, err := local.Provider.Validate(testConfig, nil)
+ c.Assert(err, gc.IsNil)
+ unknownAttrs := valid.UnknownAttrs()
+ c.Assert(unknownAttrs["storage-port"], gc.Equals, int(8040))
+}
+
  func (s *configSuite) TestNamespace(c *gc.C) {
   testConfig := minimalConfig(c)
   c.Assert(local.ConfigNamespace(testConfig), gc.Equals, "tester-test")

[Ian Booth (wallyworld)]

LGTM

https://codereview.appspot.com/14006043/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/14006043/

=== modified file 'provider/local/config.go'
--- provider/local/config.go	2013-07-17 02:40:31 +0000
+++ provider/local/config.go	2013-09-26 23:36:46 +0000
@@ -21,8 +21,8 @@
 	configFields = schema.Fields{
 		"root-dir":            schema.String(),
 		"bootstrap-ip":        schema.String(),
-		"storage-port":        schema.Int(),
-		"shared-storage-port": schema.Int(),
+		"storage-port":        schema.ForceInt(),
+		"shared-storage-port": schema.ForceInt(),
 	}
 	// The port defaults below are not entirely arbitrary.  Local user web
 	// frameworks often use 8000 or 8080, so I didn't want to use either of
@@ -102,11 +102,11 @@
 }
 
 func (c *environConfig) storagePort() int {
-	return int(c.attrs["storage-port"].(int64))
+	return c.attrs["storage-port"].(int)
 }
 
 func (c *environConfig) sharedStoragePort() int {
-	return int(c.attrs["shared-storage-port"].(int64))
+	return c.attrs["shared-storage-port"].(int)
 }
 
 func (c *environConfig) storageAddr() string {

=== modified file 'provider/local/config_test.go'
--- provider/local/config_test.go	2013-09-12 14:16:48 +0000
+++ provider/local/config_test.go	2013-09-26 23:36:46 +0000
@@ -89,6 +89,20 @@
 	c.Assert(unknownAttrs["root-dir"], gc.Equals, expectedRootDir)
 }
 
+func (s *configSuite) TestValidateConfigWithFloatPort(c *gc.C) {
+	// When the config values get serialized through JSON, the integers
+	// get coerced to float64 values.  The parsing needs to handle this.
+	values := minimalConfigValues()
+	values["storage-port"] = float64(8040)
+	testConfig, err := config.New(config.NoDefaults, values)
+	c.Assert(err, gc.IsNil)
+
+	valid, err := local.Provider.Validate(testConfig, nil)
+	c.Assert(err, gc.IsNil)
+	unknownAttrs := valid.UnknownAttrs()
+	c.Assert(unknownAttrs["storage-port"], gc.Equals, int(8040))
+}
+
 func (s *configSuite) TestNamespace(c *gc.C) {
 	testConfig := minimalConfig(c)
 	c.Assert(local.ConfigNamespace(testConfig), gc.Equals, "tester-test")