Merge lp:~themue/juju-core/049-prepare-ec2 into lp:~go-bot/juju-core/trunk

[Frank Mueller]

ec2: implemented preparing of configuration

Currently the providers don't use Prepare() for the
preparing of their configuration. Here this is changed
for EC2. So far only the existance and type of
"control-bucket" are checked. Other providers will
have different checks.

https://codereview.appspot.com/14282044/

[Frank Mueller (themue)]

Reviewers: mp+188865_code.launchpad.net,

Message:
Please take a look.

Description:
ec2: implemented preparing of configuration

Currently the providers don't use Prepare() for the
preparing of their configuration. Here this is changed
for EC2. So far only the existance and type of
"control-bucket" are checked. Other providers will
have different checks.

https://code.launchpad.net/~themue/juju-core/049-prepare-ec2/+merge/188865

(do not edit description out of merge proposal)

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

Affected files (+49, -1 lines):
   A [revision details]
   M provider/ec2/config_test.go
   M provider/ec2/ec2.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-20131002130110-6i0rmclha2rfugvw
+New revision: <email address hidden>

Index: provider/ec2/config_test.go
=== modified file 'provider/ec2/config_test.go'
--- provider/ec2/config_test.go 2013-09-27 02:00:34 +0000
+++ provider/ec2/config_test.go 2013-10-02 14:55:45 +0000
@@ -327,3 +327,33 @@
   test.err = "environment has no access-key or secret-key"
   test.check(c)
  }
+
+func (s *ConfigSuite) TestPrepare(c *gc.C) {
+ // No value for control bucket.
+ attrs := testing.FakeConfig().Merge(testing.Attrs{
+ "type": "ec2",
+ })
+ cfg, err := config.New(config.NoDefaults, attrs)
+ c.Assert(err, gc.IsNil)
+ p, err := environs.Provider(cfg.Type())
+ c.Assert(err, gc.IsNil)
+ e, err := p.Prepare(cfg)
+ c.Assert(err, gc.IsNil)
+ ecfg := e.(*environ).ecfg()
+ c.Assert(ecfg.controlBucket(), gc.Matches, "juju-[a-f0-9]{32}")
+
+ // Wrong type of control bucket value.
+ attrs = testing.FakeConfig().Merge(testing.Attrs{
+ "type": "ec2",
+ "control-bucket": 1234,
+ })
+ cfg, err = config.New(config.NoDefaults, attrs)
+ c.Assert(err, gc.IsNil)
+ p, err = environs.Provider(cfg.Type())
+ c.Assert(err, gc.IsNil)
+ e, err = p.Prepare(cfg)
+ c.Assert(err, gc.IsNil)
+ ecfg = e.(*environ).ecfg()
+ c.Assert(ecfg.controlBucket(), gc.Matches, "juju-[a-f0-9]{32}")
+
+}

Index: provider/ec2/ec2.go
=== modified file 'provider/ec2/ec2.go'
--- provider/ec2/ec2.go 2013-10-02 00:29:29 +0000
+++ provider/ec2/ec2.go 2013-10-02 14:55:45 +0000
@@ -213,7 +213,23 @@

  func (p environProvider) Prepare(cfg *config.Config) (environs.Environ,
error) {
   logger.Infof("preparing environment %q", cfg.Name())
- // TODO create/verify control bucket if necessary.
+ attrs := cfg.AllAttrs()
+
+ // Check control bucket.
+ controlBucket, ok := attrs["control-bucket"].(string)
+ if !ok || controlBucket == "" {
+ uuid, err := utils.NewUUID()
+ if err != nil {
+ panic(fmt.Errorf("error generating random id: %v", err))
+ }
+ attrs["control-bucket"] = fmt.Sprintf("juju-%x", uuid.Raw())
+ }
+
+ // Apply changes.
+ cfg, err := cfg.Apply(attrs)
+ if err != nil {
+ return nil, err
+ }
   return p.Open(cfg)
  }

[William Reade (fwereade)]

If you're on holiday now, I might try to take these over.

https://codereview.appspot.com/14282044/diff/1/provider/ec2/config_test.go
File provider/ec2/config_test.go (right):

https://codereview.appspot.com/14282044/diff/1/provider/ec2/config_test.go#newcode348
provider/ec2/config_test.go:348: "control-bucket": 1234,
I don't think we should be accepting bad data and turning it into good
-- let's stick to inserting missing data.

https://codereview.appspot.com/14282044/

[William Reade (fwereade)]

rendered moot by https://code.launchpad.net/~fwereade/juju-core/prepare-control-bucket/+merge/188955

Unmerged revisions

1928. By Frank Mueller on 2013-10-02

ec2: implemented configuration preparing

=== modified file 'provider/ec2/config_test.go'
--- provider/ec2/config_test.go	2013-09-27 02:00:34 +0000
+++ provider/ec2/config_test.go	2013-10-02 15:21:49 +0000
@@ -327,3 +327,33 @@
 	test.err = "environment has no access-key or secret-key"
 	test.check(c)
 }
+
+func (s *ConfigSuite) TestPrepare(c *gc.C) {
+	// No value for control bucket.
+	attrs := testing.FakeConfig().Merge(testing.Attrs{
+		"type": "ec2",
+	})
+	cfg, err := config.New(config.NoDefaults, attrs)
+	c.Assert(err, gc.IsNil)
+	p, err := environs.Provider(cfg.Type())
+	c.Assert(err, gc.IsNil)
+	e, err := p.Prepare(cfg)
+	c.Assert(err, gc.IsNil)
+	ecfg := e.(*environ).ecfg()
+	c.Assert(ecfg.controlBucket(), gc.Matches, "juju-[a-f0-9]{32}")
+
+	// Wrong type of control bucket value.
+	attrs = testing.FakeConfig().Merge(testing.Attrs{
+		"type":           "ec2",
+		"control-bucket": 1234,
+	})
+	cfg, err = config.New(config.NoDefaults, attrs)
+	c.Assert(err, gc.IsNil)
+	p, err = environs.Provider(cfg.Type())
+	c.Assert(err, gc.IsNil)
+	e, err = p.Prepare(cfg)
+	c.Assert(err, gc.IsNil)
+	ecfg = e.(*environ).ecfg()
+	c.Assert(ecfg.controlBucket(), gc.Matches, "juju-[a-f0-9]{32}")
+
+}

=== modified file 'provider/ec2/ec2.go'
--- provider/ec2/ec2.go	2013-10-02 00:29:29 +0000
+++ provider/ec2/ec2.go	2013-10-02 15:21:49 +0000
@@ -213,7 +213,23 @@
 
 func (p environProvider) Prepare(cfg *config.Config) (environs.Environ, error) {
 	logger.Infof("preparing environment %q", cfg.Name())
-	// TODO create/verify control bucket if necessary.
+	attrs := cfg.AllAttrs()
+
+	// Check control bucket.
+	controlBucket, ok := attrs["control-bucket"].(string)
+	if !ok || controlBucket == "" {
+		uuid, err := utils.NewUUID()
+		if err != nil {
+			panic(fmt.Errorf("error generating random id: %v", err))
+		}
+		attrs["control-bucket"] = fmt.Sprintf("juju-%x", uuid.Raw())
+	}
+
+	// Apply changes.
+	cfg, err := cfg.Apply(attrs)
+	if err != nil {
+		return nil, err
+	}
 	return p.Open(cfg)
 }