Merge lp:~frankban/charms/oneiric/buildbot-slave/use-lpsetup into lp:~yellow/charms/oneiric/buildbot-slave/trunk

Proposed by Francesco Banconi
Status: Merged
Approved by: Gary Poster
Approved revision: 32
Merged at revision: 28
Proposed branch: lp:~frankban/charms/oneiric/buildbot-slave/use-lpsetup
Merge into: lp:~yellow/charms/oneiric/buildbot-slave/trunk
Diff against target: 160 lines (+47/-15)
5 files modified
config.yaml (+9/-2)
examples/lpbuildbot.yaml (+5/-5)
hooks/install (+31/-6)
revision (+1/-1)
tests/buildbot-slave.test (+1/-1)
To merge this branch: bzr merge lp:~frankban/charms/oneiric/buildbot-slave/use-lpsetup
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+105086@code.launchpad.net

Description of the change

= Summary =

Use lpsetup in place of setuplxc to create the launchpad testing environment
used by buildbot.

== Implementation details ==

Added a new transport, *apt*, that can be used to retrieve and run a script
when the charm install hook is executed.

Renamed the *url* config option to reflect the fact that it can be either
a url or a package name: now it is called, more generically, *source*.

Updated lpbuildbot yaml file to install and run lpsetup.

Fixed a bug preventing extra packages (as listed in yaml config)
to be installed in the charm instance.

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

Hi Francesco. Cool!

We'll want to have this merged into the official charm as well (https://code.launchpad.net/~charmers/charms/precise/buildbot-slave/trunk and https://code.launchpad.net/~charmers/charms/oneiric/buildbot-slave/trunk), so some of my comments will come from that perspective.

I think we should keep url for backwards compatibility. Say that url is deprecated, and it is an error to provide both url and source. I know it makes the code messy, but I think it is the right thing to do. I'm happy to have the associated variables named "source" in the code, once the backwards compatibility hacks have happened. Also, all the examples should use the "source" variants.

In lpbuildbot.yaml, isn't --use-urandom (or similar) an option in lpsetup also? Or do we always make the urandom->random hack with lpsetup? If we always do it, we should probably change that, IMO: people will not want (or likely need?) that for their dev machines, for instance.

That's it. Thanks!

review: Approve
33. By Francesco Banconi

Backward compatible options.

Revision history for this message
Gary Poster (gary) wrote :

