Merge lp:~frankban/juju-quickstart/more-cloud-providers into lp:juju-quickstart

[Francesco Banconi]

Add support for OpenStack and Azure.

Tests: `make check`

QA: `.venv/bin/python juju-quickstart -i`
Ensure you can successfully create an openstack/HP cloud
and an azure environment.
If you already subscribed to any of those, please
check everything works ok. If not, no problem,
I already bootstrapped HP Cloud and azure.
Since this is the 1.0 version, please ensure
the environment management works well, and in general
quickstart bootstraps the environments and
deploys the GUI as expected.
Thank you!

https://codereview.appspot.com/52080044/

[Francesco Banconi (frankban)]

Reviewers: mp+201616_code.launchpad.net,

Message:
Please take a look.

Description:
Add support for OpenStack and Azure.

Tests: `make check`

QA: `.venv/bin/python juju-quickstart -i`
Ensure you can successfully create an openstack/HP cloud
and an azure environment.
If you already subscribed to any of those, please
check everything works ok. If not, no problem,
I already bootstrapped HP Cloud and azure.
Since this is the 1.0 version, please ensure
the environment management works well, and in general
quickstart bootstraps the environments and
deploys the GUI as expected.
Thank you!

https://code.launchpad.net/~frankban/juju-quickstart/more-cloud-providers/+merge/201616

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/52080044/

Affected files (+288, -55 lines):
   A [revision details]
   M quickstart/__init__.py
   M quickstart/cli/forms.py
   M quickstart/models/envs.py
   M quickstart/models/fields.py
   M quickstart/tests/cli/test_forms.py
   M quickstart/tests/models/test_envs.py
   M quickstart/tests/models/test_fields.py

[Richard Harding (rharding)]

Code looks good, a couple of comments.

Starting QA.

https://codereview.appspot.com/52080044/diff/1/quickstart/__init__.py
File quickstart/__init__.py (right):

https://codereview.appspot.com/52080044/diff/1/quickstart/__init__.py#newcode25
quickstart/__init__.py:25: VERSION = (1, 0, 0)
wahoo!

https://codereview.appspot.com/52080044/diff/1/quickstart/models/envs.py
File quickstart/models/envs.py (right):

https://codereview.appspot.com/52080044/diff/1/quickstart/models/envs.py#newcode529
quickstart/models/envs.py:529: fields.ChoiceField(
should this be a suggestion field to help users move along faster? Just
suggest a sensible default like US East which I think even things like
the AMZ web ui does? It's a suggestion field in the open stack example
above.

https://codereview.appspot.com/52080044/diff/1/quickstart/tests/cli/test_forms.py
File quickstart/tests/cli/test_forms.py (right):

https://codereview.appspot.com/52080044/diff/1/quickstart/tests/cli/test_forms.py#newcode174
quickstart/tests/cli/test_forms.py:174: class
TestCreateStringWidget(ChoicesTestsMixin, unittest.TestCase):
I think that the suggestion field is its own new class and should have
its own tests vs getting shoed into the String widget tests. This seems
like a path down like the one in the charm where the one suite of tests
contained others that weren't directly related.

https://codereview.appspot.com/52080044/

[Richard Harding (rharding)]

In creating an Azure environment there's a space issue before 'saucy' in
the list of default series.

The dual nature of this is kind of strange. There's a lot of text/links
there. I'd probably leave off the 'left_empty' since that's just
defaulting to precise. Why not just prefill that in and let users change
it?

Do you think we can skip the "Click the choices to auto-fill..." on each
field? Are the underline links a common enough/discoverable? If you
don't have/use a mouse does it help/matter?

The list of providers to create config for says 'openstack' but
everything inside of that references both openstack and HP. Should that
original selection be "openstack (HP Cloud)"?

I don't have openstack creds but looks sane.

I created 3 environments and removed two of them. One was the default.
I'd expect the last one left to be made the default now.

On the main interactive landing page it says "Use the links below to
create new environments." I don't really think of those as links. The
underlined bits in the choice fields are more "link like". I'd suggest
rephrasing this as just "Create new environments:"

Better yet, I'd try to match this up a bit. The main header can just be
"Manage Juju environments" and add a header to the top section as
"Manage existing environments:" and then the lower one could be the
"Create a new environment". I think it would flow and read a bit nicer.

When you select an existing environment the header is "name (type:
local, default). However that same information is listed below. Can it
be simpler and just be "name"?

I created a local environment, set it up, and got the gui loaded up on
it. I then changed it to be the default and selected 'use' think it
would open my existing session. Instead I got a request for my sudo
password, it went through the 'installing ppa...', asking for my ssh
passphrase, and only after that did it open. Can this shortcut to just
open it up and detect/know the other steps aren't necessary?

I removed my last environment and got the 'please create one' which is
nice. The bullet/selection for 'automatically create and bootstrap a
local' does not have a line of space between it and the above content
like the other ones do. I'd propose moving that down one newline.

One other note, removing the environment did not ask or do anything
about destroying it. Now that I've removed my config there's not a good
way for me to shut it down? Should we ask to destroy-environment when
removing a config?

