Merge lp:~frankban/juju-quickstart/code-reorg into lp:juju-quickstart
- code-reorg
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 104 |
Proposed branch: | lp:~frankban/juju-quickstart/code-reorg |
Merge into: | lp:juju-quickstart |
Diff against target: |
849 lines (+370/-222) 12 files modified
HACKING.rst (+55/-1) quickstart/app.py (+10/-21) quickstart/manage.py (+3/-15) quickstart/models/envs.py (+1/-30) quickstart/models/jenv.py (+71/-0) quickstart/serializers.py (+20/-0) quickstart/tests/helpers.py (+36/-0) quickstart/tests/models/test_envs.py (+0/-76) quickstart/tests/models/test_jenv.py (+110/-0) quickstart/tests/test_app.py (+16/-18) quickstart/tests/test_manage.py (+7/-61) quickstart/tests/test_serializers.py (+41/-0) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/code-reorg |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email:
|
Commit message
Description of the change
Code reorganization + docs.
Reorganize the jenv related code in preparation of the
bootstrap strategy change.
Separate jenv handling from the environments.yaml file
management in models.
As a consequence of only supporting juju >= 1.18 we
can now safely always retrieve the admin-secret from
the jenv file.
Also updated the HACKING docs to include a brief
description of the project structure.
Tests: `make check`.
QA: use quickstart as usual.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Francesco Banconi (frankban) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Brad Crittenden (bac) wrote : | # |
LGTM. Will QA now.
https:/
File HACKING.rst (right):
https:/
HACKING.rst:78:
A very nice introduction to the code layout. Thanks for including it.
https:/
File quickstart/
https:/
quickstart/
Why did you chose to use *args here instead of a tuple of key names?
https:/
File quickstart/
https:/
quickstart/
JUJU_HOME is set to the ancestor
typo: contextx
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Richard Harding (rharding) wrote : | # |
Thanks for the update. I just have one forward looking question that
doesn't directly effect this branch but I'm curious about while looking
at this branch.
LGTM, no qa
https:/
File HACKING.rst (right):
https:/
HACKING.rst:128: the code: go have a look!
nice ty
https:/
File quickstart/app.py (right):
https:/
quickstart/
do you know if we can use the password field in the jenv for this? Next
week we hope to start updating the gui for the new username/password
work in juju 1.21 and I know we'll need to update quickstart to pull the
username/password from the jenv file.
https:/
File quickstart/
https:/
quickstart/
section=
was this unused?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Brad Crittenden (bac) wrote : | # |
Had some QA oddities but it looks OK.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Francesco Banconi (frankban) wrote : | # |
Thanks for the reviews!
https:/
File quickstart/app.py (right):
https:/
quickstart/
On 2014/11/11 15:52:36, rharding wrote:
> do you know if we can use the password field in the jenv for this?
Next week we
> hope to start updating the gui for the new username/password work in
juju 1.21
> and I know we'll need to update quickstart to pull the
username/password from
> the jenv file.
With this change it should be easy to grab what we want from the jenv
with jenv.get_value(). We will want to use some fallback logic to
support backward compatibility.
If the user is != user-admin, we will need to update the one-show
automatic authentication with token as well in the guiserver and maybe
in the GUI itself.
https:/
File quickstart/
https:/
quickstart/
section=
On 2014/11/11 15:52:36, rharding wrote:
> was this unused?
It was used, it is no longer required now.
https:/
File quickstart/
https:/
quickstart/
On 2014/11/11 15:37:55, bac wrote:
> Why did you chose to use *args here instead of a tuple of key names?
No real reason actually, I started with just one single key as the
argument and then, after deciding to allow nested keys, it just felt
natural.
https:/
File quickstart/
https:/
quickstart/
JUJU_HOME is set to the ancestor
On 2014/11/11 15:37:55, bac wrote:
> typo: contextx
Done.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Code reorganization + docs.
Reorganize the jenv related code in preparation of the
bootstrap strategy change.
Separate jenv handling from the environments.yaml file
management in models.
As a consequence of only supporting juju >= 1.18 we
can now safely always retrieve the admin-secret from
the jenv file.
Also updated the HACKING docs to include a brief
description of the project structure.
Tests: `make check`.
QA: use quickstart as usual.
R=bac, rharding
CC=
https:/
Preview Diff
1 | === modified file 'HACKING.rst' | |||
2 | --- HACKING.rst 2014-09-01 14:49:47 +0000 | |||
3 | +++ HACKING.rst 2014-11-11 14:26:05 +0000 | |||
4 | @@ -73,6 +73,60 @@ | |||
5 | 73 | 73 | ||
6 | 74 | $ .venv/bin/python juju-quickstart --help | 74 | $ .venv/bin/python juju-quickstart --help |
7 | 75 | 75 | ||
8 | 76 | Project structure | ||
9 | 77 | ~~~~~~~~~~~~~~~~~ | ||
10 | 78 | |||
11 | 79 | Juju Quickstart is a Python project. Source files can be found in the | ||
12 | 80 | ``quickstart`` Python package included in this distribution. | ||
13 | 81 | |||
14 | 82 | A brief description of the project structure follows, including the goals of | ||
15 | 83 | each module in the ``quickstart`` package. | ||
16 | 84 | |||
17 | 85 | * ``manage.py`` includes the main entry points for the ``juju-quickstart`` | ||
18 | 86 | command. Specifically: | ||
19 | 87 | * ``manage.setup`` is used to set up the command line parser, the logging | ||
20 | 88 | infrastructure and the interactive session (if required); | ||
21 | 89 | * ``manage.run`` executes the application with the given options. | ||
22 | 90 | |||
23 | 91 | * ``app.py`` defines the main application functions, like bootstrapping an | ||
24 | 92 | environment, deploying the Juju GUI or watching the deployment progress. | ||
25 | 93 | The ``app.ProgramExit`` error can only be raised by functions in this module, | ||
26 | 94 | and it causes the immediate exit (with an error) from the application. | ||
27 | 95 | |||
28 | 96 | The ``manage.py`` and ``app.py`` modules must be considered ``juju-quickstart`` | ||
29 | 97 | execution specific: for this reason those modules are unlikely to be used as | ||
30 | 98 | libraries. All the other packages/modules in the application (except for views, | ||
31 | 99 | see below), should only include library-like code that can be reused in | ||
32 | 100 | different places by either Juju Quickstart or external programs. | ||
33 | 101 | |||
34 | 102 | * ``setting.py`` includes the application settings. Those must be considered as | ||
35 | 103 | constant values to be reused throughout the application. The settings module | ||
36 | 104 | should not import other ``quickstart`` modules. | ||
37 | 105 | |||
38 | 106 | * ``utils.py`` defines general purpose helper functions: when writing such | ||
39 | 107 | utility objects it is likely that this is the right place where they should | ||
40 | 108 | live. Separate modules are created when a set of utilities are related each | ||
41 | 109 | other: this is the case, for instance, of ``serializers.py`` (YAML/JSON | ||
42 | 110 | serialization utilities), ``ssh.py`` (SSH protocol related functions), | ||
43 | 111 | ``platform_support.py`` etc. | ||
44 | 112 | |||
45 | 113 | * ``juju.py`` defines the WebSocket API client used by Juju Quickstart to | ||
46 | 114 | connect to the Juju API server. | ||
47 | 115 | |||
48 | 116 | * the ``models`` package includes a module for each application model. Models | ||
49 | 117 | are abstractions representing the data and information managed by Juju | ||
50 | 118 | Quickstart, e.g. environment files, jenv files or charms. In the | ||
51 | 119 | implementation of models, an effort has been made to use simple data | ||
52 | 120 | structures in order to represent entities/objects, and composable functions | ||
53 | 121 | to manipulate this information. | ||
54 | 122 | |||
55 | 123 | * the ``cli`` package contains the command line interactive session handling. | ||
56 | 124 | Juju Quickstart uses Urwid to implement an ncurses-like interactive session: | ||
57 | 125 | Urwid code must only live in the ``cli`` package. | ||
58 | 126 | |||
59 | 127 | That said, module docstrings often describe the goals and usage of each part of | ||
60 | 128 | the code: go have a look! | ||
61 | 129 | |||
62 | 76 | Pre-release QA | 130 | Pre-release QA |
63 | 77 | ~~~~~~~~~~~~~~ | 131 | ~~~~~~~~~~~~~~ |
64 | 78 | 132 | ||
65 | @@ -98,7 +152,7 @@ | |||
66 | 98 | 152 | ||
67 | 99 | * Verify an environment that has already been bootstrapped is recogized and | 153 | * Verify an environment that has already been bootstrapped is recogized and |
68 | 100 | the GUI is deployed. This test also shows that a remote bundle is properly | 154 | the GUI is deployed. This test also shows that a remote bundle is properly |
70 | 101 | deployed.:: | 155 | deployed:: |
71 | 102 | 156 | ||
72 | 103 | juju bootstrap -e local | 157 | juju bootstrap -e local |
73 | 104 | juju quickstart -e local bundle:mediawiki/single | 158 | juju quickstart -e local bundle:mediawiki/single |
74 | 105 | 159 | ||
75 | === modified file 'quickstart/app.py' | |||
76 | --- quickstart/app.py 2014-11-10 11:39:40 +0000 | |||
77 | +++ quickstart/app.py 2014-11-11 14:26:05 +0000 | |||
78 | @@ -23,7 +23,6 @@ | |||
79 | 23 | 23 | ||
80 | 24 | import json | 24 | import json |
81 | 25 | import logging | 25 | import logging |
82 | 26 | import os | ||
83 | 27 | import sys | 26 | import sys |
84 | 28 | import time | 27 | import time |
85 | 29 | 28 | ||
86 | @@ -37,7 +36,7 @@ | |||
87 | 37 | utils, | 36 | utils, |
88 | 38 | watchers, | 37 | watchers, |
89 | 39 | ) | 38 | ) |
91 | 40 | from quickstart.models import envs | 39 | from quickstart.models import jenv |
92 | 41 | 40 | ||
93 | 42 | 41 | ||
94 | 43 | class ProgramExit(Exception): | 42 | class ProgramExit(Exception): |
95 | @@ -258,27 +257,17 @@ | |||
96 | 258 | raise ProgramExit('the state server is not ready:\n{}'.format(details)) | 257 | raise ProgramExit('the state server is not ready:\n{}'.format(details)) |
97 | 259 | 258 | ||
98 | 260 | 259 | ||
110 | 261 | def get_value_from_jenv(env_name, juju_home, key): | 260 | def get_admin_secret(env_name): |
111 | 262 | """Read the key from the generated environment file. | 261 | """Return the Juju admin secret for the given environment name. |
112 | 263 | 262 | ||
113 | 264 | At bootstrap, juju (v1.16 and later) writes a generated file | 263 | Parse the jenv file to retrieve the admin secret. |
114 | 265 | located in JUJU_HOME. Return the value for the key. | 264 | Raise a ProgramExit if the admin secret cannot be retrieved. |
104 | 266 | |||
105 | 267 | Raise a ValueError if: | ||
106 | 268 | - the environment file is not found; | ||
107 | 269 | - the environment file contents are not parsable by YAML; | ||
108 | 270 | - the YAML contents are not properly structured; | ||
109 | 271 | - the key is not found. | ||
115 | 272 | """ | 265 | """ |
116 | 273 | filename = '{}.jenv'.format(env_name) | ||
117 | 274 | juju_env_file = os.path.expanduser( | ||
118 | 275 | os.path.join(juju_home, 'environments', filename)) | ||
119 | 276 | jenv_db = envs.load_generated(juju_env_file) | ||
120 | 277 | try: | 266 | try: |
125 | 278 | return jenv_db[key] | 267 | return jenv.get_value(env_name, 'bootstrap-config', 'admin-secret') |
126 | 279 | except KeyError: | 268 | except ValueError as err: |
127 | 280 | msg = '{} not found in {}'.format(key, juju_env_file) | 269 | msg = b'cannot retrieve environment admin-secret: {}'.format(err) |
128 | 281 | raise ValueError(msg.encode('utf-8')) | 270 | raise ProgramExit(msg) |
129 | 282 | 271 | ||
130 | 283 | 272 | ||
131 | 284 | def get_api_url(env_name, juju_command): | 273 | def get_api_url(env_name, juju_command): |
132 | 285 | 274 | ||
133 | === modified file 'quickstart/manage.py' | |||
134 | --- quickstart/manage.py 2014-11-10 11:31:02 +0000 | |||
135 | +++ quickstart/manage.py 2014-11-11 14:26:05 +0000 | |||
136 | @@ -293,7 +293,6 @@ | |||
137 | 293 | 'this host platform does not support local environments' | 293 | 'this host platform does not support local environments' |
138 | 294 | ) | 294 | ) |
139 | 295 | # Update the options namespace with the new values. | 295 | # Update the options namespace with the new values. |
140 | 296 | options.admin_secret = env_data.get('admin-secret') | ||
141 | 297 | options.env_file = env_file | 296 | options.env_file = env_file |
142 | 298 | options.env_name = env_data['name'] | 297 | options.env_name = env_data['name'] |
143 | 299 | options.default_series = env_data.get('default-series') | 298 | options.default_series = env_data.get('default-series') |
144 | @@ -343,10 +342,9 @@ | |||
145 | 343 | """Set up the application options and logger. | 342 | """Set up the application options and logger. |
146 | 344 | 343 | ||
147 | 345 | Return the options as a namespace containing the following attributes: | 344 | Return the options as a namespace containing the following attributes: |
148 | 346 | - admin_secret: the password to use to access the Juju API or None if | ||
149 | 347 | no admin-secret is present in the $JUJU_HOME/environment.yaml file; | ||
150 | 348 | - bundle: the optional bundle (path or URL) to be deployed; | 345 | - bundle: the optional bundle (path or URL) to be deployed; |
151 | 349 | - charm_url: the Juju GUI charm URL or None if not specified; | 346 | - charm_url: the Juju GUI charm URL or None if not specified; |
152 | 347 | - constraints: the environment constrains or None if not set; | ||
153 | 350 | - debug: whether debug mode is activated; | 348 | - debug: whether debug mode is activated; |
154 | 351 | - distro_only: install Juju only using the distribution packages; | 349 | - distro_only: install Juju only using the distribution packages; |
155 | 352 | - env_file: the absolute path of the Juju environments.yaml file; | 350 | - env_file: the absolute path of the Juju environments.yaml file; |
156 | @@ -357,8 +355,7 @@ | |||
157 | 357 | - platform: The host platform; | 355 | - platform: The host platform; |
158 | 358 | - upload_tools: whether to upload local version of tools; | 356 | - upload_tools: whether to upload local version of tools; |
159 | 359 | - upload_series: the comma-separated series list for which tools will | 357 | - upload_series: the comma-separated series list for which tools will |
162 | 360 | be uploaded, or None if not set; | 358 | be uploaded, or None if not set. |
161 | 361 | - constraints: the environment constrains or None if not set. | ||
163 | 362 | 359 | ||
164 | 363 | The following attributes will also be included in the namespace if a bundle | 360 | The following attributes will also be included in the namespace if a bundle |
165 | 364 | deployment is requested: | 361 | deployment is requested: |
166 | @@ -522,16 +519,7 @@ | |||
167 | 522 | constraints=options.constraints) | 519 | constraints=options.constraints) |
168 | 523 | 520 | ||
169 | 524 | # Retrieve the admin-secret for the current environment. | 521 | # Retrieve the admin-secret for the current environment. |
180 | 525 | try: | 522 | admin_secret = app.get_admin_secret(options.env_name) |
171 | 526 | admin_secret = app.get_value_from_jenv( | ||
172 | 527 | options.env_name, settings.JUJU_HOME, 'admin-secret') | ||
173 | 528 | except ValueError as err: | ||
174 | 529 | admin_secret = options.admin_secret | ||
175 | 530 | if admin_secret is None: | ||
176 | 531 | # The admin-secret cannot be found in the jenv file and is not | ||
177 | 532 | # explicitly specified in the environments.yaml file. | ||
178 | 533 | msg = b'{} or {}'.format(err, options.env_file.encode('utf-8')) | ||
179 | 534 | raise app.ProgramExit(msg) | ||
181 | 535 | 523 | ||
182 | 536 | print('retrieving the Juju API address') | 524 | print('retrieving the Juju API address') |
183 | 537 | api_url = app.get_api_url(options.env_name, juju_command) | 525 | api_url = app.get_api_url(options.env_name, juju_command) |
184 | 538 | 526 | ||
185 | === modified file 'quickstart/models/envs.py' | |||
186 | --- quickstart/models/envs.py 2014-10-09 13:14:30 +0000 | |||
187 | +++ quickstart/models/envs.py 2014-11-11 14:26:05 +0000 | |||
188 | @@ -134,23 +134,6 @@ | |||
189 | 134 | return {'environments': {}} | 134 | return {'environments': {}} |
190 | 135 | 135 | ||
191 | 136 | 136 | ||
192 | 137 | def _load_file(env_file): | ||
193 | 138 | """Load the given file and return the YAML-parsed contents.""" | ||
194 | 139 | # Load the Juju environments file. | ||
195 | 140 | try: | ||
196 | 141 | environments_file = open(env_file.encode('utf-8')) | ||
197 | 142 | except IOError as err: | ||
198 | 143 | msg = b'unable to open environments file: {}'.format(err) | ||
199 | 144 | raise ValueError(msg) | ||
200 | 145 | # Parse the Juju environments file. | ||
201 | 146 | try: | ||
202 | 147 | contents = serializers.yaml_load(environments_file) | ||
203 | 148 | except Exception as err: | ||
204 | 149 | msg = b'unable to parse environments file {}: {}' | ||
205 | 150 | raise ValueError(msg.format(env_file.encode('utf-8'), err)) | ||
206 | 151 | return contents | ||
207 | 152 | |||
208 | 153 | |||
209 | 154 | def load(env_file): | 137 | def load(env_file): |
210 | 155 | """Load and parse the provided Juju environments.yaml file. | 138 | """Load and parse the provided Juju environments.yaml file. |
211 | 156 | 139 | ||
212 | @@ -174,7 +157,7 @@ | |||
213 | 174 | - the environment file contents are not parsable by YAML; | 157 | - the environment file contents are not parsable by YAML; |
214 | 175 | - the YAML contents are not properly structured. | 158 | - the YAML contents are not properly structured. |
215 | 176 | """ | 159 | """ |
217 | 177 | contents = _load_file(env_file) | 160 | contents = serializers.yaml_load_from_path(env_file) |
218 | 178 | if contents is None: | 161 | if contents is None: |
219 | 179 | return create_empty_env_db() | 162 | return create_empty_env_db() |
220 | 180 | # Retrieve the environment list. | 163 | # Retrieve the environment list. |
221 | @@ -199,18 +182,6 @@ | |||
222 | 199 | return env_db | 182 | return env_db |
223 | 200 | 183 | ||
224 | 201 | 184 | ||
225 | 202 | def load_generated(env_file, section='bootstrap-config'): | ||
226 | 203 | """Given the path to a YAML file, load the file and return the section.""" | ||
227 | 204 | contents = _load_file(env_file) | ||
228 | 205 | try: | ||
229 | 206 | section_contents = contents[section] | ||
230 | 207 | except (KeyError, TypeError): | ||
231 | 208 | msg = 'invalid YAML contents in {}: {}'.format(env_file, contents) | ||
232 | 209 | raise ValueError(msg.encode('utf-8')) | ||
233 | 210 | |||
234 | 211 | return section_contents | ||
235 | 212 | |||
236 | 213 | |||
237 | 214 | def save(env_file, env_db, backup_function=None): | 185 | def save(env_file, env_db, backup_function=None): |
238 | 215 | """Save the given env_db to the provided environments.yaml file. | 186 | """Save the given env_db to the provided environments.yaml file. |
239 | 216 | 187 | ||
240 | 217 | 188 | ||
241 | === added file 'quickstart/models/jenv.py' | |||
242 | --- quickstart/models/jenv.py 1970-01-01 00:00:00 +0000 | |||
243 | +++ quickstart/models/jenv.py 2014-11-11 14:26:05 +0000 | |||
244 | @@ -0,0 +1,71 @@ | |||
245 | 1 | # This file is part of the Juju Quickstart Plugin, which lets users set up a | ||
246 | 2 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). | ||
247 | 3 | # Copyright (C) 2014 Canonical Ltd. | ||
248 | 4 | # | ||
249 | 5 | # This program is free software: you can redistribute it and/or modify it under | ||
250 | 6 | # the terms of the GNU Affero General Public License version 3, as published by | ||
251 | 7 | # the Free Software Foundation. | ||
252 | 8 | # | ||
253 | 9 | # This program is distributed in the hope that it will be useful, but WITHOUT | ||
254 | 10 | # ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, | ||
255 | 11 | # SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
256 | 12 | # Affero General Public License for more details. | ||
257 | 13 | # | ||
258 | 14 | # You should have received a copy of the GNU Affero General Public License | ||
259 | 15 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
260 | 16 | |||
261 | 17 | """Juju Quickstart jenv generated files handling. | ||
262 | 18 | |||
263 | 19 | At bootstrap, Juju writes a generated file (jenv) located in JUJU_HOME. | ||
264 | 20 | This module includes functions to load and parse those jenv file contents. | ||
265 | 21 | """ | ||
266 | 22 | |||
267 | 23 | from __future__ import unicode_literals | ||
268 | 24 | |||
269 | 25 | import os | ||
270 | 26 | |||
271 | 27 | from quickstart import ( | ||
272 | 28 | serializers, | ||
273 | 29 | settings, | ||
274 | 30 | ) | ||
275 | 31 | |||
276 | 32 | |||
277 | 33 | def exists(env_name): | ||
278 | 34 | """Report whether the jenv generated file exists for the given env_name.""" | ||
279 | 35 | jenv_path = _get_jenv_path(env_name) | ||
280 | 36 | return os.path.isfile(jenv_path) | ||
281 | 37 | |||
282 | 38 | |||
283 | 39 | def get_value(env_name, *args): | ||
284 | 40 | """Read a value from the Juju generated environment file (jenv). | ||
285 | 41 | |||
286 | 42 | Return the value corresponding to the section specified in args. | ||
287 | 43 | For instance, calling get_value('ec2', 'bootstrap-config', 'admin-secret') | ||
288 | 44 | returns the value associated with the "admin-secret" key included on the | ||
289 | 45 | "bootstrap-config" section of the jenv file. | ||
290 | 46 | |||
291 | 47 | Raise a ValueError if: | ||
292 | 48 | - the environment file is not found; | ||
293 | 49 | - the environment file contents are not parsable by YAML; | ||
294 | 50 | - the YAML contents are not properly structured; | ||
295 | 51 | - one the keys in args is not found. | ||
296 | 52 | """ | ||
297 | 53 | jenv_path = _get_jenv_path(env_name) | ||
298 | 54 | data = serializers.yaml_load_from_path(jenv_path) | ||
299 | 55 | section = 'root' | ||
300 | 56 | for key in args: | ||
301 | 57 | try: | ||
302 | 58 | data = data[key] | ||
303 | 59 | except (KeyError, TypeError): | ||
304 | 60 | msg = ('invalid YAML contents in {}: ''{} key not found in the {} ' | ||
305 | 61 | 'section'.format(jenv_path, key, section)) | ||
306 | 62 | raise ValueError(msg.encode('utf-8')) | ||
307 | 63 | section = key | ||
308 | 64 | return data | ||
309 | 65 | |||
310 | 66 | |||
311 | 67 | def _get_jenv_path(env_name): | ||
312 | 68 | """Return the path to the generated jenv file for the given env_name.""" | ||
313 | 69 | filename = '{}.jenv'.format(env_name) | ||
314 | 70 | path = os.path.join(settings.JUJU_HOME, 'environments', filename) | ||
315 | 71 | return os.path.expanduser(path) | ||
316 | 0 | 72 | ||
317 | === modified file 'quickstart/serializers.py' | |||
318 | --- quickstart/serializers.py 2013-12-06 17:17:19 +0000 | |||
319 | +++ quickstart/serializers.py 2014-11-11 14:26:05 +0000 | |||
320 | @@ -45,3 +45,23 @@ | |||
321 | 45 | Always serialize collections in the block style. | 45 | Always serialize collections in the block style. |
322 | 46 | """ | 46 | """ |
323 | 47 | return yaml.safe_dump(data, stream, default_flow_style=False) | 47 | return yaml.safe_dump(data, stream, default_flow_style=False) |
324 | 48 | |||
325 | 49 | |||
326 | 50 | def yaml_load_from_path(path): | ||
327 | 51 | """Load the given file path and return the YAML-parsed contents. | ||
328 | 52 | |||
329 | 53 | Raise a ValueError if the file cannot be opened or parsed. | ||
330 | 54 | """ | ||
331 | 55 | filename = path.encode('utf-8') | ||
332 | 56 | try: | ||
333 | 57 | yaml_file = open(filename) | ||
334 | 58 | except IOError as err: | ||
335 | 59 | msg = b'unable to open file {}: {}'.format(filename, err) | ||
336 | 60 | raise ValueError(msg) | ||
337 | 61 | # Parse the Juju environments file. | ||
338 | 62 | try: | ||
339 | 63 | contents = yaml_load(yaml_file) | ||
340 | 64 | except Exception as err: | ||
341 | 65 | msg = b'unable to parse file {}: {}'.format(filename, err) | ||
342 | 66 | raise ValueError(msg) | ||
343 | 67 | return contents | ||
344 | 48 | 68 | ||
345 | === modified file 'quickstart/tests/helpers.py' | |||
346 | --- quickstart/tests/helpers.py 2014-11-10 09:08:44 +0000 | |||
347 | +++ quickstart/tests/helpers.py 2014-11-11 14:26:05 +0000 | |||
348 | @@ -136,6 +136,42 @@ | |||
349 | 136 | return env_file.name | 136 | return env_file.name |
350 | 137 | 137 | ||
351 | 138 | 138 | ||
352 | 139 | class JenvFileTestsMixin(object): | ||
353 | 140 | """Shared methods for testing Juju generated environment files (jenv).""" | ||
354 | 141 | |||
355 | 142 | jenv_data = { | ||
356 | 143 | 'user': 'admin', | ||
357 | 144 | 'state-servers': ['localhost:17070', '10.0.3.1:17070'], | ||
358 | 145 | 'bootstrap-config': { | ||
359 | 146 | 'admin-secret': 'Secret!', | ||
360 | 147 | 'api-port': 17070, | ||
361 | 148 | }, | ||
362 | 149 | 'life': {'universe': {'everything': 42}}, | ||
363 | 150 | } | ||
364 | 151 | |||
365 | 152 | @contextmanager | ||
366 | 153 | def make_jenv(self, env_name, contents): | ||
367 | 154 | """Create a temporary jenv file with the given env_name and contents. | ||
368 | 155 | |||
369 | 156 | In the contentx manager block, the JUJU_HOME is set to the ancestor | ||
370 | 157 | of the generated temporary file. | ||
371 | 158 | |||
372 | 159 | Return the jenv file path. | ||
373 | 160 | """ | ||
374 | 161 | # Create a temporary JUJU_HOME. | ||
375 | 162 | playground = tempfile.mkdtemp() | ||
376 | 163 | self.addCleanup(shutil.rmtree, playground) | ||
377 | 164 | environments_dir = os.path.join(playground, 'environments') | ||
378 | 165 | os.mkdir(environments_dir) | ||
379 | 166 | # Create the jenv file inside the temporary JUJU_HOME. | ||
380 | 167 | jenv_path = os.path.join(environments_dir, '{}.jenv'.format(env_name)) | ||
381 | 168 | with open(jenv_path, 'w') as jenv_file: | ||
382 | 169 | jenv_file.write(contents) | ||
383 | 170 | # Patch the JUJU_HOME and return the jenv file path. | ||
384 | 171 | with mock.patch('quickstart.settings.JUJU_HOME', playground): | ||
385 | 172 | yield jenv_path | ||
386 | 173 | |||
387 | 174 | |||
388 | 139 | def make_env_db(default=None, exclude_invalid=False): | 175 | def make_env_db(default=None, exclude_invalid=False): |
389 | 140 | """Create and return an env_db. | 176 | """Create and return an env_db. |
390 | 141 | 177 | ||
391 | 142 | 178 | ||
392 | === modified file 'quickstart/tests/models/test_envs.py' | |||
393 | --- quickstart/tests/models/test_envs.py 2014-10-09 15:29:46 +0000 | |||
394 | +++ quickstart/tests/models/test_envs.py 2014-11-11 14:26:05 +0000 | |||
395 | @@ -99,28 +99,6 @@ | |||
396 | 99 | self.assertEqual({'environments': {}}, env_db) | 99 | self.assertEqual({'environments': {}}, env_db) |
397 | 100 | 100 | ||
398 | 101 | 101 | ||
399 | 102 | class TestLoadFile( | ||
400 | 103 | helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin, | ||
401 | 104 | unittest.TestCase): | ||
402 | 105 | |||
403 | 106 | def test_no_file(self): | ||
404 | 107 | # A ValueError is raised if the environments file is not found. | ||
405 | 108 | expected = ( | ||
406 | 109 | "unable to open environments file: " | ||
407 | 110 | "[Errno 2] No such file or directory: '/no/such/file.yaml'" | ||
408 | 111 | ) | ||
409 | 112 | with self.assert_value_error(expected): | ||
410 | 113 | envs._load_file('/no/such/file.yaml') | ||
411 | 114 | |||
412 | 115 | def test_invalid_yaml(self): | ||
413 | 116 | # A ValueError is raised if the environments file is not a valid YAML. | ||
414 | 117 | env_file = self.make_env_file(':') | ||
415 | 118 | with self.assertRaises(ValueError) as context_manager: | ||
416 | 119 | envs._load_file(env_file) | ||
417 | 120 | expected = 'unable to parse environments file {}'.format(env_file) | ||
418 | 121 | self.assertIn(expected, bytes(context_manager.exception)) | ||
419 | 122 | |||
420 | 123 | |||
421 | 124 | class TestLoad( | 102 | class TestLoad( |
422 | 125 | helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin, | 103 | helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin, |
423 | 126 | unittest.TestCase): | 104 | unittest.TestCase): |
424 | @@ -205,60 +183,6 @@ | |||
425 | 205 | self.assertEqual(expected, env_db) | 183 | self.assertEqual(expected, env_db) |
426 | 206 | 184 | ||
427 | 207 | 185 | ||
428 | 208 | class TestLoadGenerated( | ||
429 | 209 | helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin, | ||
430 | 210 | unittest.TestCase): | ||
431 | 211 | |||
432 | 212 | def test_empty_file(self): | ||
433 | 213 | # A ValueError is raised if the environments file is empty. | ||
434 | 214 | env_file = self.make_env_file('') | ||
435 | 215 | expected = 'invalid YAML contents in {}: None'.format(env_file) | ||
436 | 216 | with self.assert_value_error(expected): | ||
437 | 217 | envs.load_generated(env_file) | ||
438 | 218 | |||
439 | 219 | def test_invalid_yaml_contents(self): | ||
440 | 220 | # A ValueError is raised if the environments file is not well formed. | ||
441 | 221 | env_file = self.make_env_file('a-string') | ||
442 | 222 | expected = 'invalid YAML contents in {}: a-string'.format(env_file) | ||
443 | 223 | with self.assert_value_error(expected): | ||
444 | 224 | envs.load_generated(env_file) | ||
445 | 225 | |||
446 | 226 | def test_section_not_found(self): | ||
447 | 227 | expected = { | ||
448 | 228 | 'shoehorn-config': { | ||
449 | 229 | 'admin-secret': 'Secret!', | ||
450 | 230 | 'type': 'ec2'}, | ||
451 | 231 | } | ||
452 | 232 | yaml_contents = expected.copy() | ||
453 | 233 | env_file = self.make_env_file(yaml.safe_dump(yaml_contents)) | ||
454 | 234 | expected = 'invalid YAML contents in {}: {}'.format( | ||
455 | 235 | env_file, yaml_contents) | ||
456 | 236 | with self.assert_value_error(expected): | ||
457 | 237 | envs.load_generated(env_file) | ||
458 | 238 | |||
459 | 239 | def test_successful_default_section(self): | ||
460 | 240 | expected = { | ||
461 | 241 | 'bootstrap-config': { | ||
462 | 242 | 'admin-secret': 'Secret!', | ||
463 | 243 | 'type': 'ec2'}, | ||
464 | 244 | } | ||
465 | 245 | yaml_contents = expected.copy() | ||
466 | 246 | env_file = self.make_env_file(yaml.safe_dump(yaml_contents)) | ||
467 | 247 | env_db = envs.load_generated(env_file) | ||
468 | 248 | self.assertEqual(expected['bootstrap-config'], env_db) | ||
469 | 249 | |||
470 | 250 | def test_successful_specified_section(self): | ||
471 | 251 | expected = { | ||
472 | 252 | 'my-config': { | ||
473 | 253 | 'admin-secret': 'Secret!', | ||
474 | 254 | 'type': 'ec2'}, | ||
475 | 255 | } | ||
476 | 256 | yaml_contents = expected.copy() | ||
477 | 257 | env_file = self.make_env_file(yaml.safe_dump(yaml_contents)) | ||
478 | 258 | env_db = envs.load_generated(env_file, section='my-config') | ||
479 | 259 | self.assertEqual(expected['my-config'], env_db) | ||
480 | 260 | |||
481 | 261 | |||
482 | 262 | class TestSave(helpers.EnvFileTestsMixin, unittest.TestCase): | 186 | class TestSave(helpers.EnvFileTestsMixin, unittest.TestCase): |
483 | 263 | 187 | ||
484 | 264 | def setUp(self): | 188 | def setUp(self): |
485 | 265 | 189 | ||
486 | === added file 'quickstart/tests/models/test_jenv.py' | |||
487 | --- quickstart/tests/models/test_jenv.py 1970-01-01 00:00:00 +0000 | |||
488 | +++ quickstart/tests/models/test_jenv.py 2014-11-11 14:26:05 +0000 | |||
489 | @@ -0,0 +1,110 @@ | |||
490 | 1 | # This file is part of the Juju Quickstart Plugin, which lets users set up a | ||
491 | 2 | # Juju environment in very few steps (https://launchpad.net/juju-quickstart). | ||
492 | 3 | # Copyright (C) 2014 Canonical Ltd. | ||
493 | 4 | # | ||
494 | 5 | # This program is free software: you can redistribute it and/or modify it under | ||
495 | 6 | # the terms of the GNU Affero General Public License version 3, as published by | ||
496 | 7 | # the Free Software Foundation. | ||
497 | 8 | # | ||
498 | 9 | # This program is distributed in the hope that it will be useful, but WITHOUT | ||
499 | 10 | # ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, | ||
500 | 11 | # SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
501 | 12 | # Affero General Public License for more details. | ||
502 | 13 | # | ||
503 | 14 | # You should have received a copy of the GNU Affero General Public License | ||
504 | 15 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
505 | 16 | |||
506 | 17 | """Tests for the Juju Quickstart jenv generated files handling.""" | ||
507 | 18 | |||
508 | 19 | from __future__ import unicode_literals | ||
509 | 20 | |||
510 | 21 | import unittest | ||
511 | 22 | |||
512 | 23 | import yaml | ||
513 | 24 | |||
514 | 25 | from quickstart.models import jenv | ||
515 | 26 | from quickstart.tests import helpers | ||
516 | 27 | |||
517 | 28 | |||
518 | 29 | class TestExists(helpers.JenvFileTestsMixin, unittest.TestCase): | ||
519 | 30 | |||
520 | 31 | def test_found(self): | ||
521 | 32 | # True is returned if the jenv file exists. | ||
522 | 33 | with self.make_jenv('ec2', ''): | ||
523 | 34 | exists = jenv.exists('ec2') | ||
524 | 35 | self.assertTrue(exists) | ||
525 | 36 | |||
526 | 37 | def test_not_found(self): | ||
527 | 38 | # False is returned if the jenv file does not exist. | ||
528 | 39 | with self.make_jenv('ec2', ''): | ||
529 | 40 | exists = jenv.exists('local') | ||
530 | 41 | self.assertFalse(exists) | ||
531 | 42 | |||
532 | 43 | |||
533 | 44 | class TestGetValue( | ||
534 | 45 | helpers.JenvFileTestsMixin, helpers.ValueErrorTestsMixin, | ||
535 | 46 | unittest.TestCase): | ||
536 | 47 | |||
537 | 48 | def test_whole_content(self): | ||
538 | 49 | # The function correctly returns the whole jenv content. | ||
539 | 50 | with self.make_jenv('local', yaml.safe_dump(self.jenv_data)): | ||
540 | 51 | data = jenv.get_value('local') | ||
541 | 52 | self.assertEqual(self.jenv_data, data) | ||
542 | 53 | |||
543 | 54 | def test_section(self): | ||
544 | 55 | # The function correctly returns a whole section. | ||
545 | 56 | with self.make_jenv('ec2', yaml.safe_dump(self.jenv_data)): | ||
546 | 57 | data = jenv.get_value('ec2', 'bootstrap-config') | ||
547 | 58 | self.assertEqual(self.jenv_data['bootstrap-config'], data) | ||
548 | 59 | |||
549 | 60 | def test_value(self): | ||
550 | 61 | # The function correctly returns a value in the root node. | ||
551 | 62 | with self.make_jenv('ec2', yaml.safe_dump(self.jenv_data)): | ||
552 | 63 | value = jenv.get_value('ec2', 'user') | ||
553 | 64 | self.assertEqual('admin', value) | ||
554 | 65 | |||
555 | 66 | def test_section_value(self): | ||
556 | 67 | # The function correctly returns a section value. | ||
557 | 68 | with self.make_jenv('ec2', yaml.safe_dump(self.jenv_data)): | ||
558 | 69 | value = jenv.get_value('ec2', 'bootstrap-config', 'admin-secret') | ||
559 | 70 | self.assertEqual('Secret!', value) | ||
560 | 71 | |||
561 | 72 | def test_nested_section(self): | ||
562 | 73 | # The function correctly returns nested section's values. | ||
563 | 74 | with self.make_jenv('hp', yaml.safe_dump(self.jenv_data)): | ||
564 | 75 | value = jenv.get_value('hp', 'life', 'universe', 'everything') | ||
565 | 76 | self.assertEqual(42, value) | ||
566 | 77 | |||
567 | 78 | def test_file_not_found(self): | ||
568 | 79 | # A ValueError is raised if the jenv file cannot be found. | ||
569 | 80 | expected_error = 'unable to open file' | ||
570 | 81 | with self.make_jenv('hp', yaml.safe_dump(self.jenv_data)): | ||
571 | 82 | with self.assertRaises(ValueError) as context_manager: | ||
572 | 83 | jenv.get_value('local') | ||
573 | 84 | self.assertIn(expected_error, bytes(context_manager.exception)) | ||
574 | 85 | |||
575 | 86 | def test_section_not_found(self): | ||
576 | 87 | # A ValueError is raised if the specified section cannot be found. | ||
577 | 88 | with self.make_jenv('local', yaml.safe_dump(self.jenv_data)) as path: | ||
578 | 89 | expected_error = ( | ||
579 | 90 | 'invalid YAML contents in {}: no-such key not found in the ' | ||
580 | 91 | 'root section'.format(path)) | ||
581 | 92 | with self.assert_value_error(expected_error): | ||
582 | 93 | jenv.get_value('local', 'no-such') | ||
583 | 94 | |||
584 | 95 | def test_subsection_not_found(self): | ||
585 | 96 | # A ValueError is raised if the specified subsection cannot be found. | ||
586 | 97 | with self.make_jenv('local', yaml.safe_dump(self.jenv_data)) as path: | ||
587 | 98 | expected_error = ( | ||
588 | 99 | 'invalid YAML contents in {}: series key not found in the ' | ||
589 | 100 | 'bootstrap-config section'.format(path)) | ||
590 | 101 | with self.assert_value_error(expected_error): | ||
591 | 102 | jenv.get_value('local', 'bootstrap-config', 'series') | ||
592 | 103 | |||
593 | 104 | def test_invalid_yaml_contents(self): | ||
594 | 105 | # A ValueError is raised if the jenv file is not well formed. | ||
595 | 106 | expected_error = 'unable to parse file' | ||
596 | 107 | with self.make_jenv('ec2', ':'): | ||
597 | 108 | with self.assertRaises(ValueError) as context_manager: | ||
598 | 109 | jenv.get_value('ec2') | ||
599 | 110 | self.assertIn(expected_error, bytes(context_manager.exception)) | ||
600 | 0 | 111 | ||
601 | === modified file 'quickstart/tests/test_app.py' | |||
602 | --- quickstart/tests/test_app.py 2014-11-10 11:39:40 +0000 | |||
603 | +++ quickstart/tests/test_app.py 2014-11-11 14:26:05 +0000 | |||
604 | @@ -673,26 +673,24 @@ | |||
605 | 673 | ] + self.make_status_calls(2)) | 673 | ] + self.make_status_calls(2)) |
606 | 674 | 674 | ||
607 | 675 | 675 | ||
620 | 676 | class TestGetValueFromJenv(unittest.TestCase): | 676 | class TestGetAdminSecret( |
621 | 677 | 677 | helpers.JenvFileTestsMixin, ProgramExitTestsMixin, unittest.TestCase): | |
610 | 678 | def test_no_key(self): | ||
611 | 679 | with mock.patch('quickstart.manage.envs.load_generated', | ||
612 | 680 | lambda x: {}): | ||
613 | 681 | with self.assertRaises(ValueError) as exc: | ||
614 | 682 | app.get_value_from_jenv( | ||
615 | 683 | 'local', '/home/bac/.juju', 'my-key') | ||
616 | 684 | expected = ( | ||
617 | 685 | u'my-key not found in ' | ||
618 | 686 | '/home/bac/.juju/environments/local.jenv') | ||
619 | 687 | self.assertIn(expected, bytes(exc.exception)) | ||
622 | 688 | 678 | ||
623 | 689 | def test_success(self): | 679 | def test_success(self): |
630 | 690 | expected = 'superchunk' | 680 | # The admin secret is successfully retrieved. |
631 | 691 | with mock.patch('quickstart.manage.envs.load_generated', | 681 | with self.make_jenv('ec2', yaml.safe_dump(self.jenv_data)): |
632 | 692 | lambda x: {'my-key': expected}): | 682 | admin_secret = app.get_admin_secret('ec2') |
633 | 693 | secret = app.get_value_from_jenv( | 683 | self.assertEqual('Secret!', admin_secret) |
634 | 694 | 'local', '~bac/.juju', 'my-key') | 684 | |
635 | 695 | self.assertEqual(expected, secret) | 685 | def test_error(self): |
636 | 686 | # A ProgramExit is raised if the admin secret cannot be retrieved. | ||
637 | 687 | with self.make_jenv('ec2', '') as path: | ||
638 | 688 | expected_error = ( | ||
639 | 689 | 'cannot retrieve environment admin-secret: invalid YAML ' | ||
640 | 690 | 'contents in {}: bootstrap-config key not found in the root ' | ||
641 | 691 | 'section'.format(path)) | ||
642 | 692 | with self.assert_program_exit(expected_error): | ||
643 | 693 | app.get_admin_secret('ec2') | ||
644 | 696 | 694 | ||
645 | 697 | 695 | ||
646 | 698 | class TestGetApiUrl( | 696 | class TestGetApiUrl( |
647 | 699 | 697 | ||
648 | === modified file 'quickstart/tests/test_manage.py' | |||
649 | --- quickstart/tests/test_manage.py 2014-11-10 11:27:04 +0000 | |||
650 | +++ quickstart/tests/test_manage.py 2014-11-11 14:26:05 +0000 | |||
651 | @@ -509,7 +509,6 @@ | |||
652 | 509 | options = self.make_options( | 509 | options = self.make_options( |
653 | 510 | env_file, env_name='aws', interactive=False) | 510 | env_file, env_name='aws', interactive=False) |
654 | 511 | manage._setup_env(options, self.parser) | 511 | manage._setup_env(options, self.parser) |
655 | 512 | self.assertEqual('Secret!', options.admin_secret) | ||
656 | 513 | self.assertEqual(env_file, options.env_file) | 512 | self.assertEqual(env_file, options.env_file) |
657 | 514 | self.assertEqual('aws', options.env_name) | 513 | self.assertEqual('aws', options.env_name) |
658 | 515 | self.assertEqual('ec2', options.env_type) | 514 | self.assertEqual('ec2', options.env_type) |
659 | @@ -550,7 +549,6 @@ | |||
660 | 550 | options = self.make_options( | 549 | options = self.make_options( |
661 | 551 | env_file, env_name='lxc', interactive=False) | 550 | env_file, env_name='lxc', interactive=False) |
662 | 552 | manage._setup_env(options, self.parser) | 551 | manage._setup_env(options, self.parser) |
663 | 553 | self.assertEqual('Secret!', options.admin_secret) | ||
664 | 554 | self.assertEqual(env_file, options.env_file) | 552 | self.assertEqual(env_file, options.env_file) |
665 | 555 | self.assertEqual('lxc', options.env_name) | 553 | self.assertEqual('lxc', options.env_name) |
666 | 556 | self.assertEqual('local', options.env_type) | 554 | self.assertEqual('local', options.env_type) |
667 | @@ -593,7 +591,6 @@ | |||
668 | 593 | mock_interactive.assert_called_once_with( | 591 | mock_interactive.assert_called_once_with( |
669 | 594 | self.parser, mock_get_env_type_db(), env_db, env_file) | 592 | self.parser, mock_get_env_type_db(), env_db, env_file) |
670 | 595 | # The options is updated with data from the selected environment. | 593 | # The options is updated with data from the selected environment. |
671 | 596 | self.assertEqual('Secret!', options.admin_secret) | ||
672 | 597 | self.assertEqual(env_file, options.env_file) | 594 | self.assertEqual(env_file, options.env_file) |
673 | 598 | self.assertEqual('aws', options.env_name) | 595 | self.assertEqual('aws', options.env_name) |
674 | 599 | self.assertEqual('ec2', options.env_type) | 596 | self.assertEqual('ec2', options.env_type) |
675 | @@ -754,7 +751,6 @@ | |||
676 | 754 | def make_options(self, **kwargs): | 751 | def make_options(self, **kwargs): |
677 | 755 | """Set up the options to be passed to the run function.""" | 752 | """Set up the options to be passed to the run function.""" |
678 | 756 | options = { | 753 | options = { |
679 | 757 | 'admin_secret': 'Secretz!', | ||
680 | 758 | 'bundle': None, | 754 | 'bundle': None, |
681 | 759 | 'bundle_id': None, | 755 | 'bundle_id': None, |
682 | 760 | 'charm_url': None, | 756 | 'charm_url': None, |
683 | @@ -767,17 +763,6 @@ | |||
684 | 767 | options.update(kwargs) | 763 | options.update(kwargs) |
685 | 768 | return mock.Mock(**options) | 764 | return mock.Mock(**options) |
686 | 769 | 765 | ||
687 | 770 | @staticmethod | ||
688 | 771 | def mock_get_value_from_jenv_success(name, home, key): | ||
689 | 772 | return 'from jenv' | ||
690 | 773 | |||
691 | 774 | @staticmethod | ||
692 | 775 | def mock_get_value_from_jenv_error(name, home, key): | ||
693 | 776 | fn = '{}.jenv'.format(name) | ||
694 | 777 | path = os.path.join(home, 'environments', fn) | ||
695 | 778 | msg = '{} not found in {}'.format(key, path) | ||
696 | 779 | raise ValueError(msg.encode('utf-8')) | ||
697 | 780 | |||
698 | 781 | def test_no_bundle(self, mock_app, mock_open): | 766 | def test_no_bundle(self, mock_app, mock_open): |
699 | 782 | # The application runs correctly if no bundle is provided. | 767 | # The application runs correctly if no bundle is provided. |
700 | 783 | mock_app.ensure_dependencies.return_value = (1, 18, 0) | 768 | mock_app.ensure_dependencies.return_value = (1, 18, 0) |
701 | @@ -790,7 +775,8 @@ | |||
702 | 790 | unit_data = {'Name': 'juju-gui/0'} | 775 | unit_data = {'Name': 'juju-gui/0'} |
703 | 791 | mock_app.check_environment.return_value = ( | 776 | mock_app.check_environment.return_value = ( |
704 | 792 | 'cs:trusty/juju-gui-42', '0', service_data, unit_data) | 777 | 'cs:trusty/juju-gui-42', '0', service_data, unit_data) |
706 | 793 | mock_app.get_value_from_jenv = self.mock_get_value_from_jenv_error | 778 | # Make mock_app.get_admin_secret return the admin secret |
707 | 779 | mock_app.get_admin_secret.return_value = 'Secret!' | ||
708 | 794 | # Make mock_app.watch return the Juju GUI unit address. | 780 | # Make mock_app.watch return the Juju GUI unit address. |
709 | 795 | mock_app.watch.return_value = '1.2.3.4' | 781 | mock_app.watch.return_value = '1.2.3.4' |
710 | 796 | # Make mock_app.create_auth_token return a fake authentication token. | 782 | # Make mock_app.create_auth_token return a fake authentication token. |
711 | @@ -810,10 +796,9 @@ | |||
712 | 810 | mock_app.get_api_url.assert_called_once_with( | 796 | mock_app.get_api_url.assert_called_once_with( |
713 | 811 | options.env_name, self.juju_command) | 797 | options.env_name, self.juju_command) |
714 | 812 | mock_app.connect.assert_has_calls([ | 798 | mock_app.connect.assert_has_calls([ |
717 | 813 | mock.call( | 799 | mock.call(mock_app.get_api_url(), 'Secret!'), |
716 | 814 | mock_app.get_api_url(), options.admin_secret), | ||
718 | 815 | mock.call().close(), | 800 | mock.call().close(), |
720 | 816 | mock.call('wss://1.2.3.4:443/ws', options.admin_secret), | 801 | mock.call('wss://1.2.3.4:443/ws', 'Secret!'), |
721 | 817 | mock.call().close(), | 802 | mock.call().close(), |
722 | 818 | ]) | 803 | ]) |
723 | 819 | mock_app.check_environment.assert_called_once_with( | 804 | mock_app.check_environment.assert_called_once_with( |
724 | @@ -906,44 +891,9 @@ | |||
725 | 906 | manage.run(options) | 891 | manage.run(options) |
726 | 907 | self.assertFalse(mock_open.called) | 892 | self.assertFalse(mock_open.called) |
727 | 908 | 893 | ||
728 | 909 | def test_admin_secret_fetched(self, mock_app, mock_open): | ||
729 | 910 | # If an admin secret is fetched from jenv it is used, even if one is | ||
730 | 911 | # found in environments.yaml, as set in options.admin_secret. | ||
731 | 912 | mock_app.get_value_from_jenv = self.mock_get_value_from_jenv_success | ||
732 | 913 | # Make mock_app.bootstrap return the already_bootstrapped flag and the | ||
733 | 914 | # bootstrap node series. | ||
734 | 915 | mock_app.bootstrap.return_value = (True, 'precise') | ||
735 | 916 | # Make mock_app.check_environment return the charm URL, the machine | ||
736 | 917 | # where to deploy the charm, the service and unit data. | ||
737 | 918 | mock_app.check_environment.return_value = ( | ||
738 | 919 | 'cs:precise/juju-gui-42', '0', None, None) | ||
739 | 920 | options = self.make_options(admin_secret='secret in environments.yaml') | ||
740 | 921 | manage.run(options) | ||
741 | 922 | mock_app.connect.assert_has_calls([ | ||
742 | 923 | mock.call(mock_app.get_api_url(), 'from jenv'), | ||
743 | 924 | ]) | ||
744 | 925 | |||
745 | 926 | def test_admin_secret_from_environments_yaml(self, mock_app, mock_open): | ||
746 | 927 | # If an admin secret is not fetched from jenv, then the one from | ||
747 | 928 | # environments.yaml is used, as found in options.admin_secret. | ||
748 | 929 | mock_app.get_value_from_jenv = self.mock_get_value_from_jenv_error | ||
749 | 930 | # Make mock_app.bootstrap return the already_bootstrapped flag and the | ||
750 | 931 | # bootstrap node series. | ||
751 | 932 | mock_app.bootstrap.return_value = (True, 'precise') | ||
752 | 933 | # Make mock_app.check_environment return the charm URL, the machine | ||
753 | 934 | # where to deploy the charm, the service and unit data. | ||
754 | 935 | mock_app.check_environment.return_value = ( | ||
755 | 936 | 'cs:precise/juju-gui-42', '0', None, None) | ||
756 | 937 | options = self.make_options(admin_secret='secret in environments.yaml') | ||
757 | 938 | manage.run(options) | ||
758 | 939 | mock_app.connect.assert_has_calls([ | ||
759 | 940 | mock.call(mock_app.get_api_url(), 'secret in environments.yaml'), | ||
760 | 941 | ]) | ||
761 | 942 | |||
762 | 943 | def test_no_admin_secret_found(self, mock_app, mock_open): | 894 | def test_no_admin_secret_found(self, mock_app, mock_open): |
766 | 944 | # If admin-secret cannot be found anywhere a ProgramExit is called. | 895 | # If admin-secret cannot be found a ProgramExit is called. |
767 | 945 | mock_app.ProgramExit = app.ProgramExit | 896 | mock_app.get_admin_secret.side_effect = app.ProgramExit('bad wolf') |
765 | 946 | mock_app.get_value_from_jenv = self.mock_get_value_from_jenv_error | ||
768 | 947 | # Make mock_app.bootstrap return the already_bootstrapped flag and the | 897 | # Make mock_app.bootstrap return the already_bootstrapped flag and the |
769 | 948 | # bootstrap node series. | 898 | # bootstrap node series. |
770 | 949 | mock_app.bootstrap.return_value = (True, 'precise') | 899 | mock_app.bootstrap.return_value = (True, 'precise') |
771 | @@ -952,15 +902,11 @@ | |||
772 | 952 | mock_app.check_environment.return_value = ( | 902 | mock_app.check_environment.return_value = ( |
773 | 953 | 'cs:precise/juju-gui-42', '0', None, None) | 903 | 'cs:precise/juju-gui-42', '0', None, None) |
774 | 954 | options = self.make_options( | 904 | options = self.make_options( |
775 | 955 | admin_secret=None, | ||
776 | 956 | env_name='local', | 905 | env_name='local', |
777 | 957 | env_file='environments.yaml') | 906 | env_file='environments.yaml') |
778 | 958 | with self.assertRaises(app.ProgramExit) as context: | 907 | with self.assertRaises(app.ProgramExit) as context: |
779 | 959 | manage.run(options) | 908 | manage.run(options) |
784 | 960 | expected = ( | 909 | self.assertEqual('bad wolf', context.exception.message) |
781 | 961 | u'admin-secret not found in {}/environments/local.jenv ' | ||
782 | 962 | 'or environments.yaml'.format(settings.JUJU_HOME)) | ||
783 | 963 | self.assertEqual(expected, context.exception.message) | ||
785 | 964 | 910 | ||
786 | 965 | def test_juju_environ_var_set(self, mock_app, mock_open): | 911 | def test_juju_environ_var_set(self, mock_app, mock_open): |
787 | 966 | mock_app.bootstrap.return_value = (True, 'precise') | 912 | mock_app.bootstrap.return_value = (True, 'precise') |
788 | 967 | 913 | ||
789 | === modified file 'quickstart/tests/test_serializers.py' | |||
790 | --- quickstart/tests/test_serializers.py 2013-12-06 18:16:02 +0000 | |||
791 | +++ quickstart/tests/test_serializers.py 2014-11-11 14:26:05 +0000 | |||
792 | @@ -18,12 +18,15 @@ | |||
793 | 18 | 18 | ||
794 | 19 | from __future__ import unicode_literals | 19 | from __future__ import unicode_literals |
795 | 20 | 20 | ||
796 | 21 | import os | ||
797 | 22 | import tempfile | ||
798 | 21 | import unittest | 23 | import unittest |
799 | 22 | 24 | ||
800 | 23 | import mock | 25 | import mock |
801 | 24 | import yaml | 26 | import yaml |
802 | 25 | 27 | ||
803 | 26 | from quickstart import serializers | 28 | from quickstart import serializers |
804 | 29 | from quickstart.tests import helpers | ||
805 | 27 | 30 | ||
806 | 28 | 31 | ||
807 | 29 | class TestYamlLoad(unittest.TestCase): | 32 | class TestYamlLoad(unittest.TestCase): |
808 | @@ -56,3 +59,41 @@ | |||
809 | 56 | # Collections are serialized in the block style. | 59 | # Collections are serialized in the block style. |
810 | 57 | contents = serializers.yaml_dump(self.data) | 60 | contents = serializers.yaml_dump(self.data) |
811 | 58 | self.assertEqual('myint: 42\nmystring: foo\n', contents) | 61 | self.assertEqual('myint: 42\nmystring: foo\n', contents) |
812 | 62 | |||
813 | 63 | |||
814 | 64 | class TestYamlLoadFromPath(helpers.ValueErrorTestsMixin, unittest.TestCase): | ||
815 | 65 | |||
816 | 66 | def make_file(self, contents): | ||
817 | 67 | """Create a temporary file with the given contents. | ||
818 | 68 | |||
819 | 69 | Return the file path. | ||
820 | 70 | """ | ||
821 | 71 | yaml_file = tempfile.NamedTemporaryFile(delete=False) | ||
822 | 72 | self.addCleanup(os.remove, yaml_file.name) | ||
823 | 73 | yaml_file.write(contents) | ||
824 | 74 | yaml_file.close() | ||
825 | 75 | return yaml_file.name | ||
826 | 76 | |||
827 | 77 | def test_resulting_contents(self): | ||
828 | 78 | # The YAML deserialized contents are correctly returned. | ||
829 | 79 | expected_data = {'myint': 42, 'mystring': 'foo'} | ||
830 | 80 | path = self.make_file(yaml.safe_dump(expected_data)) | ||
831 | 81 | obtained_data = serializers.yaml_load_from_path(path) | ||
832 | 82 | self.assertEqual(expected_data, obtained_data) | ||
833 | 83 | |||
834 | 84 | def test_no_file(self): | ||
835 | 85 | # A ValueError is raised if the given file is not found. | ||
836 | 86 | expected_error = ( | ||
837 | 87 | "unable to open file /no/such/file.yaml: " | ||
838 | 88 | "[Errno 2] No such file or directory: '/no/such/file.yaml'" | ||
839 | 89 | ) | ||
840 | 90 | with self.assert_value_error(expected_error): | ||
841 | 91 | serializers.yaml_load_from_path('/no/such/file.yaml') | ||
842 | 92 | |||
843 | 93 | def test_invalid_yaml(self): | ||
844 | 94 | # A ValueError is raised if the given file is not a valid YAML. | ||
845 | 95 | path = self.make_file(':') | ||
846 | 96 | with self.assertRaises(ValueError) as context_manager: | ||
847 | 97 | serializers.yaml_load_from_path(path) | ||
848 | 98 | expected = 'unable to parse file {}'.format(path) | ||
849 | 99 | self.assertIn(expected, bytes(context_manager.exception)) |
Reviewers: mp+241416_ code.launchpad. net,
Message:
Please take a look.
Description:
Code reorganization + docs.
Reorganize the jenv related code in preparation of the
bootstrap strategy change.
Separate jenv handling from the environments.yaml file
management in models.
As a consequence of only supporting juju >= 1.18 we
can now safely always retrieve the admin-secret from
the jenv file.
Also updated the HACKING docs to include a brief
description of the project structure.
Tests: `make check`.
QA: use quickstart as usual.
https:/ /code.launchpad .net/~frankban/ juju-quickstart /code-reorg/ +merge/ 241416
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/169380043/
Affected files (+372, -222 lines): manage. py models/ envs.py models/ jenv.py serializers. py tests/helpers. py tests/models/ test_envs. py tests/models/ test_jenv. py tests/test_ app.py tests/test_ manage. py tests/test_ serializers. py
M HACKING.rst
A [revision details]
M quickstart/app.py
M quickstart/
M quickstart/
A quickstart/
M quickstart/
M quickstart/
M quickstart/
A quickstart/
M quickstart/
M quickstart/
M quickstart/