Merge lp:~dimitern/juju-core/290-lp-1240667-cloud-tools-priority into lp:~go-bot/juju-core/trunk

[Dimiter Naydenov]

Fixed bug #1240667: pin priority for cloud-tools

This introduces several new calls in cloudinit,
which deal with apt sources, their preferences
(/etc/apt/preferences.d/) and packages that we
install from there.

Basically, when deploying a precise machine that
needs to install mongodb-server package, we add
the cloud-tools pocket, but by default this will
add it as an apt source with a higher priority
than the main archive. As a consequence, charms
that try to install packages from main, which
are also in the cloud-tools pocket get the latter,
rather than the former (i.e. the described problem
with python-django's version 1.5 in cloud-tools vs.
1.14 in main, which breaks openstack-dashboard charm).

Now, when adding the cloud-tools archive we also
change its apt preferences, so that its priority
as an apt source is lower than main, which means
when installing mongodb-server during cloudinit
we need to explicitly specity --target-release
'precise-updates/cloud-tools' to pick the version
from cloud-tools.

Live tested on EC2 and works as expected.

https://codereview.appspot.com/61410051/

R=gz, rogpeppe

Fixed bug #1240667: pin priority for cloud-tools

This introduces several new calls in cloudinit,
which deal with apt sources, their preferences
(/etc/apt/preferences.d/) and packages that we
install from there.

Basically, when deploying a precise machine that
needs to install mongodb-server package, we add
the cloud-tools pocket, but by default this will
add it as an apt source with a higher priority
than the main archive. As a consequence, charms
that try to install packages from main, which
are also in the cloud-tools pocket get the latter,
rather than the former (i.e. the described problem
with python-django's version 1.5 in cloud-tools vs.
1.14 in main, which breaks openstack-dashboard charm).

Now, when adding the cloud-tools archive we also
change its apt preferences, so that its priority
as an apt source is lower than main, which means
when installing mongodb-server during cloudinit
we need to explicitly specity --target-release
'precise-updates/cloud-tools' to pick the version
from cloud-tools.

Live tested on EC2 and works as expected.

https://codereview.appspot.com/61410051/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+205816_code.launchpad.net,

Message:
Please take a look.

Description:
Fixed bug #1240667: pin priority for cloud-tools

This introduces several new calls in cloudinit,
which deal with apt sources, their preferences
(/etc/apt/preferences.d/) and packages that we
install from there.

Basically, when deploying a precise machine that
needs to install mongodb-server package, we add
the cloud-tools pocket, but by default this will
add it as an apt source with a higher priority
than the main archive. As a consequence, charms
that try to install packages from main, which
are also in the cloud-tools pocket get the latter,
rather than the former (i.e. the described problem
with python-django's version 1.5 in cloud-tools vs.
1.14 in main, which breaks openstack-dashboard charm).

Now, when adding the cloud-tools archive we also
change its apt preferences, so that its priority
as an apt source is lower than main, which means
when installing mongodb-server during cloudinit
we need to explicitly specity --target-release
'precise-updates/cloud-tools' to pick the version
from cloud-tools.

Live tested on EC2 and works as expected.

https://code.launchpad.net/~dimitern/juju-core/290-lp-1240667-cloud-tools-priority/+merge/205816

(do not edit description out of merge proposal)

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

Affected files (+151, -22 lines):
   A [revision details]
   M cloudinit/cloudinit.go
   M cloudinit/cloudinit_test.go
   M cloudinit/options.go
   M cloudinit/sshinit/configure.go
   M cloudinit/sshinit/configure_test.go
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go

[Martin Packman (gz)]