Nice change, Francesco.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2012-02-16 16:42:57 +0000
+++ config.yaml 2012-05-09 09:26:17 +0000
@@ -1,13 +1,20 @@
1options:1options:
2 script-retrieval-method:2 script-retrieval-method:
3 description: |3 description: |
4 How the script is retrieved. You can use bzr, brz_cat, git,4 How the script is retrieved. You can use apt, bzr, brz_cat, git,
5 mercurial or wget.5 mercurial or wget.
6 type: string6 type: string
7 default: bzr7 default: bzr
8 script-source:
9 description: |
10 If script-retrieval-method is "apt", this is the name of the package to
11 install. In the other cases, this is the url where the script lives.
12 type: string
8 script-url:13 script-url:
9 description: |14 description: |
10 The url where this script lives.15 The url where the script lives. This option is DEPRECATED: use
16 `script-source` instead. If both `script-source` and `script-url` are
17 provided, the latter will be ignored.
11 type: string18 type: string
12 script-path:19 script-path:
13 description: |20 description: |
1421
=== modified file 'examples/lpbuildbot.yaml'
--- examples/lpbuildbot.yaml 2012-03-23 15:48:32 +0000
+++ examples/lpbuildbot.yaml 2012-05-09 09:26:17 +0000
@@ -1,11 +1,11 @@
1buildbot-slave:1buildbot-slave:
2 builders: lucid_lp2 builders: lucid_lp
3 script-retrieval-method: bzr_cat3 script-retrieval-method: apt
4 script-url: "http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/utilities/setuplxc.py"4 script-source: lpsetup
5 script-path: setuplxc.py5 script-path: "/usr/bin/lp-setup"
6 # The buildbot user's home directory is /var/lib/buildout.6 # The buildbot user's home directory is /var/lib/buildout.
7 # The LP dependencies will be in /var/lib/buildout/dependencies.7 # The LP dependencies will be in /var/lib/buildout/launchpad/dependencies.
8 script-args: "-u buildbot -e launchpad-pqm@canonical.com -f 'Launchpad PQM' -s launchpad_lxc_id_rsa --use-urandom {installdir}"8 script-args: "lxc-install -u buildbot -e launchpad-pqm@canonical.com -f 'Launchpad PQM' -c {installdir} -S launchpad_lxc_id_rsa --testing"
9 extra-repository: deb http://us.archive.ubuntu.com/ubuntu/ lucid main universe9 extra-repository: deb http://us.archive.ubuntu.com/ubuntu/ lucid main universe
10 buildbot-pkg: buildbot/lucid10 buildbot-pkg: buildbot/lucid
11 extra-packages: testrepository11 extra-packages: testrepository
1212
=== modified file 'hooks/install'
--- hooks/install 2012-04-16 09:23:42 +0000
+++ hooks/install 2012-05-09 09:26:17 +0000
@@ -136,6 +136,11 @@
136bzr = command('bzr')136bzr = command('bzr')
137137
138138
139def apt(source, path):
140 apt_get_install(source)
141 return path
142
143
139def bzr_fetch(source, path):144def bzr_fetch(source, path):
140 apt_get_install('bzr')145 apt_get_install('bzr')
141 target = tempfile.mktemp()146 target = tempfile.mktemp()
@@ -173,6 +178,7 @@
173178
174179
175METHODS = {180METHODS = {
181 'apt': apt,
176 'bzr': bzr_fetch,182 'bzr': bzr_fetch,
177 'bzr_cat': bzr_cat,183 'bzr_cat': bzr_cat,
178 'wget': wget,184 'wget': wget,
@@ -181,19 +187,33 @@
181 }187 }
182188
183189
184def handle_script(retrieve, url, path, args):190def handle_script(retrieve, source, path, args):
185 log('Retrieving script: {}.'.format(url))191 log('Retrieving script: {}.'.format(source))
186 script = retrieve(url, path)192 script = retrieve(source, path)
187 log('Changing script mode.')193 log('Changing script mode.')
188 run('chmod', '+x', script)194 run('chmod', '+x', script)
189 log('Executing script: {}.'.format(script))195 log('Executing script: {}.'.format(script))
190 return subprocess.call([script] + shlex.split(str(args)))196 return subprocess.call([script] + shlex.split(str(args)))
191197
192198
199def fallback_to_url(config):
200 url = config.get('script-url')
201 if url:
202 # The option `script-url` is still allowed but deprecated:
203 # print a deprecation message to stderr.
204 import warnings
205 warnings.simplefilter('default', DeprecationWarning)
206 warnings.warn(
207 'the option `script-url` is deprecated, '
208 'use `script-source` instead.',
209 DeprecationWarning)
210 return url
211
212
193def main():213def main():
194 config = get_config()214 config = get_config()
195 method = config.get('script-retrieval-method')215 method = config.get('script-retrieval-method')
196 url = config.get('script-url')216 source = config.get('script-source') or fallback_to_url(config)
197 path = config.get('script-path')217 path = config.get('script-path')
198 # This is a naive substitution. We can make it more sophisticated218 # This is a naive substitution. We can make it more sophisticated
199 # if we discover we need it. For now, simplicity wins.219 # if we discover we need it. For now, simplicity wins.
@@ -201,6 +221,7 @@
201 buildbot_pkg = config.get('buildbot-pkg')221 buildbot_pkg = config.get('buildbot-pkg')
202 extra_repo = config.get('extra-repository')222 extra_repo = config.get('extra-repository')
203 buildbot_dir = config.get('installdir')223 buildbot_dir = config.get('installdir')
224 extra_pkgs = config.get('extra-packages')
204225
205 if extra_repo:226 if extra_repo:
206 install_extra_repository(extra_repo)227 install_extra_repository(extra_repo)
@@ -215,16 +236,20 @@
215 run('addgroup', 'buildbot')236 run('addgroup', 'buildbot')
216 run('usermod', '--gid', 'buildbot', 'buildbot')237 run('usermod', '--gid', 'buildbot', 'buildbot')
217238
239 # Installing extra packages if required.
240 if extra_pkgs:
241 apt_get_install(*(pkg.strip() for pkg in extra_pkgs.split()))
242
218 config_json.set(config)243 config_json.set(config)
219244
220 retrieve = METHODS.get(method)245 retrieve = METHODS.get(method)
221 if retrieve and url and path:246 if retrieve and source and path:
222 # Make buildbot user have a shell by editing /etc/passwd.247 # Make buildbot user have a shell by editing /etc/passwd.
223 # Otherwise you cannot ssh as this user, which some scripts248 # Otherwise you cannot ssh as this user, which some scripts
224 # need (e.g. those that create lxc containers). We choose sh as249 # need (e.g. those that create lxc containers). We choose sh as
225 # a standard and basic "system" shell.250 # a standard and basic "system" shell.
226 run('usermod', '-s', '/bin/sh', 'buildbot')251 run('usermod', '-s', '/bin/sh', 'buildbot')
227 sys.exit(handle_script(retrieve, url, path, args))252 sys.exit(handle_script(retrieve, source, path, args))
228253
229254
230if __name__ == '__main__':255if __name__ == '__main__':
231256
=== modified file 'revision'
--- revision 2012-03-06 18:23:20 +0000
+++ revision 2012-05-09 09:26:17 +0000
@@ -1,1 +1,1 @@
1819
22
=== modified file 'tests/buildbot-slave.test'
--- tests/buildbot-slave.test 2012-03-01 16:32:37 +0000
+++ tests/buildbot-slave.test 2012-05-09 09:26:17 +0000
@@ -82,7 +82,7 @@
82 options = {}82 options = {}
83 options.update({83 options.update({
84 'script-retrieval-method': 'bzr_cat',84 'script-retrieval-method': 'bzr_cat',
85 'script-url':85 'script-source':
86 'http://bazaar.launchpad.net/~yellow/charms/'86 'http://bazaar.launchpad.net/~yellow/charms/'
87 'oneiric/buildbot-slave/trunk/tests/create_file.py',87 'oneiric/buildbot-slave/trunk/tests/create_file.py',
88 'script-path': 'create_file.py',88 'script-path': 'create_file.py',

Subscribers

People subscribed via source and target branches