Merge lp:~thumper/juju-core/maybe-add-cloud-archive-func into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Make MaybeAddCoudArchiveCloudTools a simple func

Simple change that will be used further down the pipe.
Will be called by the code that makes the lxc template
image, but there will be no machine config object.

https://codereview.appspot.com/72050045/

Make MaybeAddCoudArchiveCloudTools a simple func

Simple change that will be used further down the pipe.
Will be called by the code that makes the lxc template
image, but there will be no machine config object.

https://codereview.appspot.com/72050045/

[Tim Penhey (thumper)]

Reviewers: mp+209798_code.launchpad.net,

Message:
Please take a look.

Description:
Make MaybeAddCoudArchiveCloudTools a simple func

Simple change that will be used further down the pipe.
Will be called by the code that makes the lxc template
image, but there will be no machine config object.

https://code.launchpad.net/~thumper/juju-core/maybe-add-cloud-archive-func/+merge/209798

Requires:
https://code.launchpad.net/~thumper/juju-core/move-to-juju-loggo/+merge/209530

(do not edit description out of merge proposal)

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

Affected files (+5, -3 lines):
   A [revision details]
   M environs/cloudinit/cloudinit.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: <email address hidden>
+New revision: <email address hidden>

Index: environs/cloudinit/cloudinit.go
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2014-02-27 05:24:43 +0000
+++ environs/cloudinit/cloudinit.go 2014-03-06 21:55:44 +0000
@@ -326,7 +326,8 @@
   // for series that need it. This gives us up-to-date LXC,
   // MongoDB, and other infrastructure.
   if !cfg.DisablePackageCommands {
- cfg.MaybeAddCloudArchiveCloudTools(c)
+ series := cfg.Tools.Version.Series
+ MaybeAddCloudArchiveCloudTools(c, series)
   }

   if cfg.StateServer {
@@ -573,8 +574,7 @@

  // MaybeAddCloudArchiveCloudTools adds the cloud-archive cloud-tools
  // pocket to apt sources, if the series requires it.
-func (cfg *MachineConfig) MaybeAddCloudArchiveCloudTools(c
*cloudinit.Config) {
- series := cfg.Tools.Version.Series
+func MaybeAddCloudArchiveCloudTools(c *cloudinit.Config, series string) {
   if series != "precise" {
    // Currently only precise; presumably we'll
    // need to add each LTS in here as they're

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/72050045/

[Ian Booth (wallyworld)]

LGTM

https://codereview.appspot.com/72050045/

[Go Bot (go-bot)]

The attempt to merge lp:~thumper/juju-core/maybe-add-cloud-archive-func into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.050s
ok launchpad.net/juju-core/agent/mongo 0.512s
ok launchpad.net/juju-core/agent/tools 0.199s
ok launchpad.net/juju-core/bzr 5.060s
ok launchpad.net/juju-core/cert 2.940s
ok launchpad.net/juju-core/charm 0.472s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cloudinit/sshinit 0.743s
ok launchpad.net/juju-core/cmd 0.183s
ok launchpad.net/juju-core/cmd/charm-admin 0.726s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]

----------------------------------------------------------------------
FAIL: publish_test.go:75: PublishSuite.SetUpTest

publish_test.go:88:
    c.Assert(err, gc.IsNil)
... value *errors.errorString = &errors.errorString{s:"error running \"bzr init\": bzr: ERROR: Couldn't import bzrlib and dependencies.\nPlease check the directory containing bzrlib is on your PYTHONPATH.\n\nTraceback (most recent call last):\n File \"/home/tarmac/.local/bin/bzr\", line 74, in <module>\n import bzrlib\nImportError: No module named bzrlib\nexit status 1"} ("error running \"bzr init\": bzr: ERROR: Couldn't import bzrlib and dependencies.\nPlease check the directory containing bzrlib is on your PYTHONPATH.\n\nTraceback (most recent call last):\n File \"/home/tarmac/.local/bin/bzr\", line 74, in <module>\n import bzrlib\nImportError: No module named bzrlib\nexit status 1")

----------------------------------------------------------------------
PANIC: publish_test.go:159: PublishSuite.TestBrokenCharm

... Panic: Fixture has panicked (see related PANIC)
OOPS: 182 passed, 2 skipped, 1 FAILED, 17 MISSED
--- FAIL: TestPackage (171.45 seconds)
FAIL
FAIL launchpad.net/juju-core/cmd/juju 171.619s
ok launchpad.net/juju-core/cmd/jujud 64.545s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 8.274s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.158s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.024s
ok launchpad.net/juju-core/container 0.031s
ok launchpad.net/juju-core/container/factory 0.041s
ok launchpad.net/juju-core/container/kvm 0.198s
ok launchpad.net/juju-core/container/kvm/mock 0.038s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.219s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.259s
ok launchpad.net/juju-core/environs 2.510s
ok launchpad.net/juju-core/environs/bootstrap 3.097s
ok launchpad.net/juju-core/environs/cloudinit 0.424s
ok launchpad.net/juju-core/environs/config 3.887s
ok launchpad.ne...

=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2014-02-27 05:24:43 +0000
+++ environs/cloudinit/cloudinit.go	2014-03-06 23:22:52 +0000
@@ -206,6 +206,31 @@
 	return nil
 }
 
+// AddAptCommands update the cloudinit.Config instance with the necessary
+// packages, the request to do the apt-get update/upgrade on boot, and adds
+// the apt proxy settings if there are any.
+func AddAptCommands(proxy osenv.ProxySettings, c *cloudinit.Config) {
+	// Bring packages up-to-date.
+	c.SetAptUpdate(true)
+	c.SetAptUpgrade(true)
+
+	// juju requires git for managing charm directories.
+	c.AddPackage("git")
+	c.AddPackage("cpu-checker")
+	c.AddPackage("bridge-utils")
+	c.AddPackage("rsyslog-gnutls")
+
+	// Write out the apt proxy settings
+	if (proxy != osenv.ProxySettings{}) {
+		filename := utils.AptConfFile
+		c.AddBootCmd(fmt.Sprintf(
+			`[ -f %s ] || (printf '%%s\n' %s > %s)`,
+			filename,
+			shquote(utils.AptProxyContent(proxy)),
+			filename))
+	}
+}
+
 // ConfigureJuju updates the provided cloudinit.Config with configuration
 // to initialise a Juju machine agent.
 func ConfigureJuju(cfg *MachineConfig, c *cloudinit.Config) error {
@@ -230,25 +255,7 @@
 	}
 
 	if !cfg.DisablePackageCommands {
-		// Bring packages up-to-date.
-		c.SetAptUpdate(true)
-		c.SetAptUpgrade(true)
-
-		// juju requires git for managing charm directories.
-		c.AddPackage("git")
-		c.AddPackage("cpu-checker")
-		c.AddPackage("bridge-utils")
-		c.AddPackage("rsyslog-gnutls")
-
-		// Write out the apt proxy settings
-		if (cfg.AptProxySettings != osenv.ProxySettings{}) {
-			filename := utils.AptConfFile
-			c.AddBootCmd(fmt.Sprintf(
-				`[ -f %s ] || (printf '%%s\n' %s > %s)`,
-				filename,
-				shquote(utils.AptProxyContent(cfg.AptProxySettings)),
-				filename))
-		}
+		AddAptCommands(cfg.AptProxySettings, c)
 	}
 
 	// Write out the normal proxy settings so that the settings are
@@ -326,7 +333,8 @@
 	// for series that need it. This gives us up-to-date LXC,
 	// MongoDB, and other infrastructure.
 	if !cfg.DisablePackageCommands {
-		cfg.MaybeAddCloudArchiveCloudTools(c)
+		series := cfg.Tools.Version.Series
+		MaybeAddCloudArchiveCloudTools(c, series)
 	}
 
 	if cfg.StateServer {
@@ -573,8 +581,7 @@
 
 // MaybeAddCloudArchiveCloudTools adds the cloud-archive cloud-tools
 // pocket to apt sources, if the series requires it.
-func (cfg *MachineConfig) MaybeAddCloudArchiveCloudTools(c *cloudinit.Config) {
-	series := cfg.Tools.Version.Series
+func MaybeAddCloudArchiveCloudTools(c *cloudinit.Config, series string) {
 	if series != "precise" {
 		// Currently only precise; presumably we'll
 		// need to add each LTS in here as they're