Merge lp:~frankban/juju-quickstart/env-manage-base-models into lp:juju-quickstart
- env-manage-base-models
- Merge into trunk
| Status: | Merged |
|---|---|
| Merged at revision: | 32 |
| Proposed branch: | lp:~frankban/juju-quickstart/env-manage-base-models |
| Merge into: | lp:juju-quickstart |
| Diff against target: |
781 lines (+676/-5) 5 files modified
quickstart/models/envs.py (+241/-1) quickstart/tests/helpers.py (+8/-4) quickstart/tests/models/test_envs.py (+388/-0) quickstart/tests/test_utils.py (+21/-0) quickstart/utils.py (+18/-0) |
| To merge this branch: | bzr merge lp:~frankban/juju-quickstart/env-manage-base-models |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju GUI Hackers | Pending | ||
|
Review via email:
|
|||
Commit message
Description of the change
Implement base models for environments.
This is a preparation branch for the
environments management functionality.
The envs module includes a docstring describing
how those new functions will be used.
Tests: `make check`.
No QA required.
| Francesco Banconi (frankban) wrote : | # |
| Brad Crittenden (bac) wrote : | # |
LGTM with some suggestions.
https:/
File quickstart/
https:/
quickstart/
i think 'non preexisting' is redundant and should be removed.
https:/
quickstart/
The flush and fscync are not handled by the close()?
https:/
quickstart/
Because this way internal mutable
Picky, but "Why not just use env_data.copy()?" reads better.
https:/
File quickstart/
https:/
quickstart/
s/in date/at/
I think using the format "2014-02-27 07:42:47 UTC" would be better.
https:/
File quickstart/utils.py (right):
https:/
quickstart/
perhaps now.isoformat(sep=' ')?
- 38. By Francesco Banconi
-
Changes as per review.
| Francesco Banconi (frankban) wrote : | # |
Please take a look.
https:/
File quickstart/
https:/
quickstart/
On 2013/12/09 15:20:02, bac wrote:
> i think 'non preexisting' is redundant and should be removed.
Done.
https:/
quickstart/
On 2013/12/09 15:20:02, bac wrote:
> The flush and fscync are not handled by the close()?
Good question. According to
http://
it does not seem so.
https:/
quickstart/
Because this way internal mutable
On 2013/12/09 15:20:02, bac wrote:
> Picky, but "Why not just use env_data.copy()?" reads better.
Done.
https:/
File quickstart/
https:/
quickstart/
On 2013/12/09 15:20:02, bac wrote:
> s/in date/at/
> I think using the format "2014-02-27 07:42:47 UTC" would be better.
Done.
https:/
File quickstart/utils.py (right):
https:/
quickstart/
On 2013/12/09 15:20:02, bac wrote:
> perhaps now.isoformat(sep=' ')?
Done.
| Francesco Banconi (frankban) wrote : | # |
LGTM thanks for the changes.
| Francesco Banconi (frankban) wrote : | # |
On 2013/12/09 18:25:19, frankban wrote:
> LGTM thanks for the changes.
oops, wrong MP.
| Richard Harding (rharding) wrote : | # |
LGTM on the code.
I'm not 100% sold on the api itself. It seems like the envs could and
should hide away the raw config data in env_dict. Then expose an api for
others to use easily on top of that.
At the very least, I think removing the redundant 'env' in some of the
method names would help a bit.
https:/
File quickstart/
https:/
quickstart/
load('~
would it make things clearer that this is the un-touched data by naming
it raw_envs_dict?
https:/
quickstart/
This all reads a little verbose and needing to get all the parts right.
What about having the env model be a class that tracks the raw envs_dict
internally?
env = Environment.
env.raw == envs_dict
print env.name
errors = validate(env)
if errors:
env.
if env.is_dirty():
env.save()
https:/
File quickstart/
https:/
quickstart/
envs.get_
if the code is usually used as envs.XXX can we remove the env from the
function names to make the api read easier?
load/save and such do not have it so they read nicer.
| Francesco Banconi (frankban) wrote : | # |
On 2013/12/09 19:40:53, rharding wrote:
> LGTM on the code.
> I'm not 100% sold on the api itself. It seems like the envs could and
should
> hide away the raw config data in env_dict. Then expose an api for
others to use
> easily on top of that.
Hi Rick,
thanks for the review. Your suggestions are very interesting,
and I'd like to know what you think about the following
before proceeding.
This API is the result of an effort I made after a couple of
great pre-implementation calls with Gary. This implementation
is not object oriented on purpose. The idea behind this is
to expose unsurprising data-structures and reuse the basic
objects everybody is used to work with (i.e. Python dicts).
The envs_dict is exactly that, it's just the dict
representation of a YAML data structure already knew by
everyone. So instead of having classes and methods we have
basic objects and functions to manipulate those objects.
The goal is to achieve more simplicity, but simplicity is
not a "simple" concept to deal with, and I'd like to
understand from you in what ways this approach seems
confusing, and why a more object oriented one, like the
one you are suggesting, is better.
> At the very least, I think removing the redundant 'env' in some of the
method
> names would help a bit.
I am not sure about that: envs.get_data and envs.set_data
seem ambiguous to me: we need to make it clear we are
getting/setting data on a single environment. That said,
I am always open to suggestions re: naming things.
Ideas?
https:/
> quickstart/
load('~
> would it make things clearer that this is the un-touched data by
naming it
> raw_envs_dict?
As mentioned above, this is intended to be the data structure
we work with, and not just something encapsulated in an object.
https:/
> quickstart/
> This all reads a little verbose and needing to get all the parts
right.
> What about having the env model be a class that tracks the raw
envs_dict
> internally?
> env = Environment.
> env.raw == envs_dict
> print env.name
> errors = validate(env)
> if errors:
> env.normalize()
> if env.is_dirty():
> env.save()
In this case we would also need an object (e.g. a MutableSequence)
representing all the envs and another one representing
a single env. However, as mentioned above, I'll suspend this
branch because I consider this discussion important.
What do a class give us, why is it preferable in this
context?
https:/
> quickstart/
envs.get_
> 'no-such')
> if the code is usually used as envs.XXX can we remove the env from the
function
> names to make the api read easier?
> load/save and such do not have it so they read nicer.
See above, get_data seems not explicit enough to me.
Perhaps get_by_name and set_by_name? <shrug>
| Gary Poster (gary) wrote : | # |
couple of ideas. take or leave :-)
https:/
File quickstart/
https:/
quickstart/
load('~
On 2013/12/09 19:40:53, rharding wrote:
> would it make things clearer that this is the un-touched data by
naming it
> raw_envs_dict?
all_environments? environments_db? env_db?
https:/
quickstart/
Maybe
env_db = load('...')
env_type_db = get_env_type_db()
env_data = get_env_
env_metadata = get_env_
errors = validate(
| Richard Harding (rharding) wrote : | # |
On 2013/12/10 14:02:42, gary.poster wrote:
> couple of ideas. take or leave :-)
https:/
> File quickstart/
https:/
> quickstart/
load('~
> On 2013/12/09 19:40:53, rharding wrote:
> > would it make things clearer that this is the un-touched data by
naming it
> > raw_envs_dict?
> all_environments? environments_db? env_db?
Wow, I completely missed it contained "all" of the environments from the
yaml file. Because it was named so close with env_data I thought the
env_dict was the section of the yaml file that matched to the env_data
in question (one section of the yaml). So the all_environments would
help set that mindset I think.
- 39. By Francesco Banconi
-
Changes as per review.
| Francesco Banconi (frankban) wrote : | # |
Please take a look.
- 40. By Francesco Banconi
-
Checkpoint.
- 41. By Francesco Banconi
-
Merged trunk.
| Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Implement base models for environments.
This is a preparation branch for the
environments management functionality.
The envs module includes a docstring describing
how those new functions will be used.
Tests: `make check`.
No QA required.
R=bac, rharding, gary.poster
CC=
https:/
Preview Diff
| 1 | === modified file 'quickstart/models/envs.py' |
| 2 | --- quickstart/models/envs.py 2013-12-06 17:17:19 +0000 |
| 3 | +++ quickstart/models/envs.py 2013-12-10 15:44:50 +0000 |
| 4 | @@ -14,12 +14,85 @@ |
| 5 | # You should have received a copy of the GNU Affero General Public License |
| 6 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
| 7 | |
| 8 | -"""Juju Quickstart environments management.""" |
| 9 | +"""Juju Quickstart environments management. |
| 10 | + |
| 11 | +The objects included in this module help working with Juju environments. |
| 12 | + |
| 13 | +The user environments are defined in the environments.yaml file stored inside |
| 14 | +the JUJU_HOME (usually in ~/.juju/environments.yaml). |
| 15 | + |
| 16 | +The load function is used to retrieve a memory representation of the |
| 17 | +environments file. This representation is called "env_db" and it is just the |
| 18 | +Python dictionary resulting from YAML decoding the environments file contents. |
| 19 | +The env_db always includes an "environments" key and may or may not include |
| 20 | +a "default" key, which specifies the default environment name. |
| 21 | + |
| 22 | +After the environments representation is loaded in memory, it is possible to |
| 23 | +retrieve the representation of a single environment using the get_env_data |
| 24 | +function, e.g.: |
| 25 | + |
| 26 | + env_db = load('~/.juju/environments.yaml') |
| 27 | + env_data = get_env_data(env_db, 'myenv') |
| 28 | + |
| 29 | +How the env_data is different from just env_db['environments']['myenv']? |
| 30 | +The former stores two additional pieces of information: the environment name |
| 31 | +(env_data['name']) and whether or not that environment is the default one |
| 32 | +(env_data['is-default']). Retrieving an env_data also allows for easily |
| 33 | +validating and normalizing the values in the environment, e.g.: |
| 34 | + |
| 35 | + errors = validate(env_metadata, env_data) |
| 36 | + new_env_data = normalize(env_metadata, env_data) |
| 37 | + |
| 38 | +What is the "env_metadata" argument passed to the functions above? It is a |
| 39 | +dictionary storing meta information about the provider type associated with |
| 40 | +the env_data, and that can be applied to all the environments of that type. |
| 41 | +For instance, env_metadata includes a list of expected fields and a description |
| 42 | +suitable for that specific environment's type. |
| 43 | + |
| 44 | +This meta information can be retrieved as follow: |
| 45 | + |
| 46 | + env_type_db = get_env_type_db() |
| 47 | + env_metadata = get_env_metadata(env_type_db, env_data) |
| 48 | + |
| 49 | +Modifications to env_data can be easily applied to the original env_db using |
| 50 | +the "set_env_data" function, and the resulting env_db can then be saved to |
| 51 | +disk using the "save" function. In the following example an environment is |
| 52 | +retrieved, validated, normalized and then saved back to disk: |
| 53 | + |
| 54 | + env_db = load('~/.juju/environments.yaml') |
| 55 | + env_type_db = get_env_type_db() |
| 56 | + env_data = get_env_data(env_db, 'myenv') |
| 57 | + env_metadata = get_env_metadata(env_type_db, env_data) |
| 58 | + errors = validate(env_metadata, env_data) |
| 59 | + if errors: |
| 60 | + # Handle errors... |
| 61 | + new_env_data = normalize(env_metadata, env_data) |
| 62 | + if new_env_data != env_data: # The normalization process changed the data. |
| 63 | + set_env_data(env_db, 'myenv', new_env_data) |
| 64 | + save('~/.juju/environments.yaml', env_db) |
| 65 | + |
| 66 | +The set_env_data function, as seen above, needs to be passed the original name |
| 67 | +of the environment being modified. If None is passed, that means we are adding |
| 68 | +a new environment. |
| 69 | + |
| 70 | +XXX frankban 13-12-08: implement the missing functions: |
| 71 | + - get_env_type_db() |
| 72 | + - get_env_metadata(env_type_db, env_data) |
| 73 | + - map_fields_to_env_data(env_metadata, env_data) |
| 74 | + - validate(env_metadata, env_data) |
| 75 | + - normalize(env_metadata, env_data) |
| 76 | +XXX frankban 13-12-08: remove the parse_env_file function: this can be done |
| 77 | + when activating models code in manage.py. |
| 78 | +""" |
| 79 | |
| 80 | from __future__ import unicode_literals |
| 81 | |
| 82 | +import collections |
| 83 | +import copy |
| 84 | +import logging |
| 85 | import os |
| 86 | import re |
| 87 | +import tempfile |
| 88 | |
| 89 | from quickstart import ( |
| 90 | serializers, |
| 91 | @@ -60,6 +133,173 @@ |
| 92 | return match.groups()[0] |
| 93 | |
| 94 | |
| 95 | +def load(env_file): |
| 96 | + """Load and parse the provided Juju environments.yaml file. |
| 97 | + |
| 98 | + Return the decoded environments YAML as a dictionary, e.g.: |
| 99 | + |
| 100 | + { |
| 101 | + 'default': 'myenv', |
| 102 | + 'environments': { |
| 103 | + 'myenv': {'type': 'ec2', ...}, |
| 104 | + ... |
| 105 | + } |
| 106 | + } |
| 107 | + |
| 108 | + The resulting dictionary always has an "environments" keys, even if there |
| 109 | + are no environments defined in the Juju environments.yaml file. |
| 110 | + The "default" key instead is only set if the YAML includes a valid default |
| 111 | + environment. |
| 112 | + |
| 113 | + Raise a ValueError if: |
| 114 | + - the environment file is not found; |
| 115 | + - the environment file contents are not parsable by YAML; |
| 116 | + - the YAML contents are not properly structured. |
| 117 | + """ |
| 118 | + # Load the Juju environments file. |
| 119 | + try: |
| 120 | + environments_file = open(env_file.encode('utf-8')) |
| 121 | + except IOError as err: |
| 122 | + msg = b'unable to open environments file: {}'.format(err) |
| 123 | + raise ValueError(msg) |
| 124 | + # Parse the Juju environments file. |
| 125 | + try: |
| 126 | + contents = serializers.yaml_load(environments_file) |
| 127 | + except Exception as err: |
| 128 | + msg = b'unable to parse environments file {}: {}' |
| 129 | + raise ValueError(msg.format(env_file.encode('utf-8'), err)) |
| 130 | + # Retrieve the environment list. |
| 131 | + try: |
| 132 | + env_contents = contents.get('environments', {}).items() |
| 133 | + except AttributeError as err: |
| 134 | + msg = 'invalid YAML contents in {}: {}'.format(env_file, contents) |
| 135 | + raise ValueError(msg.encode('utf-8')) |
| 136 | + environments = {} |
| 137 | + for env_name, env_info in env_contents: |
| 138 | + if isinstance(env_info, collections.Mapping): |
| 139 | + environments[env_name] = env_info |
| 140 | + else: |
| 141 | + logging.warn('excluding invalid environment {}'.format(env_name)) |
| 142 | + # Build the resulting environments dict. |
| 143 | + env_db = {'environments': environments} |
| 144 | + default = contents.get('default') |
| 145 | + if default in environments: |
| 146 | + env_db['default'] = default |
| 147 | + else: |
| 148 | + logging.warn('excluding invalid default {}'.format(default)) |
| 149 | + return env_db |
| 150 | + |
| 151 | + |
| 152 | +def save(env_file, env_db): |
| 153 | + """Save the given env_db to the provided environments.yaml file. |
| 154 | + |
| 155 | + The new environments are saved to disk in the most atomic way possible. |
| 156 | + |
| 157 | + Raise an OSError if any errors occur in the process. |
| 158 | + """ |
| 159 | + # Save the contents in a temporary file, then rename to the real file. |
| 160 | + # Since the renaming operation may fail on some Unix flavors if the source |
| 161 | + # and destination files are on different file systems, use for the |
| 162 | + # temporary file the same directory where the env_file is stored. |
| 163 | + dirname = os.path.dirname(env_file) |
| 164 | + banner = utils.get_quickstart_banner() |
| 165 | + try: |
| 166 | + temp_file = tempfile.NamedTemporaryFile( |
| 167 | + prefix='quickstart-temp-envs-', dir=dirname, delete=False) |
| 168 | + temp_file.write(banner.encode('utf-8')) |
| 169 | + serializers.yaml_dump(env_db, temp_file) |
| 170 | + except Exception as err: |
| 171 | + raise OSError(err) |
| 172 | + # Ensure that all the data is written to disk. |
| 173 | + temp_file.flush() |
| 174 | + os.fsync(temp_file.fileno()) |
| 175 | + temp_file.close() |
| 176 | + # Rename the temporary file to the real environments file. |
| 177 | + os.rename(temp_file.name, env_file) |
| 178 | + |
| 179 | + |
| 180 | +def get_env_data(env_db, env_name): |
| 181 | + """Return the environment data for the given environment name. |
| 182 | + |
| 183 | + The env_data is a Python dictionary describing an environment as an entity |
| 184 | + separated from the env_db. For example, consider an env_db like this: |
| 185 | + |
| 186 | + { |
| 187 | + 'default': 'myenv', |
| 188 | + 'environments': { |
| 189 | + 'myenv': {'type': 'local', 'admin-secret': 'Secret!'}, |
| 190 | + }, |
| 191 | + } |
| 192 | + |
| 193 | + The corresponding env_data for the "myenv" environment is the following: |
| 194 | + |
| 195 | + { |
| 196 | + # The name is now included in the data structure. |
| 197 | + 'name': 'myenv', |
| 198 | + # The "is-default" field is always included. |
| 199 | + 'is-default': True, |
| 200 | + # Remaining data is left as is. |
| 201 | + 'type': 'local', |
| 202 | + 'admin-secret': 'Secret!', |
| 203 | + } |
| 204 | + |
| 205 | + Raise a ValueError if the env_db does not include an environment with the |
| 206 | + given name. |
| 207 | + """ |
| 208 | + try: |
| 209 | + info = env_db['environments'][env_name] |
| 210 | + except KeyError: |
| 211 | + raise ValueError(b'environment {!r} not found'.format(env_name)) |
| 212 | + # Why not just use env_data.copy()? Because this way internal mutable data |
| 213 | + # structures are preserved, even if they are unlikely to be found. |
| 214 | + env_data = copy.deepcopy(info) |
| 215 | + env_data.update({ |
| 216 | + 'is-default': env_db.get('default') == env_name, |
| 217 | + 'name': env_name, |
| 218 | + }) |
| 219 | + return env_data |
| 220 | + |
| 221 | + |
| 222 | +def set_env_data(env_db, env_name, env_data): |
| 223 | + """Update the environments dictionary with the given environment data. |
| 224 | + |
| 225 | + The env_name argument is used as a reference to an existing environment |
| 226 | + in the env_db. If env_name is None, a new environment is added to the |
| 227 | + env_db. Otherwise the existing environment is updated using env_data. |
| 228 | + |
| 229 | + The env_data argument is an environment data dictionary, and must include |
| 230 | + at least the "name" and "is-default" keys. |
| 231 | + |
| 232 | + Raise a ValueError if: |
| 233 | + - env_data does not include a "name" key; |
| 234 | + - env_data does not include a "is-default" key; |
| 235 | + - env_data is a new environment and its name is already used by an |
| 236 | + existing environment; |
| 237 | + - env_data is an existing environment renamed, and its name is already |
| 238 | + used by another existing environment. |
| 239 | + |
| 240 | + If any errors occur, the env_db is left untouched. |
| 241 | + Without errors, the env_db is modified in place and None is returned. |
| 242 | + """ |
| 243 | + new_env_data = copy.deepcopy(env_data) |
| 244 | + try: |
| 245 | + new_name = new_env_data.pop('name') |
| 246 | + is_default = new_env_data.pop('is-default') |
| 247 | + except KeyError: |
| 248 | + raise ValueError(b'invalid env_data: {!r}'.format(env_data)) |
| 249 | + environments = env_db['environments'] |
| 250 | + if (new_name != env_name) and (new_name in environments): |
| 251 | + raise ValueError( |
| 252 | + b'an environment named {!r} already exists'.format(new_name)) |
| 253 | + if env_name is not None: |
| 254 | + del environments[env_name] |
| 255 | + environments[new_name] = new_env_data |
| 256 | + if is_default: |
| 257 | + env_db['default'] = new_name |
| 258 | + elif env_db.get('default') == env_name: |
| 259 | + del env_db['default'] |
| 260 | + |
| 261 | + |
| 262 | def parse_env_file(env_file, env_name): |
| 263 | """Parse the provided Juju environments.yaml file. |
| 264 | |
| 265 | |
| 266 | === modified file 'quickstart/tests/helpers.py' |
| 267 | --- quickstart/tests/helpers.py 2013-12-06 17:17:19 +0000 |
| 268 | +++ quickstart/tests/helpers.py 2013-12-10 15:44:50 +0000 |
| 269 | @@ -104,10 +104,14 @@ |
| 270 | """Shared methods for testing a Juju environments file.""" |
| 271 | |
| 272 | valid_contents = yaml.safe_dump({ |
| 273 | - 'environments': {'aws': { |
| 274 | - 'admin-secret': 'Secret!', |
| 275 | - 'type': 'ec2', |
| 276 | - 'default-series': 'edgy'}}, |
| 277 | + 'default': 'aws', |
| 278 | + 'environments': { |
| 279 | + 'aws': { |
| 280 | + 'admin-secret': 'Secret!', |
| 281 | + 'type': 'ec2', |
| 282 | + 'default-series': 'edgy', |
| 283 | + }, |
| 284 | + }, |
| 285 | }) |
| 286 | |
| 287 | def make_env_file(self, contents=None): |
| 288 | |
| 289 | === modified file 'quickstart/tests/models/test_envs.py' |
| 290 | --- quickstart/tests/models/test_envs.py 2013-12-06 17:17:19 +0000 |
| 291 | +++ quickstart/tests/models/test_envs.py 2013-12-10 15:44:50 +0000 |
| 292 | @@ -18,6 +18,8 @@ |
| 293 | |
| 294 | from __future__ import unicode_literals |
| 295 | |
| 296 | +import copy |
| 297 | +import os |
| 298 | import unittest |
| 299 | |
| 300 | import mock |
| 301 | @@ -76,6 +78,392 @@ |
| 302 | mock_call.assert_called_once_with('juju', 'switch') |
| 303 | |
| 304 | |
| 305 | +class TestLoad( |
| 306 | + helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin, |
| 307 | + unittest.TestCase): |
| 308 | + |
| 309 | + def test_no_file(self): |
| 310 | + # A ValueError is raised if the environments file is not found. |
| 311 | + expected = ( |
| 312 | + 'unable to open environments file: ' |
| 313 | + "[Errno 2] No such file or directory: '/no/such/file.yaml'" |
| 314 | + ) |
| 315 | + with self.assert_value_error(expected): |
| 316 | + envs.load('/no/such/file.yaml') |
| 317 | + |
| 318 | + def test_invalid_yaml(self): |
| 319 | + # A ValueError is raised if the environments file is not a valid YAML. |
| 320 | + env_file = self.make_env_file(':') |
| 321 | + with self.assertRaises(ValueError) as context_manager: |
| 322 | + envs.load(env_file) |
| 323 | + expected = 'unable to parse environments file {}'.format(env_file) |
| 324 | + self.assertIn(expected, bytes(context_manager.exception)) |
| 325 | + |
| 326 | + def test_invalid_yaml_contents(self): |
| 327 | + # A ValueError is raised if the environments file is not well formed. |
| 328 | + env_file = self.make_env_file('a-string') |
| 329 | + expected = 'invalid YAML contents in {}: a-string'.format(env_file) |
| 330 | + with self.assert_value_error(expected): |
| 331 | + envs.load(env_file) |
| 332 | + |
| 333 | + def test_success_with_default(self): |
| 334 | + # The YAML decoded environments dictionary (including default) is |
| 335 | + # correctly generated and returned. |
| 336 | + env_file = self.make_env_file() |
| 337 | + env_db = envs.load(env_file) |
| 338 | + self.assertEqual(yaml.safe_load(self.valid_contents), env_db) |
| 339 | + |
| 340 | + def test_success_no_default(self): |
| 341 | + # The YAML decoded environments dictionary (with no default) is |
| 342 | + # correctly generated and returned. |
| 343 | + expected = { |
| 344 | + 'environments': { |
| 345 | + 'aws': {'admin-secret': 'Secret!', 'type': 'ec2'}, |
| 346 | + 'local': {'admin-secret': 'Secret!', 'type': 'local'}, |
| 347 | + }, |
| 348 | + } |
| 349 | + env_file = self.make_env_file(yaml.safe_dump(expected)) |
| 350 | + env_db = envs.load(env_file) |
| 351 | + self.assertEqual(expected, env_db) |
| 352 | + |
| 353 | + def test_success_invalid_default(self): |
| 354 | + # The YAML decoded environments dictionary is correctly generated and |
| 355 | + # returned excluding invalid default environment values. |
| 356 | + expected = { |
| 357 | + 'environments': { |
| 358 | + 'aws': {'admin-secret': 'Secret!', 'type': 'ec2'}, |
| 359 | + }, |
| 360 | + } |
| 361 | + yaml_contents = expected.copy() |
| 362 | + yaml_contents['default'] = 'no-such-env' |
| 363 | + expected_logs = ['excluding invalid default no-such-env'] |
| 364 | + env_file = self.make_env_file(yaml.safe_dump(yaml_contents)) |
| 365 | + with helpers.assert_logs(expected_logs, 'warn'): |
| 366 | + env_db = envs.load(env_file) |
| 367 | + self.assertEqual(expected, env_db) |
| 368 | + |
| 369 | + def test_success_extraneous_fields(self): |
| 370 | + # The YAML decoded environments dictionary is correctly generated and |
| 371 | + # returned preserving extraneous fields. |
| 372 | + expected = { |
| 373 | + 'environments': { |
| 374 | + 'aws': {'polluted': True, 'type': 'ec2'}, |
| 375 | + 'local': {'answer': 42}, |
| 376 | + }, |
| 377 | + } |
| 378 | + env_file = self.make_env_file(yaml.safe_dump(expected)) |
| 379 | + env_db = envs.load(env_file) |
| 380 | + self.assertEqual(expected, env_db) |
| 381 | + |
| 382 | + def test_success_excluding_envs(self): |
| 383 | + # The YAML decoded environments dictionary is correctly generated and |
| 384 | + # returned excluding invalid environments. |
| 385 | + expected = { |
| 386 | + 'default': 'aws', |
| 387 | + 'environments': { |
| 388 | + 'aws': {'admin-secret': 'Secret!', 'type': 'ec2'}, |
| 389 | + }, |
| 390 | + } |
| 391 | + yaml_contents = copy.deepcopy(expected) |
| 392 | + yaml_contents['environments']['bad'] = 42 |
| 393 | + expected_logs = ['excluding invalid environment bad'] |
| 394 | + env_file = self.make_env_file(yaml.safe_dump(yaml_contents)) |
| 395 | + with helpers.assert_logs(expected_logs, 'warn'): |
| 396 | + env_db = envs.load(env_file) |
| 397 | + self.assertEqual(expected, env_db) |
| 398 | + |
| 399 | + |
| 400 | +class TestSave(helpers.EnvFileTestsMixin, unittest.TestCase): |
| 401 | + |
| 402 | + def setUp(self): |
| 403 | + # Create an environments file. |
| 404 | + self.env_file = self.make_env_file() |
| 405 | + self.original_contents = open(self.env_file).read() |
| 406 | + |
| 407 | + def test_dump_error(self): |
| 408 | + # An OSError is raised if errors occur in the serialization process. |
| 409 | + with mock.patch('quickstart.serializers.yaml_dump') as mock_yaml_dump: |
| 410 | + mock_yaml_dump.side_effect = ValueError(b'bad wolf') |
| 411 | + with self.assertRaises(OSError) as context_manager: |
| 412 | + envs.save(self.env_file, {'new': 'contents'}) |
| 413 | + self.assertEqual(b'bad wolf', str(context_manager.exception)) |
| 414 | + # The original file contents have been left untouched. |
| 415 | + self.assertEqual(self.valid_contents, self.original_contents) |
| 416 | + |
| 417 | + def test_atomic(self): |
| 418 | + # The new contents are written in a temporary file before and then the |
| 419 | + # file is renamed to the original destination. If an error occurs, the |
| 420 | + # original contents are not influenced. |
| 421 | + def rename(source, destination): |
| 422 | + # Remove the source before actually renaming in order to simulate |
| 423 | + # some kind of disk error. |
| 424 | + os.remove(source) |
| 425 | + os.rename(source, destination) |
| 426 | + with mock.patch('os.rename', rename): |
| 427 | + with self.assertRaises(OSError) as context_manager: |
| 428 | + envs.save(self.env_file, {'new': 'contents'}) |
| 429 | + self.assertIn( |
| 430 | + 'No such file or directory', |
| 431 | + str(context_manager.exception)) |
| 432 | + # The original file contents have been left untouched. |
| 433 | + self.assertEqual(self.valid_contents, self.original_contents) |
| 434 | + |
| 435 | + def test_success(self): |
| 436 | + # The environments file is correctly updated with the new contents. |
| 437 | + envs.save(self.env_file, {'new': 'contents'}) |
| 438 | + contents = open(self.env_file).read() |
| 439 | + # The banner has been written. |
| 440 | + self.assertIn( |
| 441 | + '# This file has been generated by juju quickstart', |
| 442 | + contents) |
| 443 | + # Also the new contents have been saved. |
| 444 | + self.assertIn('new: contents', contents) |
| 445 | + |
| 446 | + |
| 447 | +class GetSetEnvDataTestsMixin(object): |
| 448 | + """Set up an initial environments dictionary.""" |
| 449 | + |
| 450 | + def setUp(self): |
| 451 | + self.env_db = { |
| 452 | + 'default': 'lxc', |
| 453 | + 'environments': { |
| 454 | + 'aws': { |
| 455 | + 'admin_secret': 'Secret!', |
| 456 | + 'default-series': 'precise', |
| 457 | + 'type': 'ec2', |
| 458 | + }, |
| 459 | + 'lxc': { |
| 460 | + 'admin_secret': 'NotSoSecret!', |
| 461 | + 'mutable': [1, 2, 3], |
| 462 | + 'type': 'local', |
| 463 | + }, |
| 464 | + }, |
| 465 | + } |
| 466 | + self.original = copy.deepcopy(self.env_db) |
| 467 | + super(GetSetEnvDataTestsMixin, self).setUp() |
| 468 | + |
| 469 | + def assert_env_db_not_modified(self): |
| 470 | + """Ensure the stored env_db is not modified.""" |
| 471 | + self.assertEqual(self.original, self.env_db) |
| 472 | + |
| 473 | + |
| 474 | +class TestGetEnvData( |
| 475 | + GetSetEnvDataTestsMixin, helpers.ValueErrorTestsMixin, |
| 476 | + unittest.TestCase): |
| 477 | + |
| 478 | + def test_env_not_found(self): |
| 479 | + # A ValueError is raised if an environment with the given name is not |
| 480 | + # found in the environments dictionary. |
| 481 | + with self.assert_value_error("environment u'no-such' not found"): |
| 482 | + envs.get_env_data(self.env_db, 'no-such') |
| 483 | + |
| 484 | + def test_resulting_env_data(self): |
| 485 | + # The resulting env_data is correctly generated. |
| 486 | + expected = { |
| 487 | + 'admin_secret': 'Secret!', |
| 488 | + 'default-series': 'precise', |
| 489 | + 'is-default': False, |
| 490 | + 'name': 'aws', |
| 491 | + 'type': 'ec2', |
| 492 | + } |
| 493 | + obtained = envs.get_env_data(self.env_db, 'aws') |
| 494 | + self.assertEqual(expected, obtained) |
| 495 | + |
| 496 | + def test_env_data_default_environments(self): |
| 497 | + # The env_data is correctly generated for a default environment. |
| 498 | + expected = { |
| 499 | + 'admin_secret': 'NotSoSecret!', |
| 500 | + 'is-default': True, |
| 501 | + 'mutable': [1, 2, 3], |
| 502 | + 'name': 'lxc', |
| 503 | + 'type': 'local', |
| 504 | + } |
| 505 | + obtained = envs.get_env_data(self.env_db, 'lxc') |
| 506 | + self.assertEqual(expected, obtained) |
| 507 | + |
| 508 | + def test_mutate(self): |
| 509 | + # Modifications to the resulting env_data do not influence the original |
| 510 | + # environments dictionary. |
| 511 | + env_data = envs.get_env_data(self.env_db, 'lxc') |
| 512 | + env_data.update({ |
| 513 | + 'admin_secret': 'AnotherSecret!', |
| 514 | + 'is-default': False, |
| 515 | + 'new-field': 'new-value' |
| 516 | + }) |
| 517 | + # Also change a mutable internal data structure. |
| 518 | + env_data['mutable'].append(42) |
| 519 | + self.assert_env_db_not_modified() |
| 520 | + |
| 521 | + |
| 522 | +class TestSetEnvData( |
| 523 | + GetSetEnvDataTestsMixin, helpers.ValueErrorTestsMixin, |
| 524 | + unittest.TestCase): |
| 525 | + |
| 526 | + def test_error_no_name_key(self): |
| 527 | + # A ValueError is raised if the env_data dictionary does not include |
| 528 | + # the "name" key. |
| 529 | + env_data = {'is-default': False} |
| 530 | + expected = "invalid env_data: {u'is-default': False}" |
| 531 | + with self.assert_value_error(expected): |
| 532 | + envs.set_env_data(self.env_db, 'aws', env_data) |
| 533 | + # The environments dictionary has not been modified. |
| 534 | + self.assert_env_db_not_modified() |
| 535 | + |
| 536 | + def test_error_no_default_key(self): |
| 537 | + # A ValueError is raised if the env_data dictionary does not include |
| 538 | + # the "is-default" key. |
| 539 | + env_data = {'name': 'aws'} |
| 540 | + expected = "invalid env_data: {u'name': u'aws'}" |
| 541 | + with self.assert_value_error(expected): |
| 542 | + envs.set_env_data(self.env_db, 'aws', env_data) |
| 543 | + # The environments dictionary has not been modified. |
| 544 | + self.assert_env_db_not_modified() |
| 545 | + |
| 546 | + def test_error_new_environment(self): |
| 547 | + # A ValueError is raised if the name of a new environment is |
| 548 | + # already in the environments dictionary. |
| 549 | + env_data = {'is-default': False, 'name': 'aws'} |
| 550 | + expected = "an environment named u'aws' already exists" |
| 551 | + with self.assert_value_error(expected): |
| 552 | + envs.set_env_data(self.env_db, None, env_data) |
| 553 | + # The environments dictionary has not been modified. |
| 554 | + self.assert_env_db_not_modified() |
| 555 | + |
| 556 | + def test_error_existing_environment(self): |
| 557 | + # A ValueError is raised if the new name of a renamed existing |
| 558 | + # environment is already in the environments dictionary. |
| 559 | + env_data = {'is-default': False, 'name': 'lxc'} |
| 560 | + expected = "an environment named u'lxc' already exists" |
| 561 | + with self.assert_value_error(expected): |
| 562 | + envs.set_env_data(self.env_db, 'aws', env_data) |
| 563 | + # The environments dictionary has not been modified. |
| 564 | + self.assert_env_db_not_modified() |
| 565 | + |
| 566 | + def test_new_environment_added(self): |
| 567 | + # A new environment is properly added to the environments dictionary. |
| 568 | + env_data = { |
| 569 | + 'default-series': 'edgy', |
| 570 | + 'is-default': False, |
| 571 | + 'name': 'new-one', |
| 572 | + } |
| 573 | + return_value = envs.set_env_data(self.env_db, None, env_data) |
| 574 | + self.assertIsNone(return_value) |
| 575 | + expected = { |
| 576 | + 'default': 'lxc', |
| 577 | + 'environments': { |
| 578 | + 'aws': { |
| 579 | + 'admin_secret': 'Secret!', |
| 580 | + 'default-series': 'precise', |
| 581 | + 'type': 'ec2', |
| 582 | + }, |
| 583 | + 'lxc': { |
| 584 | + 'admin_secret': 'NotSoSecret!', |
| 585 | + 'mutable': [1, 2, 3], |
| 586 | + 'type': 'local', |
| 587 | + }, |
| 588 | + 'new-one': { |
| 589 | + 'default-series': 'edgy', |
| 590 | + }, |
| 591 | + }, |
| 592 | + } |
| 593 | + self.assertEqual(expected, self.env_db) |
| 594 | + |
| 595 | + def test_new_default_environment_added(self): |
| 596 | + # A new default environment is properly added to the environments |
| 597 | + # dictionary. |
| 598 | + env_data = { |
| 599 | + 'default-series': 'edgy', |
| 600 | + 'is-default': True, |
| 601 | + 'name': 'new-one', |
| 602 | + } |
| 603 | + envs.set_env_data(self.env_db, None, env_data) |
| 604 | + self.assertIn('new-one', self.env_db['environments']) |
| 605 | + self.assertEqual( |
| 606 | + {'default-series': 'edgy'}, |
| 607 | + self.env_db['environments']['new-one']) |
| 608 | + self.assertEqual('new-one', self.env_db['default']) |
| 609 | + |
| 610 | + def test_existing_environment_updated(self): |
| 611 | + # An existing environment is properly updated. |
| 612 | + env_data = { |
| 613 | + 'admin_secret': 'NewSecret!', |
| 614 | + 'is-default': True, |
| 615 | + 'name': 'lxc', |
| 616 | + 'type': 'local' |
| 617 | + } |
| 618 | + return_value = envs.set_env_data(self.env_db, 'lxc', env_data) |
| 619 | + self.assertIsNone(return_value) |
| 620 | + expected = { |
| 621 | + 'default': 'lxc', |
| 622 | + 'environments': { |
| 623 | + 'aws': { |
| 624 | + 'admin_secret': 'Secret!', |
| 625 | + 'default-series': 'precise', |
| 626 | + 'type': 'ec2', |
| 627 | + }, |
| 628 | + 'lxc': { |
| 629 | + 'admin_secret': 'NewSecret!', |
| 630 | + 'type': 'local', |
| 631 | + }, |
| 632 | + }, |
| 633 | + } |
| 634 | + self.assertEqual(expected, self.env_db) |
| 635 | + |
| 636 | + def test_existing_environment_updated_changing_name(self): |
| 637 | + # An existing environment is properly updated, including its name. |
| 638 | + env_data = { |
| 639 | + 'admin_secret': 'Hash!', |
| 640 | + 'is-default': False, |
| 641 | + 'name': 'yay-the-clouds', |
| 642 | + 'type': 'ec2' |
| 643 | + } |
| 644 | + return_value = envs.set_env_data(self.env_db, 'aws', env_data) |
| 645 | + self.assertIsNone(return_value) |
| 646 | + expected = { |
| 647 | + 'default': 'lxc', |
| 648 | + 'environments': { |
| 649 | + 'lxc': { |
| 650 | + 'admin_secret': 'NotSoSecret!', |
| 651 | + 'mutable': [1, 2, 3], |
| 652 | + 'type': 'local', |
| 653 | + }, |
| 654 | + 'yay-the-clouds': { |
| 655 | + 'admin_secret': 'Hash!', |
| 656 | + 'type': 'ec2', |
| 657 | + }, |
| 658 | + }, |
| 659 | + } |
| 660 | + self.assertEqual(expected, self.env_db) |
| 661 | + |
| 662 | + def test_existing_environment_set_as_default(self): |
| 663 | + # An existing environment is correctly promoted as the default one. |
| 664 | + env_data = { |
| 665 | + 'default-series': 'edgy', |
| 666 | + 'is-default': True, |
| 667 | + 'name': 'aws', |
| 668 | + } |
| 669 | + envs.set_env_data(self.env_db, 'aws', env_data) |
| 670 | + self.assertIn('aws', self.env_db['environments']) |
| 671 | + self.assertEqual( |
| 672 | + {'default-series': 'edgy'}, |
| 673 | + self.env_db['environments']['aws']) |
| 674 | + self.assertEqual('aws', self.env_db['default']) |
| 675 | + |
| 676 | + def test_existing_environment_no_longer_default(self): |
| 677 | + # An existing environment is correctly downgraded to non-default. |
| 678 | + env_data = { |
| 679 | + 'default-series': 'edgy', |
| 680 | + 'is-default': False, |
| 681 | + 'name': 'lxc', |
| 682 | + } |
| 683 | + envs.set_env_data(self.env_db, 'lxc', env_data) |
| 684 | + self.assertIn('lxc', self.env_db['environments']) |
| 685 | + self.assertEqual( |
| 686 | + {'default-series': 'edgy'}, |
| 687 | + self.env_db['environments']['lxc']) |
| 688 | + self.assertNotIn('default', self.env_db) |
| 689 | + |
| 690 | + |
| 691 | class TestParseEnvFile( |
| 692 | helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin, |
| 693 | unittest.TestCase): |
| 694 | |
| 695 | === modified file 'quickstart/tests/test_utils.py' |
| 696 | --- quickstart/tests/test_utils.py 2013-12-06 17:17:19 +0000 |
| 697 | +++ quickstart/tests/test_utils.py 2013-12-10 15:44:50 +0000 |
| 698 | @@ -18,6 +18,7 @@ |
| 699 | |
| 700 | from __future__ import unicode_literals |
| 701 | |
| 702 | +import datetime |
| 703 | import httplib |
| 704 | import json |
| 705 | import socket |
| 706 | @@ -28,6 +29,7 @@ |
| 707 | import yaml |
| 708 | |
| 709 | from quickstart import ( |
| 710 | + get_version, |
| 711 | settings, |
| 712 | utils, |
| 713 | ) |
| 714 | @@ -237,6 +239,25 @@ |
| 715 | 'unable to find the charm URL', bytes(context_manager.exception)) |
| 716 | |
| 717 | |
| 718 | +class TestGetQuickstartBanner(unittest.TestCase): |
| 719 | + |
| 720 | + def patch_datetime(self): |
| 721 | + mock_datetime = mock.Mock() |
| 722 | + mock_datetime.utcnow.return_value = datetime.datetime( |
| 723 | + 2014, 2, 27, 7, 42, 47) |
| 724 | + return mock.patch('datetime.datetime', mock_datetime) |
| 725 | + |
| 726 | + def test_banner(self): |
| 727 | + # The banner is correctly generated. |
| 728 | + with self.patch_datetime(): |
| 729 | + obtained = utils.get_quickstart_banner() |
| 730 | + expected = ( |
| 731 | + '# This file has been generated by juju quickstart v{}\n' |
| 732 | + '# at 2014-02-27 07:42:47 UTC.\n\n' |
| 733 | + ).format(get_version()) |
| 734 | + self.assertEqual(expected, obtained) |
| 735 | + |
| 736 | + |
| 737 | class TestGetServiceInfo(helpers.WatcherDataTestsMixin, unittest.TestCase): |
| 738 | |
| 739 | def test_service_and_unit(self): |
| 740 | |
| 741 | === modified file 'quickstart/utils.py' |
| 742 | --- quickstart/utils.py 2013-12-06 17:17:19 +0000 |
| 743 | +++ quickstart/utils.py 2013-12-10 15:44:50 +0000 |
| 744 | @@ -22,6 +22,7 @@ |
| 745 | ) |
| 746 | |
| 747 | import collections |
| 748 | +import datetime |
| 749 | import httplib |
| 750 | import json |
| 751 | import logging |
| 752 | @@ -30,6 +31,7 @@ |
| 753 | import subprocess |
| 754 | import urllib2 |
| 755 | |
| 756 | +import quickstart |
| 757 | from quickstart import ( |
| 758 | serializers, |
| 759 | settings, |
| 760 | @@ -133,6 +135,22 @@ |
| 761 | return charm_url |
| 762 | |
| 763 | |
| 764 | +def get_quickstart_banner(): |
| 765 | + """Return a quickstart banner suitable for being included in files. |
| 766 | + |
| 767 | + The banner is returned as a string, e.g.: |
| 768 | + |
| 769 | + # This file has been generated by juju quickstart v0.42.0 |
| 770 | + # in date 2013-12-31 23:59:00 UTC. |
| 771 | + """ |
| 772 | + now = datetime.datetime.utcnow() |
| 773 | + formatted_date = now.isoformat(sep=b' ').split('.')[0] |
| 774 | + version = quickstart.get_version() |
| 775 | + return ( |
| 776 | + '# This file has been generated by juju quickstart v{}\n' |
| 777 | + '# at {} UTC.\n\n'.format(version, formatted_date)) |
| 778 | + |
| 779 | + |
| 780 | def get_service_info(status, service_name): |
| 781 | """Retrieve information on the given service and on its first alive unit. |
| 782 |
Reviewers: mp+198265_
Message:
Please take a look.
Description:
Implement base models for environments.
This is a preparation branch for the
environments management functionality.
The envs module includes a docstring describing
how those new functions will be used.
Tests: `make check`.
No QA required.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+669, -5 lines):
A [revision details]
M quickstart/
M quickstart/
M quickstart/
M quickstart/
M quickstart/utils.py