Merge lp:~frankban/juju-quickstart/handle-jenv-envs into lp:juju-quickstart
- handle-jenv-envs
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 109 |
Proposed branch: | lp:~frankban/juju-quickstart/handle-jenv-envs |
Merge into: | lp:juju-quickstart |
Diff against target: |
725 lines (+364/-80) 8 files modified
quickstart/app.py (+13/-15) quickstart/manage.py (+26/-12) quickstart/models/jenv.py (+85/-7) quickstart/tests/helpers.py (+49/-9) quickstart/tests/models/test_jenv.py (+141/-7) quickstart/tests/test_app.py (+22/-23) quickstart/tests/test_juju.py (+9/-0) quickstart/tests/test_manage.py (+19/-7) |
To merge this branch: | bzr merge lp:~frankban/juju-quickstart/handle-jenv-envs |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email: mp+244880@code.launchpad.net |
Commit message
Description of the change
Promote jenv files as first class envs.
Quickstart no longer refuses to use an environment
which is only present as a jenv file.
Add some more helper functions to the
Also retrieve the environment type, in the case
the environment is already bootstrapped, from
the WebSocket connection and not from the jenv:
jenv files are not assumed to include the type.
Tests: `make check`.
QA:
- use quickstart to bootstrap an environment:
`.venv/bin/python juju-quickstart`;
- re-run quickstart again to reopen the same environment:
`.venv/bin/python juju-quickstart`;
- in both cases, check auto-login works and the output
is sane;
- generate a new environment user and put the
resulting jenv in your Juju home:
`juju user add myuser --generate -o ~/.juju/
- use quickstart with the new environment:
`.venv/bin/python juju-quickstart -e myenv`;
- check that the new credentials are printed to stdout
and that the auto-login still works;
- destroy the environment.
Thanks a lot!
Francesco Banconi (frankban) wrote : | # |
Brad Crittenden (bac) wrote : | # |
LGTM. WIll QA next.
https:/
File quickstart/
https:/
quickstart/
environments.yaml file, we can also
This comment seems wrong since we could've come through the second path,
meaning it was not in environments.yaml.
Brad Crittenden (bac) wrote : | # |
QA was OK.
Note juju will not let you destroy the environment with the generated
name. If 'local' was originally booted and a mytest.jenv file was
generated by 'juju user add' then 'juju destroy-environment mytest'
prints "removing empty environment file" but does not destroy the
environment.
Not a quickstart issue but interesting.
Richard Harding (rharding) wrote : | # |
Francesco Banconi (frankban) wrote : | # |
Thanks for the reviews!
https:/
File quickstart/
https:/
quickstart/
environments.yaml file, we can also
On 2014/12/16 17:36:37, bac wrote:
> This comment seems wrong since we could've come through the second
path, meaning
> it was not in environments.yaml.
But this point is never reached in that case: the second path (outer
except block) always returns and exits the function.
Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Promote jenv files as first class envs.
Quickstart no longer refuses to use an environment
which is only present as a jenv file.
Add some more helper functions to the
Also retrieve the environment type, in the case
the environment is already bootstrapped, from
the WebSocket connection and not from the jenv:
jenv files are not assumed to include the type.
Tests: `make check`.
QA:
- use quickstart to bootstrap an environment:
`.venv/
- re-run quickstart again to reopen the same environment:
`.venv/
- in both cases, check auto-login works and the output
is sane;
- generate a new environment user and put the
resulting jenv in your Juju home:
`juju user add myuser --generate -o ~/.juju/
- use quickstart with the new environment:
`.venv/
- check that the new credentials are printed to stdout
and that the auto-login still works;
- destroy the environment.
Thanks a lot!
R=bac, rharding
CC=
https:/
Preview Diff
1 | === modified file 'quickstart/app.py' |
2 | --- quickstart/app.py 2014-12-16 11:10:10 +0000 |
3 | +++ quickstart/app.py 2014-12-16 16:32:00 +0000 |
4 | @@ -278,21 +278,6 @@ |
5 | raise ProgramExit('the state server is not ready:\n{}'.format(details)) |
6 | |
7 | |
8 | -def get_env_type(env_name): |
9 | - """Return the Juju environment type for the given environment name. |
10 | - |
11 | - Since the environment type is retrieved by parsing the jenv file, the |
12 | - environment must be already bootstrapped. |
13 | - |
14 | - Raise a ProgramExit if the environment type cannot be retrieved. |
15 | - """ |
16 | - try: |
17 | - return jenv.get_value(env_name, 'bootstrap-config', 'type') |
18 | - except ValueError as err: |
19 | - msg = b'cannot retrieve environment type: {}'.format(err) |
20 | - raise ProgramExit(msg) |
21 | - |
22 | - |
23 | def get_credentials(env_name): |
24 | """Return the Juju credentials for the given environment name. |
25 | |
26 | @@ -353,6 +338,19 @@ |
27 | return env |
28 | |
29 | |
30 | +def get_env_type(env): |
31 | + """Return the Juju environment type for the given environment connection. |
32 | + |
33 | + Raise a ProgramExit if the environment type cannot be retrieved. |
34 | + """ |
35 | + try: |
36 | + info = env.info() |
37 | + except jujuclient.EnvError as err: |
38 | + msg = 'cannot retrieve the environment type: {}'.format(err.message) |
39 | + raise ProgramExit(msg) |
40 | + return info['ProviderType'] |
41 | + |
42 | + |
43 | def create_auth_token(env): |
44 | """Return a new authentication token. |
45 | |
46 | |
47 | === modified file 'quickstart/manage.py' |
48 | --- quickstart/manage.py 2014-12-15 17:31:23 +0000 |
49 | +++ quickstart/manage.py 2014-12-16 16:32:00 +0000 |
50 | @@ -42,6 +42,7 @@ |
51 | from quickstart.models import ( |
52 | charms, |
53 | envs, |
54 | + jenv, |
55 | ) |
56 | |
57 | |
58 | @@ -222,16 +223,23 @@ |
59 | return env_data |
60 | |
61 | |
62 | -def _retrieve_env_data(parser, env_type_db, env_db, env_name): |
63 | +def _retrieve_env_data(parser, env_type_db, env_db, jenv_db, env_name): |
64 | """Retrieve and return the env_data corresponding to the given env_name. |
65 | |
66 | Invoke a parser error if the environment does not exist or is not valid. |
67 | """ |
68 | try: |
69 | env_data = envs.get_env_data(env_db, env_name) |
70 | - except ValueError as err: |
71 | - # The specified environment does not exist. |
72 | - return parser.error(bytes(err)) |
73 | + except ValueError: |
74 | + # The specified environment does not exist in the environments file. |
75 | + # Check if this is an imported environment. |
76 | + try: |
77 | + return envs.get_env_data(jenv_db, env_name) |
78 | + except ValueError as err: |
79 | + # The environment cannot be found anywhere, exit with an error. |
80 | + return parser.error(bytes(err)) |
81 | + # If the environment was found in the environments.yaml file, we can also |
82 | + # validate it. |
83 | env_metadata = envs.get_env_metadata(env_type_db, env_data) |
84 | errors = envs.validate(env_metadata, env_data) |
85 | if errors: |
86 | @@ -275,6 +283,7 @@ |
87 | |
88 | # Retrieve the environment database (or create an in-memory empty one). |
89 | env_db = _retrieve_env_db(parser, env_file if env_file_exists else None) |
90 | + jenv_db = jenv.get_env_db() |
91 | |
92 | # Validate the environment. |
93 | env_type_db = envs.get_env_type_db() |
94 | @@ -285,7 +294,8 @@ |
95 | else: |
96 | # This is a non-interactive session and we need to validate the |
97 | # selected environment before proceeding. |
98 | - env_data = _retrieve_env_data(parser, env_type_db, env_db, env_name) |
99 | + env_data = _retrieve_env_data( |
100 | + parser, env_type_db, env_db, jenv_db, env_name) |
101 | # Check for local support, if requested. |
102 | options.env_type = env_data['type'] |
103 | no_local_support = not platform_support.supports_local(options.platform) |
104 | @@ -510,8 +520,7 @@ |
105 | env_type = options.env_type |
106 | api_url = app.check_bootstrapped(options.env_name) |
107 | if api_url is None: |
108 | - print('bootstrapping the {} environment (type: {})'.format( |
109 | - options.env_name, env_type)) |
110 | + print('bootstrapping the {} environment'.format(options.env_name)) |
111 | if env_type == 'local': |
112 | # If this is a local environment, notify the user that "sudo" will |
113 | # be required by Juju to bootstrap the environment. |
114 | @@ -524,11 +533,8 @@ |
115 | upload_series=options.upload_series, |
116 | constraints=options.constraints) |
117 | if already_bootstrapped: |
118 | - # Retrieve the environment type from the jenv file: it may be different |
119 | - # from the one declared on the environments.yaml file. |
120 | - env_type = app.get_env_type(options.env_name) |
121 | - print('reusing the already bootstrapped {} environment ' |
122 | - '(type: {})'.format(options.env_name, env_type)) |
123 | + print('reusing the already bootstrapped {} environment'.format( |
124 | + options.env_name)) |
125 | |
126 | # Retrieve the environment status, ensure it is in a ready state and |
127 | # contextually fetch the bootstrap node series. |
128 | @@ -548,6 +554,14 @@ |
129 | print('connecting to {}'.format(api_url)) |
130 | env = app.connect(api_url, username, password) |
131 | |
132 | + if already_bootstrapped: |
133 | + # Retrieve the environment type from the live environment: it may be |
134 | + # different from the one declared on the environments.yaml file. |
135 | + # Moreover, an imported jenv file could be in use, in which case the |
136 | + # environment type is probably still unknown. |
137 | + env_type = app.get_env_type(env) |
138 | + print('environment type: {}'.format(env_type)) |
139 | + |
140 | # Inspect the environment and deploy the charm if required. |
141 | charm_url, machine, service_data, unit_data = app.check_environment( |
142 | env, settings.JUJU_GUI_SERVICE_NAME, options.charm_url, |
143 | |
144 | === modified file 'quickstart/models/jenv.py' |
145 | --- quickstart/models/jenv.py 2014-12-16 11:10:10 +0000 |
146 | +++ quickstart/models/jenv.py 2014-12-16 16:32:00 +0000 |
147 | @@ -33,6 +33,8 @@ |
148 | |
149 | # Define the default Juju user when an environment is initially bootstrapped. |
150 | JUJU_DEFAULT_USER = 'admin' |
151 | +# Define an env type to use when the real type is not included in the jenv. |
152 | +UNKNOWN_ENV_TYPE = '__unknown__' |
153 | |
154 | |
155 | def exists(env_name): |
156 | @@ -78,7 +80,20 @@ |
157 | """ |
158 | jenv_path = _get_jenv_path(env_name) |
159 | data = serializers.yaml_load_from_path(jenv_path) |
160 | - |
161 | + try: |
162 | + return _get_credentials(data) |
163 | + except ValueError as err: |
164 | + msg = b'cannot parse {}: {}'.format(jenv_path.encode('utf-8'), err) |
165 | + raise ValueError(msg) |
166 | + |
167 | + |
168 | +def _get_credentials(data): |
169 | + """Return the Juju environment credentials from the YAML decoded data. |
170 | + |
171 | + Raise a ValueError if the credentials cannot be found. |
172 | + See get_credentials for further information on how the credentials are |
173 | + retrieved. |
174 | + """ |
175 | # Retrieve the user name. |
176 | try: |
177 | username = _get_value_from_yaml(data, 'user') |
178 | @@ -86,9 +101,8 @@ |
179 | # This is probably an old version of Juju not supporting multiple |
180 | # users. Return the default user name. |
181 | logging.warn( |
182 | - 'cannot retrieve the user name from {}: {}: ' |
183 | - 'falling back to the default user name' .format( |
184 | - jenv_path, bytes(err).decode('utf-8'))) |
185 | + b'cannot retrieve the user name: {}: ' |
186 | + b'falling back to the default user name'.format(err)) |
187 | username = JUJU_DEFAULT_USER |
188 | |
189 | # Retrieve the password. |
190 | @@ -97,9 +111,8 @@ |
191 | except ValueError as err: |
192 | # This is probably an old version of Juju not supporting multiple |
193 | # users. Fall back to the admin secret. |
194 | - msg = b'cannot retrieve the password from {}: '.format( |
195 | - jenv_path.encode('utf-8')) |
196 | - logging.debug(msg + bytes(err)) |
197 | + msg = b'cannot retrieve the password: ' |
198 | + logging.debug(msg + bytes(err) + ': trying with the admin-secret') |
199 | try: |
200 | password = _get_value_from_yaml( |
201 | data, 'bootstrap-config', 'admin-secret') |
202 | @@ -109,6 +122,71 @@ |
203 | return username, password |
204 | |
205 | |
206 | +def get_env_db(): |
207 | + """Return an environment database parsing the existing jenv files. |
208 | + |
209 | + The returned db is similar to what is returned by models.envs.load(). |
210 | + |
211 | + When a jenv file is created using "juju user add", the resulting YAML data |
212 | + is very concise, not even including the environment type. |
213 | + For this reason, the environment database returned by this function does |
214 | + not contain the usual fields used as bootstrap options, but just the |
215 | + environment name and the type. If the environment type is not included in |
216 | + the jenv, UNKNOWN_ENV_TYPE is used. |
217 | + """ |
218 | + db = {'environments': {}} |
219 | + path = os.path.expanduser(os.path.join(settings.JUJU_HOME, 'environments')) |
220 | + # Check if the Juju home is already configured. |
221 | + if not os.path.isdir(path): |
222 | + logging.debug('environments directory not found in the Juju home') |
223 | + return db |
224 | + # Collect the environments. |
225 | + environments = db['environments'] |
226 | + for filename in os.listdir(path): |
227 | + fullpath = os.path.join(path, filename) |
228 | + # Check that the current file is a jenv file. |
229 | + if not os.path.isfile(fullpath): |
230 | + continue |
231 | + name, ext = os.path.splitext(filename) |
232 | + if ext != '.jenv': |
233 | + continue |
234 | + # Validate the jenv contents. |
235 | + try: |
236 | + data = serializers.yaml_load_from_path(fullpath) |
237 | + validate(data) |
238 | + except ValueError as err: |
239 | + logging.warn('ignoring invalid jenv file {}: {}'. format( |
240 | + filename, bytes(err).decode('utf-8'))) |
241 | + continue |
242 | + # Try to retrieve the environment type from the jenv data. |
243 | + try: |
244 | + env_type = _get_value_from_yaml(data, 'bootstrap-config', 'type') |
245 | + except ValueError: |
246 | + # This is expected when a jenv is generated with "juju user add". |
247 | + env_type = UNKNOWN_ENV_TYPE |
248 | + environments[name] = {'type': env_type} |
249 | + return db |
250 | + |
251 | + |
252 | +def validate(data): |
253 | + """Validate the given YAML decoded jenv data. |
254 | + |
255 | + This is a weak validation: from the quickstart point of view a jenv file is |
256 | + valid if it includes the juju environment credentials and the API servers. |
257 | + |
258 | + Raise a ValueError if: |
259 | + - the environment file is not found; |
260 | + - the environment file contents are not parsable by YAML; |
261 | + - the environment data is not valid. |
262 | + """ |
263 | + _get_credentials(data) |
264 | + servers = data.get('state-servers') |
265 | + if not isinstance(servers, (list, tuple)): |
266 | + raise ValueError(b'invalid state-servers field') |
267 | + if not len(servers): |
268 | + raise ValueError(b'no state-servers found') |
269 | + |
270 | + |
271 | def _get_value_from_yaml(data, *args): |
272 | """Read and return a value from the given YAML decoded data. |
273 | |
274 | |
275 | === modified file 'quickstart/tests/helpers.py' |
276 | --- quickstart/tests/helpers.py 2014-11-12 12:21:09 +0000 |
277 | +++ quickstart/tests/helpers.py 2014-12-16 16:32:00 +0000 |
278 | @@ -142,6 +142,7 @@ |
279 | |
280 | jenv_data = { |
281 | 'user': 'admin', |
282 | + 'password': 'Secret!', |
283 | 'state-servers': ['localhost:17070', '10.0.3.1:17070'], |
284 | 'bootstrap-config': { |
285 | 'admin-secret': 'Secret!', |
286 | @@ -151,6 +152,24 @@ |
287 | 'life': {'universe': {'everything': 42}}, |
288 | } |
289 | |
290 | + def _make_playground(self): |
291 | + """Create and return a mock Juju home.""" |
292 | + playground = tempfile.mkdtemp() |
293 | + self.addCleanup(shutil.rmtree, playground) |
294 | + os.mkdir(os.path.join(playground, 'environments')) |
295 | + return playground |
296 | + |
297 | + def write_jenv(self, juju_home, name, contents): |
298 | + """Create a jenv file in the given Juju home. |
299 | + |
300 | + Use the given name and YAML encoded contents to create the jenv. |
301 | + Return the resulting jenv path. |
302 | + """ |
303 | + path = os.path.join(juju_home, 'environments', '{}.jenv'.format(name)) |
304 | + with open(path, 'w') as jenv_file: |
305 | + jenv_file.write(contents) |
306 | + return path |
307 | + |
308 | @contextmanager |
309 | def make_jenv(self, env_name, contents): |
310 | """Create a temporary jenv file with the given env_name and contents. |
311 | @@ -160,19 +179,30 @@ |
312 | |
313 | Return the jenv file path. |
314 | """ |
315 | - # Create a temporary JUJU_HOME. |
316 | - playground = tempfile.mkdtemp() |
317 | - self.addCleanup(shutil.rmtree, playground) |
318 | - environments_dir = os.path.join(playground, 'environments') |
319 | - os.mkdir(environments_dir) |
320 | - # Create the jenv file inside the temporary JUJU_HOME. |
321 | - jenv_path = os.path.join(environments_dir, '{}.jenv'.format(env_name)) |
322 | - with open(jenv_path, 'w') as jenv_file: |
323 | - jenv_file.write(contents) |
324 | + playground = self._make_playground() |
325 | + jenv_path = self.write_jenv(playground, env_name, contents) |
326 | # Patch the JUJU_HOME and return the jenv file path. |
327 | with mock.patch('quickstart.settings.JUJU_HOME', playground): |
328 | yield jenv_path |
329 | |
330 | + @contextmanager |
331 | + def make_multiple_jenvs(self, data): |
332 | + """Create multiple temporary jenv files with the data. |
333 | + |
334 | + Data is a dict mapping env names to jenv contents. |
335 | + |
336 | + In the context manager block, the JUJU_HOME is set to the ancestor |
337 | + of the generated temporary files. |
338 | + |
339 | + Return the Juju home path. |
340 | + """ |
341 | + playground = self._make_playground() |
342 | + for name, contents in data.items(): |
343 | + self.write_jenv(playground, name, contents) |
344 | + # Patch the JUJU_HOME and return the jenv file path. |
345 | + with mock.patch('quickstart.settings.JUJU_HOME', playground): |
346 | + yield playground |
347 | + |
348 | |
349 | def make_env_db(default=None, exclude_invalid=False): |
350 | """Create and return an env_db. |
351 | @@ -229,6 +259,16 @@ |
352 | return env_db |
353 | |
354 | |
355 | +def make_jenv_db(): |
356 | + """Create and return a jenv files database.""" |
357 | + environments = { |
358 | + 'ec2-west': {'type': '__unknown__'}, |
359 | + 'lxc': {'type': 'local'}, |
360 | + 'test-jenv': {'type': '__unknown__'}, |
361 | + } |
362 | + return {'environments': environments} |
363 | + |
364 | + |
365 | # Mock the builtin print function. |
366 | mock_print = mock.patch('__builtin__.print') |
367 | |
368 | |
369 | === modified file 'quickstart/tests/models/test_jenv.py' |
370 | --- quickstart/tests/models/test_jenv.py 2014-12-16 11:10:10 +0000 |
371 | +++ quickstart/tests/models/test_jenv.py 2014-12-16 16:32:00 +0000 |
372 | @@ -18,8 +18,10 @@ |
373 | |
374 | from __future__ import unicode_literals |
375 | |
376 | +import os |
377 | import unittest |
378 | |
379 | +import mock |
380 | import yaml |
381 | |
382 | from quickstart.models import jenv |
383 | @@ -126,12 +128,12 @@ |
384 | # The default user name is returned if it's not possible to retrieve it |
385 | # otherwise. |
386 | data = {'password': 'Secret!'} |
387 | - with self.make_jenv('local', yaml.safe_dump(data)) as path: |
388 | + with self.make_jenv('local', yaml.safe_dump(data)): |
389 | expected_logs = [ |
390 | - 'cannot retrieve the user name from {}: ' |
391 | + 'cannot retrieve the user name: ' |
392 | 'invalid YAML contents: ' |
393 | 'user key not found in the root section: ' |
394 | - 'falling back to the default user name'.format(path)] |
395 | + 'falling back to the default user name'] |
396 | with helpers.assert_logs(expected_logs, 'warn'): |
397 | username, password = jenv.get_credentials('local') |
398 | self.assertEqual('admin', username) |
399 | @@ -144,11 +146,12 @@ |
400 | 'user': 'who', |
401 | 'bootstrap-config': {'admin-secret': 'Admin!'}, |
402 | } |
403 | - with self.make_jenv('local', yaml.safe_dump(data)) as path: |
404 | + with self.make_jenv('local', yaml.safe_dump(data)): |
405 | expected_logs = [ |
406 | - 'cannot retrieve the password from {}: ' |
407 | + 'cannot retrieve the password: ' |
408 | 'invalid YAML contents: ' |
409 | - 'password key not found in the root section'.format(path)] |
410 | + 'password key not found in the root section: ' |
411 | + 'trying with the admin-secret'] |
412 | with helpers.assert_logs(expected_logs, 'debug'): |
413 | username, password = jenv.get_credentials('local') |
414 | self.assertEqual('Admin!', password) |
415 | @@ -157,8 +160,139 @@ |
416 | # A ValueError is raised if the password cannot be found anywhere. |
417 | with self.make_jenv('local', yaml.safe_dump({})) as path: |
418 | expected_error = ( |
419 | - 'cannot retrieve the password from {}: ' |
420 | + 'cannot parse {}: ' |
421 | + 'cannot retrieve the password: ' |
422 | 'invalid YAML contents: bootstrap-config key ' |
423 | 'not found in the root section'.format(path)) |
424 | with self.assert_value_error(expected_error): |
425 | jenv.get_credentials('local') |
426 | + |
427 | + |
428 | +class TestGetEnvDb(helpers.JenvFileTestsMixin, unittest.TestCase): |
429 | + |
430 | + def test_no_juju_home(self): |
431 | + # An empty db is returned if the Juju home is not set up. |
432 | + with mock.patch('quickstart.settings.JUJU_HOME', '/no/such/dir'): |
433 | + jenv_db = jenv.get_env_db() |
434 | + self.assertEqual({'environments': {}}, jenv_db) |
435 | + |
436 | + def test_no_jenv_files(self): |
437 | + # An empty db is returned if there are no jenv files. |
438 | + with self.make_multiple_jenvs({}): |
439 | + jenv_db = jenv.get_env_db() |
440 | + self.assertEqual({'environments': {}}, jenv_db) |
441 | + |
442 | + def test_no_valid_jenv_files(self): |
443 | + # An empty db is returned if there are no valid jenv files. |
444 | + with self.make_jenv('local', yaml.safe_dump({})): |
445 | + expected_logs = [ |
446 | + 'ignoring invalid jenv file local.jenv: ' |
447 | + 'cannot retrieve the password: invalid YAML contents: ' |
448 | + 'bootstrap-config key not found in the root section'] |
449 | + with helpers.assert_logs(expected_logs, 'warn'): |
450 | + jenv_db = jenv.get_env_db() |
451 | + self.assertEqual({'environments': {}}, jenv_db) |
452 | + |
453 | + def test_single_valid_jenv(self): |
454 | + # Only the valid environments are returned. |
455 | + with self.make_multiple_jenvs({ |
456 | + 'local': yaml.safe_dump({}), |
457 | + 'ec2': yaml.safe_dump(self.jenv_data), |
458 | + }): |
459 | + jenv_db = jenv.get_env_db() |
460 | + self.assertEqual({ |
461 | + 'environments': {'ec2': {'type': 'ec2'}}, |
462 | + }, jenv_db) |
463 | + |
464 | + def test_multiple_jenv_files(self): |
465 | + # Multiple environments are correctly returned. |
466 | + jenv_data1 = { |
467 | + 'user': 'admin', |
468 | + 'password': 'Secret!', |
469 | + 'state-servers': ['localhost:17070'], |
470 | + 'bootstrap-config': {'type': 'hp'}, |
471 | + } |
472 | + jenv_data2 = { |
473 | + 'user': 'admin', |
474 | + 'password': 'Secret!', |
475 | + 'state-servers': ['localhost:17070'], |
476 | + 'bootstrap-config': {'type': 'maas'}, |
477 | + } |
478 | + with self.make_multiple_jenvs({ |
479 | + 'hp': yaml.safe_dump(jenv_data1), |
480 | + 'maas': yaml.safe_dump(jenv_data2), |
481 | + }): |
482 | + jenv_db = jenv.get_env_db() |
483 | + self.assertEqual({ |
484 | + 'environments': { |
485 | + 'hp': {'type': 'hp'}, |
486 | + 'maas': {'type': 'maas'}, |
487 | + }, |
488 | + }, jenv_db) |
489 | + |
490 | + def test_unknown_env_type(self): |
491 | + # If the jenv file does not include the env type, jenv.UNKNOWN_ENV_TYPE |
492 | + # is used as the environment type. |
493 | + jenv_data = { |
494 | + 'user': 'admin', |
495 | + 'password': 'Secret!', |
496 | + 'state-servers': ['localhost:17070'], |
497 | + } |
498 | + with self.make_jenv('local', yaml.safe_dump(jenv_data)): |
499 | + jenv_db = jenv.get_env_db() |
500 | + self.assertEqual({ |
501 | + 'environments': {'local': {'type': jenv.UNKNOWN_ENV_TYPE}}, |
502 | + }, jenv_db) |
503 | + |
504 | + def test_extraneous_files(self): |
505 | + # Extraneous files are ignored. |
506 | + with self.make_multiple_jenvs({}) as juju_home: |
507 | + envs_dir = os.path.join(juju_home, 'environments') |
508 | + os.mkdir(os.path.join(envs_dir, 'local.jenv')) |
509 | + with open(os.path.join(envs_dir, 'not-a-jenv'), 'w') as stream: |
510 | + stream.write('bad wolf') |
511 | + jenv_db = jenv.get_env_db() |
512 | + self.assertEqual({'environments': {}}, jenv_db) |
513 | + |
514 | + |
515 | +class TestValidate( |
516 | + helpers.JenvFileTestsMixin, helpers.ValueErrorTestsMixin, |
517 | + unittest.TestCase): |
518 | + |
519 | + def test_validation_success(self): |
520 | + # A valid jenv file is successfully validated. |
521 | + jenv.validate(self.jenv_data) |
522 | + |
523 | + def test_invalid_credentials(self): |
524 | + # A ValueError is raised if the credentials cannot be retrieved. |
525 | + expected_error = ( |
526 | + 'cannot retrieve the password: invalid YAML contents: ' |
527 | + 'bootstrap-config key not found in the root section') |
528 | + with self.assert_value_error(expected_error): |
529 | + jenv.validate({}) |
530 | + |
531 | + def test_missing_state_servers(self): |
532 | + # A ValueError is raised if the Juju state servers cannot be retrieved. |
533 | + with self.assert_value_error('invalid state-servers field'): |
534 | + jenv.validate({ |
535 | + 'user': 'admin', |
536 | + 'password': 'Secret!', |
537 | + }) |
538 | + |
539 | + def test_invalid_state_servers(self): |
540 | + # A ValueError is raised if the Juju servers have an invalid type. |
541 | + with self.assert_value_error('invalid state-servers field'): |
542 | + jenv.validate({ |
543 | + 'user': 'admin', |
544 | + 'password': 'Secret!', |
545 | + 'state-servers': 'NO!', |
546 | + }) |
547 | + |
548 | + def test_no_state_servers(self): |
549 | + # A ValueError is raised if the state server list is empty. |
550 | + with self.assert_value_error('no state-servers found'): |
551 | + jenv.validate({ |
552 | + 'user': 'admin', |
553 | + 'password': 'Secret!', |
554 | + 'state-servers': [], |
555 | + }) |
556 | |
557 | === modified file 'quickstart/tests/test_app.py' |
558 | --- quickstart/tests/test_app.py 2014-12-16 11:10:10 +0000 |
559 | +++ quickstart/tests/test_app.py 2014-12-16 16:32:00 +0000 |
560 | @@ -700,26 +700,6 @@ |
561 | mock_call.assert_has_calls(expected_calls) |
562 | |
563 | |
564 | -class TestGetEnvType( |
565 | - helpers.JenvFileTestsMixin, ProgramExitTestsMixin, unittest.TestCase): |
566 | - |
567 | - def test_success(self): |
568 | - # The environment type is successfully retrieved. |
569 | - with self.make_jenv('ec2', yaml.safe_dump(self.jenv_data)): |
570 | - env_type = app.get_env_type('ec2') |
571 | - self.assertEqual('ec2', env_type) |
572 | - |
573 | - def test_error(self): |
574 | - # A ProgramExit is raised if the environment type cannot be retrieved. |
575 | - with self.make_jenv('aws', '') as path: |
576 | - expected_error = ( |
577 | - 'cannot retrieve environment type: cannot read {}: invalid ' |
578 | - 'YAML contents: bootstrap-config key not found in the root ' |
579 | - 'section'.format(path)) |
580 | - with self.assert_program_exit(expected_error): |
581 | - app.get_env_type('aws') |
582 | - |
583 | - |
584 | class TestGetCredentials( |
585 | helpers.JenvFileTestsMixin, ProgramExitTestsMixin, unittest.TestCase): |
586 | |
587 | @@ -734,9 +714,10 @@ |
588 | # A ProgramExit is raised if the credentials cannot be retrieved. |
589 | with self.make_jenv('ec2', '') as path: |
590 | expected_error = ( |
591 | - 'cannot retrieve environment credentials: cannot retrieve the ' |
592 | - 'password from {}: invalid YAML contents: bootstrap-config ' |
593 | - 'key not found in the root section'.format(path)) |
594 | + 'cannot retrieve environment credentials: cannot parse {}: ' |
595 | + 'cannot retrieve the password: invalid YAML contents: ' |
596 | + 'bootstrap-config key not found in the root section' |
597 | + ''.format(path)) |
598 | with self.assert_program_exit(expected_error): |
599 | app.get_credentials('ec2') |
600 | |
601 | @@ -842,6 +823,24 @@ |
602 | self.assertIs(error, context_manager.exception) |
603 | |
604 | |
605 | +class TestGetEnvType(ProgramExitTestsMixin, unittest.TestCase): |
606 | + |
607 | + def test_success(self): |
608 | + # The environment type is successfully retrieved. |
609 | + env = mock.Mock() |
610 | + env.info.return_value = {'ProviderType': 'ec2'} |
611 | + env_type = app.get_env_type(env) |
612 | + self.assertEqual('ec2', env_type) |
613 | + |
614 | + def test_error(self): |
615 | + # A ProgramExit is raised if the environment type cannot be retrieved. |
616 | + env = mock.Mock() |
617 | + env.info.side_effect = self.make_env_error('bad wolf') |
618 | + expected_error = 'cannot retrieve the environment type: bad wolf' |
619 | + with self.assert_program_exit(expected_error): |
620 | + app.get_env_type(env) |
621 | + |
622 | + |
623 | class TestCreateAuthToken(unittest.TestCase): |
624 | |
625 | def test_success(self): |
626 | |
627 | === modified file 'quickstart/tests/test_juju.py' |
628 | --- quickstart/tests/test_juju.py 2014-12-16 11:10:10 +0000 |
629 | +++ quickstart/tests/test_juju.py 2014-12-16 16:32:00 +0000 |
630 | @@ -277,6 +277,15 @@ |
631 | mock_rpc.assert_called_once_with(expected) |
632 | |
633 | @patch_rpc |
634 | + def test_info(self, mock_rpc): |
635 | + # The EnvironmentInfo API call is properly generated. |
636 | + self.env.info() |
637 | + mock_rpc.assert_called_once_with({ |
638 | + 'Type': 'Client', |
639 | + 'Request': 'EnvironmentInfo', |
640 | + }) |
641 | + |
642 | + @patch_rpc |
643 | def test_create_auth_token(self, mock_rpc): |
644 | self.env.create_auth_token() |
645 | expected = dict(Type='GUIToken', Request='Create') |
646 | |
647 | === modified file 'quickstart/tests/test_manage.py' |
648 | --- quickstart/tests/test_manage.py 2014-12-15 17:31:23 +0000 |
649 | +++ quickstart/tests/test_manage.py 2014-12-16 16:32:00 +0000 |
650 | @@ -428,26 +428,37 @@ |
651 | self.parser = mock.Mock() |
652 | self.env_type_db = envs.get_env_type_db() |
653 | self.env_db = helpers.make_env_db() |
654 | + self.jenv_db = helpers.make_jenv_db() |
655 | |
656 | def test_resulting_env_data(self): |
657 | # The env_data is correctly validated and returned. |
658 | expected_env_data = envs.get_env_data(self.env_db, 'lxc') |
659 | env_data = manage._retrieve_env_data( |
660 | - self.parser, self.env_type_db, self.env_db, 'lxc') |
661 | + self.parser, self.env_type_db, self.env_db, self.jenv_db, 'lxc') |
662 | + self.assertEqual(expected_env_data, env_data) |
663 | + |
664 | + def test_jenv_data(self): |
665 | + # The env_data is correctly retrieved from the jenv database. |
666 | + expected_env_data = envs.get_env_data(self.jenv_db, 'test-jenv') |
667 | + env_data = manage._retrieve_env_data( |
668 | + self.parser, self.env_type_db, self.env_db, self.jenv_db, |
669 | + 'test-jenv') |
670 | self.assertEqual(expected_env_data, env_data) |
671 | |
672 | def test_error_environment_not_found(self): |
673 | # A parser error is invoked if the provided environment is not included |
674 | # in the environments database. |
675 | manage._retrieve_env_data( |
676 | - self.parser, self.env_type_db, self.env_db, 'no-such') |
677 | + self.parser, self.env_type_db, self.env_db, self.jenv_db, |
678 | + 'no-such') |
679 | self.parser.error.assert_called_once_with( |
680 | 'environment no-such not found') |
681 | |
682 | def test_error_environment_not_valid(self): |
683 | # A parser error is invoked if the selected environment is not valid. |
684 | manage._retrieve_env_data( |
685 | - self.parser, self.env_type_db, self.env_db, 'local-with-errors') |
686 | + self.parser, self.env_type_db, self.env_db, self.jenv_db, |
687 | + 'local-with-errors') |
688 | self.parser.error.assert_called_once_with( |
689 | 'cannot use the local-with-errors environment:\n' |
690 | 'the storage port field requires an integer value') |
691 | @@ -860,13 +871,14 @@ |
692 | |
693 | def test_already_bootstrapped(self, mock_app, mock_open): |
694 | # The application correctly reuses an already bootstrapped environment. |
695 | - self.configure_app(mock_app, check_bootstrapped='wss://example.com') |
696 | + env = self.configure_app( |
697 | + mock_app, check_bootstrapped='wss://example.com') |
698 | # Run the application. |
699 | options = self.make_options() |
700 | with self.patch_get_juju_command(): |
701 | manage.run(options) |
702 | # The environment type is retrieved from the jenv. |
703 | - mock_app.get_env_type.assert_called_once_with(options.env_name) |
704 | + mock_app.get_env_type.assert_called_once_with(env) |
705 | # No reason to call bootstrap or get_api_url functions. |
706 | self.assertFalse(mock_app.bootstrap.called) |
707 | self.assertFalse(mock_app.get_api_url.called) |
708 | @@ -875,7 +887,7 @@ |
709 | # The application correctly reuses an already bootstrapped environment. |
710 | # In this case, the environment seems not bootstrapped at first, but |
711 | # it ended up being up and running later. |
712 | - self.configure_app(mock_app, bootstrap=True) |
713 | + env = self.configure_app(mock_app, bootstrap=True) |
714 | # Run the application. |
715 | options = self.make_options() |
716 | with self.patch_get_juju_command(): |
717 | @@ -891,7 +903,7 @@ |
718 | constraints=options.constraints) |
719 | mock_app.get_api_url.assert_called_once_with( |
720 | options.env_name, self.juju_command) |
721 | - mock_app.get_env_type.assert_called_once_with(options.env_name) |
722 | + mock_app.get_env_type.assert_called_once_with(env) |
723 | |
724 | def test_no_token(self, mock_app, mock_open): |
725 | # The process continues even if the authentication token cannot be |
Reviewers: mp+244880_ code.launchpad. net,
Message:
Please take a look.
Description:
Promote jenv files as first class envs.
Quickstart no longer refuses to use an environment
which is only present as a jenv file.
Add some more helper functions to the
Also retrieve the environment type, in the case
the environment is already bootstrapped, from
the WebSocket connection and not from the jenv:
jenv files are not assumed to include the type.
Tests: `make check`.
QA: bin/python juju-quickstart`;
- use quickstart to bootstrap an environment:
`.venv/
- re-run quickstart again to reopen the same environment: bin/python juju-quickstart`;
`.venv/
- in both cases, check auto-login works and the output
is sane;
- generate a new environment user and put the environments/ myenv.jenv` ;
resulting jenv in your Juju home:
`juju user add myuser --generate -o ~/.juju/
- use quickstart with the new environment: bin/python juju-quickstart -e myenv`;
`.venv/
- check that the new credentials are printed to stdout
and that the auto-login still works;
- destroy the environment.
Thanks a lot!
https:/ /code.launchpad .net/~frankban/ juju-quickstart /handle- jenv-envs/ +merge/ 244880
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/188300043/
Affected files (+366, -80 lines): manage. py models/ jenv.py tests/helpers. py tests/models/ test_jenv. py tests/test_ app.py tests/test_ juju.py tests/test_ manage. py
A [revision details]
M quickstart/app.py
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/