LGTM if we get sign-off from smoser.

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go
File cloudinit/options.go (right):

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go#newcode134
cloudinit/options.go:134: cfg.AddPackage(fmt.Sprintf("--target-release
'%s' '%s'", targetRelease, packageName))
This is the only dodgy bit of the operation, I'm not sure if we should
really rely on cloud-init passing through flags on the packages key like
this, but given our tie to a specific older cloud-init on precise, as it
happens to work we can probably get away with it. Let's check with
Scott.

https://codereview.appspot.com/61410051/

[Roger Peppe (rogpeppe)]

LGTM modulo mgz's concerns and some suggestions below.

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go
File cloudinit/options.go (right):

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go#newcode17
cloudinit/options.go:17: // AptPreferencesTemplate defines the format
used to create an
I don't really like using printf format strings as poor-man's
templates, particularly when the data is already in a struct,
and we have text/template.

I'd do something like:

var aptPreferences = template.Must(template.New("").Parse(
`Explanation: {{.Explanation}}
Package: {{.Package}}
Pin: {{.Pin}}
Pin-Priority: {{.PinPriority}}
`))

Then below, you can do:

if prefs != nil {
     var buf bytes.Buffer
     err := aptPreferencesTemplate.Execute(&buf, prefs)
     if err != nil {
         panic(err)
     }
     cfg.AddFile(prefs.Path, buf.String(), 0644)
}

then the template is easier to read and easier to change
in the future.

Also, rather than exporting the template, you could
define a method on AptPreferences to return the
file contents:

e.g.

// FileContents returns file contents suitable
// for setting up the given apt preferences.
func (prefs *AptPreferences) FileContents() string

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go#newcode134
cloudinit/options.go:134: cfg.AddPackage(fmt.Sprintf("--target-release
'%s' '%s'", targetRelease, packageName))
+1 to mgz's remarks.
Also:

cfg.AddPackage(fmt.Sprintf("--target-release %s %s",
utils.ShQuote(targetRelease), utils.ShQuote(packageName)))

https://codereview.appspot.com/61410051/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/61410051/diff/1/environs/cloudinit/cloudinit.go#newcode213
environs/cloudinit/cloudinit.go:213: c.AddFile(noncefile,
cfg.MachineNonce, 0644)
+1 to this drive-by, assuming it's tested.

https://codereview.appspot.com/61410051/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go
File cloudinit/options.go (right):

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go#newcode17
cloudinit/options.go:17: // AptPreferencesTemplate defines the format
used to create an
On 2014/02/11 17:31:54, rog wrote:
> I don't really like using printf format strings as poor-man's
> templates, particularly when the data is already in a struct,
> and we have text/template.

> I'd do something like:

> var aptPreferences = template.Must(template.New("").Parse(
> `Explanation: {{.Explanation}}
> Package: {{.Package}}
> Pin: {{.Pin}}
> Pin-Priority: {{.PinPriority}}
> `))

> Then below, you can do:

> if prefs != nil {
> var buf bytes.Buffer
> err := aptPreferencesTemplate.Execute(&buf, prefs)
> if err != nil {
> panic(err)
> }
> cfg.AddFile(prefs.Path, buf.String(), 0644)
> }

> then the template is easier to read and easier to change
> in the future.

> Also, rather than exporting the template, you could
> define a method on AptPreferences to return the
> file contents:

> e.g.

> // FileContents returns file contents suitable
> // for setting up the given apt preferences.
> func (prefs *AptPreferences) FileContents() string

Good point, done!

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go#newcode134
cloudinit/options.go:134: cfg.AddPackage(fmt.Sprintf("--target-release
'%s' '%s'", targetRelease, packageName))
On 2014/02/11 17:05:01, gz wrote:
> This is the only dodgy bit of the operation, I'm not sure if we should
really
> rely on cloud-init passing through flags on the packages key like
this, but
> given our tie to a specific older cloud-init on precise, as it happens
to work
> we can probably get away with it. Let's check with Scott.

As discussed on IRC with smoser, this is fine and I tested it to do the
right thing. It's much simpler than passing an entire certificate file
with rigid formatting through cloudinit. I added tests for all newly
generated code.

https://codereview.appspot.com/61410051/diff/1/cloudinit/options.go#newcode134
cloudinit/options.go:134: cfg.AddPackage(fmt.Sprintf("--target-release
'%s' '%s'", targetRelease, packageName))
On 2014/02/11 17:31:54, rog wrote:
> +1 to mgz's remarks.
> Also:

> cfg.AddPackage(fmt.Sprintf("--target-release %s %s",
> utils.ShQuote(targetRelease), utils.ShQuote(packageName)))

I don't want to shquote these here, because they weren't escaped before
and they get escaped at rendering time, somewhat differently in
cloudinit and sshinit, after doing some checks.

https://codereview.appspot.com/61410051/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/61410051/diff/1/environs/cloudinit/cloudinit.go#newcode213
environs/cloudinit/cloudinit.go:213: c.AddFile(noncefile,
cfg.MachineNonce, 0644)
On 2014/02/11 17:31:54, rog wrote:
> +1 to this drive-by, assuming it's tested.

Yep, it is - works fine.

https://codereview.appspot.com/61410051/

[Andrew Wilkins (axwalk)]

https://codereview.appspot.com/61410051/diff/20001/cloudinit/options.go
File cloudinit/options.go (right):

https://codereview.appspot.com/61410051/diff/20001/cloudinit/options.go#newcode91
cloudinit/options.go:91: cfg.AddFile(prefs.Path, prefs.FileContents(),
0644)
Did you live test both add-machine and bootstrap?

I don't see how this could have worked as expected in a live test of
add-machine (which uses cloud-init all the way, as opposed to cloud-init
+ "sshinit"). AddFile is implemented in terms of runcmd; runcmd runs
after packages are installed.

I'm really not keen on this approach at all. I think environs/cloudinit
should be modified to add a bootcmd in ConfigureBasic. Bootcmds are run
before packages are installed.

https://codereview.appspot.com/61410051/diff/20001/cloudinit/sshinit/configure.go
File cloudinit/sshinit/configure.go (right):

https://codereview.appspot.com/61410051/diff/20001/cloudinit/sshinit/configure.go#newcode144
cloudinit/sshinit/configure.go:144: if src.Prefs != nil {
Is there a good reason to duplicate this logic here and in
cloudinit/options.go? Anything that's not core cloud-init functionality
should really belong in environs/cloudinit.

https://codereview.appspot.com/61410051/

[Dimiter Naydenov (dimitern)]

https://codereview.appspot.com/61410051/diff/20001/cloudinit/options.go
File cloudinit/options.go (right):

https://codereview.appspot.com/61410051/diff/20001/cloudinit/options.go#newcode91
cloudinit/options.go:91: cfg.AddFile(prefs.Path, prefs.FileContents(),
0644)
On 2014/02/12 01:25:42, axw wrote:
> Did you live test both add-machine and bootstrap?

> I don't see how this could have worked as expected in a live test of
add-machine
> (which uses cloud-init all the way, as opposed to cloud-init +
"sshinit").
> AddFile is implemented in terms of runcmd; runcmd runs after packages
are
> installed.

> I'm really not keen on this approach at all. I think
environs/cloudinit should
> be modified to add a bootcmd in ConfigureBasic. Bootcmds are run
before packages
> are installed.

It works all the same - the repository is added and with it the prefs
file gets created. Just tested it carefully again with add-machine and a
fresh precise VM.

About the approach, since it fixes the bug, which I was aiming to do,
it's adequate, if not the best one, I agree. If you feel strongly that
it should be changed, please propose a follow-up?

https://codereview.appspot.com/61410051/diff/20001/cloudinit/sshinit/configure.go
File cloudinit/sshinit/configure.go (right):

https://codereview.appspot.com/61410051/diff/20001/cloudinit/sshinit/configure.go#newcode144
cloudinit/sshinit/configure.go:144: if src.Prefs != nil {
On 2014/02/12 01:25:42, axw wrote:
> Is there a good reason to duplicate this logic here and in
cloudinit/options.go?
> Anything that's not core cloud-init functionality should really belong
in
> environs/cloudinit.

This is due to the fact that runcmds are run after everything else, as
you mentioned. So I had to pull it out here in order for it to work.

https://codereview.appspot.com/61410051/

=== modified file 'cloudinit/cloudinit.go'
--- cloudinit/cloudinit.go	2013-11-26 12:24:48 +0000
+++ cloudinit/cloudinit.go	2014-02-11 20:01:44 +0000
@@ -7,6 +7,9 @@
 package cloudinit
 
 import (
+	"bytes"
+	"text/template"
+
 	yaml "launchpad.net/goyaml"
 )
 
@@ -38,10 +41,41 @@
 }
 
 // AptSource is an apt(8) source, comprising a source location,
-// with an optional Key.
+// with an optional Key, and optional apt_preferences(5).
 type AptSource struct {
-	Source string `yaml:"source"`
-	Key    string `yaml:"key,omitempty"`
+	Source string          `yaml:"source"`
+	Key    string          `yaml:"key,omitempty"`
+	Prefs  *AptPreferences `yaml:"-"`
+}
+
+// AptPreferences is a set of apt_preferences(5) compatible
+// preferences for an apt source. It can be used to override the
+// default priority for the source. Path where the file will be
+// created (usually in /etc/apt/preferences.d/).
+type AptPreferences struct {
+	Path        string
+	Explanation string
+	Package     string
+	Pin         string
+	PinPriority int
+}
+
+// FileContents generates an apt_preferences(5) file from the fields
+// in prefs.
+func (prefs *AptPreferences) FileContents() string {
+	const prefTemplate = `
+Explanation: {{.Explanation}}
+Package: {{.Package}}
+Pin: {{.Pin}}
+Pin-Priority: {{.PinPriority}}
+`
+	var buf bytes.Buffer
+	t := template.Must(template.New("").Parse(prefTemplate[1:]))
+	err := t.Execute(&buf, prefs)
+	if err != nil {
+		panic(err)
+	}
+	return buf.String()
 }
 
 // command represents a shell command.

=== modified file 'cloudinit/cloudinit_test.go'
--- cloudinit/cloudinit_test.go	2013-12-16 07:20:01 +0000
+++ cloudinit/cloudinit_test.go	2014-02-11 20:01:44 +0000
@@ -177,7 +177,35 @@
 		"AptSources",
 		"apt_sources:\n- source: keyName\n  key: someKey\n",
 		func(cfg *cloudinit.Config) {
-			cfg.AddAptSource("keyName", "someKey")
+			cfg.AddAptSource("keyName", "someKey", nil)
+		},
+	},
+	{
+		"AptSources with preferences",
+		`apt_sources:
+- source: keyName
+  key: someKey
+runcmd:
+- install -D -m 644 /dev/null '/some/path'
+- 'printf ''%s\n'' ''Explanation: test
+
+  Package: *
+
+  Pin: release n=series
+
+  Pin-Priority: 123
+
+  '' > ''/some/path'''
+`,
+		func(cfg *cloudinit.Config) {
+			prefs := &cloudinit.AptPreferences{
+				Path:        "/some/path",
+				Explanation: "test",
+				Package:     "*",
+				Pin:         "release n=series",
+				PinPriority: 123,
+			}
+			cfg.AddAptSource("keyName", "someKey", prefs)
 		},
 	},
 	{
@@ -189,6 +217,13 @@
 		},
 	},
 	{
+		"Packages with --target-release",
+		"packages:\n- --target-release 'precise-updates/cloud-tools' 'mongodb-server'\n",
+		func(cfg *cloudinit.Config) {
+			cfg.AddPackageFromTargetRelease("mongodb-server", "precise-updates/cloud-tools")
+		},
+	},
+	{
 		"BootCmd",
 		"bootcmd:\n- ls > /dev\n- - ls\n  - '>with space'\n",
 		func(cfg *cloudinit.Config) {
@@ -276,7 +311,11 @@
 	c.Assert(cfg.Packages(), gc.HasLen, 0)
 	cfg.AddPackage("a b c")
 	cfg.AddPackage("d!")
-	c.Assert(cfg.Packages(), gc.DeepEquals, []string{"a b c", "d!"})
+	expectedPackages := []string{"a b c", "d!"}
+	c.Assert(cfg.Packages(), gc.DeepEquals, expectedPackages)
+	cfg.AddPackageFromTargetRelease("package", "series")
+	expectedPackages = append(expectedPackages, "--target-release 'series' 'package'")
+	c.Assert(cfg.Packages(), gc.DeepEquals, expectedPackages)
 }
 
 func (S) TestSetOutput(c *gc.C) {

=== modified file 'cloudinit/options.go'
--- cloudinit/options.go	2013-12-20 00:30:15 +0000
+++ cloudinit/options.go	2014-02-11 20:01:44 +0000
@@ -10,6 +10,10 @@
 	"launchpad.net/juju-core/utils/ssh"
 )
 
+// CloudToolsPrefsPath defines the default location of
+// apt_preferences(5) file for the cloud-tools pocket.
+const CloudToolsPrefsPath = "/etc/apt/preferences.d/50-cloud-tools"
+
 // SetAttr sets an arbitrary attribute in the cloudinit config.
 // If value is nil the attribute will be deleted; otherwise
 // the value will be marshalled according to the rules
@@ -73,13 +77,19 @@
 
 // AddAptSource adds an apt source. The key holds the
 // public key of the source, in the form expected by apt-key(8).
-func (cfg *Config) AddAptSource(name, key string) {
+func (cfg *Config) AddAptSource(name, key string, prefs *AptPreferences) {
 	src, _ := cfg.attrs["apt_sources"].([]*AptSource)
 	cfg.attrs["apt_sources"] = append(src,
 		&AptSource{
 			Source: name,
 			Key:    key,
-		})
+			Prefs:  prefs,
+		},
+	)
+	if prefs != nil {
+		// Create the apt preferences file.
+		cfg.AddFile(prefs.Path, prefs.FileContents(), 0644)
+	}
 }
 
 // AptSources returns the apt sources added with AddAptSource.
@@ -102,6 +112,13 @@
 	cfg.attrs["packages"] = append(cfg.Packages(), name)
 }
 
+// AddPackageFromTargetRelease adds a package to be installed using
+// the given release, passed to apt-get with the --target-release
+// argument.
+func (cfg *Config) AddPackageFromTargetRelease(packageName, targetRelease string) {
+	cfg.AddPackage(fmt.Sprintf("--target-release '%s' '%s'", targetRelease, packageName))
+}
+
 // Packages returns a list of packages that will be
 // installed on first boot.
 func (cfg *Config) Packages() []string {

=== modified file 'cloudinit/sshinit/configure.go'
--- cloudinit/sshinit/configure.go	2014-01-28 04:58:43 +0000
+++ cloudinit/sshinit/configure.go	2014-02-11 20:01:44 +0000
@@ -141,6 +141,12 @@
 		}
 		cmds = append(cmds, cloudinit.LogProgressCmd("Adding apt repository: %s", src.Source))
 		cmds = append(cmds, "add-apt-repository -y "+utils.ShQuote(src.Source))
+		if src.Prefs != nil {
+			path := utils.ShQuote(src.Prefs.Path)
+			contents := utils.ShQuote(src.Prefs.FileContents())
+			cmds = append(cmds, "install -D -m 644 /dev/null "+path)
+			cmds = append(cmds, `printf '%s\n' `+contents+` > `+path)
+		}
 	}
 	if len(cfg.AptSources()) > 0 || cfg.AptUpdate() {
 		cmds = append(cmds, cloudinit.LogProgressCmd("Running apt-get update"))
@@ -152,7 +158,12 @@
 	}
 	for _, pkg := range cfg.Packages() {
 		cmds = append(cmds, cloudinit.LogProgressCmd("Installing package: %s", pkg))
-		cmd := fmt.Sprintf(aptget+"install %s", utils.ShQuote(pkg))
+		if !strings.Contains(pkg, "--target-release") {
+			// We only need to shquote the package name if it does not
+			// contain additional arguments.
+			pkg = utils.ShQuote(pkg)
+		}
+		cmd := fmt.Sprintf(aptget+"install %s", pkg)
 		cmds = append(cmds, cmd)
 	}
 	if len(cmds) > 0 {

=== modified file 'cloudinit/sshinit/configure_test.go'
--- cloudinit/sshinit/configure_test.go	2014-01-16 07:10:50 +0000
+++ cloudinit/sshinit/configure_test.go	2014-02-11 20:01:44 +0000
@@ -103,6 +103,16 @@
 			checkIff(gc.Matches, needsCloudTools),
 			"(.|\n)*add-apt-repository.*cloud-tools(.|\n)*",
 		)
+		c.Assert(
+			script,
+			checkIff(gc.Matches, needsCloudTools),
+			"(.|\n)*Pin: release n=precise-updates/cloud-tools\nPin-Priority: 400(.|\n)*",
+		)
+		c.Assert(
+			script,
+			checkIff(gc.Matches, needsCloudTools),
+			"(.|\n)*install -D -m 644 /dev/null '/etc/apt/preferences.d/50-cloud-tools'(.|\n)*",
+		)
 
 		// Only Quantal requires the PPA (for mongo).
 		needsJujuPPA := series == "quantal"
@@ -143,7 +153,7 @@
 	cfg.SetAptUpdate(true)
 	assertScriptMatches(c, cfg, aptGetUpdatePattern, true)
 	cfg.SetAptUpdate(false)
-	cfg.AddAptSource("source", "key")
+	cfg.AddAptSource("source", "key", nil)
 	assertScriptMatches(c, cfg, aptGetUpdatePattern, true)
 }
 
@@ -152,7 +162,7 @@
 	aptGetUpgradePattern := aptgetRegexp + "upgrade(.|\n)*"
 	cfg := cloudinit.New()
 	cfg.SetAptUpdate(true)
-	cfg.AddAptSource("source", "key")
+	cfg.AddAptSource("source", "key", nil)
 	assertScriptMatches(c, cfg, aptGetUpgradePattern, false)
 	cfg.SetAptUpgrade(true)
 	assertScriptMatches(c, cfg, aptGetUpgradePattern, true)

=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2014-02-05 15:41:44 +0000
+++ environs/cloudinit/cloudinit.go	2014-02-11 20:01:44 +0000
@@ -209,11 +209,8 @@
 	// Note: this must be the last runcmd we do in ConfigureBasic, as
 	// the presence of the nonce file is used to gate the remainder
 	// of synchronous bootstrap.
-	noncefile := shquote(path.Join(cfg.DataDir, NonceFile))
-	c.AddScripts(
-		fmt.Sprintf("install -D -m %o /dev/null %s", 0644, noncefile),
-		fmt.Sprintf(`printf '%%s\n' %s > %s`, shquote(cfg.MachineNonce), noncefile),
-	)
+	noncefile := path.Join(cfg.DataDir, NonceFile)
+	c.AddFile(noncefile, cfg.MachineNonce, 0644)
 	return nil
 }
 
@@ -348,9 +345,17 @@
 
 			if cfg.NeedMongoPPA() {
 				const key = "" // key is loaded from PPA
-				c.AddAptSource("ppa:juju/stable", key)
-			}
-			c.AddPackage("mongodb-server")
+				c.AddAptSource("ppa:juju/stable", key, nil)
+			}
+			if cfg.Tools.Version.Series == "precise" {
+				// In precise we add the cloud-tools pocket and
+				// pin it with a lower priority, so we need to
+				// explicitly specify the target release when
+				// installing mongodb-server from there.
+				c.AddPackageFromTargetRelease("mongodb-server", "precise-updates/cloud-tools")
+			} else {
+				c.AddPackage("mongodb-server")
+			}
 		}
 		certKey := string(cfg.StateServerCert) + string(cfg.StateServerKey)
 		c.AddFile(cfg.dataFile("server.pem"), certKey, 0600)
@@ -601,7 +606,14 @@
 	}
 	const url = "http://ubuntu-cloud.archive.canonical.com/ubuntu"
 	name := fmt.Sprintf("deb %s %s-updates/cloud-tools main", url, series)
