Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(65)

Issue 90570044: Support trusty environments.

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 2 months ago by frankban
Modified:
5 years, 2 months ago
Reviewers:
mp+216921, j.c.sackett, rharding
Visibility:
Public.

Description

Support trusty environments. Add the ability to deploy the trusty charm. Introduced the concept of multiple supported series for the Juju GUI charm. Split the app.deploy_gui function in two separate function: - check_environment inspects the environment and returns the data required to deploy the GUI; - deploy_gui's only responsibility is to return when the GUI service is deployed/exposed and the unit created. Include the default-series field in the auto-generated local environment. This is the environment that quickstart offers to automatically create when no other environments are found. Also propose "trusty" as the default series when manually creating new environments. Bump version up: while this branch incidentally fixes bug 1306537 [1], the ability to deploy the GUI on trusty can be considered a new feature. My apologies for the long diff. Tests: `make check`. QA: Use quickstart like the following: `.venv/bin/python juju-quickstart [-i]`. You should be able to deploy the trusty GUI charm. If you are on trusty, the trusty charm should be deployed when the default-series field is missing. This must be tested also using the local provider, in which case Juju is currently not able to deploy precise charms when the bootstrap node is trusty (bug 1306537). In general quickstart should deploy the charm series corresponding to the bootstrap node series: so on trusty environments the trusty charm should be installed, on precise environments the precise one. This way, at least when the bootstrap node series is precise or trusty, quickstart is able to add the GUI unit to machine 0. You can test it using, e.g. an ec2 environment. This is true also when providing a custom charm URL, e.g.: `.venv/bin/python juju-quickstart --gui-charm-url cs:~juju-gui/trusty/juju-gui-1` In all the other cases, quickstart uses the trusty charm. You can test this by using quickstart with an ec2 environment having "default-series: saucy": a trusty GUI should be deployed on machine 1. Two final checks: - try to create a new environment with quickstart: the default-series field should be pre-filled with "trusty"; - move temporarily your environments.yaml somewhere else, and let quickstart auto-generate a local environment for you: the deployment process should succeed and the environment should include the "trusty" default series. Thanks a lot for all of this, and sorry for the long QA: this is going to be released in trusty, so your efforts are really appreciated! [1] https://bugs.launchpad.net/juju-core/+bug/1306537 https://code.launchpad.net/~frankban/juju-quickstart/trusty-charm/+merge/216921 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Support trusty environments. #

Total comments: 7

Patch Set 3 : Support trusty environments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+606 lines, -294 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M quickstart/__init__.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
M quickstart/app.py View 1 4 chunks +76 lines, -24 lines 0 comments Download
M quickstart/cli/views.py View 1 chunk +3 lines, -2 lines 0 comments Download
M quickstart/manage.py View 3 chunks +12 lines, -14 lines 0 comments Download
M quickstart/models/envs.py View 2 chunks +8 lines, -2 lines 0 comments Download
M quickstart/settings.py View 2 chunks +18 lines, -13 lines 0 comments Download
M quickstart/tests/cli/test_views.py View 1 chunk +1 line, -1 line 0 comments Download
M quickstart/tests/helpers.py View 2 chunks +23 lines, -10 lines 0 comments Download
M quickstart/tests/models/test_envs.py View 3 chunks +15 lines, -4 lines 0 comments Download
M quickstart/tests/test_app.py View 4 chunks +324 lines, -190 lines 0 comments Download
M quickstart/tests/test_manage.py View 10 chunks +77 lines, -7 lines 0 comments Download
M quickstart/tests/test_utils.py View 5 chunks +23 lines, -13 lines 0 comments Download
M quickstart/utils.py View 2 chunks +23 lines, -13 lines 0 comments Download

Messages

Total messages: 9
frankban
Please take a look.
5 years, 2 months ago (2014-04-23 16:14:37 UTC) #1
j.c.sackett
LGTM, Francesco. I have one comment that is probably more about clearing up my own ...
5 years, 2 months ago (2014-04-24 15:21:27 UTC) #2
frankban
Please take a look. https://codereview.appspot.com/90570044/diff/1/quickstart/app.py File quickstart/app.py (right): https://codereview.appspot.com/90570044/diff/1/quickstart/app.py#newcode387 quickstart/app.py:387: charm_url = service_data['CharmURL'] On 2014/04/24 ...
5 years, 2 months ago (2014-04-24 15:33:54 UTC) #3
j.c.sackett
On 2014/04/24 15:33:54, frankban wrote: > Yeah, basically if service_data is not None it means ...
5 years, 2 months ago (2014-04-24 15:36:13 UTC) #4
rharding
The code looks good. I can't help but feel like we're doing things to make ...
5 years, 2 months ago (2014-04-24 17:33:19 UTC) #5
rharding
QA is ok.
5 years, 2 months ago (2014-04-24 18:52:41 UTC) #6
rharding
LGTM
5 years, 2 months ago (2014-04-24 18:57:27 UTC) #7
frankban
On 2014/04/24 17:33:19, rharding wrote: > The code looks good. I can't help but feel ...
5 years, 2 months ago (2014-04-25 11:19:41 UTC) #8
frankban
5 years, 2 months ago (2014-04-30 18:20:05 UTC) #9
*** Submitted:

Support trusty environments.

Add the ability to deploy the trusty charm.
Introduced the concept of multiple supported
series for the Juju GUI charm.

Split the app.deploy_gui function in two
separate function:
- check_environment inspects the environment
  and returns the data required to deploy the GUI;
- deploy_gui's only responsibility is to
  return when the GUI service is deployed/exposed
  and the unit created.

Include the default-series field in the auto-generated
local environment. This is the environment that
quickstart offers to automatically create when no other
environments are found.

Also propose "trusty" as the default series when manually 
creating new environments.

Bump version up: while this branch 
incidentally fixes bug 1306537 [1],
the ability to deploy the GUI on trusty
can be considered a new feature.

My apologies for the long diff.

Tests: `make check`.

QA:
Use quickstart like the following:
  `.venv/bin/python juju-quickstart [-i]`.

You should be able to deploy the trusty GUI charm.

If you are on trusty, the trusty charm should be deployed
when the default-series field is missing. 
This must be tested also using the local provider, in which case
Juju is currently not able to deploy precise charms when the 
bootstrap node is trusty (bug 1306537).

In general quickstart should deploy the charm series corresponding
to the bootstrap node series: so on trusty environments the trusty
charm should be installed, on precise environments the precise one.

This way, at least when the bootstrap node series is precise or trusty,
quickstart is able to add the GUI unit to machine 0. You can test it
using, e.g. an ec2 environment.

This is true also when providing a custom charm URL, e.g.:
  `.venv/bin/python juju-quickstart --gui-charm-url
cs:~juju-gui/trusty/juju-gui-1`

In all the other cases, quickstart uses the trusty charm. You can test this
by using quickstart with an ec2 environment having "default-series: saucy":
a trusty GUI should be deployed on machine 1.

Two final checks:
- try to create a new environment with quickstart: the default-series
  field should be pre-filled with "trusty";
- move temporarily your environments.yaml somewhere else, and let quickstart
  auto-generate a local environment for you: the deployment process should
  succeed and the environment should include the "trusty" default series.

Thanks a lot for all of this, and sorry for the long QA: this is going to
be released in trusty, so your efforts are really appreciated!

[1] https://bugs.launchpad.net/juju-core/+bug/1306537

R=
CC=
https://codereview.appspot.com/90570044
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b