Merge lp:~gary/charms/precise/juju-gui/update-for-jujucharms into lp:~juju-gui/charms/precise/juju-gui/trunk
- Precise Pangolin (12.04)
- update-for-jujucharms
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 48 |
Proposed branch: | lp:~gary/charms/precise/juju-gui/update-for-jujucharms |
Merge into: | lp:~juju-gui/charms/precise/juju-gui/trunk |
Diff against target: |
313 lines (+105/-20) 10 files modified
.bzrignore (+1/-0) config.yaml (+6/-0) config/config.js.template (+1/-0) hooks/backend.py (+2/-2) hooks/install (+23/-5) hooks/utils.py (+13/-4) hooks/web-relation-joined (+23/-0) revision (+1/-1) tests/deploy.test (+4/-4) tests/test_utils.py (+31/-4) |
To merge this branch: | bzr merge lp:~gary/charms/precise/juju-gui/update-for-jujucharms |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
charmers | Pending | ||
Review via email: mp+161298@code.launchpad.net |
Commit message
Description of the change
Fix charm for jujucharms.com use
- Merge work from mews to support exec.d directory and relative file paths. Add tests.
- Merge work from benji to support charmworld option needed by upcoming gui changes. Make a small fix.
- Fix deploy tests.
Tests are still fragile, but I got them all to pass with the current branch at one time, and verified manually that the various steps still work.
Gary Poster (gary) wrote : | # |
Gary Poster (gary) wrote : | # |
Reviewers: mp+161298_
Message:
Please take a look.
Description:
Fix charm for jujucharms.com use
- Merge work from mews to support exec.d directory and relative file
paths. Add tests.
- Merge work from benji to support charmworld option needed by upcoming
gui changes. Make a small fix.
- Fix deploy tests.
Tests are still fragile, but I got them all to pass with the current
branch at one time, and verified manually that the various steps still
work.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files:
M .bzrignore
A [revision details]
M config.yaml
M config/
M hooks/backend.py
M hooks/install
M hooks/utils.py
A hooks/web-
M revision
M tests/deploy.test
M tests/test_utils.py
Richard Harding (rharding) wrote : | # |
LGTM, thanks for working through it.
- 51. By Gary Poster
-
respond to review
Gary Poster (gary) wrote : | # |
*** Submitted:
Fix charm for jujucharms.com use
- Merge work from mews to support exec.d directory and relative file
paths. Add tests.
- Merge work from benji to support charmworld option needed by upcoming
gui changes. Make a small fix.
- Fix deploy tests.
Tests are still fragile, but I got them all to pass with the current
branch at one time, and verified manually that the various steps still
work.
R=rharding, benji
CC=
https:/
https:/
File hooks/install (right):
https:/
hooks/install:46: for module in os.listdir(
On 2013/04/27 20:29:30, benji wrote:
> Usually ".d" files are to be run in lexicographic order, so throwing a
sort()
> around the listdir would be good.
Good idea in the abstract, and I changed it.
FWIW, this code is provided by IS for their use, and I don't like the
pattern and at least mew has agreed with me, so I'm not super excited
about improving it. Your ideas were good, though, so I grudgingly did
them anyway. :-)
https:/
hooks/install:49: except OSError:
On 2013/04/27 20:29:30, benji wrote:
> It would be safer if we knew which OSErrors in particular we should
ignore. It
> would also be nice to know why we are ignoring them (i.e., a comment).
OK, good point...I did it, because I agree with you, though I had to
guess at their intent. Hopefully I got it right. :-/
https:/
File hooks/utils.py (right):
https:/
hooks/utils.py:195: """Log when a hook starts and stops its execution.
On 2013/04/27 20:29:30, benji wrote:
> Yay! I hate the "an" before non-silent "h" thing.
> I know, I know; I'm weird.
heh
https:/
hooks/utils.py:228: if source[0] != '/':
On 2013/04/27 20:29:30, benji wrote:
> I don't really prefer it, but just in case you do:
> if not source.
Yeah, I do, thanks. Changed.
https:/
File hooks/web-
https:/
hooks/web-
On 2013/04/27 20:29:30, benji wrote:
> Unnecessary newline.
I was doing as the Romans do, but eh, I don't think the Romans will
notice. Deleted.
https:/
hooks/web-
On 2013/04/27 20:29:30, benji wrote:
> You can use a one-liner here.
Again, I was following the local patterns in the charm, but eh, I agree.
Changed.
https:/
hooks/web-
On 2013/04/27 20:29:30, benji wrote:
> I'm pretty sure lines 16-18 are equivalent to
> hostname = socket.getfqdn()
I stared at this for a bit. I think you are probably right. What I had
here is what the haproxy or apache2 charms are...
Gary Poster (gary) wrote : | # |
thank you both!
Preview Diff
1 | === modified file '.bzrignore' |
2 | --- .bzrignore 2013-04-15 17:47:54 +0000 |
3 | +++ .bzrignore 2013-04-28 01:09:28 +0000 |
4 | @@ -1,2 +1,3 @@ |
5 | .emacs* |
6 | tags |
7 | +exec.d/* |
8 | |
9 | === modified file 'config.yaml' |
10 | --- config.yaml 2013-04-17 19:31:56 +0000 |
11 | +++ config.yaml 2013-04-28 01:09:28 +0000 |
12 | @@ -112,3 +112,9 @@ |
13 | the source: hooks/install and hooks/backend.py in particular. |
14 | type: boolean |
15 | default: true |
16 | + charmworld-url: |
17 | + description: | |
18 | + The URL of the charm catalog site ("charmworld") from which charm catalog |
19 | + data will be drawn. |
20 | + type: string |
21 | + default: http://staging.jujucharms.com |
22 | |
23 | === modified file 'config/config.js.template' |
24 | --- config/config.js.template 2013-04-12 10:09:32 +0000 |
25 | +++ config/config.js.template 2013-04-28 01:09:28 +0000 |
26 | @@ -11,6 +11,7 @@ |
27 | // SubApps. |
28 | consoleEnabled: {{console_enabled}}, |
29 | charm_store_url: 'https://jujucharms.com/', |
30 | + charmworldURL: {{charmworld_url}}, |
31 | // socket_url is only honored in older versions of the GUI. |
32 | socket_url: '{{raw_protocol}}://{{address}}/ws', |
33 | // socket_protocol is used instead by newer versions of the GUI to |
34 | |
35 | === added directory 'exec.d' |
36 | === modified file 'hooks/backend.py' |
37 | --- hooks/backend.py 2013-04-24 13:18:22 +0000 |
38 | +++ hooks/backend.py 2013-04-28 01:09:28 +0000 |
39 | @@ -107,8 +107,8 @@ |
40 | start_gui( |
41 | config['juju-gui-console-enabled'], config['login-help'], |
42 | config['read-only'], config['staging'], config['ssl-cert-path'], |
43 | - config['serve-tests'], secure=config['secure'], |
44 | - sandbox=config['sandbox']) |
45 | + config['charmworld-url'], config['serve-tests'], |
46 | + secure=config['secure'], sandbox=config['sandbox']) |
47 | open_port(80) |
48 | open_port(443) |
49 | |
50 | |
51 | === modified file 'hooks/install' |
52 | --- hooks/install 2013-04-24 10:46:20 +0000 |
53 | +++ hooks/install 2013-04-28 01:09:28 +0000 |
54 | @@ -1,8 +1,9 @@ |
55 | #!/usr/bin/env python2 |
56 | # -*- python -*- |
57 | |
58 | +import errno |
59 | import json |
60 | -from subprocess import check_call |
61 | +import os |
62 | |
63 | # If the user's environment has "juju-origin: ppa" set, they will |
64 | # automatically have access to python-charmhelpers and python-shelltoolbox. |
65 | @@ -27,18 +28,35 @@ |
66 | 'python-apt', 'python-charmhelpers', 'python-launchpadlib', |
67 | 'python-shelltoolbox', 'python-tempita', |
68 | ) |
69 | -check_call(('apt-get', 'install', '-y') + PYTHON_DEPENDENCIES) |
70 | - |
71 | - |
72 | +bootstrap_utils.run(*(('apt-get', 'install', '-y') + PYTHON_DEPENDENCIES)) |
73 | + |
74 | + |
75 | +from charmhelpers import log |
76 | from utils import ( |
77 | config_json, |
78 | log_hook, |
79 | ) |
80 | - |
81 | from backend import Backend |
82 | |
83 | |
84 | def main(): |
85 | + # Run pre-install tasks, if available. Please do not rely on the |
86 | + # exec.d interface without conferring with the Juju GUI team: it may |
87 | + # change after upcoming discussion with Canonical IS. |
88 | + if os.path.isdir('exec.d'): |
89 | + dirnames = os.listdir('exec.d') |
90 | + dirnames.sort() |
91 | + for module in dirnames: |
92 | + filename = os.path.join('exec.d', module, 'charm-pre-install') |
93 | + try: |
94 | + bootstrap_utils.run(filename) |
95 | + except OSError, e: |
96 | + # If the exec.d file does not exist or is not runnable, |
97 | + # assume we can recover. Log the problem and proceed. |
98 | + if e.errno in (errno.ENOENT, errno.EACCES): |
99 | + log('{}: {}'.format(e.strerror, filename)) |
100 | + else: |
101 | + raise |
102 | config = get_config() |
103 | backend = Backend(config) |
104 | backend.install() |
105 | |
106 | === modified file 'hooks/utils.py' |
107 | --- hooks/utils.py 2013-04-25 16:46:07 +0000 |
108 | +++ hooks/utils.py 2013-04-28 01:09:28 +0000 |
109 | @@ -45,6 +45,7 @@ |
110 | import shutil |
111 | from subprocess import CalledProcessError |
112 | import tempfile |
113 | +from urlparse import urlparse |
114 | |
115 | from launchpadlib.launchpad import Launchpad |
116 | from shelltoolbox import ( |
117 | @@ -191,7 +192,7 @@ |
118 | |
119 | @contextmanager |
120 | def log_hook(): |
121 | - """Log when an hook starts and stops its execution. |
122 | + """Log when a hook starts and stops its execution. |
123 | |
124 | Also log to stdout possible CalledProcessError exceptions raised executing |
125 | the hook. |
126 | @@ -217,10 +218,17 @@ |
127 | - ('stable', '0.1.0'): stable release v0.1.0; |
128 | - ('trunk', None): latest trunk release; |
129 | - ('trunk', '0.1.0+build.1'): trunk release v0.1.0 bzr revision 1; |
130 | - - ('branch', 'lp:juju-gui'): release is made from a branch. |
131 | + - ('branch', 'lp:juju-gui'): release is made from a branch; |
132 | + - ('url', 'http://example.com/gui'): release from a downloaded file. |
133 | """ |
134 | if source.startswith('url:'): |
135 | - return 'url', source[4:] |
136 | + source = source[4:] |
137 | + # Support file paths, including relative paths. |
138 | + if urlparse(source).scheme == '': |
139 | + if not source.startswith('/'): |
140 | + source = os.path.join(os.path.abspath(CURRENT_DIR), source) |
141 | + source = "file://%s" % source |
142 | + return 'url', source |
143 | if source in ('stable', 'trunk'): |
144 | return source, None |
145 | if source.startswith('lp:') or source.startswith('http://'): |
146 | @@ -310,7 +318,7 @@ |
147 | |
148 | def start_gui( |
149 | console_enabled, login_help, readonly, in_staging, ssl_cert_path, |
150 | - serve_tests, haproxy_path='/etc/haproxy/haproxy.cfg', |
151 | + charmworld_url, serve_tests, haproxy_path='/etc/haproxy/haproxy.cfg', |
152 | config_js_path=None, secure=True, sandbox=False): |
153 | """Set up and start the Juju GUI server.""" |
154 | with su('root'): |
155 | @@ -350,6 +358,7 @@ |
156 | 'user': json.dumps(user), |
157 | 'protocol': json.dumps(protocol), |
158 | 'sandbox': json.dumps(sandbox), |
159 | + 'charmworld_url': json.dumps(charmworld_url), |
160 | } |
161 | if config_js_path is None: |
162 | config_js_path = os.path.join( |
163 | |
164 | === added file 'hooks/web-relation-joined' |
165 | --- hooks/web-relation-joined 1970-01-01 00:00:00 +0000 |
166 | +++ hooks/web-relation-joined 2013-04-28 01:09:28 +0000 |
167 | @@ -0,0 +1,23 @@ |
168 | +#!/usr/bin/env python2 |
169 | +#-*- python -*- |
170 | + |
171 | +import socket |
172 | +from charmhelpers import ( |
173 | + get_config, |
174 | + relation_set, |
175 | +) |
176 | +from utils import log_hook |
177 | + |
178 | + |
179 | +def main(): |
180 | + hostname = socket.getfqdn() |
181 | + config = get_config() |
182 | + if config['secure']: |
183 | + port = 443 |
184 | + else: |
185 | + port = 80 |
186 | + relation_set(port=port, hostname=hostname) |
187 | + |
188 | +if __name__ == '__main__': |
189 | + with log_hook(): |
190 | + main() |
191 | |
192 | === modified file 'revision' |
193 | --- revision 2013-04-23 07:32:46 +0000 |
194 | +++ revision 2013-04-28 01:09:28 +0000 |
195 | @@ -1,1 +1,1 @@ |
196 | -34 |
197 | +40 |
198 | |
199 | === modified file 'tests/deploy.test' |
200 | --- tests/deploy.test 2013-03-13 21:24:31 +0000 |
201 | +++ tests/deploy.test 2013-04-28 01:09:28 +0000 |
202 | @@ -166,7 +166,7 @@ |
203 | # XXX 2012-11-29 frankban bug=872264: see *stop_services* above. |
204 | self.addCleanup( |
205 | self.stop_services, |
206 | - hostname, ['haproxy', 'nginx', 'juju-api-agent']) |
207 | + hostname, ['haproxy', 'apache2', 'juju-api-agent']) |
208 | self.navigate_to(hostname) |
209 | self.handle_browser_warning() |
210 | self.assertEnvironmentIsConnected() |
211 | @@ -177,7 +177,7 @@ |
212 | # XXX 2012-11-29 frankban bug=872264: see *stop_services* above. |
213 | self.addCleanup( |
214 | self.stop_services, |
215 | - hostname, ['haproxy', 'nginx', 'juju-api-improv']) |
216 | + hostname, ['haproxy', 'apache2', 'juju-api-improv']) |
217 | self.navigate_to(hostname) |
218 | self.handle_browser_warning() |
219 | self.assertEnvironmentIsConnected() |
220 | @@ -190,7 +190,7 @@ |
221 | # XXX 2012-11-29 frankban bug=872264: see *stop_services* above. |
222 | self.addCleanup( |
223 | self.stop_services, |
224 | - hostname, ['haproxy', 'nginx', 'juju-api-agent']) |
225 | + hostname, ['haproxy', 'apache2', 'juju-api-agent']) |
226 | self.navigate_to(hostname) |
227 | self.handle_browser_warning() |
228 | self.assertEnvironmentIsConnected() |
229 | @@ -222,7 +222,7 @@ |
230 | # XXX 2012-11-29 frankban bug=872264: see *stop_services* above. |
231 | self.addCleanup( |
232 | self.stop_services, |
233 | - hostname, ['haproxy', 'nginx', 'juju-api-agent']) |
234 | + hostname, ['haproxy', 'apache2', 'juju-api-agent']) |
235 | self.navigate_to(hostname) |
236 | self.handle_browser_warning() |
237 | self.assertEnvironmentIsConnected() |
238 | |
239 | === modified file 'tests/test_utils.py' |
240 | --- tests/test_utils.py 2013-04-23 16:55:56 +0000 |
241 | +++ tests/test_utils.py 2013-04-28 01:09:28 +0000 |
242 | @@ -366,6 +366,15 @@ |
243 | |
244 | class ParseSourceTest(unittest.TestCase): |
245 | |
246 | + def setUp(self): |
247 | + # Monkey patch utils.CURRENT_DIR. |
248 | + self.original_current_dir = utils.CURRENT_DIR |
249 | + utils.CURRENT_DIR = '/current/dir' |
250 | + |
251 | + def tearDown(self): |
252 | + # Restore the original utils.CURRENT_DIR. |
253 | + utils.CURRENT_DIR = self.original_current_dir |
254 | + |
255 | def test_latest_stable_release(self): |
256 | # Ensure the latest stable release is correctly parsed. |
257 | expected = ('stable', None) |
258 | @@ -392,6 +401,19 @@ |
259 | for source in sources: |
260 | self.assertTupleEqual(('branch', source), parse_source(source)) |
261 | |
262 | + def test_url(self): |
263 | + expected = ('url', 'http://example.com/gui') |
264 | + self.assertTupleEqual( |
265 | + expected, parse_source('url:http://example.com/gui')) |
266 | + |
267 | + def test_file_url(self): |
268 | + expected = ('url', 'file:///foo/bar') |
269 | + self.assertTupleEqual(expected, parse_source('url:/foo/bar')) |
270 | + |
271 | + def test_relative_file_url(self): |
272 | + expected = ('url', 'file:///current/dir/foo/bar') |
273 | + self.assertTupleEqual(expected, parse_source('url:foo/bar')) |
274 | + |
275 | |
276 | class RenderToFileTest(unittest.TestCase): |
277 | |
278 | @@ -539,9 +561,11 @@ |
279 | |
280 | def test_start_gui(self): |
281 | ssl_cert_path = '/tmp/certificates/' |
282 | + charmworld_url = 'http://charmworld.example' |
283 | start_gui( |
284 | - False, 'This is login help.', True, True, ssl_cert_path, True, |
285 | - haproxy_path='haproxy', config_js_path='config') |
286 | + False, 'This is login help.', True, True, ssl_cert_path, |
287 | + charmworld_url, True, haproxy_path='haproxy', |
288 | + config_js_path='config') |
289 | haproxy_conf = self.files['haproxy'] |
290 | self.assertIn('ca-base {0}'.format(ssl_cert_path), haproxy_conf) |
291 | self.assertIn('crt-base {0}'.format(ssl_cert_path), haproxy_conf) |
292 | @@ -558,6 +582,7 @@ |
293 | self.assertIn('readOnly: true', js_conf) |
294 | self.assertIn("socket_url: 'wss://", js_conf) |
295 | self.assertIn('socket_protocol: "wss"', js_conf) |
296 | + self.assertIn('charmworldURL: "http://charmworld.example"', js_conf) |
297 | apache_conf = self.files['juju-gui'] |
298 | self.assertIn('juju-gui/build-', apache_conf) |
299 | self.assertIn('VirtualHost *:{0}'.format(WEB_PORT), apache_conf) |
300 | @@ -565,9 +590,11 @@ |
301 | |
302 | def test_start_gui_insecure(self): |
303 | ssl_cert_path = '/tmp/certificates/' |
304 | + charmworld_url = 'http://charmworld.example' |
305 | start_gui( |
306 | - False, 'This is login help.', True, True, ssl_cert_path, True, |
307 | - haproxy_path='haproxy', config_js_path='config', secure=False) |
308 | + False, 'This is login help.', True, True, ssl_cert_path, |
309 | + charmworld_url, True, haproxy_path='haproxy', |
310 | + config_js_path='config', secure=False) |
311 | js_conf = self.files['config'] |
312 | self.assertIn("socket_url: 'ws://", js_conf) |
313 | self.assertIn('socket_protocol: "ws"', js_conf) |
Please take a look.