Handle cases where default-series is problematic.

Handle cases where default-series is problematic.

If the bootstrap node is not the default (currently precise) then it is bad
for deploying the Juju GUI since the charm is only currently available for
precise.

If an environment is already booted to non-precise or the default-series is
set to something other than precise then special care needs to be taken.  All
of the scenarios are summarized in this note:

http://bit.ly/1cXiPIW

The three QA steps will all re-use the following command to invoke Quickstart:

% .venv/bin/python juju-quickstart \
   --gui-charm-url cs:~juju-gui/precise/juju-gui-136 \
   bundle:~bac/muletrain/6/wiki

All of the following use ec2, so:

% juju switch ec2

1) No default-series.

Destroy any existing environment.
Edit ~/.juju/environments.yaml and comment out default-series for ec2.
Run Quickstart as above.

Expect: Three machines (BSN and GUI on 0) all precise.


2) Non-precise default-series

Destroy any existing environment.
Edit ~/.juju/environments.yaml and set 'default-series: raring' for ec2.
Run Quickstart as above.

Expect: Four machines
	0 - bootstrap node, raring
	1 - GUI, precise
	2 & 3, bundle services, precise because that's what their charms are.

Look at the output generated.  You should see

setting the default-series to raring

Also do a
% juju get-env
The default-series should be raring.


3) Existing environment with non-precise.

Destroy any existing environment.
Edit ~/.juju/environments.yaml and set 'default-series: raring' for ec2.
% juju bootstrap
Run Quickstart as above.

Expect: As #2 without the message about setting the default-series.

https://code.launchpad.net/~bac/juju-quickstart/default-series/+merge/197791

(do not edit description out of merge proposal)

[bac, 2013-12-04 19:46:14]

Please take a look.

[bac, 2013-12-04 19:54:03]

Developer notes added.

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode178
quickstart/app.py:178: def set_environment(env_name, **kwargs):
Docstring!

It would say something like
"""Invoke juju set-environment.  Keys in kwargs that contain underscores will be
converted to dashes.
"""

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode254
quickstart/app.py:254: check_preexisting=False):
This line should not have been wrapped.

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py#newcode284
quickstart/manage.py:284: 
This entire section may be unnecessary now.

Since 'juju bootstrap' no equivalent of the 'default-series' being set in the
environment.yaml, the bootstrap now just uses the default, which is sanely
'precise'. 

If there is a default-series set, then the bootstrap does not somehow override
it and it remains in place.  Thus this block is a no-op.

I've left it in for review as I discovered it late and want confirmation.

[gary.poster, 2013-12-04 21:00:39]

LGTM with fully ignorable whining about names and such.

QA good.  When you rip out code from our misunderstanding (sorry again), maybe
we should also rip out the code that does the "setting the default-series to
raring" bit, right?

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode164
quickstart/app.py:164: agent_state = utils.get_agent_state(output)
get_agent_state_from_status ?  Long, I know, but that's what it is.

get_agent_state_from_status_output ?

parse_status_output(output, 'agent_string') ?  And raise NotImplemented if
second arg is not agent_string?

Sigh.  Or leave it. <shrug>

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode178
quickstart/app.py:178: def set_environment(env_name, **kwargs):
On 2013/12/04 19:54:03, bac wrote:
> Docstring!

+1


"""Invoke juju set-environment.

Kwargs are used to set environment values.  Keys in kwargs that contain
underscores are converted to dashes."""

?

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py#newcode284
quickstart/manage.py:284: 
On 2013/12/04 19:54:03, bac wrote:
> This entire section may be unnecessary now.
> 
> Since 'juju bootstrap' no equivalent of the 'default-series' being set in the
> environment.yaml, the bootstrap now just uses the default, which is sanely
> 'precise'. 
> 
> If there is a default-series set, then the bootstrap does not somehow override
> it and it remains in place.  Thus this block is a no-op.
> 
> I've left it in for review as I discovered it late and want confirmation.

From whom? :-) If that's what you found then sorry for misguiding you, but let's
clean this out.

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py#newcode293
quickstart/manage.py:293: bsn_series != settings.JUJU_GUI_PREFERRED_SERIES)
wouldn't this be bsn_series not in settings.JUJU_GUI_SUPPORTED_SERIES?

https://codereview.appspot.com/37320043/diff/1/quickstart/tests/test_utils.py
File quickstart/tests/test_utils.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/tests/test_utils.py...
quickstart/tests/test_utils.py:476: def test_no_bsn_series(self):
I must admit that I have no idea what bsn is, other than the fact that I assume
"s" is series.

[Later] Oh!  Bootstrap Node!

+1 on just spelling it out. :-)