Overall this is really great and nice to use. I'm with-holding the L G T
M because I think the destroy question and the re-opening without asking
might require enough code changes (if there's agreement to make those
changes) to want to re-review.

https://codereview.appspot.com/52080044/

[Madison Scott-Clary (makyo)]

I agree that some of the points Rick brings up - notably destroying an
environment - are worth considering, though I think they may be worth
their own branches. However, if that's the case, then I think we should
withhold the move to 1.0.0 until the destroy confirmation is made, since
that's potentially big. That means, either include that here or push to
another branch with 1.0.0; otherwise, LGTM! Thanks for the work!

https://codereview.appspot.com/52080044/

[lp:~frankban/juju-quickstart/more-cloud-providers]

50. By Francesco Banconi on 2014-01-16

Checkpoint.

51. By Francesco Banconi on 2014-01-16

UI changes as per review.

52. By Francesco Banconi on 2014-01-16

All changes as per review.

53. By Francesco Banconi on 2014-01-16

Checkpoint.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/52080044/diff/1/quickstart/models/envs.py
File quickstart/models/envs.py (right):

https://codereview.appspot.com/52080044/diff/1/quickstart/models/envs.py#newcode529
quickstart/models/envs.py:529: fields.ChoiceField(
On 2014/01/15 19:29:04, rharding wrote:
> should this be a suggestion field to help users move along faster?
Just suggest
> a sensible default like US East which I think even things like the AMZ
web ui
> does? It's a suggestion field in the open stack example above.

Suggestion fields are used to suggest a value while leaving the field
open to arbitrary values. The choice field here means validation is
involved. AFAICT Azure does not use US East as default.

https://codereview.appspot.com/52080044/diff/1/quickstart/tests/cli/test_forms.py
File quickstart/tests/cli/test_forms.py (right):

https://codereview.appspot.com/52080044/diff/1/quickstart/tests/cli/test_forms.py#newcode174
quickstart/tests/cli/test_forms.py:174: class
TestCreateStringWidget(ChoicesTestsMixin, unittest.TestCase):
On 2014/01/15 19:29:04, rharding wrote:
> I think that the suggestion field is its own new class and should have
its own
> tests vs getting shoed into the String widget tests. This seems like a
path down
> like the one in the charm where the one suite of tests contained
others that
> weren't directly related.

I am not sure I understood this comment. The suggestion field itself
already has its own tests in test_fields.py. Here we are just testing
that suggestions are included in the string widget if required.

https://codereview.appspot.com/52080044/

[Francesco Banconi (frankban)]

This is really good stuff Rick and Matthew.
Thank you both for your great reviews!
Please see the inline comments below, and,
if you can, please QA the new changes.

On 2014/01/15 20:51:45, rharding wrote:
> In creating an Azure environment there's a space issue before 'saucy'
in the
> list of default series.

Unfortunately this is due to the fact we are
using a Grid widget to display choices.
This is done so that line wrapping works as
expected on small terminals. This can be better
seen, e.g., in the azure location choices.
There is no easy way to fix this in Urwid.
I agree it's a bit ugly but I also think
we can live with that for 1.0. What do you think?

> The dual nature of this is kind of strange. There's a lot of
text/links there.
> I'd probably leave off the 'left_empty' since that's just defaulting
to precise.
> Why not just prefill that in and let users change it?

For optional fields, rather than pre-filling
values, I'd prefer to leave the field empty
(and consequently expose an option to empty
the field). Defaults can change over time,
and pre-filling with a value is semantically
different than not writing the field at all
in the envs.yaml file. The default series is
a good example of this. Leaving the field
empty means that juju-core, in a few months, will
bootstrap trusty machines. Explicitly selecting
precise avoids this behavior, and we want to
support both needs.

> Do you think we can skip the "Click the choices to auto-fill..." on
each field?
> Are the underline links a common enough/discoverable? If you don't
have/use a
> mouse does it help/matter?

This makes the UI more explicit and has been
requested in previous reviews.

> The list of providers to create config for says 'openstack' but
everything
> inside of that references both openstack and HP. Should that original
selection
> be "openstack (HP Cloud)"?

This is a very good point Rick.
Fixed, now the labels for new environments
are more human readable.

> I don't have openstack creds but looks sane.

Cool.

> I created 3 environments and removed two of them. One was the default.
I'd
> expect the last one left to be made the default now.

Good idea, done.

> On the main interactive landing page it says "Use the links below to
create new
> environments." I don't really think of those as links. The underlined
bits in
> the choice fields are more "link like". I'd suggest rephrasing this as
just
> "Create new environments:"

Done.

> Better yet, I'd try to match this up a bit. The main header can just
be "Manage
> Juju environments" and add a header to the top section as "Manage
existing
> environments:" and then the lower one could be the "Create a new
environment". I
> think it would flow and read a bit nicer.

Done.

> When you select an existing environment the header is "name (type:
local,
> default). However that same information is listed below. Can it be
simpler and
> just be "name"?

I find that summary useful, especially when the
environment is the default one: most of the times
you get all the info you require without having
to read the table below. I'll change it if you are
not convinced.

