launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/juju-core/cloudinit-cleanup into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

Commit message:
Add cloudinit.Configure() and use that in the MAASprovider.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/juju-core/cloudinit-cleanup/+merge/157306

This is a follow-up to the branch https://code.launchpad.net/~rvb/juju-core/instanceID/+merge/157059. As advised by William Reade, instead of adding a new AddRunCmdPrefix method, I created a cloudinit.Configure() method so that it's possible to get a cloudinit config with custom run commands added at the beginning of the list.
-- 
https://code.launchpad.net/~rvb/juju-core/cloudinit-cleanup/+merge/157306
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/juju-core/cloudinit-cleanup into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

=== modified file 'cloudinit/cloudinit_test.go'
--- cloudinit/cloudinit_test.go	2013-04-04 12:18:51 +0000
+++ cloudinit/cloudinit_test.go	2013-04-05 09:20:35 +0000
@@ -182,14 +182,6 @@
 		},
 	},
 	{
-		"AddRunCmdPrefix",
-		"runcmd:\n- testprefix\n- test\n",
-		func(cfg *cloudinit.Config) {
-			cfg.AddRunCmd("test")
-			cfg.AddRunCmdPrefix("testprefix")
-		},
-	},
-	{
 		"Mounts",
 		"mounts:\n- - x\n  - \"y\"\n- - z\n  - w\n",
 		func(cfg *cloudinit.Config) {

=== modified file 'cloudinit/options.go'
--- cloudinit/options.go	2013-04-04 12:18:51 +0000
+++ cloudinit/options.go	2013-04-05 09:20:35 +0000
@@ -89,11 +89,6 @@
 	cfg.attrs[kind] = append(cmds, c)
 }
 
-func (cfg *Config) addCmdPrefix(kind string, c *command) {
-	cmds, _ := cfg.attrs[kind].([]*command)
-	cfg.attrs[kind] = append([]*command{c}, cmds...)
-}
-
 // AddRunCmd adds a command to be executed
 // at first boot. The command will be run
 // by the shell with any metacharacters retaining
@@ -102,16 +97,6 @@
 	cfg.addCmd("runcmd", &command{literal: cmd})
 }
 
-// AddRunCmdPrefix adds a command to be executed
-// at first boot. AddRunCmdPrefix adds the command at
-// the beginning of the list of the commands to be run.
-// The command will be run by the shell with any metacharacters
-// retaining their special meaning (that is, no quoting takes
-// place).
-func (cfg *Config) AddRunCmdPrefix(cmd string) {
-	cfg.addCmdPrefix("runcmd", &command{literal: cmd})
-}
-
 // AddRunCmdArgs is like AddRunCmd except that the command
 // will be executed with the given arguments properly quoted.
 func (cfg *Config) AddRunCmdArgs(args ...string) {

=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2013-02-27 13:28:43 +0000
+++ environs/cloudinit/cloudinit.go	2013-04-05 09:20:35 +0000
@@ -96,7 +96,13 @@
 		return nil, err
 	}
 	c := cloudinit.New()
+	return Configure(cfg, c)
+}
 
+func Configure(cfg *MachineConfig, c *cloudinit.Config) (*cloudinit.Config, error) {
+	if err := verifyConfig(cfg); err != nil {
+		return nil, err
+	}
 	c.AddSSHAuthorizedKeys(cfg.AuthorizedKeys)
 	c.AddPackage("git")
 

=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go	2013-02-27 13:28:43 +0000
+++ environs/cloudinit/cloudinit_test.go	2013-04-05 09:20:35 +0000
@@ -4,6 +4,7 @@
 	"encoding/base64"
 	. "launchpad.net/gocheck"
 	"launchpad.net/goyaml"
+	cloudinit_core "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/state"
@@ -200,6 +201,30 @@
 	}
 }
 
+func (*cloudinitSuite) TestCloudInitConfigure(c *C) {
+	for i, test := range cloudinitTests {
+		c.Logf("test %d (Configure)", i)
+		// Create a simple cloudinit config with a 'runcmd' statement.
+		cloudcfg := cloudinit_core.New()
+		script := "test script"
+		cloudcfg.AddRunCmd(script)
+
+		ci, err := cloudinit.Configure(&test.cfg, cloudcfg)
+		c.Assert(err, IsNil)
+		c.Check(ci, NotNil)
+		data, err := ci.Render()
+		c.Assert(err, IsNil)
+
+		ciContent := make(map[interface{}]interface{})
+		err = goyaml.Unmarshal(data, &ciContent)
+		c.Assert(err, IsNil)
+		// The 'runcmd' statement is at the beginning of the list
+		// of 'runcmd' statements.
+		runCmd := ciContent["runcmd"].([]interface{})
+		c.Check(runCmd[0], Equals, script)
+	}
+}
+
 func getScripts(x map[interface{}]interface{}) []string {
 	var scripts []string
 	for _, s := range x["runcmd"].([]interface{}) {

=== modified file 'environs/maas/util.go'
--- environs/maas/util.go	2013-04-04 12:18:51 +0000
+++ environs/maas/util.go	2013-04-05 09:20:35 +0000
@@ -1,6 +1,7 @@
 package maas
 
 import (
+	cloudinit_core "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
@@ -30,10 +31,11 @@
 
 // userData returns a zipped cloudinit config.
 func userData(cfg *cloudinit.MachineConfig, scripts ...string) ([]byte, error) {
-	cloudcfg, err := cloudinit.New(cfg)
+	cloudcfg := cloudinit_core.New()
 	for _, script := range scripts {
-		cloudcfg.AddRunCmdPrefix(script)
+		cloudcfg.AddRunCmd(script)
 	}
+	cloudcfg, err := cloudinit.Configure(cfg, cloudcfg)
 	if err != nil {
 		return nil, err
 	}

=== modified file 'environs/maas/util_test.go'
--- environs/maas/util_test.go	2013-04-04 12:18:51 +0000
+++ environs/maas/util_test.go	2013-04-05 09:20:35 +0000
@@ -86,6 +86,6 @@
 	// The scripts given to userData where added as the first
 	// commands to be run.
 	runCmd := config["runcmd"].([]interface{})
-	c.Check(runCmd[0], Equals, script2)
-	c.Check(runCmd[1], Equals, script1)
+	c.Check(runCmd[0], Equals, script1)
+	c.Check(runCmd[1], Equals, script2)
 }