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 on 2012-05-09

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
1=== modified file 'config.yaml'
2--- config.yaml 2012-02-16 16:42:57 +0000
3+++ config.yaml 2012-05-09 09:26:17 +0000
4@@ -1,13 +1,20 @@
5 options:
6 script-retrieval-method:
7 description: |
8- How the script is retrieved. You can use bzr, brz_cat, git,
9+ How the script is retrieved. You can use apt, bzr, brz_cat, git,
10 mercurial or wget.
11 type: string
12 default: bzr
13+ script-source:
14+ description: |
15+ If script-retrieval-method is "apt", this is the name of the package to
16+ install. In the other cases, this is the url where the script lives.
17+ type: string
18 script-url:
19 description: |
20- The url where this script lives.
21+ The url where the script lives. This option is DEPRECATED: use
22+ `script-source` instead. If both `script-source` and `script-url` are
23+ provided, the latter will be ignored.
24 type: string
25 script-path:
26 description: |
27
28=== modified file 'examples/lpbuildbot.yaml'
29--- examples/lpbuildbot.yaml 2012-03-23 15:48:32 +0000
30+++ examples/lpbuildbot.yaml 2012-05-09 09:26:17 +0000
31@@ -1,11 +1,11 @@
32 buildbot-slave:
33 builders: lucid_lp
34- script-retrieval-method: bzr_cat
35- script-url: "http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/utilities/setuplxc.py"
36- script-path: setuplxc.py
37+ script-retrieval-method: apt
38+ script-source: lpsetup
39+ script-path: "/usr/bin/lp-setup"
40 # The buildbot user's home directory is /var/lib/buildout.
41- # The LP dependencies will be in /var/lib/buildout/dependencies.
42- script-args: "-u buildbot -e launchpad-pqm@canonical.com -f 'Launchpad PQM' -s launchpad_lxc_id_rsa --use-urandom {installdir}"
43+ # The LP dependencies will be in /var/lib/buildout/launchpad/dependencies.
44+ script-args: "lxc-install -u buildbot -e launchpad-pqm@canonical.com -f 'Launchpad PQM' -c {installdir} -S launchpad_lxc_id_rsa --testing"
45 extra-repository: deb http://us.archive.ubuntu.com/ubuntu/ lucid main universe
46 buildbot-pkg: buildbot/lucid
47 extra-packages: testrepository
48
49=== modified file 'hooks/install'
50--- hooks/install 2012-04-16 09:23:42 +0000
51+++ hooks/install 2012-05-09 09:26:17 +0000
52@@ -136,6 +136,11 @@
53 bzr = command('bzr')
54
55
56+def apt(source, path):
57+ apt_get_install(source)
58+ return path
59+
60+
61 def bzr_fetch(source, path):
62 apt_get_install('bzr')
63 target = tempfile.mktemp()
64@@ -173,6 +178,7 @@
65
66
67 METHODS = {
68+ 'apt': apt,
69 'bzr': bzr_fetch,
70 'bzr_cat': bzr_cat,
71 'wget': wget,
72@@ -181,19 +187,33 @@
73 }
74
75
76-def handle_script(retrieve, url, path, args):
77- log('Retrieving script: {}.'.format(url))
78- script = retrieve(url, path)
79+def handle_script(retrieve, source, path, args):
80+ log('Retrieving script: {}.'.format(source))
81+ script = retrieve(source, path)
82 log('Changing script mode.')
83 run('chmod', '+x', script)
84 log('Executing script: {}.'.format(script))
85 return subprocess.call([script] + shlex.split(str(args)))
86
87
88+def fallback_to_url(config):
89+ url = config.get('script-url')
90+ if url:
91+ # The option `script-url` is still allowed but deprecated:
92+ # print a deprecation message to stderr.
93+ import warnings
94+ warnings.simplefilter('default', DeprecationWarning)
95+ warnings.warn(
96+ 'the option `script-url` is deprecated, '
97+ 'use `script-source` instead.',
98+ DeprecationWarning)
99+ return url
100+
101+
102 def main():
103 config = get_config()
104 method = config.get('script-retrieval-method')
105- url = config.get('script-url')
106+ source = config.get('script-source') or fallback_to_url(config)
107 path = config.get('script-path')
108 # This is a naive substitution. We can make it more sophisticated
109 # if we discover we need it. For now, simplicity wins.
110@@ -201,6 +221,7 @@
111 buildbot_pkg = config.get('buildbot-pkg')
112 extra_repo = config.get('extra-repository')
113 buildbot_dir = config.get('installdir')
114+ extra_pkgs = config.get('extra-packages')
115
116 if extra_repo:
117 install_extra_repository(extra_repo)
118@@ -215,16 +236,20 @@
119 run('addgroup', 'buildbot')
120 run('usermod', '--gid', 'buildbot', 'buildbot')
121
122+ # Installing extra packages if required.
123+ if extra_pkgs:
124+ apt_get_install(*(pkg.strip() for pkg in extra_pkgs.split()))
125+
126 config_json.set(config)
127
128 retrieve = METHODS.get(method)
129- if retrieve and url and path:
130+ if retrieve and source and path:
131 # Make buildbot user have a shell by editing /etc/passwd.
132 # Otherwise you cannot ssh as this user, which some scripts
133 # need (e.g. those that create lxc containers). We choose sh as
134 # a standard and basic "system" shell.
135 run('usermod', '-s', '/bin/sh', 'buildbot')
136- sys.exit(handle_script(retrieve, url, path, args))
137+ sys.exit(handle_script(retrieve, source, path, args))
138
139
140 if __name__ == '__main__':
141
142=== modified file 'revision'
143--- revision 2012-03-06 18:23:20 +0000
144+++ revision 2012-05-09 09:26:17 +0000
145@@ -1,1 +1,1 @@
146-8
147+9
148
149=== modified file 'tests/buildbot-slave.test'
150--- tests/buildbot-slave.test 2012-03-01 16:32:37 +0000
151+++ tests/buildbot-slave.test 2012-05-09 09:26:17 +0000
152@@ -82,7 +82,7 @@
153 options = {}
154 options.update({
155 'script-retrieval-method': 'bzr_cat',
156- 'script-url':
157+ 'script-source':
158 'http://bazaar.launchpad.net/~yellow/charms/'
159 'oneiric/buildbot-slave/trunk/tests/create_file.py',
160 'script-path': 'create_file.py',

Subscribers

People subscribed via source and target branches