> I created a local environment, set it up, and got the gui loaded up o...

[Richard Harding (rharding)]

Thanks for the out of band email and updates.

LGTM with the notes that this is 0.9 and 1.0 will have the checking for
catching Juju errors in bringing up the machine as a follow up card.

https://codereview.appspot.com/52080044/

[Francesco Banconi (frankban)]

*** Submitted:

Add support for OpenStack and Azure.

Tests: `make check`

QA: `.venv/bin/python juju-quickstart -i`
Ensure you can successfully create an openstack/HP cloud
and an azure environment.
If you already subscribed to any of those, please
check everything works ok. If not, no problem,
I already bootstrapped HP Cloud and azure.
Since this is the 1.0 version, please ensure
the environment management works well, and in general
quickstart bootstraps the environments and
deploys the GUI as expected.
Thank you!

R=rharding, matthew.scott
CC=
https://codereview.appspot.com/52080044

https://codereview.appspot.com/52080044/

=== modified file 'quickstart/__init__.py'
--- quickstart/__init__.py	2014-01-09 11:04:03 +0000
+++ quickstart/__init__.py	2014-01-16 12:48:07 +0000
@@ -22,7 +22,7 @@
 from __future__ import unicode_literals
 
 
-VERSION = (0, 6, 0)
+VERSION = (1, 0, 0)
 
 
 def get_version():

=== modified file 'quickstart/cli/forms.py'
--- quickstart/cli/forms.py	2014-01-08 12:00:44 +0000
+++ quickstart/cli/forms.py	2014-01-16 12:48:07 +0000
@@ -56,23 +56,34 @@
     ])
 
 
-def _create_choices_widget(choices, required, edit_widget):
-    """Create and return a choices widget.
+def _create_buttons_grid_widget(choices, edit_widget):
+    """Create and return a grid of button widgets.
 
-    The widget displays the given choices for a specific form field. Clicking a
-    choice updates the corresponding edit widget.
+    Buttons are associated with the given choices, and when clicked, udpate
+    the given edit_widget's text.
     """
-    widgets = [urwid.Text('possible values are:')]
     callback = edit_widget.set_edit_text
     buttons = [
         ui.MenuButton(('field button', choice), ui.thunk(callback, choice))
         for choice in choices
     ]
     cell_width = max(button.base_widget.pack()[0] for button in buttons)
-    widgets.append(urwid.GridFlow(buttons, cell_width, 1, 0, 'left'))
+    return urwid.GridFlow(buttons, cell_width, 1, 0, 'left')
+
+
+def _create_choices_widget(choices, required, edit_widget):
+    """Create and return a choices widget.
+
+    The widget displays the given choices for a specific form field. Clicking a
+    choice updates the corresponding edit widget.
+    """
+    widgets = [urwid.Text('possible values are:')]
+    buttons_grid = _create_buttons_grid_widget(choices, edit_widget)
+    widgets.append(buttons_grid)
     if not required:
         button = ui.MenuButton(
-            ('field button', 'left empty'), ui.thunk(callback, ''))
+            ('field button', 'left empty'),
+            ui.thunk(edit_widget.set_edit_text, ''))
         widgets.append(urwid.Columns([
             ('pack', urwid.Text('but this field can also be ')), button,
         ]))
@@ -118,18 +129,23 @@
         # Display the field help message below the edit widget.
         widgets.append(urwid.Text(field.help))
     if not field.readonly:
+        # Can we display suggestions for this field?
+        suggestions = getattr(field, 'suggestions', ())
+        if suggestions:
+            widgets.append(
+                _create_buttons_grid_widget(suggestions, edit_widget))
         # Can the value be automatically generated?
         generate_callable = getattr(field, 'generate', None)
         if generate_callable is not None:
             widgets.append(
                 _create_generate_widget(generate_callable, edit_widget))
-    # If the value must be in a range of choices, display the possible choices
-    # as part of the help message.
-    choices = getattr(field, 'choices', None)
-    if choices is not None:
-        choices_widget = _create_choices_widget(
-            tuple(choices), field.required, edit_widget)
-        widgets.append(choices_widget)
+        # If the value must be in a range of choices, display the possible
+        # choices as part of the help message.
+        choices = getattr(field, 'choices', None)
+        if choices is not None:
+            choices_widget = _create_choices_widget(
+                tuple(choices), field.required, edit_widget)
+            widgets.append(choices_widget)
     if field.default is not None:
         widgets.append(
             urwid.Text('default if not set: {}'.format(field.default)))

=== modified file 'quickstart/cli/views.py'
--- quickstart/cli/views.py	2014-01-09 18:10:40 +0000
+++ quickstart/cli/views.py	2014-01-16 12:48:07 +0000
@@ -172,7 +172,10 @@
 
     if environments:
         title = 'Select an existing Juju environment or create a new one'
-        widgets = []
+        widgets = [
+            urwid.Text(('highlight', 'Manage existing environments:')),
+            urwid.Divider(),
+        ]
     else:
         title = 'No Juju environments already set up: please create one'
         widgets = [
@@ -190,6 +193,7 @@
                 'start your Juju experience in a local environment (LXC), '
                 'just click the link below:'
             ]),
+            urwid.Divider(),
             ui.MenuButton(
                 '\N{BULLET} automatically create and bootstrap a local '
                 'environment', ui.thunk(create_and_start_local_env)),
@@ -202,12 +206,14 @@
     # time consuming.
     focus_position = None
     errors_found = default_found = False
+    existing_widgets_num = len(widgets)
     for position, env_data in enumerate(environments):
         bullet = '\N{BULLET}'
         # Is this environment the default one?
         if env_data['is-default']:
             default_found = True
-            focus_position = position
+            # The first two positions are the section header and the divider.
+            focus_position = position + existing_widgets_num
             bullet = '\N{CHECK MARK}'
         # Is this environment valid?
         env_metadata = envs.get_env_metadata(env_type_db, env_data)
@@ -223,7 +229,7 @@
     widgets.extend([
         urwid.Divider(),
         urwid.Text((
-            'highlight', 'Use the links below to create new environments')),
+            'highlight', 'Create a new environment:')),
         urwid.Divider(),
     ])
     # The Juju GUI can be safely installed in the bootstrap node only if its
@@ -231,11 +237,11 @@
     preferred_series = settings.JUJU_GUI_PREFERRED_SERIES
     widgets.extend([
         ui.MenuButton(
-            ['\N{BULLET} new ', ('highlight', env_type), ' environment'],
+            ['\N{BULLET} new ', ('highlight', label), ' environment'],
             ui.thunk(edit_view, {
                 'type': env_type, 'default-series': preferred_series})
         )
-        for env_type in envs.get_supported_env_types(env_type_db)
+        for env_type, label in envs.get_supported_env_types(env_type_db)
     ])
     # Set up the application status messages.
     status = [' \N{UPWARDS ARROW LEFTWARDS OF DOWNWARDS ARROW} navigate ']

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-01-13 15:46:27 +0000
+++ quickstart/manage.py	2014-01-16 12:48:07 +0000
@@ -430,9 +430,12 @@
     gui_env.close()
     print(
         'done!\n\n'
-        'Run "juju quickstart -e {}" again if you want\n'
+        'Run "juju quickstart -e {env_name}" again if you want\n'
         'to reopen and log in to the GUI browser later.\n'
         'Run "juju quickstart -i" if you want to manage\n'
         'or bootstrap your Juju environments using the\n'
-        'interactive session.'.format(options.env_name)
+        'interactive session.\n'
+        'Run "juju destroy-environment -e {env_name} [-y]"\n'
+        'to destroy the environment you just bootstrapped.'.format(
+            env_name=options.env_name)
     )

=== modified file 'quickstart/models/envs.py'
--- quickstart/models/envs.py	2014-01-10 15:44:36 +0000
+++ quickstart/models/envs.py	2014-01-16 12:48:07 +0000
@@ -341,13 +341,17 @@
     Raise a ValueError if the environment is not present in env_db.
     Without errors, the env_db is modified in place and None is returned.
     """
