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

Proposed by Dimiter Naydenov
Status: Merged
Approved by: Dimiter Naydenov
Approved revision: no longer in the source branch.
Merged at revision: 2312
Proposed branch: lp:~dimitern/juju-core/290-lp-1240667-cloud-tools-priority
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 344 lines (+149/-22)
7 files modified
cloudinit/cloudinit.go (+37/-3)
cloudinit/cloudinit_test.go (+41/-2)
cloudinit/options.go (+19/-2)
cloudinit/sshinit/configure.go (+12/-1)
cloudinit/sshinit/configure_test.go (+12/-2)
environs/cloudinit/cloudinit.go (+21/-9)
environs/cloudinit/cloudinit_test.go (+7/-3)
To merge this branch: bzr merge lp:~dimitern/juju-core/290-lp-1240667-cloud-tools-priority
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+205816@code.launchpad.net

Commit message

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

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

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

Revision history for this message
Martin Packman (gz) wrote :

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/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

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/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

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/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
7package cloudinit7package cloudinit
88
9import (9import (
10 "bytes"
11 "text/template"
12
10 yaml "launchpad.net/goyaml"13 yaml "launchpad.net/goyaml"
11)14)
1215
@@ -38,10 +41,41 @@
38}41}
3942
40// AptSource is an apt(8) source, comprising a source location,43// AptSource is an apt(8) source, comprising a source location,
41// with an optional Key.44// with an optional Key, and optional apt_preferences(5).
42type AptSource struct {45type AptSource struct {
43 Source string `yaml:"source"`46 Source string `yaml:"source"`
44 Key string `yaml:"key,omitempty"`47 Key string `yaml:"key,omitempty"`
48 Prefs *AptPreferences `yaml:"-"`
49}
50
51// AptPreferences is a set of apt_preferences(5) compatible
52// preferences for an apt source. It can be used to override the
53// default priority for the source. Path where the file will be
54// created (usually in /etc/apt/preferences.d/).
55type AptPreferences struct {
56 Path string
57 Explanation string
58 Package string
59 Pin string
60 PinPriority int
61}
62
63// FileContents generates an apt_preferences(5) file from the fields
64// in prefs.
65func (prefs *AptPreferences) FileContents() string {
66 const prefTemplate = `
67Explanation: {{.Explanation}}
68Package: {{.Package}}
69Pin: {{.Pin}}
70Pin-Priority: {{.PinPriority}}
71`
72 var buf bytes.Buffer
73 t := template.Must(template.New("").Parse(prefTemplate[1:]))
74 err := t.Execute(&buf, prefs)
75 if err != nil {
76 panic(err)
77 }
78 return buf.String()
45}79}
4680
47// command represents a shell command.81// command represents a shell command.
4882
=== 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 @@
177 "AptSources",177 "AptSources",
178 "apt_sources:\n- source: keyName\n key: someKey\n",178 "apt_sources:\n- source: keyName\n key: someKey\n",
179 func(cfg *cloudinit.Config) {179 func(cfg *cloudinit.Config) {
180 cfg.AddAptSource("keyName", "someKey")180 cfg.AddAptSource("keyName", "someKey", nil)
181 },
182 },
183 {
184 "AptSources with preferences",
185 `apt_sources:
186- source: keyName
187 key: someKey
188runcmd:
189- install -D -m 644 /dev/null '/some/path'
190- 'printf ''%s\n'' ''Explanation: test
191
192 Package: *
193
194 Pin: release n=series
195
196 Pin-Priority: 123
197
198 '' > ''/some/path'''
199`,
200 func(cfg *cloudinit.Config) {
201 prefs := &cloudinit.AptPreferences{
202 Path: "/some/path",
203 Explanation: "test",
204 Package: "*",
205 Pin: "release n=series",
206 PinPriority: 123,
207 }
208 cfg.AddAptSource("keyName", "someKey", prefs)
181 },209 },
182 },210 },
183 {211 {
@@ -189,6 +217,13 @@
189 },217 },
190 },218 },
191 {219 {
220 "Packages with --target-release",
221 "packages:\n- --target-release 'precise-updates/cloud-tools' 'mongodb-server'\n",
222 func(cfg *cloudinit.Config) {
223 cfg.AddPackageFromTargetRelease("mongodb-server", "precise-updates/cloud-tools")
224 },
225 },
226 {
192 "BootCmd",227 "BootCmd",
193 "bootcmd:\n- ls > /dev\n- - ls\n - '>with space'\n",228 "bootcmd:\n- ls > /dev\n- - ls\n - '>with space'\n",
194 func(cfg *cloudinit.Config) {229 func(cfg *cloudinit.Config) {
@@ -276,7 +311,11 @@
276 c.Assert(cfg.Packages(), gc.HasLen, 0)311 c.Assert(cfg.Packages(), gc.HasLen, 0)
277 cfg.AddPackage("a b c")312 cfg.AddPackage("a b c")
278 cfg.AddPackage("d!")313 cfg.AddPackage("d!")
279 c.Assert(cfg.Packages(), gc.DeepEquals, []string{"a b c", "d!"})314 expectedPackages := []string{"a b c", "d!"}
315 c.Assert(cfg.Packages(), gc.DeepEquals, expectedPackages)
316 cfg.AddPackageFromTargetRelease("package", "series")
317 expectedPackages = append(expectedPackages, "--target-release 'series' 'package'")
318 c.Assert(cfg.Packages(), gc.DeepEquals, expectedPackages)
280}319}
281320
282func (S) TestSetOutput(c *gc.C) {321func (S) TestSetOutput(c *gc.C) {
283322
=== 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 @@
10 "launchpad.net/juju-core/utils/ssh"10 "launchpad.net/juju-core/utils/ssh"
11)11)
1212
13// CloudToolsPrefsPath defines the default location of
14// apt_preferences(5) file for the cloud-tools pocket.
15const CloudToolsPrefsPath = "/etc/apt/preferences.d/50-cloud-tools"
16
13// SetAttr sets an arbitrary attribute in the cloudinit config.17// SetAttr sets an arbitrary attribute in the cloudinit config.
14// If value is nil the attribute will be deleted; otherwise18// If value is nil the attribute will be deleted; otherwise
15// the value will be marshalled according to the rules19// the value will be marshalled according to the rules
@@ -73,13 +77,19 @@
7377
74// AddAptSource adds an apt source. The key holds the78// AddAptSource adds an apt source. The key holds the
75// public key of the source, in the form expected by apt-key(8).79// public key of the source, in the form expected by apt-key(8).
76func (cfg *Config) AddAptSource(name, key string) {80func (cfg *Config) AddAptSource(name, key string, prefs *AptPreferences) {
77 src, _ := cfg.attrs["apt_sources"].([]*AptSource)81 src, _ := cfg.attrs["apt_sources"].([]*AptSource)
78 cfg.attrs["apt_sources"] = append(src,82 cfg.attrs["apt_sources"] = append(src,
79 &AptSource{83 &AptSource{
80 Source: name,84 Source: name,
81 Key: key,85 Key: key,
82 })86 Prefs: prefs,
87 },
88 )
89 if prefs != nil {
90 // Create the apt preferences file.
91 cfg.AddFile(prefs.Path, prefs.FileContents(), 0644)
92 }
83}93}
8494
85// AptSources returns the apt sources added with AddAptSource.95// AptSources returns the apt sources added with AddAptSource.
@@ -102,6 +112,13 @@
102 cfg.attrs["packages"] = append(cfg.Packages(), name)112 cfg.attrs["packages"] = append(cfg.Packages(), name)
103}113}
104114
115// AddPackageFromTargetRelease adds a package to be installed using
116// the given release, passed to apt-get with the --target-release
117// argument.
118func (cfg *Config) AddPackageFromTargetRelease(packageName, targetRelease string) {
119 cfg.AddPackage(fmt.Sprintf("--target-release '%s' '%s'", targetRelease, packageName))
120}
121
105// Packages returns a list of packages that will be122// Packages returns a list of packages that will be
106// installed on first boot.123// installed on first boot.
107func (cfg *Config) Packages() []string {124func (cfg *Config) Packages() []string {
108125
=== 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 @@
141 }141 }
142 cmds = append(cmds, cloudinit.LogProgressCmd("Adding apt repository: %s", src.Source))142 cmds = append(cmds, cloudinit.LogProgressCmd("Adding apt repository: %s", src.Source))
143 cmds = append(cmds, "add-apt-repository -y "+utils.ShQuote(src.Source))143 cmds = append(cmds, "add-apt-repository -y "+utils.ShQuote(src.Source))
144 if src.Prefs != nil {
145 path := utils.ShQuote(src.Prefs.Path)
146 contents := utils.ShQuote(src.Prefs.FileContents())
147 cmds = append(cmds, "install -D -m 644 /dev/null "+path)
148 cmds = append(cmds, `printf '%s\n' `+contents+` > `+path)
149 }
144 }150 }
145 if len(cfg.AptSources()) > 0 || cfg.AptUpdate() {151 if len(cfg.AptSources()) > 0 || cfg.AptUpdate() {
146 cmds = append(cmds, cloudinit.LogProgressCmd("Running apt-get update"))152 cmds = append(cmds, cloudinit.LogProgressCmd("Running apt-get update"))
@@ -152,7 +158,12 @@
152 }158 }
153 for _, pkg := range cfg.Packages() {159 for _, pkg := range cfg.Packages() {
154 cmds = append(cmds, cloudinit.LogProgressCmd("Installing package: %s", pkg))160 cmds = append(cmds, cloudinit.LogProgressCmd("Installing package: %s", pkg))
155 cmd := fmt.Sprintf(aptget+"install %s", utils.ShQuote(pkg))161 if !strings.Contains(pkg, "--target-release") {
162 // We only need to shquote the package name if it does not
163 // contain additional arguments.
164 pkg = utils.ShQuote(pkg)
165 }
166 cmd := fmt.Sprintf(aptget+"install %s", pkg)
156 cmds = append(cmds, cmd)167 cmds = append(cmds, cmd)
157 }168 }
158 if len(cmds) > 0 {169 if len(cmds) > 0 {
159170
=== 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 @@
103 checkIff(gc.Matches, needsCloudTools),103 checkIff(gc.Matches, needsCloudTools),
104 "(.|\n)*add-apt-repository.*cloud-tools(.|\n)*",104 "(.|\n)*add-apt-repository.*cloud-tools(.|\n)*",
105 )105 )
106 c.Assert(
107 script,
108 checkIff(gc.Matches, needsCloudTools),
109 "(.|\n)*Pin: release n=precise-updates/cloud-tools\nPin-Priority: 400(.|\n)*",
110 )
111 c.Assert(
112 script,
113 checkIff(gc.Matches, needsCloudTools),
114 "(.|\n)*install -D -m 644 /dev/null '/etc/apt/preferences.d/50-cloud-tools'(.|\n)*",
115 )
106116
107 // Only Quantal requires the PPA (for mongo).117 // Only Quantal requires the PPA (for mongo).
108 needsJujuPPA := series == "quantal"118 needsJujuPPA := series == "quantal"
@@ -143,7 +153,7 @@
143 cfg.SetAptUpdate(true)153 cfg.SetAptUpdate(true)
144 assertScriptMatches(c, cfg, aptGetUpdatePattern, true)154 assertScriptMatches(c, cfg, aptGetUpdatePattern, true)
145 cfg.SetAptUpdate(false)155 cfg.SetAptUpdate(false)
146 cfg.AddAptSource("source", "key")156 cfg.AddAptSource("source", "key", nil)
147 assertScriptMatches(c, cfg, aptGetUpdatePattern, true)157 assertScriptMatches(c, cfg, aptGetUpdatePattern, true)
148}158}
149159
@@ -152,7 +162,7 @@
152 aptGetUpgradePattern := aptgetRegexp + "upgrade(.|\n)*"162 aptGetUpgradePattern := aptgetRegexp + "upgrade(.|\n)*"
153 cfg := cloudinit.New()163 cfg := cloudinit.New()
154 cfg.SetAptUpdate(true)164 cfg.SetAptUpdate(true)
155 cfg.AddAptSource("source", "key")165 cfg.AddAptSource("source", "key", nil)
156 assertScriptMatches(c, cfg, aptGetUpgradePattern, false)166 assertScriptMatches(c, cfg, aptGetUpgradePattern, false)
157 cfg.SetAptUpgrade(true)167 cfg.SetAptUpgrade(true)
158 assertScriptMatches(c, cfg, aptGetUpgradePattern, true)168 assertScriptMatches(c, cfg, aptGetUpgradePattern, true)
159169
=== 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 @@
209 // Note: this must be the last runcmd we do in ConfigureBasic, as209 // Note: this must be the last runcmd we do in ConfigureBasic, as
210 // the presence of the nonce file is used to gate the remainder210 // the presence of the nonce file is used to gate the remainder
211 // of synchronous bootstrap.211 // of synchronous bootstrap.
212 noncefile := shquote(path.Join(cfg.DataDir, NonceFile))212 noncefile := path.Join(cfg.DataDir, NonceFile)
213 c.AddScripts(213 c.AddFile(noncefile, cfg.MachineNonce, 0644)
214 fmt.Sprintf("install -D -m %o /dev/null %s", 0644, noncefile),
215 fmt.Sprintf(`printf '%%s\n' %s > %s`, shquote(cfg.MachineNonce), noncefile),
216 )
217 return nil214 return nil
218}215}
219216
@@ -348,9 +345,17 @@
348345
349 if cfg.NeedMongoPPA() {346 if cfg.NeedMongoPPA() {
350 const key = "" // key is loaded from PPA347 const key = "" // key is loaded from PPA
351 c.AddAptSource("ppa:juju/stable", key)348 c.AddAptSource("ppa:juju/stable", key, nil)
352 }349 }
353 c.AddPackage("mongodb-server")350 if cfg.Tools.Version.Series == "precise" {
351 // In precise we add the cloud-tools pocket and
352 // pin it with a lower priority, so we need to
353 // explicitly specify the target release when
354 // installing mongodb-server from there.
355 c.AddPackageFromTargetRelease("mongodb-server", "precise-updates/cloud-tools")
356 } else {
357 c.AddPackage("mongodb-server")
358 }
354 }359 }
355 certKey := string(cfg.StateServerCert) + string(cfg.StateServerKey)360 certKey := string(cfg.StateServerCert) + string(cfg.StateServerKey)
356 c.AddFile(cfg.dataFile("server.pem"), certKey, 0600)361 c.AddFile(cfg.dataFile("server.pem"), certKey, 0600)
@@ -601,7 +606,14 @@
601 }606 }
602 const url = "http://ubuntu-cloud.archive.canonical.com/ubuntu"607 const url = "http://ubuntu-cloud.archive.canonical.com/ubuntu"
603 name := fmt.Sprintf("deb %s %s-updates/cloud-tools main", url, series)608 name := fmt.Sprintf("deb %s %s-updates/cloud-tools main", url, series)
604 c.AddAptSource(name, CanonicalCloudArchiveSigningKey)609 prefs := &cloudinit.AptPreferences{
610 Path: cloudinit.CloudToolsPrefsPath,
611 Explanation: "Pin with lower priority, not to interfere with charms",
612 Package: "*",
613 Pin: fmt.Sprintf("release n=%s-updates/cloud-tools", series),
614 PinPriority: 400,
615 }
616 c.AddAptSource(name, CanonicalCloudArchiveSigningKey, prefs)
605}617}
606618
607func (cfg *MachineConfig) NeedMongoPPA() bool {619func (cfg *MachineConfig) NeedMongoPPA() bool {
608620
=== 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 @@
119printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'119printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'
120install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'120install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
121printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'121printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
122install -D -m 644 /dev/null '/etc/apt/preferences\.d/50-cloud-tools'
123printf '%s\\n' '.*' > '/etc/apt/preferences\.d/50-cloud-tools'
122install -D -m 600 /dev/null '/var/lib/juju/system-identity'124install -D -m 600 /dev/null '/var/lib/juju/system-identity'
123printf '%s\\n' '.*' > '/var/lib/juju/system-identity'125printf '%s\\n' '.*' > '/var/lib/juju/system-identity'
124install -D -m 600 /dev/null '/var/lib/juju/server\.pem'126install -D -m 600 /dev/null '/var/lib/juju/server\.pem'
@@ -559,7 +561,7 @@
559 }561 }
560}562}
561563
562// CheckPackage checks that the cloudinit will or won't install the given564// checkPackage checks that the cloudinit will or won't install the given
563// package, depending on the value of match.565// package, depending on the value of match.
564func checkPackage(c *gc.C, x map[interface{}]interface{}, pkg string, match bool) {566func checkPackage(c *gc.C, x map[interface{}]interface{}, pkg string, match bool) {
565 pkgs0 := x["packages"]567 pkgs0 := x["packages"]
@@ -575,7 +577,9 @@
575 found := false577 found := false
576 for _, p0 := range pkgs {578 for _, p0 := range pkgs {
577 p := p0.(string)579 p := p0.(string)
578 if p == pkg {580 hasTargetRelease := strings.Contains(p, "--target-release")
581 hasQuotedPkg := strings.Contains(p, "'"+pkg+"'")
582 if p == pkg || (hasTargetRelease && hasQuotedPkg) {
579 found = true583 found = true
580 }584 }
581 }585 }
@@ -587,7 +591,7 @@
587 }591 }
588}592}
589593
590// CheckAptSources checks that the cloudinit will or won't install the given594// checkAptSources checks that the cloudinit will or won't install the given
591// source, depending on the value of match.595// source, depending on the value of match.
592func checkAptSource(c *gc.C, x map[interface{}]interface{}, source, key string, match bool) {596func checkAptSource(c *gc.C, x map[interface{}]interface{}, source, key string, match bool) {
593 sources0 := x["apt_sources"]597 sources0 := x["apt_sources"]

Subscribers

People subscribed via source and target branches

to status/vote changes: