Merge lp:~frankban/juju-quickstart/charm-url-warning into lp:juju-quickstart
- charm-url-warning
- Merge into trunk
| Status: | Merged |
|---|---|
| Merged at revision: | 21 |
| Proposed branch: | lp:~frankban/juju-quickstart/charm-url-warning |
| Merge into: | lp:juju-quickstart |
| Diff against target: |
899 lines (+636/-39) 10 files modified
quickstart/__init__.py (+1/-1) quickstart/app.py (+2/-6) quickstart/charms.py (+118/-0) quickstart/manage.py (+40/-2) quickstart/settings.py (+10/-1) quickstart/tests/test_app.py (+129/-25) quickstart/tests/test_charms.py (+205/-0) quickstart/tests/test_manage.py (+74/-1) quickstart/tests/test_utils.py (+31/-1) quickstart/utils.py (+26/-2) |
| To merge this branch: | bzr merge lp:~frankban/juju-quickstart/charm-url-warning |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju GUI Hackers | Pending | ||
|
Review via email:
|
|||
Commit message
Description of the change
Improve charm URL handling.
Validate the user provided GUI charm URL.
Also add a missing test for complete coverage
(obsessive-
Tests: `make check`.
QA:
1)
Provide invalid charm URLs, the program
should immediately stop the execution
and exit with an error, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url invalid
.venv/bin/python juju-quickstart --gui-charm-url local:precise/
.venv/bin/python juju-quickstart --gui-charm-url http:~juju-
.venv/bin/python juju-quickstart --gui-charm-url cs:precise/
.venv/bin/python juju-quickstart --gui-charm-url cs:saucy/
2)
Run the program passing a customized charm URL, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url cs:~juju-
You should see the "using a customized juju-gui charm" warning
printed during the service deployment step.
Re-execute the command above: quickstart should reuse the
service in the environment and the warning is printed again.
Destroy the environment.
3)
Now manually deploy an outdated version of the GUI charm:
(sudo) juju bootstrap
juju deploy cs:precise/
Run quickstart:
.venv/bin/python juju-quickstart
You should see the "charm is outdated" warning, then quickstart
waits for the outdated GUI to be deployed and ready.
Destroy the environment.
4)
Run quickstart normally:
.venv/bin/python juju-quickstart
The last official GUI charm (cs:precise/
is installed and no warnings are logged.
Destroy the environment.
Done, thank you!
| Francesco Banconi (frankban) wrote : | # |
| Richard Harding (rharding) wrote : | # |
I'm worried about the dupe code. We've got another project doing charm
parsing and such. This is done in the juju store and charmworld. I
wonder if we can cheat and check if the charm can be found via a network
connection to either of those stores rather than dupe the code.
The hard coded series support just seems like a land mine to forget to
update the deployer and then get bug reports down the road. Maybe we can
at least put in precise/trusty now and file a bug on the charm we need
to test and get it into trusty?
The code looks ok with a couple of notes below, but I'd like to discuss
if there's a lighter maintenance way to go.
https:/
File quickstart/app.py (right):
https:/
quickstart/
I'm a bit hmmm on the check method just being inline. Other things are
doing logging, raising exeptions, etc. I'd have expected this to check
it and, based on the response, take some logic to exit from here.
https:/
File quickstart/
https:/
quickstart/
yuck, another place of duped logic for urls. Is there any way we
can/should push this up to charmworld?
https:/
File quickstart/
https:/
quickstart/
settings.
is settings coming from juju or is there another copy of the list in
here?
https:/
quickstart/
should this be moved to some _checkIsValidBundle method?
https:/
File quickstart/
https:/
quickstart/
is correctly found and logged.
Is there a test for a non-precise charm?
https:/
File quickstart/utils.py (right):
https:/
quickstart/
I see, it's just logging. I still thing it reads strange in app to check
and then do nothing with the check. Maybe it's a warn_invalid_
method?
| Richard Harding (rharding) wrote : | # |
Per irc chat, LGTM on the code. It'll be a nice day when we all share a
single proof library for that stuff.
Will QA now.
| Brad Crittenden (bac) wrote : | # |
LGTM modulo small suggestions and Rick's concerns. QA OK though I did
not do #3.
https:/
File quickstart/
https:/
quickstart/
This function looks very complete. But as Rick said it duplicates a lot
of functionality used by Charmworld. Would be nice to consolidate
them...but where?
https:/
File quickstart/
https:/
quickstart/
{}'.format(charm))
If you just printed the charm.series it would be more consistent with
other error messages.
https:/
File quickstart/
https:/
quickstart/
Since the word 'series' is both singular and plural, just for clarity
I'd change the above to say "The set of series..." to indicate in the
future it may be more than one. May be overkill since you clearly
define a tuple below...
| Richard Harding (rharding) wrote : | # |
QA on #3 is good.
- 27. By Francesco Banconi
-
Changes as per review.
- 28. By Francesco Banconi
-
Merged trunk.
| Francesco Banconi (frankban) wrote : | # |
*** Submitted:
Improve charm URL handling.
Validate the user provided GUI charm URL.
Also add a missing test for complete coverage
(obsessive-
Tests: `make check`.
QA:
1)
Provide invalid charm URLs, the program
should immediately stop the execution
and exit with an error, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url invalid
.venv/bin/python juju-quickstart --gui-charm-url
local:precise/
.venv/bin/python juju-quickstart --gui-charm-url
http:~juju-
.venv/bin/python juju-quickstart --gui-charm-url cs:precise/
bundle:
.venv/bin/python juju-quickstart --gui-charm-url cs:saucy/
2)
Run the program passing a customized charm URL, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url
cs:~juju-
You should see the "using a customized juju-gui charm" warning
printed during the service deployment step.
Re-execute the command above: quickstart should reuse the
service in the environment and the warning is printed again.
Destroy the environment.
3)
Now manually deploy an outdated version of the GUI charm:
(sudo) juju bootstrap
juju deploy cs:precise/
Run quickstart:
.venv/bin/python juju-quickstart
You should see the "charm is outdated" warning, then quickstart
waits for the outdated GUI to be deployed and ready.
Destroy the environment.
4)
Run quickstart normally:
.venv/bin/python juju-quickstart
The last official GUI charm (cs:precise/
is installed and no warnings are logged.
Destroy the environment.
Done, thank you!
R=rharding, bac
CC=
https:/
https:/
File quickstart/
https:/
quickstart/
On 2013/11/22 14:31:51, bac wrote:
> This function looks very complete. But as Rick said it duplicates a
lot of
> functionality used by Charmworld. Would be nice to consolidate
them...but
> where?
Agreed. We discussed on IRC about the need for an external
charms/bundles validation library where to refactor this kind of code.
Many projects can then take advantage of such a library, e.g.
quickstart, proof, deployer, guiserver, charmworld...
https:/
File quickstart/
https:/
quickstart/
settings.
On 2013/11/22 13:47:33, rharding wrote:
> is settings coming from juju or is there another copy of the list in
here?
The list is defined in quickstart.
https:/
quickstart/
{}'.format(charm))
On 2013/11/22 14:31:51, bac wrote:
> If you just printed the charm.series it would be more consistent with
other
> error messages.
Done.
https:/
quickstart/
| Francesco Banconi (frankban) wrote : | # |
Hey Rick and Brad,
thank you for the great suggestions in you reviews!
Preview Diff
| 1 | === modified file 'quickstart/__init__.py' |
| 2 | --- quickstart/__init__.py 2013-11-18 22:50:55 +0000 |
| 3 | +++ quickstart/__init__.py 2013-11-22 15:07:41 +0000 |
| 4 | @@ -19,7 +19,7 @@ |
| 5 | that it can be managed using a Web interface (the Juju GUI). |
| 6 | """ |
| 7 | |
| 8 | -VERSION = (0, 4, 1) |
| 9 | +VERSION = (0, 4, 2) |
| 10 | |
| 11 | |
| 12 | def get_version(): |
| 13 | |
| 14 | === modified file 'quickstart/app.py' |
| 15 | --- quickstart/app.py 2013-11-21 19:45:01 +0000 |
| 16 | +++ quickstart/app.py 2013-11-22 15:07:41 +0000 |
| 17 | @@ -233,7 +233,7 @@ |
| 18 | msg = 'unable to retrieve the {} charm URL from the API: {}' |
| 19 | logging.warn(msg.format(service_name, err)) |
| 20 | charm_url = settings.DEFAULT_CHARM_URL |
| 21 | - print('charm URL: {}'.format(charm_url)) |
| 22 | + utils.check_gui_charm_url(charm_url) |
| 23 | # Deploy the service without units. |
| 24 | try: |
| 25 | env.deploy(service_name, charm_url, num_units=0) |
| 26 | @@ -244,11 +244,7 @@ |
| 27 | else: |
| 28 | # We already have the service in the environment. |
| 29 | print('service {} already deployed'.format(service_name)) |
| 30 | - charm_url = service_data['CharmURL'] |
| 31 | - print('charm URL: {}'.format(charm_url)) |
| 32 | - # XXX frankban: warn the user if the actual charm_url != the given |
| 33 | - # charm_url, and warn louder if the charm URL does not match |
| 34 | - # /\/juju-gui-\d+$/. |
| 35 | + utils.check_gui_charm_url(service_data['CharmURL']) |
| 36 | service_exposed = service_data.get('Exposed', False) |
| 37 | # At this point the service is surely deployed in the environment: expose |
| 38 | # it if necessary and add a unit if it is missing. |
| 39 | |
| 40 | === added file 'quickstart/charms.py' |
| 41 | --- quickstart/charms.py 1970-01-01 00:00:00 +0000 |
| 42 | +++ quickstart/charms.py 2013-11-22 15:07:41 +0000 |
| 43 | @@ -0,0 +1,118 @@ |
| 44 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
| 45 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
| 46 | +# Copyright (C) 2013 Canonical Ltd. |
| 47 | +# |
| 48 | +# This program is free software: you can redistribute it and/or modify it under |
| 49 | +# the terms of the GNU Affero General Public License version 3, as published by |
| 50 | +# the Free Software Foundation. |
| 51 | +# |
| 52 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
| 53 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
| 54 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
| 55 | +# Affero General Public License for more details. |
| 56 | +# |
| 57 | +# You should have received a copy of the GNU Affero General Public License |
| 58 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
| 59 | + |
| 60 | +"""Juju Quickstart charms management.""" |
| 61 | + |
| 62 | +import re |
| 63 | + |
| 64 | + |
| 65 | +# The following regular expressions are the same used in juju-core: see |
| 66 | +# http://bazaar.launchpad.net/~go-bot/juju-core/trunk/view/head:/charm/url.go. |
| 67 | +valid_user = re.compile(r'^[a-z0-9][a-zA-Z0-9+.-]+$').match |
| 68 | +valid_series = re.compile(r'^[a-z]+([a-z-]+[a-z])?$').match |
| 69 | +valid_name = re.compile(r'^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*$').match |
| 70 | + |
| 71 | + |
| 72 | +def parse_url(url): |
| 73 | + """Parse the given charm URL. |
| 74 | + |
| 75 | + Return a tuple containing the charm URL fragments: schema, user, series, |
| 76 | + name and revision. Each fragment is a string except revision (int). |
| 77 | + |
| 78 | + Raise a ValueError with a descriptive message if the given URL is not a |
| 79 | + valid charm URL. |
| 80 | + """ |
| 81 | + # Retrieve the schema. |
| 82 | + try: |
| 83 | + schema, remaining = url.split(':', 1) |
| 84 | + except ValueError: |
| 85 | + raise ValueError('charm URL has no schema: {!r}'.format(url)) |
| 86 | + if schema not in ('cs', 'local'): |
| 87 | + raise ValueError('charm URL has invalid schema: {!r}'.format(schema)) |
| 88 | + # Retrieve the optional user, the series, name and revision. |
| 89 | + parts = remaining.split('/') |
| 90 | + parts_length = len(parts) |
| 91 | + if parts_length == 3: |
| 92 | + user, series, name_revision = parts |
| 93 | + if not user.startswith('~'): |
| 94 | + raise ValueError( |
| 95 | + 'charm URL has invalid user name form: {!r}'.format(user)) |
| 96 | + user = user[1:] |
| 97 | + if not valid_user(user): |
| 98 | + raise ValueError( |
| 99 | + 'charm URL has invalid user name: {!r}'.format(user)) |
| 100 | + if schema == 'local': |
| 101 | + raise ValueError( |
| 102 | + 'local charm URL with user name: {!r}'.format(url)) |
| 103 | + elif parts_length == 2: |
| 104 | + user = '' |
| 105 | + series, name_revision = parts |
| 106 | + else: |
| 107 | + raise ValueError('charm URL has invalid form: {!r}'.format(url)) |
| 108 | + # Validate the series. |
| 109 | + if not valid_series(series): |
| 110 | + raise ValueError('charm URL has invalid series: {!r}'.format(series)) |
| 111 | + # Validate name and revision. |
| 112 | + try: |
| 113 | + name, revision = name_revision.rsplit('-', 1) |
| 114 | + except ValueError: |
| 115 | + raise ValueError( |
| 116 | + 'charm URL has no revision: {!r}'.format(url)) |
| 117 | + if not valid_name(name): |
| 118 | + raise ValueError('charm URL has invalid name: {!r}'.format(name)) |
| 119 | + try: |
| 120 | + revision = int(revision) |
| 121 | + except ValueError: |
| 122 | + raise ValueError( |
| 123 | + 'charm URL has invalid revision: {!r}'.format(revision)) |
| 124 | + return schema, user, series, name, revision |
| 125 | + |
| 126 | + |
| 127 | +class Charm(object): |
| 128 | + """Represent the charm information stored in the charm URL.""" |
| 129 | + |
| 130 | + def __init__(self, schema, user, series, name, revision): |
| 131 | + """Initialize the charm. Receives the URL fragments.""" |
| 132 | + self.schema = schema |
| 133 | + self.user = user |
| 134 | + self.series = series |
| 135 | + self.name = name |
| 136 | + self.revision = int(revision) |
| 137 | + |
| 138 | + @classmethod |
| 139 | + def from_url(cls, url): |
| 140 | + """Given a charm URL, create and return a Charm instance. |
| 141 | + |
| 142 | + Raise a ValueError if the charm URL is not valid. |
| 143 | + """ |
| 144 | + return cls(*parse_url(url)) |
| 145 | + |
| 146 | + def __str__(self): |
| 147 | + """The string representation of a charm is its URL.""" |
| 148 | + return self.url() |
| 149 | + |
| 150 | + def __repr__(self): |
| 151 | + return '<Charm: {}>'.format(str(self)) |
| 152 | + |
| 153 | + def url(self): |
| 154 | + """Return the charm URL.""" |
| 155 | + user_part = '~{}/'.format(self.user) if self.user else '' |
| 156 | + return '{}:{}{}/{}-{}'.format( |
| 157 | + self.schema, user_part, self.series, self.name, self.revision) |
| 158 | + |
| 159 | + def is_local(self): |
| 160 | + """Return True if this is a local charm, False otherwise.""" |
| 161 | + return self.schema == 'local' |
| 162 | |
| 163 | === modified file 'quickstart/manage.py' |
| 164 | --- quickstart/manage.py 2013-11-21 18:41:16 +0000 |
| 165 | +++ quickstart/manage.py 2013-11-22 15:07:41 +0000 |
| 166 | @@ -25,6 +25,7 @@ |
| 167 | import quickstart |
| 168 | from quickstart import ( |
| 169 | app, |
| 170 | + charms, |
| 171 | settings, |
| 172 | utils, |
| 173 | ) |
| 174 | @@ -88,6 +89,41 @@ |
| 175 | options.bundle_id = bundle_id |
| 176 | |
| 177 | |
| 178 | +def _validate_charm_url(options, parser): |
| 179 | + """Validate the provided charm URL option. |
| 180 | + |
| 181 | + Exit with an error if: |
| 182 | + - the URL is not a valid charm URL; |
| 183 | + - the URL represents a local charm; |
| 184 | + - the charm series is not supported; |
| 185 | + - a bundle deployment has been requested but the provided charm does |
| 186 | + not support bundles. |
| 187 | + |
| 188 | + Leave the options namespace untouched. |
| 189 | + """ |
| 190 | + try: |
| 191 | + charm = charms.Charm.from_url(options.charm_url) |
| 192 | + except ValueError as err: |
| 193 | + return parser.error(str(err)) |
| 194 | + if charm.is_local(): |
| 195 | + return parser.error('local charms are not allowed: {}'.format(charm)) |
| 196 | + if charm.series not in settings.JUJU_GUI_SUPPORTED_SERIES: |
| 197 | + return parser.error( |
| 198 | + 'unsupported charm series: {!r}'.format(charm.series)) |
| 199 | + if ( |
| 200 | + # The user requested a bundle deployment. |
| 201 | + options.bundle and |
| 202 | + # This is the official Juju GUI charm. |
| 203 | + charm.name == settings.JUJU_GUI_CHARM_NAME and |
| 204 | + not charm.user and |
| 205 | + # The charm at this revision does not support bundle deployments. |
| 206 | + charm.revision < settings.MINIMUM_CHARM_REVISION_FOR_BUNDLES |
| 207 | + ): |
| 208 | + return parser.error( |
| 209 | + 'bundle deployments not supported by the requested charm ' |
| 210 | + 'revision: {}'.format(charm)) |
| 211 | + |
| 212 | + |
| 213 | def _validate_env(options, parser): |
| 214 | """Validate and process the provided environment related options. |
| 215 | |
| 216 | @@ -210,6 +246,8 @@ |
| 217 | _validate_env(options, parser) |
| 218 | if options.bundle is not None: |
| 219 | _validate_bundle(options, parser) |
| 220 | + if options.charm_url is not None: |
| 221 | + _validate_charm_url(options, parser) |
| 222 | # Set up logging. |
| 223 | _configure_logging(logging.DEBUG if options.debug else logging.INFO) |
| 224 | return options |
| 225 | @@ -235,8 +273,8 @@ |
| 226 | # local provider. |
| 227 | machine = None if options.env_type == 'local' else '0' |
| 228 | unit_name = app.deploy_gui( |
| 229 | - env, settings.JUJU_GUI_NAME, machine, charm_url=options.charm_url, |
| 230 | - check_preexisting=already_bootstrapped) |
| 231 | + env, settings.JUJU_GUI_SERVICE_NAME, machine, |
| 232 | + charm_url=options.charm_url, check_preexisting=already_bootstrapped) |
| 233 | address = app.watch(env, unit_name) |
| 234 | url = 'https://{}'.format(address) |
| 235 | print('url: {}\npassword: {}'.format(url, options.admin_secret)) |
| 236 | |
| 237 | === modified file 'quickstart/settings.py' |
| 238 | --- quickstart/settings.py 2013-11-14 10:52:31 +0000 |
| 239 | +++ quickstart/settings.py 2013-11-22 15:07:41 +0000 |
| 240 | @@ -32,5 +32,14 @@ |
| 241 | # Retrieve the current juju-core home. |
| 242 | JUJU_HOME = os.getenv('JUJU_HOME', '~/.juju') |
| 243 | |
| 244 | +# The name of the Juju GUI charm. |
| 245 | +JUJU_GUI_CHARM_NAME = 'juju-gui' |
| 246 | + |
| 247 | # The name of the Juju GUI service. |
| 248 | -JUJU_GUI_NAME = 'juju-gui' |
| 249 | +JUJU_GUI_SERVICE_NAME = JUJU_GUI_CHARM_NAME |
| 250 | + |
| 251 | +# The set of series supported by the Juju GUI charm. |
| 252 | +JUJU_GUI_SUPPORTED_SERIES = ('precise',) |
| 253 | + |
| 254 | +# The minimum Juju GUI charm revision supporting bundle deployments. |
| 255 | +MINIMUM_CHARM_REVISION_FOR_BUNDLES = 80 |
| 256 | |
| 257 | === modified file 'quickstart/tests/test_app.py' |
| 258 | --- quickstart/tests/test_app.py 2013-11-21 18:41:16 +0000 |
| 259 | +++ quickstart/tests/test_app.py 2013-11-22 15:07:41 +0000 |
| 260 | @@ -61,6 +61,7 @@ |
| 261 | return jujuclient.EnvError({'Error': message}) |
| 262 | |
| 263 | |
| 264 | +@mock_print |
| 265 | class TestEnsureDependencies( |
| 266 | helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase): |
| 267 | |
| 268 | @@ -69,7 +70,7 @@ |
| 269 | app.ensure_dependencies() |
| 270 | return mock_call |
| 271 | |
| 272 | - def test_success_install(self): |
| 273 | + def test_success_install(self, mock_print): |
| 274 | call = self.call_ensure_dependencies( |
| 275 | ( |
| 276 | (127, '', 'no juju'), |
| 277 | @@ -83,14 +84,18 @@ |
| 278 | call.assert_has_calls([ |
| 279 | mock.call('juju', 'version'), |
| 280 | mock.call('lxc-ls'), |
| 281 | - mock.call('sudo', 'apt-add-repository', '-y', |
| 282 | - 'ppa:juju/stable'), |
| 283 | + mock.call('sudo', 'apt-add-repository', '-y', 'ppa:juju/stable'), |
| 284 | mock.call('sudo', 'apt-get', 'update'), |
| 285 | - mock.call('sudo', 'apt-get', 'install', '-y', 'juju-core', |
| 286 | - 'lxc'), |
| 287 | + mock.call('sudo', 'apt-get', 'install', '-y', 'juju-core', 'lxc'), |
| 288 | + ]) |
| 289 | + mock_print.assert_has_calls([ |
| 290 | + mock.call( |
| 291 | + 'The following packages need to be installed: juju-core, lxc'), |
| 292 | + mock.call( |
| 293 | + 'sudo privileges are required for package installation.'), |
| 294 | ]) |
| 295 | |
| 296 | - def test_success_no_install(self): |
| 297 | + def test_success_no_install(self, mock_print): |
| 298 | call = self.call_ensure_dependencies( |
| 299 | [ |
| 300 | (0, '1.16', ''), |
| 301 | @@ -102,7 +107,30 @@ |
| 302 | ) |
| 303 | self.assertEqual(call.call_count, 2) |
| 304 | |
| 305 | - def test_failure(self): |
| 306 | + def test_success_one_install(self, mock_print): |
| 307 | + # One missing installation is correctly handled. |
| 308 | + call = self.call_ensure_dependencies([ |
| 309 | + (0, '1.16', ''), |
| 310 | + (127, '', 'no lxc'), |
| 311 | + (0, 'add repo', ''), |
| 312 | + (0, 'update', ''), |
| 313 | + (0, 'install', ''), |
| 314 | + ]) |
| 315 | + self.assertEqual(call.call_count, 5) |
| 316 | + call.assert_has_calls([ |
| 317 | + mock.call('juju', 'version'), |
| 318 | + mock.call('lxc-ls'), |
| 319 | + mock.call('sudo', 'apt-add-repository', '-y', 'ppa:juju/stable'), |
| 320 | + mock.call('sudo', 'apt-get', 'update'), |
| 321 | + mock.call('sudo', 'apt-get', 'install', '-y', 'lxc'), |
| 322 | + ]) |
| 323 | + mock_print.assert_has_calls([ |
| 324 | + mock.call('The following package needs to be installed: lxc'), |
| 325 | + mock.call( |
| 326 | + 'sudo privileges are required for package installation.'), |
| 327 | + ]) |
| 328 | + |
| 329 | + def test_failure(self, mock_print): |
| 330 | with self.assert_program_exit('install failure'): |
| 331 | call = self.call_ensure_dependencies( |
| 332 | [ |
| 333 | @@ -374,7 +402,7 @@ |
| 334 | ProgramExitTestsMixin, helpers.WatcherDataTestsMixin, |
| 335 | unittest.TestCase): |
| 336 | |
| 337 | - charm_url = 'cs:precise/juju-gui-42' |
| 338 | + charm_url = 'cs:precise/juju-gui-100' |
| 339 | |
| 340 | def make_env(self, unit_name=None, service_data=None, unit_data=None): |
| 341 | """Create and return a mock environment object. |
| 342 | @@ -403,6 +431,44 @@ |
| 343 | mock_get_charm_url = mock.Mock(side_effect=side_effect) |
| 344 | return mock.patch('quickstart.utils.get_charm_url', mock_get_charm_url) |
| 345 | |
| 346 | + def check_provided_charm_url( |
| 347 | + self, charm_url, mock_print, expected_logs=None): |
| 348 | + """Ensure the service is deployed and exposed with the given charm URL. |
| 349 | + |
| 350 | + Also check the expected warnings, if they are provided, are logged. |
| 351 | + """ |
| 352 | + env = self.make_env(unit_name='my-gui/42') |
| 353 | + with helpers.assert_logs(expected_logs or [], level='warn'): |
| 354 | + app.deploy_gui(env, 'my-gui', '0', charm_url=charm_url) |
| 355 | + env.assert_has_calls([ |
| 356 | + mock.call.deploy('my-gui', charm_url, num_units=0), |
| 357 | + mock.call.expose('my-gui'), |
| 358 | + mock.call.add_unit('my-gui', machine_spec='0'), |
| 359 | + ]) |
| 360 | + mock_print.assert_has_calls([ |
| 361 | + mock.call('requesting my-gui deployment'), |
| 362 | + mock.call('charm URL: {}'.format(charm_url)), |
| 363 | + ]) |
| 364 | + |
| 365 | + def check_existing_charm_url( |
| 366 | + self, charm_url, mock_print, expected_logs=None): |
| 367 | + """Ensure the service is correctly found with the given charm URL. |
| 368 | + |
| 369 | + Also check the expected warnings, if they are provided, are logged. |
| 370 | + """ |
| 371 | + service_data = {'CharmURL': charm_url} |
| 372 | + env = self.make_env(unit_name='my-gui/42', service_data=service_data) |
| 373 | + with helpers.assert_logs(expected_logs or [], level='warn'): |
| 374 | + app.deploy_gui(env, 'my-gui', '0', check_preexisting=True) |
| 375 | + env.assert_has_calls([ |
| 376 | + mock.call.get_status(), |
| 377 | + mock.call.add_unit('my-gui', machine_spec='0'), |
| 378 | + ]) |
| 379 | + mock_print.assert_has_calls([ |
| 380 | + mock.call('service my-gui already deployed'), |
| 381 | + mock.call('charm URL: {}'.format(charm_url)), |
| 382 | + ]) |
| 383 | + |
| 384 | def test_deployment(self, mock_print): |
| 385 | # The function correctly deploys and exposes the service, retrieving |
| 386 | # the charm URL from the charmworld API. |
| 387 | @@ -461,22 +527,6 @@ |
| 388 | mock.call('charm URL: {}'.format(settings.DEFAULT_CHARM_URL)), |
| 389 | ]) |
| 390 | |
| 391 | - def test_provided_charm_url(self, mock_print): |
| 392 | - # The function correctly deploys and exposes the service using a user |
| 393 | - # provided Juju GUI charm URL. |
| 394 | - env = self.make_env(unit_name='my-gui/42') |
| 395 | - charm_url = 'cs:~juju-gui/precise/juju-gui-116' |
| 396 | - app.deploy_gui(env, 'my-gui', '0', charm_url=charm_url) |
| 397 | - env.assert_has_calls([ |
| 398 | - mock.call.deploy('my-gui', charm_url, num_units=0), |
| 399 | - mock.call.expose('my-gui'), |
| 400 | - mock.call.add_unit('my-gui', machine_spec='0'), |
| 401 | - ]) |
| 402 | - mock_print.assert_has_calls([ |
| 403 | - mock.call('requesting my-gui deployment'), |
| 404 | - mock.call('charm URL: {}'.format(charm_url)), |
| 405 | - ]) |
| 406 | - |
| 407 | def test_existing_service(self, mock_print): |
| 408 | # The deployment is executed reusing an already deployed service. |
| 409 | env = self.make_env(unit_name='my-gui/42', service_data={}) |
| 410 | @@ -551,6 +601,60 @@ |
| 411 | mock.call.add_unit('my-gui', machine_spec=None), |
| 412 | ]) |
| 413 | |
| 414 | + def test_offical_charm_url_provided(self, mock_print): |
| 415 | + # The function correctly deploys and exposes the service using a user |
| 416 | + # provided revision of the Juju GUI charm URL. |
| 417 | + self.check_provided_charm_url('cs:precise/juju-gui-4242', mock_print) |
| 418 | + |
| 419 | + def test_customized_charm_url_provided(self, mock_print): |
| 420 | + # A customized charm URL is correctly recognized and logged if provided |
| 421 | + # by the user. |
| 422 | + self.check_provided_charm_url( |
| 423 | + 'cs:~juju-gui/precise/juju-gui-42', mock_print, |
| 424 | + expected_logs=['using a customized juju-gui charm']) |
| 425 | + |
| 426 | + def test_outdated_charm_url_provided(self, mock_print): |
| 427 | + # An outdated charm URL is correctly recognized and logged if provided |
| 428 | + # by the user. |
| 429 | + self.check_provided_charm_url( |
| 430 | + 'cs:precise/juju-gui-1', mock_print, |
| 431 | + expected_logs=[ |
| 432 | + 'charm is outdated and may not support bundle deployments']) |
| 433 | + |
| 434 | + def test_unexpected_charm_url_provided(self, mock_print): |
| 435 | + # An unexpected charm URL is correctly recognized and logged if |
| 436 | + # provided by the user. |
| 437 | + self.check_provided_charm_url( |
| 438 | + 'cs:precise/exterminate-the-gui-666', mock_print, |
| 439 | + expected_logs=[ |
| 440 | + 'unexpected URL for the juju-gui charm: ' |
| 441 | + 'the service may not work as expected']) |
| 442 | + |
| 443 | + def test_offical_charm_url_existing(self, mock_print): |
| 444 | + # An existing official charm URL is correctly found. |
| 445 | + self.check_existing_charm_url('cs:precise/juju-gui-4242', mock_print) |
| 446 | + |
| 447 | + def test_customized_charm_url_existing(self, mock_print): |
| 448 | + # An existing customized charm URL is correctly found and logged. |
| 449 | + self.check_existing_charm_url( |
| 450 | + 'cs:~juju-gui/precise/juju-gui-42', mock_print, |
| 451 | + expected_logs=['using a customized juju-gui charm']) |
| 452 | + |
| 453 | + def test_outdated_charm_url_existing(self, mock_print): |
| 454 | + # An existing but outdated charm URL is correctly found and logged. |
| 455 | + self.check_existing_charm_url( |
| 456 | + 'cs:precise/juju-gui-1', mock_print, |
| 457 | + expected_logs=[ |
| 458 | + 'charm is outdated and may not support bundle deployments']) |
| 459 | + |
| 460 | + def test_unexpected_charm_url_existing(self, mock_print): |
| 461 | + # An existing but unexpected charm URL is correctly found and logged. |
| 462 | + self.check_existing_charm_url( |
| 463 | + 'cs:precise/exterminate-the-gui-666', mock_print, |
| 464 | + expected_logs=[ |
| 465 | + 'unexpected URL for the juju-gui charm: ' |
| 466 | + 'the service may not work as expected']) |
| 467 | + |
| 468 | def test_status_error(self, mock_print): |
| 469 | # A ProgramExit is raised if an error occurs in the status API call. |
| 470 | env = self.make_env() |
| 471 | @@ -597,7 +701,7 @@ |
| 472 | with self.assertRaises(ValueError) as context_manager: |
| 473 | app.deploy_gui(env, 'juju-gui', '0') |
| 474 | env.deploy.assert_called_once_with( |
| 475 | - 'juju-gui', 'cs:precise/juju-gui-42', num_units=0) |
| 476 | + 'juju-gui', self.charm_url, num_units=0) |
| 477 | env.expose.assert_called_once_with('juju-gui') |
| 478 | self.assertIs(error, context_manager.exception) |
| 479 | |
| 480 | |
| 481 | === added file 'quickstart/tests/test_charms.py' |
| 482 | --- quickstart/tests/test_charms.py 1970-01-01 00:00:00 +0000 |
| 483 | +++ quickstart/tests/test_charms.py 2013-11-22 15:07:41 +0000 |
| 484 | @@ -0,0 +1,205 @@ |
| 485 | +# This file is part of the Juju Quickstart Plugin, which lets users set up a |
| 486 | +# Juju environment in very few steps (https://launchpad.net/juju-quickstart). |
| 487 | +# Copyright (C) 2013 Canonical Ltd. |
| 488 | +# |
| 489 | +# This program is free software: you can redistribute it and/or modify it under |
| 490 | +# the terms of the GNU Affero General Public License version 3, as published by |
| 491 | +# the Free Software Foundation. |
| 492 | +# |
| 493 | +# This program is distributed in the hope that it will be useful, but WITHOUT |
| 494 | +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
| 495 | +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
| 496 | +# Affero General Public License for more details. |
| 497 | +# |
| 498 | +# You should have received a copy of the GNU Affero General Public License |
| 499 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
| 500 | + |
| 501 | +"""Tests for the Juju Quickstart charms management.""" |
| 502 | + |
| 503 | +import unittest |
| 504 | + |
| 505 | +from quickstart import charms |
| 506 | +from quickstart.tests import helpers |
| 507 | + |
| 508 | + |
| 509 | +class TestParseUrl(helpers.ValueErrorTestsMixin, unittest.TestCase): |
| 510 | + |
| 511 | + def test_no_schema_error(self): |
| 512 | + # A ValueError is raised if the URL schema is missing. |
| 513 | + expected = "charm URL has no schema: 'precise/juju-gui'" |
| 514 | + with self.assert_value_error(expected): |
| 515 | + charms.parse_url('precise/juju-gui') |
| 516 | + |
| 517 | + def test_invalid_schema_error(self): |
| 518 | + # A ValueError is raised if the URL schema is not valid. |
| 519 | + expected = "charm URL has invalid schema: 'http'" |
| 520 | + with self.assert_value_error(expected): |
| 521 | + charms.parse_url('http:precise/juju-gui') |
| 522 | + |
| 523 | + def test_invalid_user_form_error(self): |
| 524 | + # A ValueError is raised if the user form is not valid. |
| 525 | + expected = "charm URL has invalid user name form: 'jean-luc'" |
| 526 | + with self.assert_value_error(expected): |
| 527 | + charms.parse_url('cs:jean-luc/precise/juju-gui') |
| 528 | + |
| 529 | + def test_invalid_user_name_error(self): |
| 530 | + # A ValueError is raised if the user name is not valid. |
| 531 | + expected = "charm URL has invalid user name: 'jean:luc'" |
| 532 | + with self.assert_value_error(expected): |
| 533 | + charms.parse_url('cs:~jean:luc/precise/juju-gui') |
| 534 | + |
| 535 | + def test_local_user_name_error(self): |
| 536 | + # A ValueError is raised if a user is specified on a local charm. |
| 537 | + expected = ( |
| 538 | + 'local charm URL with user name: ' |
| 539 | + "'local:~jean-luc/precise/juju-gui'") |
| 540 | + with self.assert_value_error(expected): |
| 541 | + charms.parse_url('local:~jean-luc/precise/juju-gui') |
| 542 | + |
| 543 | + def test_invalid_form_error(self): |
| 544 | + # A ValueError is raised if the URL is not valid. |
| 545 | + expected = "charm URL has invalid form: 'cs:~user/series/name/what-?'" |
| 546 | + with self.assert_value_error(expected): |
| 547 | + charms.parse_url('cs:~user/series/name/what-?') |
| 548 | + |
| 549 | + def test_invalid_series_error(self): |
| 550 | + # A ValueError is raised if the series is not valid. |
| 551 | + expected = "charm URL has invalid series: 'boo!'" |
| 552 | + with self.assert_value_error(expected): |
| 553 | + charms.parse_url('cs:boo!/juju-gui-42') |
| 554 | + |
| 555 | + def test_no_revision_error(self): |
| 556 | + # A ValueError is raised if the charm revision is missing. |
| 557 | + expected = "charm URL has no revision: 'cs:series/name'" |
| 558 | + with self.assert_value_error(expected): |
| 559 | + charms.parse_url('cs:series/name') |
| 560 | + |
| 561 | + def test_invalid_revision_error(self): |
| 562 | + # A ValueError is raised if the charm revision is not valid. |
| 563 | + expected = "charm URL has invalid revision: 'revision'" |
| 564 | + with self.assert_value_error(expected): |
| 565 | + charms.parse_url('cs:series/name-revision') |
| 566 | + |
| 567 | + def test_invalid_name_error(self): |
| 568 | + # A ValueError is raised if the charm name is not valid. |
| 569 | + expected = "charm URL has invalid name: 'not:valid'" |
| 570 | + with self.assert_value_error(expected): |
| 571 | + charms.parse_url('cs:precise/not:valid-42') |
| 572 | + |
| 573 | + def test_success_with_user(self): |
| 574 | + # A charm URL including the user is correctly parsed. |
| 575 | + schema, user, series, name, revision = charms.parse_url( |
| 576 | + 'cs:~who/precise/juju-gui-42') |
| 577 | + self.assertEqual('cs', schema) |
| 578 | + self.assertEqual('who', user) |
| 579 | + self.assertEqual('precise', series) |
| 580 | + self.assertEqual('juju-gui', name) |
| 581 | + self.assertEqual(42, revision) |
| 582 | + |
| 583 | + def test_success_without_user(self): |
| 584 | + # A charm URL not including the user is correctly parsed. |
| 585 | + schema, user, series, name, revision = charms.parse_url( |
| 586 | + 'cs:trusty/django-1') |
| 587 | + self.assertEqual('cs', schema) |
| 588 | + self.assertEqual('', user) |
| 589 | + self.assertEqual('trusty', series) |
| 590 | + self.assertEqual('django', name) |
| 591 | + self.assertEqual(1, revision) |
| 592 | + |
| 593 | + def test_success_local_charm(self): |
| 594 | + # A local charm URL is correctly parsed. |
| 595 | + schema, user, series, name, revision = charms.parse_url( |
| 596 | + 'local:saucy/wordpress-100') |
| 597 | + self.assertEqual('local', schema) |
| 598 | + self.assertEqual('', user) |
| 599 | + self.assertEqual('saucy', series) |
| 600 | + self.assertEqual('wordpress', name) |
| 601 | + self.assertEqual(100, revision) |
| 602 | + |
| 603 | + |
| 604 | +class TestCharm(helpers.ValueErrorTestsMixin, unittest.TestCase): |
| 605 | + |
| 606 | + def make_charm( |
| 607 | + self, schema='cs', user='myuser', series='precise', |
| 608 | + name='juju-gui', revision=42): |
| 609 | + """Create and return a Charm instance.""" |
| 610 | + return charms.Charm(schema, user, series, name, revision) |
| 611 | + |
| 612 | + def test_attributes(self): |
| 613 | + # All charm attributes are correctly stored. |
| 614 | + charm = self.make_charm() |
| 615 | + self.assertEqual('cs', charm.schema) |
| 616 | + self.assertEqual('myuser', charm.user) |
| 617 | + self.assertEqual('precise', charm.series) |
| 618 | + self.assertEqual('juju-gui', charm.name) |
| 619 | + self.assertEqual(42, charm.revision) |
| 620 | + |
| 621 | + def test_revision_as_string(self): |
| 622 | + # Revision is converted to an int. |
| 623 | + charm = self.make_charm(revision='47') |
| 624 | + self.assertEqual(47, charm.revision) |
| 625 | + |
| 626 | + def test_from_url(self): |
| 627 | + # A Charm can be instantiated from a charm URL. |
| 628 | + charm = charms.Charm.from_url('cs:~who/trusty/django-1') |
| 629 | + self.assertEqual('cs', charm.schema) |
| 630 | + self.assertEqual('who', charm.user) |
| 631 | + self.assertEqual('trusty', charm.series) |
| 632 | + self.assertEqual('django', charm.name) |
| 633 | + self.assertEqual(1, charm.revision) |
| 634 | + |
| 635 | + def test_from_url_without_user(self): |
| 636 | + # Official charm store URLs are properly handled. |
| 637 | + charm = charms.Charm.from_url('cs:saucy/django-123') |
| 638 | + self.assertEqual('cs', charm.schema) |
| 639 | + self.assertEqual('', charm.user) |
| 640 | + self.assertEqual('saucy', charm.series) |
| 641 | + self.assertEqual('django', charm.name) |
| 642 | + self.assertEqual(123, charm.revision) |
| 643 | + |
| 644 | + def test_from_url_local(self): |
| 645 | + # Local charms URLs are properly handled. |
| 646 | + charm = charms.Charm.from_url('local:precise/my-local-charm-42') |
| 647 | + self.assertEqual('local', charm.schema) |
| 648 | + self.assertEqual('', charm.user) |
| 649 | + self.assertEqual('precise', charm.series) |
| 650 | + self.assertEqual('my-local-charm', charm.name) |
| 651 | + self.assertEqual(42, charm.revision) |
| 652 | + |
| 653 | + def test_from_url_error(self): |
| 654 | + # A ValueError is raised by the from_url class method if the provided |
| 655 | + # URL is not a valid charm URL. |
| 656 | + expected = "charm URL has invalid form: 'cs:not-a-charm-url'" |
| 657 | + with self.assert_value_error(expected): |
| 658 | + charms.Charm.from_url('cs:not-a-charm-url') |
| 659 | + |
| 660 | + def test_string(self): |
| 661 | + # The string representation of a charm instance is its URL. |
| 662 | + charm = self.make_charm() |
| 663 | + self.assertEqual('cs:~myuser/precise/juju-gui-42', str(charm)) |
| 664 | + |
| 665 | + def test_repr(self): |
| 666 | + # A charm instance is correctly represented. |
| 667 | + charm = self.make_charm() |
| 668 | + self.assertEqual( |
| 669 | + '<Charm: cs:~myuser/precise/juju-gui-42>', repr(charm)) |
| 670 | + |
| 671 | + def test_charm_store_url(self): |
| 672 | + # A charm store URL is correctly returned. |
| 673 | + charm = self.make_charm(schema='cs') |
| 674 | + self.assertEqual('cs:~myuser/precise/juju-gui-42', charm.url()) |
| 675 | + |
| 676 | + def test_local_url(self): |
| 677 | + # A local charm URL is correctly returned. |
| 678 | + charm = self.make_charm(schema='local', user='') |
| 679 | + self.assertEqual('local:precise/juju-gui-42', charm.url()) |
| 680 | + |
| 681 | + def test_charm_store_charm(self): |
| 682 | + # The is_local method returns False for charm store charms. |
| 683 | + charm = self.make_charm(schema='cs') |
| 684 | + self.assertFalse(charm.is_local()) |
| 685 | + |
| 686 | + def test_local_charm(self): |
| 687 | + # The is_local method returns True for local charms. |
| 688 | + charm = self.make_charm(schema='local') |
| 689 | + self.assertTrue(charm.is_local()) |
| 690 | |
| 691 | === modified file 'quickstart/tests/test_manage.py' |
| 692 | --- quickstart/tests/test_manage.py 2013-11-20 11:40:10 +0000 |
| 693 | +++ quickstart/tests/test_manage.py 2013-11-22 15:07:41 +0000 |
| 694 | @@ -178,6 +178,69 @@ |
| 695 | self.parser.error.assert_called_once_with(expected) |
| 696 | |
| 697 | |
| 698 | +class TestValidateCharmUrl(unittest.TestCase): |
| 699 | + |
| 700 | + def setUp(self): |
| 701 | + self.parser = mock.Mock() |
| 702 | + |
| 703 | + def make_options(self, charm_url, has_bundle=False): |
| 704 | + """Return a mock options object which includes the passed arguments.""" |
| 705 | + options = mock.Mock(charm_url=charm_url, bundle=None) |
| 706 | + if has_bundle: |
| 707 | + options.bundle = 'bundle:~who/django/42/django' |
| 708 | + return options |
| 709 | + |
| 710 | + def test_invalid_url_error(self): |
| 711 | + # A parser error is invoked if the charm URL is not valid. |
| 712 | + options = self.make_options('cs:invalid') |
| 713 | + manage._validate_charm_url(options, self.parser) |
| 714 | + expected = "charm URL has invalid form: 'cs:invalid'" |
| 715 | + self.parser.error.assert_called_once_with(expected) |
| 716 | + |
| 717 | + def test_local_charm_error(self): |
| 718 | + # A parser error is invoked if a local charm is provided. |
| 719 | + options = self.make_options('local:precise/juju-gui-100') |
| 720 | + manage._validate_charm_url(options, self.parser) |
| 721 | + expected = 'local charms are not allowed: local:precise/juju-gui-100' |
| 722 | + self.parser.error.assert_called_once_with(expected) |
| 723 | + |
| 724 | + def test_unsupported_series_error(self): |
| 725 | + # A parser error is invoked if the charm series is not supported. |
| 726 | + options = self.make_options('cs:nosuch/juju-gui-100') |
| 727 | + manage._validate_charm_url(options, self.parser) |
| 728 | + expected = "unsupported charm series: 'nosuch'" |
| 729 | + self.parser.error.assert_called_once_with(expected) |
| 730 | + |
| 731 | + def test_outdated_charm_error(self): |
| 732 | + # A parser error is invoked if a bundle deployment has been requested |
| 733 | + # but the provided charm does not support bundles. |
| 734 | + options = self.make_options('cs:precise/juju-gui-1', has_bundle=True) |
| 735 | + manage._validate_charm_url(options, self.parser) |
| 736 | + expected = ( |
| 737 | + 'bundle deployments not supported by the requested charm ' |
| 738 | + 'revision: cs:precise/juju-gui-1') |
| 739 | + self.parser.error.assert_called_once_with(expected) |
| 740 | + |
| 741 | + def test_outdated_allowed_without_bundles(self): |
| 742 | + # An outdated charm is allowed if no bundles are provided. |
| 743 | + options = self.make_options('cs:precise/juju-gui-1', has_bundle=False) |
| 744 | + manage._validate_charm_url(options, self.parser) |
| 745 | + self.assertFalse(self.parser.error.called) |
| 746 | + |
| 747 | + def test_success(self): |
| 748 | + # The functions completes without error if the charm URL is valid. |
| 749 | + good = ( |
| 750 | + 'cs:precise/juju-gui-100', |
| 751 | + 'cs:~juju-gui/precise/juju-gui-42', |
| 752 | + 'cs:~who/precise/juju-gui-42', |
| 753 | + 'cs:~who/precise/my-juju-gui-42', |
| 754 | + ) |
| 755 | + for charm_url in good: |
| 756 | + options = self.make_options(charm_url) |
| 757 | + manage._validate_charm_url(options, self.parser) |
| 758 | + self.assertFalse(self.parser.error.called, charm_url) |
| 759 | + |
| 760 | + |
| 761 | class TestValidateEnv(helpers.EnvFileTestsMixin, unittest.TestCase): |
| 762 | |
| 763 | def setUp(self): |
| 764 | @@ -316,6 +379,16 @@ |
| 765 | self.assertIsInstance(options, argparse.Namespace) |
| 766 | self.assertIsInstance(parser, argparse.ArgumentParser) |
| 767 | |
| 768 | + @mock.patch('quickstart.manage._validate_charm_url') |
| 769 | + def test_charm_url(self, mock_validate_charm_url): |
| 770 | + # The charm URL validation process is started if a URL is provided. |
| 771 | + self.call_setup( |
| 772 | + ['--gui-charm-url', 'cs:precise/juju-gui-42'], exit_called=False) |
| 773 | + self.assertTrue(mock_validate_charm_url.called) |
| 774 | + options, parser = mock_validate_charm_url.call_args_list[0][0] |
| 775 | + self.assertIsInstance(options, argparse.Namespace) |
| 776 | + self.assertIsInstance(parser, argparse.ArgumentParser) |
| 777 | + |
| 778 | def test_configure_logging(self): |
| 779 | # Logging is properly set up at the info level. |
| 780 | logger = logging.getLogger() |
| 781 | @@ -359,7 +432,7 @@ |
| 782 | mock_app.connect.assert_called_once_with( |
| 783 | mock_app.get_api_url(), options.admin_secret) |
| 784 | mock_app.deploy_gui.assert_called_once_with( |
| 785 | - mock_app.connect(), settings.JUJU_GUI_NAME, '0', |
| 786 | + mock_app.connect(), settings.JUJU_GUI_SERVICE_NAME, '0', |
| 787 | charm_url=options.charm_url, |
| 788 | check_preexisting=mock_app.bootstrap()) |
| 789 | mock_app.watch.assert_called_once_with( |
| 790 | |
| 791 | === modified file 'quickstart/tests/test_utils.py' |
| 792 | --- quickstart/tests/test_utils.py 2013-11-20 20:30:08 +0000 |
| 793 | +++ quickstart/tests/test_utils.py 2013-11-22 15:07:41 +0000 |
| 794 | @@ -78,6 +78,36 @@ |
| 795 | utils.call('echo', 'we are the borg!') |
| 796 | |
| 797 | |
| 798 | +@mock.patch('__builtin__.print', mock.Mock()) |
| 799 | +class TestCheckGuiCharmUrl(unittest.TestCase): |
| 800 | + |
| 801 | + def test_customized(self): |
| 802 | + # A customized charm URL is properly logged. |
| 803 | + expected = 'using a customized juju-gui charm' |
| 804 | + with helpers.assert_logs([expected], level='warn'): |
| 805 | + utils.check_gui_charm_url('cs:~juju-gui/precise/juju-gui-28') |
| 806 | + |
| 807 | + def test_outdated(self): |
| 808 | + # An outdated charm URL is properly logged. |
| 809 | + expected = 'charm is outdated and may not support bundle deployments' |
| 810 | + with helpers.assert_logs([expected], level='warn'): |
| 811 | + utils.check_gui_charm_url('cs:precise/juju-gui-1') |
| 812 | + |
| 813 | + def test_unexpected(self): |
| 814 | + # An unexpected charm URL is properly logged. |
| 815 | + expected = ( |
| 816 | + 'unexpected URL for the juju-gui charm: the service may not work ' |
| 817 | + 'as expected') |
| 818 | + with helpers.assert_logs([expected], level='warn'): |
| 819 | + utils.check_gui_charm_url('cs:precise/another-gui-42') |
| 820 | + |
| 821 | + def test_official(self): |
| 822 | + # No warnings are logged if an up to date charm is passed. |
| 823 | + with mock.patch('logging.warn') as mock_warn: |
| 824 | + utils.check_gui_charm_url('cs:precise/juju-gui-100') |
| 825 | + self.assertFalse(mock_warn.called) |
| 826 | + |
| 827 | + |
| 828 | class TestConvertBundleUrl(helpers.ValueErrorTestsMixin, unittest.TestCase): |
| 829 | |
| 830 | def test_conversion(self): |
| 831 | @@ -338,7 +368,7 @@ |
| 832 | def test_yaml_gui_in_services(self): |
| 833 | # A ValueError is raised if the bundle contains juju-gui. |
| 834 | contents = yaml.safe_dump({ |
| 835 | - 'mybundle': {'services': {'juju-gui': {}}}, |
| 836 | + 'mybundle': {'services': {settings.JUJU_GUI_SERVICE_NAME: {}}}, |
| 837 | }) |
| 838 | expected = 'bundle mybundle contains an instance of juju-gui. ' \ |
| 839 | 'quickstart will install the latest version of the Juju GUI ' \ |
| 840 | |
| 841 | === modified file 'quickstart/utils.py' |
| 842 | --- quickstart/utils.py 2013-11-20 20:30:08 +0000 |
| 843 | +++ quickstart/utils.py 2013-11-22 15:07:41 +0000 |
| 844 | @@ -16,6 +16,7 @@ |
| 845 | |
| 846 | """Juju Quickstart utility functions and classes.""" |
| 847 | |
| 848 | +from __future__ import print_function |
| 849 | import collections |
| 850 | import httplib |
| 851 | import json |
| 852 | @@ -29,7 +30,10 @@ |
| 853 | |
| 854 | import yaml |
| 855 | |
| 856 | -from quickstart import settings |
| 857 | +from quickstart import ( |
| 858 | + charms, |
| 859 | + settings |
| 860 | +) |
| 861 | |
| 862 | # Compile the regular expression used to parse the "juju switch" output. |
| 863 | _juju_switch_expression = re.compile(r'Current environment: "([\w-]+)"\n') |
| 864 | @@ -59,6 +63,26 @@ |
| 865 | return retcode, output, error |
| 866 | |
| 867 | |
| 868 | +def check_gui_charm_url(charm_url): |
| 869 | + """Print (to stdout or to logs) info and warnings about the charm URL.""" |
| 870 | + print('charm URL: {}'.format(charm_url)) |
| 871 | + charm = charms.Charm.from_url(charm_url) |
| 872 | + charm_name = settings.JUJU_GUI_CHARM_NAME |
| 873 | + if charm.name == charm_name: |
| 874 | + if charm.user or charm.is_local(): |
| 875 | + # This is not the official Juju GUI charm. |
| 876 | + logging.warn('using a customized {} charm'.format(charm_name)) |
| 877 | + elif charm.revision < settings.MINIMUM_CHARM_REVISION_FOR_BUNDLES: |
| 878 | + # This is the official Juju GUI charm, but it is outdated. |
| 879 | + logging.warn( |
| 880 | + 'charm is outdated and may not support bundle deployments') |
| 881 | + else: |
| 882 | + # This does not seem to be a Juju GUI charm. |
| 883 | + logging.warn( |
| 884 | + 'unexpected URL for the {} charm: ' |
| 885 | + 'the service may not work as expected'.format(charm_name)) |
| 886 | + |
| 887 | + |
| 888 | def convert_bundle_url(bundle_url): |
| 889 | """Return the equivalent YAML HTTPS location for the fiven bundle URL. |
| 890 | |
| 891 | @@ -189,7 +213,7 @@ |
| 892 | if not bundle_services: |
| 893 | msg = 'bundle {} does not include any services'.format(bundle_name) |
| 894 | raise ValueError(msg) |
| 895 | - if settings.JUJU_GUI_NAME in bundle_services: |
| 896 | + if settings.JUJU_GUI_SERVICE_NAME in bundle_services: |
| 897 | raise ValueError('bundle {} contains an instance of juju-gui. ' |
| 898 | 'quickstart will install the latest version of the ' |
| 899 | 'Juju GUI automatically, please remove juju-gui from ' |
Reviewers: mp+196244_
Message:
Please take a look.
Description:
Improve charm URL handling.
Validate the user provided GUI charm URL.
Also add a missing test for complete coverage
(obsessive-
Tests: `make check`.
QA:
1)
Provide invalid charm URLs, the program
should immediately stop the execution
and exit with an error, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url invalid
.venv/bin/python juju-quickstart --gui-charm-url
local:precise/
.venv/bin/python juju-quickstart --gui-charm-url
http:~juju-
.venv/bin/python juju-quickstart --gui-charm-url cs:precise/
bundle:
.venv/bin/python juju-quickstart --gui-charm-url cs:saucy/
2)
Run the program passing a customized charm URL, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url
cs:~juju-
You should see the "using a customized juju-gui charm" warning
printed during the service deployment step.
Re-execute the command above: quickstart should reuse the
service in the environment and the warning is printed again.
Destroy the environment.
3)
Now manually deploy an outdated version of the GUI charm:
(sudo) juju bootstrap
juju deploy cs:precise/
Run quickstart:
.venv/bin/python juju-quickstart
You should see the "charm is outdated" warning, then quickstart
waits for the outdated GUI to be deployed and ready.
Destroy the environment.
4)
Run quickstart normally:
.venv/bin/python juju-quickstart
The last official GUI charm (cs:precise/
is installed and no warnings are logged.
Destroy the environment.
Done, thank you!
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+637, -39 lines):
A [revision details]
M quickstart/
M quickstart/app.py
A quickstart/
M quickstart/
M quickstart/
M quickstart/
A quickstart/
M quickstart/
M quickstart/
M quickstart/utils.py