+    environments = env_db['environments']
     try:
-        del env_db['environments'][env_name]
+        del environments[env_name]
     except KeyError:
         raise ValueError(
             b'the environment named {!r} does not exist'.format(env_name))
     if env_db.get('default') == env_name:
         del env_db['default']
+    # If only one environment remains, set it as default.
+    if len(environments) == 1:
+        env_db['default'] = environments.keys()[0]
 
 
 def get_env_type_db():
@@ -384,6 +388,11 @@
         'eu-west-1', 'sa-east-1',
         'us-east-1', 'us-west-1', 'us-west-2',
     )
+    hp_regions = (
+        'az-1.region-a.geo-1', 'az-2.region-a.geo-1', 'az-3.region-a.geo-1')
+    azure_locations = (
+        'East US', 'West US', 'West Europe', 'North Europe',
+        'Southeast Asia', 'East Asia')
     # Define the env_type_db dictionary: this is done inside this function in
     # order to avoid instantiating fields at import time.
     # This is an ordered dict so that views can expose options to create new
@@ -405,6 +414,7 @@
         },
     })
     env_type_db['ec2'] = {
+        'label': 'Amazon EC2',
         'description': (
             'The ec2 provider enable you to run Juju on the EC2 cloud. '
             'This process requires you to have an Amazon Web Services (AWS) '
@@ -443,7 +453,114 @@
             is_default_field,
         ),
     }
