Merge lp:~thumper/juju-core/container-userdata into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Move the writing of the user-data to container.

Writing out the user-data is container agnostic.

https://codereview.appspot.com/24790043/

Move the writing of the userdata to container.

Writing out the userdata is container agnostic.

https://codereview.appspot.com/24790043/

[Tim Penhey (thumper)]

Reviewers: mp+194759_code.launchpad.net,

Message:
Please take a look.

Description:
Move the writing of the userdata to container.

Writing out the userdata is container agnostic.

https://code.launchpad.net/~thumper/juju-core/container-userdata/+merge/194759

Requires:
https://code.launchpad.net/~thumper/juju-core/container-interface/+merge/194757

(do not edit description out of merge proposal)

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

Affected files (+84, -63 lines):
   A [revision details]
   M container/lxc/lxc.go
   A container/userdata.go

[Ian Booth (wallyworld)]

LGTM

https://codereview.appspot.com/24790043/

[Go Bot (go-bot)]

The prerequisite https://code.launchpad.net/~thumper/juju-core/container-interface has not yet been merged into https://code.launchpad.net/~go-bot/juju-core/trunk.

=== modified file 'container/lxc/lxc.go'
--- container/lxc/lxc.go	2013-11-12 00:35:51 +0000
+++ container/lxc/lxc.go	2013-11-12 00:35:51 +0000
@@ -8,7 +8,6 @@
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"launchpad.net/golxc"
@@ -18,14 +17,12 @@
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/names"
-	"launchpad.net/juju-core/utils"
 )
 
 var logger = loggo.GetLogger("juju.container.lxc")
 
 var (
 	defaultTemplate     = "ubuntu-cloud"
-	aptHTTPProxyRE      = regexp.MustCompile(`(?i)^Acquire::HTTP::Proxy\s+"([^"]+)";$`)
 	ContainerDir        = "/var/lib/juju/containers"
 	RemovedContainerDir = "/var/lib/juju/removed-containers"
 	LxcContainerDir     = "/var/lib/lxc"
@@ -85,7 +82,7 @@
 		return nil, err
 	}
 	logger.Tracef("write cloud-init")
-	userDataFilename, err := writeUserData(machineConfig, directory)
+	userDataFilename, err := container.WriteUserData(machineConfig, directory)
 	if err != nil {
 		logger.Errorf("failed to write user data: %v", err)
 		return nil, err
@@ -248,65 +245,6 @@
 	return configFilename, nil
 }
 
-func writeUserData(machineConfig *cloudinit.MachineConfig, directory string) (string, error) {
-	userData, err := cloudInitUserData(machineConfig)
-	if err != nil {
-		logger.Errorf("failed to create user data: %v", err)
-		return "", err
-	}
-	userDataFilename := filepath.Join(directory, "cloud-init")
-	if err := ioutil.WriteFile(userDataFilename, userData, 0644); err != nil {
-		logger.Errorf("failed to write user data: %v", err)
-		return "", err
-	}
-	return userDataFilename, nil
-}
-
-func cloudInitUserData(machineConfig *cloudinit.MachineConfig) ([]byte, error) {
-	machineConfig.DataDir = "/var/lib/juju"
-	cloudConfig, err := cloudinit.New(machineConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	// Run apt-config to fetch proxy settings from host. If no proxy
-	// settings are configured, then we don't set up any proxy information
-	// on the container.
-	proxyConfig, err := utils.AptConfigProxy()
-	if err != nil {
-		return nil, err
-	}
-	if proxyConfig != "" {
-		var proxyLines []string
-		for _, line := range strings.Split(proxyConfig, "\n") {
-			line = strings.TrimSpace(line)
-			if len(line) > 0 {
-				if m := aptHTTPProxyRE.FindStringSubmatch(line); m != nil {
-					cloudConfig.SetAptProxy(m[1])
-				} else {
-					proxyLines = append(proxyLines, line)
-				}
-			}
-		}
-		if len(proxyLines) > 0 {
-			cloudConfig.AddFile(
-				"/etc/apt/apt.conf.d/99proxy-extra",
-				strings.Join(proxyLines, "\n"),
-				0644)
-		}
-	}
-
-	// Run ifconfig to get the addresses of the internal container at least
-	// logged in the host.
-	cloudConfig.AddRunCmd("ifconfig")
-
-	data, err := cloudConfig.Render()
-	if err != nil {
-		return nil, err
-	}
-	return data, nil
-}
-
 // uniqueDirectory returns "path/name" if that directory doesn't exist.  If it
 // does, the method starts appending .1, .2, etc until a unique name is found.
 func uniqueDirectory(path, name string) (string, error) {

=== added file 'container/userdata.go'
--- container/userdata.go	1970-01-01 00:00:00 +0000
+++ container/userdata.go	2013-11-12 00:35:51 +0000
@@ -0,0 +1,81 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package container
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"regexp"
+	"strings"
+
+	"launchpad.net/loggo"
+
+	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/utils"
+)
+
+var (
+	logger         = loggo.GetLogger("juju.container.lxc")
+	aptHTTPProxyRE = regexp.MustCompile(`(?i)^Acquire::HTTP::Proxy\s+"([^"]+)";$`)
+)
+
+func WriteUserData(machineConfig *cloudinit.MachineConfig, directory string) (string, error) {
+	userData, err := cloudInitUserData(machineConfig)
+	if err != nil {
+		logger.Errorf("failed to create user data: %v", err)
+		return "", err
+	}
+	userDataFilename := filepath.Join(directory, "cloud-init")
+	if err := ioutil.WriteFile(userDataFilename, userData, 0644); err != nil {
+		logger.Errorf("failed to write user data: %v", err)
+		return "", err
+	}
+	return userDataFilename, nil
+}
+
+func cloudInitUserData(machineConfig *cloudinit.MachineConfig) ([]byte, error) {
+	// consider not having this line hardcoded...
+	machineConfig.DataDir = "/var/lib/juju"
+	cloudConfig, err := cloudinit.New(machineConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	// Run apt-config to fetch proxy settings from host. If no proxy
+	// settings are configured, then we don't set up any proxy information
+	// on the container.
+	proxyConfig, err := utils.AptConfigProxy()
+	if err != nil {
+		return nil, err
+	}
+	if proxyConfig != "" {
+		var proxyLines []string
+		for _, line := range strings.Split(proxyConfig, "\n") {
+			line = strings.TrimSpace(line)
+			if len(line) > 0 {
+				if m := aptHTTPProxyRE.FindStringSubmatch(line); m != nil {
+					cloudConfig.SetAptProxy(m[1])
+				} else {
+					proxyLines = append(proxyLines, line)
+				}
+			}
+		}
+		if len(proxyLines) > 0 {
+			cloudConfig.AddFile(
+				"/etc/apt/apt.conf.d/99proxy-extra",
+				strings.Join(proxyLines, "\n"),
+				0644)
+		}
+	}
+
+	// Run ifconfig to get the addresses of the internal container at least
+	// logged in the host.
+	cloudConfig.AddRunCmd("ifconfig")
+
+	data, err := cloudConfig.Render()
+	if err != nil {
+		return nil, err
+	}
+	return data, nil
+}