-	c.AddAptSource(name, CanonicalCloudArchiveSigningKey)
+	prefs := &cloudinit.AptPreferences{
+		Path:        cloudinit.CloudToolsPrefsPath,
+		Explanation: "Pin with lower priority, not to interfere with charms",
+		Package:     "*",
+		Pin:         fmt.Sprintf("release n=%s-updates/cloud-tools", series),
+		PinPriority: 400,
+	}
+	c.AddAptSource(name, CanonicalCloudArchiveSigningKey, prefs)
 }
 
 func (cfg *MachineConfig) NeedMongoPPA() bool {

=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go	2014-01-30 21:20:10 +0000
+++ environs/cloudinit/cloudinit_test.go	2014-02-11 20:01:44 +0000
@@ -119,6 +119,8 @@
 printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'
 install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
 printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
+install -D -m 644 /dev/null '/etc/apt/preferences\.d/50-cloud-tools'
+printf '%s\\n' '.*' > '/etc/apt/preferences\.d/50-cloud-tools'
 install -D -m 600 /dev/null '/var/lib/juju/system-identity'
 printf '%s\\n' '.*' > '/var/lib/juju/system-identity'
 install -D -m 600 /dev/null '/var/lib/juju/server\.pem'
@@ -559,7 +561,7 @@
 	}
 }
 
-// CheckPackage checks that the cloudinit will or won't install the given
+// checkPackage checks that the cloudinit will or won't install the given
 // package, depending on the value of match.
 func checkPackage(c *gc.C, x map[interface{}]interface{}, pkg string, match bool) {
 	pkgs0 := x["packages"]
@@ -575,7 +577,9 @@
 	found := false
 	for _, p0 := range pkgs {
 		p := p0.(string)
-		if p == pkg {
+		hasTargetRelease := strings.Contains(p, "--target-release")
+		hasQuotedPkg := strings.Contains(p, "'"+pkg+"'")
+		if p == pkg || (hasTargetRelease && hasQuotedPkg) {
 			found = true
 		}
 	}
@@ -587,7 +591,7 @@
 	}
 }
 
-// CheckAptSources checks that the cloudinit will or won't install the given
+// checkAptSources checks that the cloudinit will or won't install the given
 // source, depending on the value of match.
 func checkAptSource(c *gc.C, x map[interface{}]interface{}, source, key string, match bool) {
 	sources0 := x["apt_sources"]