+    env_type_db['openstack'] = {
+        'label': 'OpenStack (or HP Public Cloud)',
+        'description': (
+            'The openstack provider enable you to run Juju on OpenStack '
+            'private and public clouds. Use this also if you want to '
+            'set up Juju on HP Public Cloud. See '
+            'https://juju.ubuntu.com/docs/config-openstack.html and '
+            'https://juju.ubuntu.com/docs/config-hpcloud.html for more '
+            'details on the openstack provider configuration.'
+        ),
+        'fields': (
+            provider_field,
+            name_field,
+            admin_secret_field,
+            default_series_field,
+            fields.BoolField(
+                'use-floating-ip', label='use floating IP', allow_mixed=False,
+                required=True,
+                help='Specifies whether the use of a floating IP address is '
+                     'required to give the nodes a public IP address. '
+                     'Some installations assign public IP addresses by '
+                     'default without requiring a floating IP address.'),
+            fields.AutoGeneratedStringField(
+                'control-bucket', label='control bucket', required=True,
+                help='the globally unique swift bucket name'),
+            fields.SuggestionsStringField(
+                'auth-url', label='authentication URL', required=True,
+                help='The Keystone URL to use in the authentication process. '
+                     'For HP Public Cloud, use the value suggested below:',
+                suggestions=['https://region-a.geo-1.identity.hpcloudsvc.com'
+                             ':35357/v2.0/']),
+            fields.StringField(
+                'tenant-name', label='tenant name', required=True,
+                help='The OpenStack tenant name. For HP Public Cloud, this is '
+                     'listed as the project name on the '
+                     'https://account.hpcloud.com/projects page.'),
+            fields.SuggestionsStringField(
+                'region', label='region', required=True,
+                help='The OpenStack region to use. '
+                     'For HP Public Cloud, use one of the following:',
+                suggestions=hp_regions),
+            fields.ChoiceField(
+                'auth-mode', label='authentication mode', required=False,
+                default='userpass', choices=('userpass', 'keypair'),
+                help='The way Juju authenticates to OpenStack. The userpass '
+                     'authentication requires you to fill in your user name '
+                     'and password. The keypair mode requires access key and '
+                     'secret key to be properly set up. For HP Public Cloud '
+                     'these information can be retrieved on the '
+                     'https://account.hpcloud.com/account/api_keys page.'),
+            fields.StringField(
+                'username', label='user name', required=False,
+                help='the user name to use for the userpass authentication'),
+            fields.PasswordField(
+                'password', label='password', required=False,
+                help='the user name to use for the userpass authentication'),
+            fields.StringField(
+                'access-key', label='access key', required=False,
+                help='the access key to use for the keypair authentication'),
+            fields.PasswordField(
+                'secret-key', label='secret key', required=False,
+                help='the secret key to use for the keypair authentication'),
+            is_default_field,
+        ),
+    }
+    env_type_db['azure'] = {
+        'label': 'Windows Azure',
+        'description': (
+            'The azure provider enable you to run Juju on Windows Azure. '
+            'It requires you to have an Windows Azure account. If you have '
+            'not signed up for one yet, it can obtained at '
+            'http://www.windowsazure.com/. See '
+            'https://juju.ubuntu.com/docs/config-azure.html for more '
+            'details on the azure provider configuration.'
+        ),
+        'fields': (
+            provider_field,
+            name_field,
+            admin_secret_field,
+            default_series_field,
+            fields.ChoiceField(
+                'location', choices=azure_locations, label='location',
+                required=True, help='the region to use'),
+            fields.StringField(
+                'management-subscription-id', required=True,
+                label='management subscription ID',
+                help='this information can be retrieved from '
+                     'https://manage.windowsazure.com (Settings)'),
+            fields.StringField(
+                'management-certificate-path', required=True,
+                label='management certificate path',
+                help='the path to the pem file associated to the certificate '
+                     'uploaded in the Azure management console: '
+                     'https://manage.windowsazure.com '
+                     '(Settings -> Management Certificates)'),
+            fields.StringField(
+                'storage-account-name', required=True,
+                label='storage account name',
+                help='the name you used when creating a storage account in '
+                     'the Azure management console: '
+                     'https://manage.windowsazure.com (Storage). '
+                     'You must create the storage account in the same '
+                     'region/location specified by the location key value.'),
+            is_default_field,
+        ),
+    }
     env_type_db['local'] = {
+        'label': 'local (LXC)',
         'description': (
             'The LXC local provider enables you to run Juju on a single '
             'system like your local computer or a single server. '
@@ -479,8 +596,15 @@
 
 
 def get_supported_env_types(env_type_db):
-    """Return a list of supported provider type names."""
-    return [env_type for env_type in env_type_db if env_type != '__fallback__']
+    """Return a list of supported (provider type, label) tuples.
+
+    Each tuple represents an environment type supported by Quickstart.
+    """
+    return [
+        (env_type, metadata['label'])
+        for env_type, metadata in env_type_db.items()
+        if env_type != '__fallback__'
+    ]
 
 
 def get_env_metadata(env_type_db, env_data):

=== modified file 'quickstart/models/fields.py'
--- quickstart/models/fields.py	2014-01-07 15:41:55 +0000
+++ quickstart/models/fields.py	2014-01-16 12:48:07 +0000
@@ -293,6 +293,15 @@
             )
 
 
+class SuggestionsStringField(StringField):
+    """A string field storing possible value suggestions."""
+
+    def __init__(self, name, suggestions=(), **kwargs):
+        """Initialize the choices field with the given choices."""
+        super(SuggestionsStringField, self).__init__(name, **kwargs)
+        self.suggestions = tuple(suggestions)
+
+
 class AutoGeneratedStringField(StringField):
     """Can automatically generate string values if they are not provided.
 

=== modified file 'quickstart/tests/cli/test_forms.py'
--- quickstart/tests/cli/test_forms.py	2014-01-08 12:00:44 +0000
+++ quickstart/tests/cli/test_forms.py	2014-01-16 12:48:07 +0000
@@ -18,6 +18,7 @@
 
 from __future__ import unicode_literals
 
+import collections
 import unittest
 
 import mock
@@ -85,6 +86,38 @@
         self.assertEqual(expected_choices, obtained_choices)
 
 
+class TestCreateButtonsGridWidget(unittest.TestCase):
+
+    def setUp(self):
+        # Set up a mock edit widget and choices.
+        self.mock_edit_widget = mock.Mock()
+        self.choices = ['Kirk', 'Picard', 'Sisko']
+        self.widget = forms._create_buttons_grid_widget(
+            self.choices, self.mock_edit_widget)
+
+    def get_buttons(self):
+        """Return a list of buttons included in self.widget."""
+        return [button for button, _ in self.widget.contents]
+
+    def test_widget_structure(self):
+        # The resulting widget is a urwid.GridFlow including all the expected
+        # button widgets.
+        self.assertIsInstance(self.widget, urwid.GridFlow)
+        buttons = self.get_buttons()
+        self.assertEqual(len(self.choices), len(buttons))
+        self.assertEqual(
+            self.choices, map(cli_helpers.get_button_caption, buttons))
+
+    def test_button_click(self):
+        # The given edit widget is updated when buttons are clicked.
+        for button in self.get_buttons():
+            choice = cli_helpers.get_button_caption(button)
+            # Click the button.
+            cli_helpers.emit(button)
+            # Ensure the edit widget has been updated accordingly.
+            self.mock_edit_widget.set_edit_text.assert_called_with(choice)
+
+
 class TestCreateChoicesWidget(ChoicesTestsMixin, unittest.TestCase):
 
     def setUp(self):
@@ -141,42 +174,49 @@
 class TestCreateStringWidget(ChoicesTestsMixin, unittest.TestCase):
 
     def inspect_widget(self, widget, field):
-        """Return a list of sub-widgets composing the given string widget.
-
-        The sub-widgets are:
-            - the wrapper edit widget;
-            - the base edit widget;
-            - the caption text widget;
-            - the help widget (or None if not present);
-            - the error text widget (or None if not present);
-            - the generate text widget (or None if not present);
-            - the choices text widget (or None if not present);
-            - the default text widget (or None if not present).
+        """Return a dict of sub-widgets composing the given string widget.
+
+        The dictionary includes the following keys:
+
+            - wrapper: the wrapper edit widget;
+            - edit: the base edit widget;
+            - caption: the caption text widget;
+            - help: the help widget (or None if not present);
+            - error: the error text widget (or None if not present);
+            - suggestions: the suggested values grid (or None if not present);
+            - generate: the generate text widget (or None if not present);
+            - choices: the choices text widget (or None if not present);
+            - default: the default text widget (or None if not present).
         """
-        help = error = generate = choices = default = None
+        widgets = collections.defaultdict(lambda: None)
         # Retrieve the Pile contents ignoring the last Divider widget.
         contents = list(widget.contents)[:-1]
         first_widget = contents.pop(0)[0]
         if isinstance(first_widget, urwid.Text):
             # This is the error message.
-            error = first_widget
-            wrapper = contents.pop(0)[0]
+            widgets.update({
+                'error': first_widget,
+                'wrapper': contents.pop(0)[0]
+            })
         else:
             # The widget has no errors.
-            wrapper = first_widget
-        caption_attrs, edit_attrs = wrapper.base_widget.contents
+            widgets['wrapper'] = first_widget
+        caption_attrs, edit_attrs = widgets['wrapper'].base_widget.contents
+        widgets.update({
+            'edit': edit_attrs[0],
+            'caption': caption_attrs[0],
+        })
         if field.help:
-            help = contents.pop(0)[0]
+            widgets['help'] = contents.pop(0)[0]
+        if hasattr(field, 'suggestions'):
+            widgets['suggestions'] = contents.pop(0)[0]
         if hasattr(field, 'generate'):
-            generate = contents.pop(0)[0]
+            widgets['generate'] = contents.pop(0)[0]
         if hasattr(field, 'choices'):
-            choices = contents.pop(0)[0]
+            widgets['choices'] = contents.pop(0)[0]
         if field.default is not None:
-            default = contents.pop(0)[0]
-        return (
-            wrapper, edit_attrs[0], caption_attrs[0],
-            help, error, generate, choices, default,
-        )
+            widgets['default'] = contents.pop(0)[0]
+        return widgets
 
     def test_widget_structure(self):
         # The widget includes all the information about a field.
@@ -184,28 +224,29 @@
             'first-name', label='first name', help='your first name',
             default='Jean')
         widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
