launchpad-reviewers team mailing list archive

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

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

Commit message:
Implement InstanceID().

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/juju-core/instanceID/+merge/157059

As suggested by William (and this is also the solution Jeroen and I had in mind for this),  this branch changes the userdata script so that a file containing the instanceID will be written by cloudinit, this file will then be picked up by jujud when running maasEnvironProvider.InstanceId so that jujud will be able to know the instanceID of the machine on which it is running.

The only "trick" is that I had to add another utility method in cloudinit.go so that the creation of the instanceID file will happen *before* jujud is started (jujud is indeed started by cloudinit as well).

This was tested in the lab and a bootstrap node (with the correct machineID file) successfully started.
-- 
https://code.launchpad.net/~rvb/juju-core/instanceID/+merge/157059
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/juju-core/instanceID into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

=== modified file 'cloudinit/cloudinit_test.go'
--- cloudinit/cloudinit_test.go	2013-01-16 14:27:13 +0000
+++ cloudinit/cloudinit_test.go	2013-04-04 12:23:20 +0000
@@ -182,6 +182,14 @@
 		},
 	},
 	{
+		"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	2012-02-15 08:13:22 +0000
+++ cloudinit/options.go	2013-04-04 12:23:20 +0000
@@ -89,6 +89,11 @@
 	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
@@ -97,6 +102,16 @@
 	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/maas/environ.go'
--- environs/maas/environ.go	2013-04-03 15:04:01 +0000
+++ environs/maas/environ.go	2013-04-04 12:23:20 +0000
@@ -176,12 +176,7 @@
 	mcfg.MongoURL = mongoURL
 	mcfg.Config = config
 
-	userdata, err := userData(mcfg)
-	if err != nil {
-		msg := fmt.Errorf("could not compose userdata for bootstrap node: %v", err)
-		return nil, msg
-	}
-	inst, err := env.obtainNode(machineID, &stateInfo, &apiInfo, tools, userdata)
+	inst, err := env.obtainNode(machineID, &stateInfo, &apiInfo, tools, mcfg)
 	if err != nil {
 		return nil, fmt.Errorf("cannot start bootstrap instance: %v", err)
 	}
@@ -358,9 +353,11 @@
 	return err
 }
 
+var _MAASInstanceIDFilename = jujuDataDir + "/MAASmachineID.txt"
+
 // obtainNode allocates and starts a MAAS node.  It is used both for the
 // implementation of StartInstance, and to initialize the bootstrap node.
-func (environ *maasEnviron) obtainNode(machineId string, stateInfo *state.Info, apiInfo *api.Info, tools *state.Tools, userdata []byte) (*maasInstance, error) {
+func (environ *maasEnviron) obtainNode(machineId string, stateInfo *state.Info, apiInfo *api.Info, tools *state.Tools, mcfg *cloudinit.MachineConfig) (*maasInstance, error) {
 
 	log.Printf("environs/maas: starting machine %s in $q running tools version %q from %q", machineId, environ.name, tools.Binary, tools.URL)
 
@@ -370,6 +367,12 @@
 	}
 	instance := maasInstance{&node, environ}
 
+	script := fmt.Sprintf(`mkdir -p %s; echo -n %s > %s`, trivial.ShQuote(jujuDataDir), trivial.ShQuote(string(instance.Id())), trivial.ShQuote(_MAASInstanceIDFilename))
+	userdata, err := userData(mcfg, script)
+	if err != nil {
+		msg := fmt.Errorf("could not compose userdata for bootstrap node: %v", err)
+		return nil, msg
+	}
 	err = environ.startNode(node, tools, userdata)
 	if err != nil {
 		environ.StopInstances([]environs.Instance{&instance})
@@ -391,12 +394,7 @@
 	}
 
 	mcfg := environ.makeMachineConfig(machineID, stateInfo, apiInfo, tools)
-	userdata, err := userData(mcfg)
-	if err != nil {
-		msg := fmt.Errorf("could not compose user data: %v", err)
-		return nil, msg
-	}
-	return environ.obtainNode(machineID, stateInfo, apiInfo, tools, userdata)
+	return environ.obtainNode(machineID, stateInfo, apiInfo, tools, mcfg)
 }
 
 // StopInstances is specified in the Environ interface.

