Merge lp:~frankban/juju-quickstart/prepare-new-release into lp:juju-quickstart
- prepare-new-release
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 54 |
Proposed branch: | lp:~frankban/juju-quickstart/prepare-new-release |
Merge into: | lp:juju-quickstart |
Diff against target: |
354 lines (+96/-71) 9 files modified
HACKING.rst (+5/-0) quickstart/__init__.py (+2/-2) quickstart/app.py (+22/-2) quickstart/manage.py (+2/-11) quickstart/settings.py (+3/-3) quickstart/tests/test_app.py (+52/-5) quickstart/tests/test_manage.py (+8/-14) quickstart/tests/test_utils.py (+1/-24) quickstart/utils.py (+1/-10) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/prepare-new-release |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+207145@code.launchpad.net |
Commit message
Description of the change
Minor changes in preparation for release.
Bump version up to 1.1.0.
Re-organize the "requires sudo" code.
Restore 100% unit test coverage (compulsive mode on).
Update license headers of modified files.
Tests: `make check`
QA: use quickstart as usual, bootstrapping
both ec2 and local environments.
Francesco Banconi (frankban) wrote : | # |
Richard Harding (rharding) wrote : | # |
LGTM with the one note on potential room for a docs update.
Will QA.
https:/
File quickstart/
https:/
quickstart/
app.bootstrap_
yay for functions that can be tested in isolation.
https:/
File quickstart/
https:/
quickstart/
'raring', 'saucy', 'trusty')
good call, are these tweaks needed to keep it up to date noted? Maybe a
releases section in the hacking doc can note to check the latest charm
version and such?
Richard Harding (rharding) wrote : | # |
- 55. By Francesco Banconi
-
Changes as per review.
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Minor changes in preparation for release.
Bump version up to 1.1.0.
Re-organize the "requires sudo" code.
Restore 100% unit test coverage (compulsive mode on).
Update license headers of modified files.
Tests: `make check`
QA: use quickstart as usual, bootstrapping
both ec2 and local environments.
R=rharding
CC=
https:/
https:/
File quickstart/
https:/
quickstart/
'raring', 'saucy', 'trusty')
On 2014/02/19 13:29:09, rharding wrote:
> good call, are these tweaks needed to keep it up to date noted? Maybe
a releases
> section in the hacking doc can note to check the latest charm version
and such?
Done.
Francesco Banconi (frankban) wrote : | # |
Thank you!
Preview Diff
1 | === modified file 'HACKING.rst' |
2 | --- HACKING.rst 2014-01-30 12:26:02 +0000 |
3 | +++ HACKING.rst 2014-02-19 13:55:43 +0000 |
4 | @@ -134,6 +134,11 @@ |
5 | precise to saucy), or in the Juju Quickstart Beta PPA: see |
6 | <https://launchpad.net/~juju-gui/+archive/quickstart-beta/+packages>. |
7 | |
8 | +Please also keep up to date the possible values for the environments.yaml |
9 | +default-series field (see ``quickstart.settings.JUJU_DEFAULT_SERIES``) and the |
10 | +set of series supported by the Juju GUI charm |
11 | +(see ``quickstart.settings.JUJU_GUI_SUPPORTED_SERIES``). |
12 | + |
13 | Debugging bundle support |
14 | ~~~~~~~~~~~~~~~~~~~~~~~~ |
15 | |
16 | |
17 | === modified file 'quickstart/__init__.py' |
18 | --- quickstart/__init__.py 2014-01-14 09:28:17 +0000 |
19 | +++ quickstart/__init__.py 2014-02-19 13:55:43 +0000 |
20 | @@ -1,6 +1,6 @@ |
21 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
22 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
23 | -# Copyright (C) 2013 Canonical Ltd. |
24 | +# Copyright (C) 2013-2014 Canonical Ltd. |
25 | # |
26 | # This program is free software: you can redistribute it and/or modify it under |
27 | # the terms of the GNU Affero General Public License version 3, as published by |
28 | @@ -22,7 +22,7 @@ |
29 | from __future__ import unicode_literals |
30 | |
31 | |
32 | -VERSION = (1, 0, 0) |
33 | +VERSION = (1, 1, 0) |
34 | |
35 | |
36 | def get_version(): |
37 | |
38 | === modified file 'quickstart/app.py' |
39 | --- quickstart/app.py 2014-01-31 21:00:06 +0000 |
40 | +++ quickstart/app.py 2014-02-19 13:55:43 +0000 |
41 | @@ -1,6 +1,6 @@ |
42 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
43 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
44 | -# Copyright (C) 2013 Canonical Ltd. |
45 | +# Copyright (C) 2013-2014 Canonical Ltd. |
46 | # |
47 | # This program is free software: you can redistribute it and/or modify it under |
48 | # the terms of the GNU Affero General Public License version 3, as published by |
49 | @@ -216,6 +216,27 @@ |
50 | print('a new ssh key was generated in {}'.format(key_file)) |
51 | |
52 | |
53 | +def bootstrap_requires_sudo(is_local): |
54 | + """Return True if "sudo" is required to bootstrap the Juju environment. |
55 | + |
56 | + Return False otherwise. |
57 | + Raise a ProgramExit if any error occurs retrieving the Juju version. |
58 | + """ |
59 | + if not is_local: |
60 | + return False |
61 | + # If this is a local environment, notify the user that "sudo" will be |
62 | + # required to bootstrap the application, even in newer Juju versions where |
63 | + # "sudo" is invoked by juju-core itself. |
64 | + print('sudo privileges required to bootstrap the environment') |
65 | + # If the Juju core version is less than 1.17.2 then use sudo for local |
66 | + # deployments. |
67 | + try: |
68 | + major, minor, patch = utils.get_juju_version() |
69 | + except ValueError as err: |
70 | + raise ProgramExit(bytes(err)) |
71 | + return (major, minor, patch) < (1, 17, 2) |
72 | + |
73 | + |
74 | def bootstrap(env_name, requires_sudo=False, debug=False): |
75 | """Bootstrap the Juju environment with the given name. |
76 | |
77 | @@ -238,7 +259,6 @@ |
78 | already_bootstrapped = False |
79 | cmd = ['/usr/bin/juju', 'bootstrap', '-e', env_name] |
80 | if requires_sudo: |
81 | - print('sudo privileges required to bootstrap the environment') |
82 | cmd.insert(0, 'sudo') |
83 | if debug: |
84 | cmd.append('--debug') |
85 | |
86 | === modified file 'quickstart/manage.py' |
87 | --- quickstart/manage.py 2014-01-31 20:51:16 +0000 |
88 | +++ quickstart/manage.py 2014-02-19 13:55:43 +0000 |
89 | @@ -1,6 +1,6 @@ |
90 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
91 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
92 | -# Copyright (C) 2013 Canonical Ltd. |
93 | +# Copyright (C) 2013-2014 Canonical Ltd. |
94 | # |
95 | # This program is free software: you can redistribute it and/or modify it under |
96 | # the terms of the GNU Affero General Public License version 3, as published by |
97 | @@ -389,16 +389,7 @@ |
98 | print('bootstrapping the {} environment (type: {})'.format( |
99 | options.env_name, options.env_type)) |
100 | is_local = options.env_type == 'local' |
101 | - requires_sudo = False |
102 | - if is_local: |
103 | - # If the Juju core version is less than 1.17.2 then |
104 | - # use sudo for local deployments. |
105 | - try: |
106 | - major, minor, patch = utils.get_juju_version() |
107 | - except ValueError as err: |
108 | - raise app.ProgramExit(bytes(err)) |
109 | - requires_sudo = utils.local_bootstrap_requires_sudo( |
110 | - major, minor, patch) |
111 | + requires_sudo = app.bootstrap_requires_sudo(is_local) |
112 | already_bootstrapped, bsn_series = app.bootstrap( |
113 | options.env_name, requires_sudo=requires_sudo, debug=options.debug) |
114 | |
115 | |
116 | === modified file 'quickstart/settings.py' |
117 | --- quickstart/settings.py 2013-12-10 17:22:44 +0000 |
118 | +++ quickstart/settings.py 2014-02-19 13:55:43 +0000 |
119 | @@ -1,6 +1,6 @@ |
120 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
121 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
122 | -# Copyright (C) 2013 Canonical Ltd. |
123 | +# Copyright (C) 2013-2014 Canonical Ltd. |
124 | # |
125 | # This program is free software: you can redistribute it and/or modify it under |
126 | # the terms of the GNU Affero General Public License version 3, as published by |
127 | @@ -26,13 +26,13 @@ |
128 | |
129 | # The default Juju GUI charm URL to use when it is not possible to retrieve it |
130 | # from the charmworld API, e.g. due to temporary connection/charmworld errors. |
131 | -DEFAULT_CHARM_URL = 'cs:precise/juju-gui-80' |
132 | +DEFAULT_CHARM_URL = 'cs:precise/juju-gui-83' |
133 | |
134 | # The quickstart app short description. |
135 | DESCRIPTION = 'set up a Juju environment (including the GUI) in very few steps' |
136 | |
137 | # The possible values for the environments.yaml default-series field. |
138 | -JUJU_DEFAULT_SERIES = ('precise', 'quantal', 'raring', 'saucy') |
139 | +JUJU_DEFAULT_SERIES = ('precise', 'quantal', 'raring', 'saucy', 'trusty') |
140 | |
141 | # Retrieve the current juju-core home. |
142 | JUJU_HOME = os.getenv('JUJU_HOME', '~/.juju') |
143 | |
144 | === modified file 'quickstart/tests/test_app.py' |
145 | --- quickstart/tests/test_app.py 2014-01-31 21:38:59 +0000 |
146 | +++ quickstart/tests/test_app.py 2014-02-19 13:55:43 +0000 |
147 | @@ -1,6 +1,6 @@ |
148 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
149 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
150 | -# Copyright (C) 2013 Canonical Ltd. |
151 | +# Copyright (C) 2013-2014 Canonical Ltd. |
152 | # |
153 | # This program is free software: you can redistribute it and/or modify it under |
154 | # the terms of the GNU Affero General Public License version 3, as published by |
155 | @@ -400,6 +400,56 @@ |
156 | |
157 | |
158 | @helpers.mock_print |
159 | +class TestBootstrapRequiresSudo(ProgramExitTestsMixin, unittest.TestCase): |
160 | + |
161 | + sudo_message = 'sudo privileges required to bootstrap the environment' |
162 | + |
163 | + def patch_get_juju_version(self, return_value): |
164 | + """Patch the quickstart.utils.get_juju_version function.""" |
165 | + if isinstance(return_value, Exception): |
166 | + mock_get_juju_version = mock.Mock(side_effect=return_value) |
167 | + else: |
168 | + mock_get_juju_version = mock.Mock(return_value=return_value) |
169 | + return mock.patch( |
170 | + 'quickstart.utils.get_juju_version', mock_get_juju_version) |
171 | + |
172 | + def test_not_local(self, mock_print): |
173 | + # Sudo privileges are never required for non-local environments. |
174 | + self.assertFalse(app.bootstrap_requires_sudo(False)) |
175 | + |
176 | + def test_sudo_required(self, mock_print): |
177 | + # Sudo privileges are required if the Juju version is < 1.17.2. |
178 | + versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)] |
179 | + for version in versions: |
180 | + with self.patch_get_juju_version(version): |
181 | + requires_sudo = app.bootstrap_requires_sudo(True) |
182 | + self.assertTrue(requires_sudo, version) |
183 | + # On local environments the "sudo privileges required" message is |
184 | + # always printed. |
185 | + mock_print.assert_called_once_with(self.sudo_message) |
186 | + mock_print.reset_mock() |
187 | + |
188 | + def test_sudo_not_required(self, mock_print): |
189 | + # Sudo privileges are not required if the Juju version is >= 1.17.2. |
190 | + versions = [ |
191 | + (1, 17, 2), (1, 17, 10), (1, 18, 0), (1, 18, 2), (2, 16, 1)] |
192 | + for version in versions: |
193 | + with self.patch_get_juju_version(version): |
194 | + requires_sudo = app.bootstrap_requires_sudo(True) |
195 | + self.assertFalse(requires_sudo, version) |
196 | + # On local environments the "sudo privileges required" message is |
197 | + # always printed. |
198 | + mock_print.assert_called_once_with(self.sudo_message) |
199 | + mock_print.reset_mock() |
200 | + |
201 | + def test_invalid_version(self, mock_print): |
202 | + # A ProgramExit is raised if the Juju version is not valid. |
203 | + with self.patch_get_juju_version(ValueError(b'bad wolf')): |
204 | + with self.assert_program_exit('bad wolf'): |
205 | + app.bootstrap_requires_sudo(True) |
206 | + |
207 | + |
208 | +@helpers.mock_print |
209 | class TestBootstrap( |
210 | helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase): |
211 | |
212 | @@ -459,10 +509,7 @@ |
213 | mock_call.assert_has_calls([ |
214 | mock.call('sudo', self.juju, 'bootstrap', '-e', self.env_name), |
215 | ] + self.make_status_calls(1)) |
216 | - mock_print.assert_has_calls([ |
217 | - mock.call('sudo privileges required to bootstrap the environment'), |
218 | - mock.call(self.status_message), |
219 | - ]) |
220 | + mock_print.assert_called_once_with(self.status_message) |
221 | |
222 | def test_success_debug(self, mock_print): |
223 | # The environment is successfully bootstrapped in debug mode. |
224 | |
225 | === modified file 'quickstart/tests/test_manage.py' |
226 | --- quickstart/tests/test_manage.py 2014-01-31 18:09:48 +0000 |
227 | +++ quickstart/tests/test_manage.py 2014-02-19 13:55:43 +0000 |
228 | @@ -1,6 +1,6 @@ |
229 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
230 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
231 | -# Copyright (C) 2013 Canonical Ltd. |
232 | +# Copyright (C) 2013-2014 Canonical Ltd. |
233 | # |
234 | # This program is free software: you can redistribute it and/or modify it under |
235 | # the terms of the GNU Affero General Public License version 3, as published by |
236 | @@ -669,12 +669,6 @@ |
237 | msg = 'admin-secret not found in {}'.format(path) |
238 | raise ValueError(msg.encode('utf-8')) |
239 | |
240 | - @staticmethod |
241 | - def patch_get_juju_version(major, minor, patch): |
242 | - """Patch the quickstart.utils.get_juju_version function.""" |
243 | - mock_call = mock.Mock(return_value=(major, minor, patch)) |
244 | - return mock.patch('quickstart.utils.get_juju_version', mock_call) |
245 | - |
246 | def test_no_bundle(self, mock_app, mock_open): |
247 | # The application runs correctly if no bundle is provided. |
248 | token = 'AUTHTOKEN' |
249 | @@ -683,12 +677,12 @@ |
250 | mock_app.bootstrap.return_value = (True, 'precise') |
251 | mock_app.get_admin_secret = self.mock_get_admin_secret_error |
252 | options = self.make_options() |
253 | - with self.patch_get_juju_version(1, 17, 1): |
254 | - manage.run(options) |
255 | + manage.run(options) |
256 | mock_app.ensure_dependencies.assert_called() |
257 | mock_app.ensure_ssh_keys.assert_called() |
258 | mock_app.bootstrap.assert_called_once_with( |
259 | - options.env_name, requires_sudo=False, debug=options.debug) |
260 | + options.env_name, requires_sudo=mock_app.bootstrap_requires_sudo(), |
261 | + debug=options.debug) |
262 | mock_app.get_api_url.assert_called_once_with(options.env_name) |
263 | mock_app.connect.assert_has_calls([ |
264 | mock.call(mock_app.get_api_url(), options.admin_secret), |
265 | @@ -731,9 +725,9 @@ |
266 | # The application correctly handles working with local providers with |
267 | # new Juju versions not requiring "sudo" to bootstrap the environment. |
268 | options = self.make_options(env_type='local') |
269 | + mock_app.bootstrap_requires_sudo.return_value = False |
270 | mock_app.bootstrap.return_value = (True, 'precise') |
271 | - with self.patch_get_juju_version(1, 18, 0): |
272 | - manage.run(options) |
273 | + manage.run(options) |
274 | mock_app.bootstrap.assert_called_once_with( |
275 | options.env_name, requires_sudo=False, debug=options.debug) |
276 | |
277 | @@ -741,9 +735,9 @@ |
278 | # The application correctly handles working with local providers when |
279 | # Juju requires an external "sudo" call to bootstrap the environment. |
280 | options = self.make_options(env_type='local') |
281 | + mock_app.bootstrap_requires_sudo.return_value = True |
282 | mock_app.bootstrap.return_value = (True, 'precise') |
283 | - with self.patch_get_juju_version(1, 17, 1): |
284 | - manage.run(options) |
285 | + manage.run(options) |
286 | mock_app.bootstrap.assert_called_once_with( |
287 | options.env_name, requires_sudo=True, debug=options.debug) |
288 | |
289 | |
290 | === modified file 'quickstart/tests/test_utils.py' |
291 | --- quickstart/tests/test_utils.py 2014-01-31 18:09:48 +0000 |
292 | +++ quickstart/tests/test_utils.py 2014-02-19 13:55:43 +0000 |
293 | @@ -1,6 +1,6 @@ |
294 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
295 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
296 | -# Copyright (C) 2013 Canonical Ltd. |
297 | +# Copyright (C) 2013-2014 Canonical Ltd. |
298 | # |
299 | # This program is free software: you can redistribute it and/or modify it under |
300 | # the terms of the GNU Affero General Public License version 3, as published by |
301 | @@ -710,26 +710,3 @@ |
302 | utils.get_juju_version() |
303 | msg = 'invalid version string: 1.17' |
304 | self.assertEqual(msg, bytes(context_manager.exception)) |
305 | - |
306 | - |
307 | -class TestLocalBootstrapRequiresSudo(unittest.TestCase): |
308 | - |
309 | - def test_returns_true_on_lower_than_1_17_2(self): |
310 | - # If Juju version is lower than 1.17.2. |
311 | - value = utils.local_bootstrap_requires_sudo(1, 17, 0) |
312 | - self.assertTrue(value) |
313 | - value = utils.local_bootstrap_requires_sudo(1, 16, 5) |
314 | - self.assertTrue(value) |
315 | - |
316 | - def test_returns_false_higher_than_1_17_1(self): |
317 | - # If juju version is higher than 1.17.1. |
318 | - value = utils.local_bootstrap_requires_sudo(1, 17, 2) |
319 | - self.assertFalse(value) |
320 | - value = utils.local_bootstrap_requires_sudo(1, 17, 10) |
321 | - self.assertFalse(value) |
322 | - value = utils.local_bootstrap_requires_sudo(1, 18, 0) |
323 | - self.assertFalse(value) |
324 | - value = utils.local_bootstrap_requires_sudo(1, 18, 2) |
325 | - self.assertFalse(value) |
326 | - value = utils.local_bootstrap_requires_sudo(2, 16, 1) |
327 | - self.assertFalse(value) |
328 | |
329 | === modified file 'quickstart/utils.py' |
330 | --- quickstart/utils.py 2014-01-31 18:09:48 +0000 |
331 | +++ quickstart/utils.py 2014-02-19 13:55:43 +0000 |
332 | @@ -1,6 +1,6 @@ |
333 | # This file is part of the Juju Quickstart Plugin, which lets users set up a |
334 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
335 | -# Copyright (C) 2013 Canonical Ltd. |
336 | +# Copyright (C) 2013-2014 Canonical Ltd. |
337 | # |
338 | # This program is free software: you can redistribute it and/or modify it under |
339 | # the terms of the GNU Affero General Public License version 3, as published by |
340 | @@ -348,15 +348,6 @@ |
341 | raise ValueError(msg.encode('utf-8')) |
342 | |
343 | |
344 | -def local_bootstrap_requires_sudo(major, minor, patch): |
345 | - """Parse Juju version to check if sudo is required for local deployment. |
346 | - |
347 | - Before Juju version 1.17.2 sudo is required for bootstrapping |
348 | - local environments. |
349 | - """ |
350 | - return (major, minor, patch) < (1, 17, 2) |
351 | - |
352 | - |
353 | def run_once(function): |
354 | """Return a decorated version of the given function which runs only once. |
355 |
Reviewers: mp+207145_ code.launchpad. net,
Message:
Please take a look.
Description:
Minor changes in preparation for release.
Bump version up to 1.1.0.
Re-organize the "requires sudo" code.
Restore 100% unit test coverage (compulsive mode on).
Update license headers of modified files.
Tests: `make check`
QA: use quickstart as usual, bootstrapping
both ec2 and local environments.
https:/ /code.launchpad .net/~frankban/ juju-quickstart /prepare- new-release/ +merge/ 207145
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/65970043/
Affected files (+93, -71 lines): __init_ _.py manage. py settings. py tests/test_ app.py tests/test_ manage. py tests/test_ utils.py
A [revision details]
M quickstart/
M quickstart/app.py
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/utils.py