-        wrapper, edit, caption, help, error, generate, choices, default = (
-            self.inspect_widget(widget, field))
+        widgets = self.inspect_widget(widget, field)
         # Since the field is not read-only, the widget is properly enabled.
-        self.assertNotIsInstance(wrapper, urwid.WidgetDisable)
+        self.assertNotIsInstance(widgets['wrapper'], urwid.WidgetDisable)
         # The edit widget is set to the given value.
-        self.assertEqual('Luc', edit.get_edit_text())
+        self.assertEqual('Luc', widgets['edit'].get_edit_text())
         # The caption and help are properly set.
-        self.assertEqual('\N{BULLET} first name: ', caption.text)
-        self.assertEqual('your first name', help.text)
+        self.assertEqual('\N{BULLET} first name: ', widgets['caption'].text)
+        self.assertEqual('your first name', widgets['help'].text)
         # The error is displayed.
-        self.assertEqual('invalid name', error.text)
-        # The field is not able to generate a value, and there are no choices.
-        self.assertIsNone(generate)
-        self.assertIsNone(choices)
+        self.assertEqual('invalid name', widgets['error'].text)
+        # The field is not able to generate a value, and there are no choices
+        # or suggestions.
+        self.assertIsNone(widgets['generate'])
+        self.assertIsNone(widgets['suggestions'])
+        self.assertIsNone(widgets['choices'])
         # The default value is properly displayed.