=== modified file 'environs/maas/environprovider.go'
--- environs/maas/environprovider.go	2013-03-11 15:01:02 +0000
+++ environs/maas/environprovider.go	2013-04-04 12:23:20 +0000
@@ -1,6 +1,7 @@
 package maas
 
 import (
+	"io/ioutil"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/log"
@@ -50,5 +51,9 @@
 
 // InstanceId is specified in the EnvironProvider interface.
 func (*maasEnvironProvider) InstanceId() (state.InstanceId, error) {
-	panic("Not implemented.")
+	content, err := ioutil.ReadFile(_MAASInstanceIDFilename)
+	if err != nil {
+		return "", err
+	}
+	return state.InstanceId(string(content)), nil
 }

=== modified file 'environs/maas/environprovider_test.go'
--- environs/maas/environprovider_test.go	2013-03-12 02:46:32 +0000
+++ environs/maas/environprovider_test.go	2013-04-04 12:23:20 +0000
@@ -1,8 +1,11 @@
 package maas
 
 import (
+	"io/ioutil"
 	. "launchpad.net/gocheck"
 	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/state"
+	"os"
 )
 
 type EnvironProviderSuite struct {
@@ -28,3 +31,25 @@
 	expectedAttrs := map[string]interface{}{"maas-oauth": oauth}
 	c.Check(secretAttrs, DeepEquals, expectedAttrs)
 }
+
+func (suite *EnvironProviderSuite) TestInstanceIdReadsInstanceIdFile(c *C) {
+	instanceId := "instance-id"
+	// Create a temporary file to act as the file where the instanceID
+	// is stored.
+	file, err := ioutil.TempFile("", "")
+	c.Assert(err, IsNil)
+	filename := file.Name()
+	defer os.Remove(filename)
+	err = ioutil.WriteFile(filename, []byte(instanceId), 0644)
+	c.Assert(err, IsNil)
+	// "Monkey patch" the value of _MAASInstanceIDFilename with the path
+	// to the temporary file.
+	old_MAASInstanceIDFilename := _MAASInstanceIDFilename
+	_MAASInstanceIDFilename = filename
+	defer func() { _MAASInstanceIDFilename = old_MAASInstanceIDFilename }()
+
+	provider := suite.environ.Provider()
+	returnedInstanceId, err := provider.InstanceId()
+	c.Assert(err, IsNil)
+	c.Check(returnedInstanceId, Equals, state.InstanceId(instanceId))
+}

=== modified file 'environs/maas/util.go'
--- environs/maas/util.go	2013-03-11 17:03:47 +0000
+++ environs/maas/util.go	2013-04-04 12:23:20 +0000
@@ -29,8 +29,11 @@
 }
 
 // userData returns a zipped cloudinit config.
-func userData(cfg *cloudinit.MachineConfig) ([]byte, error) {
+func userData(cfg *cloudinit.MachineConfig, scripts ...string) ([]byte, error) {
 	cloudcfg, err := cloudinit.New(cfg)
+	for _, script := range scripts {
+		cloudcfg.AddRunCmdPrefix(script)
+	}
 	if err != nil {
 		return nil, err
 	}

=== modified file 'environs/maas/util_test.go'
--- environs/maas/util_test.go	2013-03-11 17:23:36 +0000
+++ environs/maas/util_test.go	2013-04-04 12:23:20 +0000
@@ -68,7 +68,10 @@
 		APIPort:     apiPort,
 		StateServer: true,
 	}
-	result, err := userData(cfg)
+	script1 := "script1"
+	script2 := "script2"
+	scripts := []string{script1, script2}
+	result, err := userData(cfg, scripts...)
 	c.Assert(err, IsNil)
 
 	unzipped, err := trivial.Gunzip(result)
@@ -80,4 +83,9 @@
 
 	// Just check that the cloudinit config looks good.
 	c.Check(config["apt_upgrade"], Equals, true)
+	// 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)
 }