https://codereview.appspot.com/37320043/diff/1/quickstart/utils.py
File quickstart/utils.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/utils.py#newcode225
quickstart/utils.py:225: def parse_status_output(output, keys=None):
Oh, nice.

Maybe use this directly and drop get_agent_state and get_bootstrap_node_series?

I dunno, feel free to ignore me.  But parse_status_output now does what it says,
while the other two are harder to understand.

what about a usage of
get_bootstrap_node_agent_state(parse_status_output(output))
and similar?

I dunno.

[bac, 2013-12-04 22:33:35]

*** Submitted:

Handle cases where default-series is problematic.

If the bootstrap node is not the default (currently precise) then it is bad
for deploying the Juju GUI since the charm is only currently available for
precise.

If an environment is already booted to non-precise or the default-series is
set to something other than precise then special care needs to be taken.  All
of the scenarios are summarized in this note:

http://bit.ly/1cXiPIW

The three QA steps will all re-use the following command to invoke Quickstart:

% .venv/bin/python juju-quickstart \
   --gui-charm-url cs:~juju-gui/precise/juju-gui-136 \
   bundle:~bac/muletrain/6/wiki

All of the following use ec2, so:

% juju switch ec2

1) No default-series.

Destroy any existing environment.
Edit ~/.juju/environments.yaml and comment out default-series for ec2.
Run Quickstart as above.

Expect: Three machines (BSN and GUI on 0) all precise.


2) Non-precise default-series

Destroy any existing environment.
Edit ~/.juju/environments.yaml and set 'default-series: raring' for ec2.
Run Quickstart as above.

Expect: Four machines
	0 - bootstrap node, raring
	1 - GUI, precise
	2 & 3, bundle services, precise because that's what their charms are.

Look at the output generated.  You should see

setting the default-series to raring

Also do a
% juju get-env
The default-series should be raring.


3) Existing environment with non-precise.

Destroy any existing environment.
Edit ~/.juju/environments.yaml and set 'default-series: raring' for ec2.
% juju bootstrap
Run Quickstart as above.

Expect: As #2 without the message about setting the default-series.

R=gary.poster
CC=
https://codereview.appspot.com/37320043

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode164
quickstart/app.py:164: agent_state = utils.get_agent_state(output)
I'm not opposed to making changes.  But I don't think the suggestion really
improves clarity that much. I rarely invoke the tie-goes-to-the-runner rule but
will this time.

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode178
quickstart/app.py:178: def set_environment(env_name, **kwargs):
YAGNI -- deleting the whole method.

https://codereview.appspot.com/37320043/diff/1/quickstart/app.py#newcode254
quickstart/app.py:254: check_preexisting=False):
On 2013/12/04 19:54:03, bac wrote:
> This line should not have been wrapped.

Done.

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py#newcode284
quickstart/manage.py:284: 
On 2013/12/04 21:00:39, gary.poster wrote:
> On 2013/12/04 19:54:03, bac wrote:
> > This entire section may be unnecessary now.
> > 
> > Since 'juju bootstrap' no equivalent of the 'default-series' being set in
the
> > environment.yaml, the bootstrap now just uses the default, which is sanely
> > 'precise'. 
> > 
> > If there is a default-series set, then the bootstrap does not somehow
override
> > it and it remains in place.  Thus this block is a no-op.
> > 
> > I've left it in for review as I discovered it late and want confirmation.
> 
> From whom? :-) If that's what you found then sorry for misguiding you, but
let's
> clean this out.

Done.

https://codereview.appspot.com/37320043/diff/1/quickstart/manage.py#newcode293
quickstart/manage.py:293: bsn_series != settings.JUJU_GUI_PREFERRED_SERIES)
I thought that originally.

But we have lots of ways to talk about the charm.  We try to get one from
charmworld and if that fails we have a default we use.  Each of those is tied to
a series.  

So even if we do have charms for precise and trusty you can't put a trusty charm
on a bootnode running precise.  So this test is more restrictive but I think
more correct.

The fragility is that those other places where we specify the charm must match
this setting.

https://codereview.appspot.com/37320043/diff/1/quickstart/tests/test_utils.py
File quickstart/tests/test_utils.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/tests/test_utils.py...
quickstart/tests/test_utils.py:476: def test_no_bsn_series(self):
On 2013/12/04 21:00:39, gary.poster wrote:
> I must admit that I have no idea what bsn is, other than the fact that I
assume
> "s" is series.
> 
> [Later] Oh!  Bootstrap Node!
> 
> +1 on just spelling it out. :-)

Done.

https://codereview.appspot.com/37320043/diff/1/quickstart/utils.py
File quickstart/utils.py (right):

https://codereview.appspot.com/37320043/diff/1/quickstart/utils.py#newcode225
quickstart/utils.py:225: def parse_status_output(output, keys=None):
I think the helpers are useful.