-        self.assertEqual('default if not set: Jean', default.text)
+        self.assertEqual('default if not set: Jean', widgets['default'].text)
 
     def test_value_getter(self):
         # The returned value getter function returns the current widget value.
         field = fields.StringField('first-name')
         widget, value_getter = forms.create_string_widget(field, 'Luc', None)
-        edit = self.inspect_widget(widget, field)[1]
+        edit = self.inspect_widget(widget, field)['edit']
         self.assertEqual('Luc', value_getter())
         # The value getter is lazy and always returns the current value.
         edit.set_edit_text('Jean-Luc')
@@ -227,15 +268,28 @@
         # The widget is disabled if the field is read-only.
         field = fields.StringField('first-name', readonly=True)
         widget, _ = forms.create_string_widget(field, 'Jean-Luc', None)
-        wrapper = self.inspect_widget(widget, field)[0]
+        wrapper = self.inspect_widget(widget, field)['wrapper']
         self.assertIsInstance(wrapper, urwid.WidgetDisable)
 
+    def test_suggestions(self):
+        # Suggested values, if present, are properly displayed below the edit
+        # widget.
+        field = fields.SuggestionsStringField(
+            'captain', suggestions=('Kirk', 'Picard'))
+        widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
+        suggestions = self.inspect_widget(widget, field)['suggestions']
+        captions = [
+            cli_helpers.get_button_caption(button)
+            for button, attrs in suggestions.contents
+        ]
+        self.assertEqual(['Kirk', 'Picard'], captions)
+
     def test_autogenerated_field(self):
         # The widgets allows for automatically generating field values.
         field = fields.AutoGeneratedStringField('password')
         with mock.patch.object(field, 'generate', lambda: 'auto-generated!'):
             widget, value_getter = forms.create_string_widget(field, '', None)
-        generate = self.inspect_widget(widget, field)[5]
+        generate = self.inspect_widget(widget, field)['generate']
         # The generate button is the first widget in the urwid.Columns.
         generate_button = generate.contents[0][0]
         # Click the generate button, and ensure the value has been generated.
@@ -246,7 +300,7 @@
         # Possible choices are properly displayed below the edit widget.
         field = fields.ChoiceField('captain', choices=('Kirk', 'Picard'))
         widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
-        choices = self.inspect_widget(widget, field)[6]
+        choices = self.inspect_widget(widget, field)['choices']
         self.assert_choices(['Kirk', 'Picard', 'left empty'], choices)
 
     def test_choices_required_field(self):
@@ -255,7 +309,7 @@
         field = fields.ChoiceField(
             'captain', required=True, choices=('Janeway', 'Sisko'))
         widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
-        choices = self.inspect_widget(widget, field)[6]
+        choices = self.inspect_widget(widget, field)['choices']
         self.assert_choices(['Janeway', 'Sisko'], choices)
 
 

=== modified file 'quickstart/tests/cli/test_views.py'
--- quickstart/tests/cli/test_views.py	2014-01-09 16:18:18 +0000
+++ quickstart/tests/cli/test_views.py	2014-01-16 12:48:07 +0000
@@ -200,9 +200,10 @@
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         env_types = envs.get_supported_env_types(self.env_type_db)
-        for env_type, button in zip(env_types, buttons[-len(env_types):]):
-            # The caption includes the environment type.
-            expected_caption = 'new {} environment'.format(env_type)
+        for type_tuple, button in zip(env_types, buttons[-len(env_types):]):
+            env_type, label = type_tuple
+            # The caption includes the environment type label.
+            expected_caption = 'new {} environment'.format(label)
             caption = cli_helpers.get_button_caption(button)
             self.assertIn(expected_caption, caption)
             # When the button is clicked, the edit view is called passing the

=== modified file 'quickstart/tests/models/test_envs.py'
--- quickstart/tests/models/test_envs.py	2014-01-10 15:44:36 +0000
+++ quickstart/tests/models/test_envs.py	2014-01-16 12:48:07 +0000
@@ -584,14 +584,29 @@
 
     def test_default_environment_removal(self):
         # An environment is successfully removed even if it is the default one.
+        environments = self.env_db['environments']
+        environments['a-third-one'] = {'type': 'openstack'}
         envs.remove_env(self.env_db, 'lxc')
-        environments = self.env_db['environments']
         self.assertNotIn('lxc', environments)
         # The other environments are not removed.
         self.assertIn('aws', environments)
-        # The environments database no longer includes a default environment.
+        # Since there are two remaining environments, the environments database
+        # no longer includes a default environment.
         self.assertNotIn('default', self.env_db)
 
+    def test_one_remaining_environment(self):
+        # If, after a removal, only one environment remains, it is
+        # automatically set as default.
+        envs.remove_env(self.env_db, 'lxc')
+        self.assertEqual(1, len(self.env_db['environments']))
+        self.assertEqual('aws', self.env_db['default'])
+
+    def test_remove_all_environments(self):
+        # Removing all the environments results in an empty environments db.
+        for env_name in list(self.env_db['environments'].keys()):
+            envs.remove_env(self.env_db, env_name)
+        self.assertEqual(envs.create_empty_env_db(), self.env_db)
+
     def test_invalid_environment_name(self):
         # A ValueError is raised if the environment is not present in env_db.
         expected = "the environment named u'no-such' does not exist"
@@ -621,6 +636,11 @@
         # provider type key.
         self.assertNotEqual(0, len(self.env_type_db))
         for provider_type, env_metadata in self.env_type_db.items():
+            # Check the label metadata (not present in the fallback provider).
+            if provider_type != '__fallback__':
+                self.assertIn('label', env_metadata, provider_type)
+                self.assertIsInstance(
+                    env_metadata['label'], unicode, provider_type)
             # Check the description metadata.
             self.assertIn('description', env_metadata, provider_type)
             self.assertIsInstance(
@@ -665,18 +685,51 @@
         self.assert_fields(expected, env_metadata)
         self.assert_required_fields(expected_required, env_metadata)
 
+    def test_openstack_environment(self):
+        # The openstack environment metadata includes the expected fields.
+        self.assertIn('openstack', self.env_type_db)
+        env_metadata = self.env_type_db['openstack']
+        expected = [
+            'type', 'name', 'admin-secret', 'default-series',
+            'use-floating-ip', 'control-bucket', 'auth-url', 'tenant-name',
+            'region', 'auth-mode', 'username', 'password', 'access-key',
+            'secret-key', 'is-default']
+        expected_required = [
+            'type', 'name', 'admin-secret', 'use-floating-ip',
+            'control-bucket', 'auth-url', 'tenant-name', 'region',
+            'is-default']
+        self.assert_fields(expected, env_metadata)
+        self.assert_required_fields(expected_required, env_metadata)
+
+    def test_azure_environment(self):
+        # The azure environment metadata includes the expected fields.
+        self.assertIn('azure', self.env_type_db)
+        env_metadata = self.env_type_db['azure']
+        expected = [
+            'type', 'name', 'admin-secret', 'default-series', 'location',
+            'management-subscription-id', 'management-certificate-path',
+            'storage-account-name', 'is-default']
+        expected_required = [
+            'type', 'name', 'admin-secret', 'location',
+            'management-subscription-id', 'management-certificate-path',
+            'storage-account-name', 'is-default']
+        self.assert_fields(expected, env_metadata)
+        self.assert_required_fields(expected_required, env_metadata)
+
 
 class TestGetSupportedEnvTypes(unittest.TestCase):
 
     def test_env_types(self):
         # All the supported env_types but the fallback one are returned.
         env_type_db = envs.get_env_type_db()
-        expected = set(env_type_db)
-        expected.remove('__fallback__')
-        supported_env_types = envs.get_supported_env_types(env_type_db)
-        obtained = set(supported_env_types)
-        self.assertEqual(len(obtained), len(supported_env_types))
-        self.assertEqual(expected, obtained)
+        expected_env_types = [
+            ('ec2', 'Amazon EC2'),
+            ('openstack', 'OpenStack (or HP Public Cloud)'),
+            ('azure', 'Windows Azure'),
+            ('local', 'local (LXC)'),
+        ]
+        obtained_env_types = envs.get_supported_env_types(env_type_db)
+        self.assertEqual(expected_env_types, obtained_env_types)
 
 
 class TestGetEnvMetadata(unittest.TestCase):

=== modified file 'quickstart/tests/models/test_fields.py'
--- quickstart/tests/models/test_fields.py	2014-01-07 15:41:55 +0000
+++ quickstart/tests/models/test_fields.py	2014-01-16 12:48:07 +0000
@@ -457,6 +457,18 @@
             field.validate(None)
 
 
+class TestSuggestionsStringField(TestStringField):
+
+    field_class = fields.SuggestionsStringField
+
+    def test_suggestions(self):
+        # Suggested values are properly stored as a field attribute.
+        suggestions = ('these', 'are', 'the', 'voyages')
+        field = self.field_class(
+            'word', suggestions=suggestions, label='selected word')
+        self.assertEqual(suggestions, field.suggestions)
+
+
 class TestPasswordField(TestStringField):
 
     field_class = fields.PasswordField