Merge lp:~frankban/juju-quickstart/version-handling into lp:juju-quickstart

[Francesco Banconi]

Improve version handling.

Modify the app.ensure_dependencies function
so that it returns the Juju version tuple.
This way we allow for reusing the version
through the app and we can avoid calling
"juju version" multiple times.

Support local environments by installing
the juju-local meta-package in place of
specific packages (e.g. lxc, mongo).
This way we don't have to update quickstart
if core devs introduce new local env
dependencies.

Fix the destroy-environment message displayed
at the end of the process. The -e flag is
included when an old Juju is used.

Tests: `make check`.

QA: no strictly required.
Use .venv/bin/python juju-quickstart, check everything
works well, check the destroy-environment message
at the end of the process.

https://codereview.appspot.com/74010044/

[Francesco Banconi (frankban)]

Reviewers: mp+210395_code.launchpad.net,

Message:
Please take a look.

Description:
Improve version handling.

Modify the app.ensure_dependencies function
so that it returns the Juju version tuple.
This way we allow for reusing the version
through the app and we can avoid calling
"juju version" multiple times.

Support local environments by installing
the juju-local meta-package in place of
specific packages (e.g. lxc, mongo).
This way we don't have to update quickstart
if core devs introduce new local env
dependencies.

Fix the destroy-environment message displayed
at the end of the process. The -e flag is
included when an old Juju is used.

Tests: `make check`.

QA: no strictly required.
Use .venv/bin/python juju-quickstart, check everything
works well, check the destroy-environment message
at the end of the process.

https://code.launchpad.net/~frankban/juju-quickstart/version-handling/+merge/210395

(do not edit description out of merge proposal)

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

Affected files (+161, -159 lines):
   A [revision details]
   M quickstart/__init__.py
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/settings.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

[Richard Harding (rharding)]

LGTM will qa as well.

https://codereview.appspot.com/74010044/diff/1/quickstart/settings.py
File quickstart/settings.py (right):

https://codereview.appspot.com/74010044/diff/1/quickstart/settings.py#newcode34
quickstart/settings.py:34: # The path to the Juju command.
can/should we get this via `which juju`? I guess it shouldn't change.

https://codereview.appspot.com/74010044/

[Richard Harding (rharding)]

QA ok doing a local env post 1.17.2 thanks!

https://codereview.appspot.com/74010044/

[Francesco Banconi (frankban)]

Thank you for the review!

https://codereview.appspot.com/74010044/diff/1/quickstart/settings.py
File quickstart/settings.py (right):

https://codereview.appspot.com/74010044/diff/1/quickstart/settings.py#newcode34
quickstart/settings.py:34: # The path to the Juju command.
On 2014/03/11 13:24:24, rharding wrote:
> can/should we get this via `which juju`? I guess it shouldn't change.

We can but I am not sure we should. We call juju with sudo, and we use
the complete path for that security reason. AFAICT using `which` would
reintroduce the problem ,and at that point it would be easier to just
call `juju`.

https://codereview.appspot.com/74010044/

[Madison Scott-Clary (makyo)]

LGTM, thanks for the clean-up!

https://codereview.appspot.com/74010044/

[Francesco Banconi (frankban)]

*** Submitted:

Improve version handling.

Modify the app.ensure_dependencies function
so that it returns the Juju version tuple.
This way we allow for reusing the version
through the app and we can avoid calling
"juju version" multiple times.

Support local environments by installing
the juju-local meta-package in place of
specific packages (e.g. lxc, mongo).
This way we don't have to update quickstart
if core devs introduce new local env
dependencies.

Fix the destroy-environment message displayed
at the end of the process. The -e flag is
included when an old Juju is used.

Tests: `make check`.

QA: no strictly required.
Use .venv/bin/python juju-quickstart, check everything
works well, check the destroy-environment message
at the end of the process.

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

https://codereview.appspot.com/74010044/

[Francesco Banconi (frankban)]

Thank you!

https://codereview.appspot.com/74010044/

=== modified file 'quickstart/__init__.py'
--- quickstart/__init__.py	2014-02-19 10:55:39 +0000
+++ quickstart/__init__.py	2014-03-11 13:05:34 +0000
@@ -22,7 +22,7 @@
 from __future__ import unicode_literals
 
 
-VERSION = (1, 1, 0)
+VERSION = (1, 1, 1)
 
 
 def get_version():

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2014-03-10 14:40:21 +0000
+++ quickstart/app.py	2014-03-11 13:05:34 +0000
@@ -63,32 +63,49 @@
     If juju is not found in the PATH, then add the juju stable PPA and install
     both juju and lxc.
 
-    Return when juju is available.
-    Raise a ProgramExit if an error occurs installing.
+    Return the Juju version tuple when Juju is available.
+
+    Raise a ProgramExit if an error occurs installing packages or retrieving
+    the Juju version.
     """
     required_packages = []
-    retcode = utils.call('/usr/bin/juju', 'version')[0]
-    if retcode > 0:
-        required_packages.append('juju-core')
-    retcode = utils.call('/usr/bin/lxc-ls')[0]
-    if retcode > 0:
-        # The lxc package is required to create the LXC containers used by
-        # the local provider. When the local provider is enabled, the host
-        # itself is used as bootstrap node. For this reason, the mongodb-server
-        # package from the Juju stable PPA is also required.
-        required_packages.extend(['lxc', 'mongodb-server'])
+    # Check if juju is installed.
+    try:
+        juju_version = utils.get_juju_version()
+    except ValueError:
+        # Juju is not installed or configured properly. To ensure everything
+        # is set up correctly, also install packages required to run
+        # environments using the local provider.
+        required_packages.extend(['juju-core', 'juju-local'])
+        juju_version = None
+    else:
+        # Check if LXC is installed.
+        retcode = utils.call('/usr/bin/lxc-ls')[0]
+        if retcode:
+            # Some packages (e.g. lxc and mongodb-server) are required to
+            # support bootstrapping environments using the local provider.
+            # All those packages are installed as juju-local dependencies.
+            required_packages.append('juju-local')
     if required_packages:
         try:
             utils.add_apt_repository('ppa:juju/stable')
         except OSError as err:
             raise ProgramExit(bytes(err))
-        print('installing the following packages: {} '
-              '(this can take a while)'.format(', '.join(required_packages)))
-        print('sudo privileges will be used for package installation')
+        print('sudo privileges will be used for the installation of \n'
+              'the following packages: {}\n'
+              'this can take a while...'.format(', '.join(required_packages)))
         retcode, _, error = utils.call(
             'sudo', '/usr/bin/apt-get', 'install', '-y', *required_packages)
         if retcode:
             raise ProgramExit(error)
+    # Return the current Juju version.
+    if juju_version is None:
+        # Juju has been just installed, retrieve its version.
+        try:
+            juju_version = utils.get_juju_version()
+        except ValueError as err:
+            raise ProgramExit(bytes(err))
+    return juju_version
 
 
 def ensure_ssh_keys():
@@ -139,27 +156,6 @@
         raise ProgramExit(bytes(err))
 
 
-def bootstrap_requires_sudo(is_local):
-    """Return True if "sudo" is required to bootstrap the Juju environment.
-
-    Return False otherwise.
-    Raise a ProgramExit if any error occurs retrieving the Juju version.
-    """
-    if not is_local:
-        return False
-    # If this is a local environment, notify the user that "sudo" will be
-    # required to bootstrap the application, even in newer Juju versions where
-    # "sudo" is invoked by juju-core itself.
-    print('sudo privileges required to bootstrap the environment')
-    # If the Juju core version is less than 1.17.2 then use sudo for local
-    # deployments.
-    try:
-        major, minor, patch = utils.get_juju_version()
-    except ValueError as err:
-        raise ProgramExit(bytes(err))
-    return (major, minor, patch) < (1, 17, 2)
-
-
 def bootstrap(env_name, requires_sudo=False, debug=False):
     """Bootstrap the Juju environment with the given name.
 
@@ -180,7 +176,7 @@
     Raise a ProgramExit if any error occurs in the bootstrap process.
     """
     already_bootstrapped = False
-    cmd = ['/usr/bin/juju', 'bootstrap', '-e', env_name]
+    cmd = [settings.JUJU_CMD, 'bootstrap', '-e', env_name]
     if requires_sudo:
         cmd.insert(0, 'sudo')
     if debug:
@@ -217,7 +213,7 @@
     timeout = time.time() + (60*10)
     while time.time() < timeout:
         retcode, output, error = utils.call(
-            '/usr/bin/juju', 'status', '-e', env_name, '--format', 'yaml')
+            settings.JUJU_CMD, 'status', '-e', env_name, '--format', 'yaml')
         if retcode:
             continue
         # Ensure the state server is up and the agent is started.
@@ -266,7 +262,7 @@
     Raise a ProgramExit if any error occurs.
     """
     retcode, output, error = utils.call(
-        '/usr/bin/juju', 'api-endpoints', '-e', env_name, '--format', 'json')
+        settings.JUJU_CMD, 'api-endpoints', '-e', env_name, '--format', 'json')
     if retcode:
         raise ProgramExit(error)
     # Assuming there is always at least one API address, grab the first one

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-02-19 10:55:39 +0000
+++ quickstart/manage.py	2014-03-11 13:05:34 +0000
@@ -383,13 +383,22 @@
     """Run the application."""
     print('juju quickstart v{}'.format(version))
     logging.debug('ensuring juju and lxc are installed')
-    app.ensure_dependencies()
+    juju_version = app.ensure_dependencies()
+
     logging.debug('ensuring SSH keys are available')
     app.ensure_ssh_keys()
+
     print('bootstrapping the {} environment (type: {})'.format(
         options.env_name, options.env_type))
     is_local = options.env_type == 'local'
-    requires_sudo = app.bootstrap_requires_sudo(is_local)
+    requires_sudo = False
+    if is_local:
+        # If this is a local environment, notify the user that "sudo" will be
+        # required to bootstrap the application, even in newer Juju versions
+        # where "sudo" is invoked by juju-core itself.
+        print('sudo privileges will be required to bootstrap the environment')
+        # If the Juju version is less than 1.17.2 then use sudo for local envs.
+        requires_sudo = juju_version < (1, 17, 2)
     already_bootstrapped, bsn_series = app.bootstrap(
         options.env_name, requires_sudo=requires_sudo, debug=options.debug)
 
@@ -457,5 +466,5 @@
         'to destroy the environment you just bootstrapped.'.format(
             env_name=options.env_name,
             sudo='sudo ' if requires_sudo else '',
-            eflag='-e ' if requires_sudo else '')
+            eflag='-e ' if juju_version < (1, 17, 2) else '')
     )

=== modified file 'quickstart/settings.py'
--- quickstart/settings.py	2014-02-19 10:55:39 +0000
+++ quickstart/settings.py	2014-03-11 13:05:34 +0000
@@ -31,6 +31,9 @@
 # The quickstart app short description.
 DESCRIPTION = 'set up a Juju environment (including the GUI) in very few steps'
 
+# The path to the Juju command.
+JUJU_CMD = '/usr/bin/juju'
+
 # The possible values for the environments.yaml default-series field.
 JUJU_DEFAULT_SERIES = ('precise', 'quantal', 'raring', 'saucy', 'trusty')
 

=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2014-03-10 13:44:20 +0000
+++ quickstart/tests/test_app.py	2014-03-11 13:05:34 +0000
@@ -68,58 +68,68 @@
     apt_get = '/usr/bin/apt-get'
 
     def call_ensure_dependencies(self, call_effects):
+        """Execute the quickstart.app.ensure_dependencies call.
+
+        The call_effects argument is used to customize the values returned by
+        quickstart.utils.call invocations.
+
+        Return the mock call object and the ensure_dependencies return value.
+        """
         with self.patch_multiple_calls(call_effects) as mock_call:
-            app.ensure_dependencies()
-        return mock_call
+            juju_version = app.ensure_dependencies()
+        return mock_call, juju_version
 
     def test_success_install(self, mock_print):
         # All the missing packages are installed from the PPA.
         side_effects = (
-            (127, '', 'no juju'),  # Check the juju command.
-            (127, '', 'no lxc'),  # Check the lxc-ls command.
+            (127, '', 'no juju'),  # Retrieve the Juju version.
             (0, 'saucy', ''),  # Retrieve the Ubuntu release codename.
             (0, 'install add repo', ''),  # Install add-apt-repository.
             (0, 'add repo', ''),  # Add the juju stable repository.
             (0, 'update', ''),  # Update the repository with new sources.
             (0, 'install', ''),  # Install missing packages.
+            (0, '1.18.0', ''),  # Retrieve the version again.
         )
-        mock_call = self.call_ensure_dependencies(side_effects)
+        mock_call, juju_version = self.call_ensure_dependencies(side_effects)
         self.assertEqual(len(side_effects), mock_call.call_count)
         mock_call.assert_has_calls([
-            mock.call('/usr/bin/juju', 'version'),
-            mock.call('/usr/bin/lxc-ls'),
+            mock.call(settings.JUJU_CMD, 'version'),
             mock.call('lsb_release', '-cs'),
             mock.call('sudo', self.apt_get, 'install', '-y',
                       'software-properties-common'),
             mock.call('sudo', self.add_repository, '-y', 'ppa:juju/stable'),
             mock.call('sudo', self.apt_get, 'update'),
             mock.call('sudo', self.apt_get, 'install', '-y',
-                      'juju-core', 'lxc', 'mongodb-server'),
+                      'juju-core', 'juju-local'),
+            mock.call(settings.JUJU_CMD, 'version'),
         ])
         mock_print.assert_has_calls([
-            mock.call('sudo privileges are required for PPA installation'),
-            mock.call('installing the following packages: juju-core, '
-                      'lxc, mongodb-server (this can take a while)'),
-            mock.call('sudo privileges will be used for package installation'),
+            mock.call('adding the ppa:juju/stable PPA repository'),
+            mock.call('sudo privileges will be required for PPA installation'),
+            mock.call('sudo privileges will be used for the installation of \n'
+                      'the following packages: juju-core, juju-local\n'
+                      'this can take a while...'),
         ])
+        self.assertEqual((1, 18, 0), juju_version)
 
     def test_success_no_install(self, mock_print):
         # There is no need to install packages/PPAs if everything is already
         # set up.
         side_effects = (
-            (0, '1.16', ''),  # Check the juju command.
+            (0, '1.16.2-amd64', ''),  # Check the juju command.
             (0, '', ''),  # Check the lxc-ls command.
             # The remaining call should be ignored.
             (1, '', 'not ignored'),
         )
-        mock_call = self.call_ensure_dependencies(side_effects)
+        mock_call, juju_version = self.call_ensure_dependencies(side_effects)
         self.assertEqual(2, mock_call.call_count)
         self.assertFalse(mock_print.called)
+        self.assertEqual((1, 16, 2), juju_version)
 
     def test_success_partial_install(self, mock_print):
         # One missing installation is correctly handled.
         side_effects = (
-            (0, '1.16', ''),  # Check the juju command.
+            (0, '1.16.42', ''),  # Check the juju command.
             (127, '', 'no lxc'),  # Check the lxc-ls command.
             (0, 'saucy', ''),  # Retrieve the Ubuntu release codename.
             (0, 'install add repo', ''),  # Install add-apt-repository.
@@ -127,44 +137,43 @@
             (0, 'update', ''),  # Update the repository with new sources.
             (0, 'install', ''),  # Install missing packages.
         )
-        mock_call = self.call_ensure_dependencies(side_effects)
+        mock_call, juju_version = self.call_ensure_dependencies(side_effects)
         self.assertEqual(len(side_effects), mock_call.call_count)
         mock_call.assert_has_calls([
-            mock.call('/usr/bin/juju', 'version'),
+            mock.call(settings.JUJU_CMD, 'version'),
             mock.call('/usr/bin/lxc-ls'),
             mock.call('lsb_release', '-cs'),
             mock.call('sudo', self.apt_get, 'install', '-y',
                       'software-properties-common'),
             mock.call('sudo', self.add_repository, '-y', 'ppa:juju/stable'),
             mock.call('sudo', self.apt_get, 'update'),
-            mock.call('sudo', self.apt_get, 'install', '-y',
-                      'lxc', 'mongodb-server'),
+            mock.call('sudo', self.apt_get, 'install', '-y', 'juju-local'),
         ])
         mock_print.assert_has_calls([
-            mock.call('sudo privileges are required for PPA installation'),
-            mock.call('installing the following packages: '
-                      'lxc, mongodb-server (this can take a while)'),
-            mock.call('sudo privileges will be used for package installation'),
+            mock.call('adding the ppa:juju/stable PPA repository'),
+            mock.call('sudo privileges will be required for PPA installation'),
+            mock.call('sudo privileges will be used for the installation of \n'
+                      'the following packages: juju-local\n'
+                      'this can take a while...'),
         ])
+        self.assertEqual((1, 16, 42), juju_version)
 
     def test_add_repository_failure(self, mock_print):
         # A ProgramExit is raised if the PPA is not successfully installed.
         side_effects = (
             (127, '', 'no juju'),  # Check the juju command.
-            (127, '', 'no lxc'),  # Check the lxc-ls command.
             (0, 'saucy', ''),  # Retrieve the Ubuntu release codename.
             (0, 'install add repo', ''),  # Install add-apt-repository.
             (1, '', 'add repo error'),  # Add the juju stable repository.
         )
         with self.assert_program_exit('add repo error'):
-            mock_call = self.call_ensure_dependencies(side_effects)
+            mock_call = self.call_ensure_dependencies(side_effects)[0]
             self.assertEqual(3, mock_call.call_count)
 
     def test_install_failure(self, mock_print):
         # A ProgramExit is raised if the packages installation fails.
         side_effects = (
             (127, '', 'no juju'),  # Check the juju command.
-            (127, '', 'no lxc'),  # Check the lxc-ls command.
             (0, 'saucy', ''),  # Retrieve the Ubuntu release codename.
             (0, 'install add repo', ''),  # Install add-apt-repository.
             (0, 'add repo', ''),  # Add the juju stable repository.
@@ -172,7 +181,23 @@
             (1, '', 'install error'),  # Install missing packages.
         )
         with self.assert_program_exit('install error'):
-            mock_call = self.call_ensure_dependencies(side_effects)
+            mock_call = self.call_ensure_dependencies(side_effects)[0]
+            self.assertEqual(3, mock_call.call_count)
+
+    def test_juju_version_failure(self, mock_print):
+        # A ProgramExit is raised if an error occurs while retrieving the Juju
+        # version after the packages installation.
+        side_effects = (
+            (127, '', 'no juju'),  # Check the juju command.
+            (0, 'saucy', ''),  # Retrieve the Ubuntu release codename.
+            (0, 'install add repo', ''),  # Install add-apt-repository.
+            (0, 'add repo', ''),  # Add the juju stable repository.
+            (0, 'update', ''),  # Update the repository with new sources.
+            (0, 'install', ''),  # Install missing packages.
+            (127, '', 'no juju (again)'),  # Retrieve the Juju version.
+        )
+        with self.assert_program_exit('no juju (again)'):
+            mock_call = self.call_ensure_dependencies(side_effects)[0]
             self.assertEqual(3, mock_call.call_count)
 
 
@@ -311,61 +336,10 @@
 
 
 @helpers.mock_print
-class TestBootstrapRequiresSudo(ProgramExitTestsMixin, unittest.TestCase):
-
-    sudo_message = 'sudo privileges required to bootstrap the environment'
-
-    def patch_get_juju_version(self, return_value):
-        """Patch the quickstart.utils.get_juju_version function."""
-        if isinstance(return_value, Exception):
-            mock_get_juju_version = mock.Mock(side_effect=return_value)
-        else:
-            mock_get_juju_version = mock.Mock(return_value=return_value)
-        return mock.patch(
-            'quickstart.utils.get_juju_version', mock_get_juju_version)
-
-    def test_not_local(self, mock_print):
-        # Sudo privileges are never required for non-local environments.
-        self.assertFalse(app.bootstrap_requires_sudo(False))
-
-    def test_sudo_required(self, mock_print):
-        # Sudo privileges are required if the Juju version is < 1.17.2.
-        versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)]
-        for version in versions:
-            with self.patch_get_juju_version(version):
-                requires_sudo = app.bootstrap_requires_sudo(True)
-            self.assertTrue(requires_sudo, version)
-            # On local environments the "sudo privileges required" message is
-            # always printed.
-            mock_print.assert_called_once_with(self.sudo_message)
-            mock_print.reset_mock()
-
-    def test_sudo_not_required(self, mock_print):
-        # Sudo privileges are not required if the Juju version is >= 1.17.2.
-        versions = [
-            (1, 17, 2), (1, 17, 10), (1, 18, 0), (1, 18, 2), (2, 16, 1)]
-        for version in versions:
-            with self.patch_get_juju_version(version):
-                requires_sudo = app.bootstrap_requires_sudo(True)
-            self.assertFalse(requires_sudo, version)
-            # On local environments the "sudo privileges required" message is
-            # always printed.
-            mock_print.assert_called_once_with(self.sudo_message)
-            mock_print.reset_mock()
-
-    def test_invalid_version(self, mock_print):
-        # A ProgramExit is raised if the Juju version is not valid.
-        with self.patch_get_juju_version(ValueError(b'bad wolf')):
-            with self.assert_program_exit('bad wolf'):
-                app.bootstrap_requires_sudo(True)
-
-
[e-mail]_print
 class TestBootstrap(
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
 
     env_name = 'my-juju-env'
-    juju = '/usr/bin/juju'
     status_message = 'retrieving the environment status'
 
     def make_status_output(self, agent_state, series='hoary'):
@@ -378,7 +352,8 @@
     def make_status_calls(self, number):
         """Return a list containing the given number of status calls."""
         call = mock.call(
-            self.juju, 'status', '-e', self.env_name, '--format', 'yaml')
+            settings.JUJU_CMD, 'status', '-e', self.env_name,
+            '--format', 'yaml')
         return [call for _ in range(number)]
 
     def make_side_effects(self):
@@ -396,7 +371,7 @@
         with self.patch_multiple_calls(side_effects) as mock_call:
             app.bootstrap(self.env_name)
         mock_call.assert_has_calls([
-            mock.call(self.juju, 'bootstrap', '-e', self.env_name),
+            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(5))
 
     def test_success(self, mock_print):
@@ -406,7 +381,7 @@
         self.assertFalse(already_bootstrapped)
         self.assertEqual(series, 'hoary')
         mock_call.assert_has_calls([
-            mock.call(self.juju, 'bootstrap', '-e', self.env_name),
+            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
         mock_print.assert_called_once_with(self.status_message)
 
@@ -418,7 +393,8 @@
         self.assertFalse(already_bootstrapped)
         self.assertEqual(series, 'hoary')
         mock_call.assert_has_calls([
-            mock.call('sudo', self.juju, 'bootstrap', '-e', self.env_name),
+            mock.call(
+                'sudo', settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
         mock_print.assert_called_once_with(self.status_message)
 
@@ -430,7 +406,9 @@
         self.assertFalse(already_bootstrapped)
         self.assertEqual(series, 'hoary')
         mock_call.assert_has_calls([
-            mock.call(self.juju, 'bootstrap', '-e', self.env_name, '--debug'),
+            mock.call(
+                settings.JUJU_CMD, 'bootstrap', '-e', self.env_name,
+                '--debug'),
         ] + self.make_status_calls(1))
 
     def test_already_bootstrapped(self, mock_print):
@@ -445,7 +423,7 @@
         self.assertTrue(already_bootstrapped)
         self.assertEqual(series, 'precise')
         mock_call.assert_has_calls([
-            mock.call(self.juju, 'bootstrap', '-e', self.env_name),
+            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
         existing_message = 'reusing the already bootstrapped {} environment'
         mock_print.assert_has_calls([
@@ -459,7 +437,7 @@
             with self.assert_program_exit('bad wolf'):
                 app.bootstrap(self.env_name)
         mock_call.assert_called_once_with(
-            self.juju, 'bootstrap', '-e', self.env_name),
+            settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
 
     def test_status_retry_error(self, mock_print):
         # Before raising a ProgramExit, the functions tries to call
@@ -520,7 +498,7 @@
             with self.assert_program_exit(expected):
                 app.bootstrap(self.env_name)
         mock_call.assert_has_calls([
-            mock.call(self.juju, 'bootstrap', '-e', self.env_name),
+            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
 
     def test_status_failure(self, mock_print):
@@ -546,7 +524,7 @@
                 with self.assert_program_exit(expected):
                     app.bootstrap(self.env_name)
         mock_call.assert_has_calls([
-            mock.call(self.juju, 'bootstrap', '-e', self.env_name),
+            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(2))
 
 
@@ -582,7 +560,7 @@
             api_url = app.get_api_url(self.env_name)
         self.assertEqual('wss://api.example.com:17070', api_url)
         mock_call.assert_called_once_with(
-            '/usr/bin/juju', 'api-endpoints', '-e', self.env_name,
+            settings.JUJU_CMD, 'api-endpoints', '-e', self.env_name,
             '--format', 'json')
 
     def test_failure(self):
@@ -591,7 +569,7 @@
             with self.assert_program_exit('bad wolf'):
                 app.get_api_url(self.env_name)
         mock_call.assert_called_once_with(
-            '/usr/bin/juju', 'api-endpoints', '-e', self.env_name,
+            settings.JUJU_CMD, 'api-endpoints', '-e', self.env_name,
             '--format', 'json')
 
 

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-02-19 10:55:39 +0000
+++ quickstart/tests/test_manage.py	2014-03-11 13:05:34 +0000
@@ -671,18 +671,17 @@
 
     def test_no_bundle(self, mock_app, mock_open):
         # The application runs correctly if no bundle is provided.
-        token = 'AUTHTOKEN'
-        mock_app.create_auth_token.return_value = token
+        mock_app.ensure_dependencies.return_value = (1, 18, 0)
+        mock_app.bootstrap.return_value = (True, 'precise')
+        mock_app.get_admin_secret = self.mock_get_admin_secret_error
         mock_app.watch.return_value = '1.2.3.4'
-        mock_app.bootstrap.return_value = (True, 'precise')
-        mock_app.get_admin_secret = self.mock_get_admin_secret_error
+        mock_app.create_auth_token.return_value = 'AUTHTOKEN'
         options = self.make_options()
         manage.run(options)
         mock_app.ensure_dependencies.assert_called()
         mock_app.ensure_ssh_keys.assert_called()
         mock_app.bootstrap.assert_called_once_with(
-            options.env_name, requires_sudo=mock_app.bootstrap_requires_sudo(),
-            debug=options.debug)
+            options.env_name, requires_sudo=False, debug=options.debug)
         mock_app.get_api_url.assert_called_once_with(options.env_name)
         mock_app.connect.assert_has_calls([
             mock.call(mock_app.get_api_url(), options.admin_secret),
@@ -698,7 +697,7 @@
             mock_app.connect(), mock_app.deploy_gui())
         mock_app.create_auth_token.assert_called_once_with(mock_app.connect())
         mock_open.assert_called_once_with(
-            'https://{}/?authtoken={}'.format(mock_app.watch(), token))
+            'https://{}/?authtoken={}'.format(mock_app.watch(), 'AUTHTOKEN'))
         self.assertFalse(mock_app.deploy_bundle.called)
 
     def test_no_token(self, mock_app, mock_open):
@@ -724,22 +723,40 @@
     def test_local_provider_no_sudo(self, mock_app, mock_open):
         # The application correctly handles working with local providers with
         # new Juju versions not requiring "sudo" to bootstrap the environment.
+        # Sudo privileges are not required if the Juju version is >= 1.17.2.
         options = self.make_options(env_type='local')
-        mock_app.bootstrap_requires_sudo.return_value = False
+        versions = [
+            (1, 17, 2), (1, 17, 10), (1, 18, 0), (1, 18, 2), (2, 16, 1)]
         mock_app.bootstrap.return_value = (True, 'precise')
-        manage.run(options)
-        mock_app.bootstrap.assert_called_once_with(
-            options.env_name, requires_sudo=False, debug=options.debug)
+        for version in versions:
+            mock_app.ensure_dependencies.return_value = version
+            manage.run(options)
+            mock_app.bootstrap.assert_called_once_with(
+                options.env_name, requires_sudo=False, debug=options.debug)
+            mock_app.bootstrap.reset_mock()
 
-    def test_local_provider_req_sudo(self, mock_app, mock_open):
+    def test_local_provider_requiring_sudo(self, mock_app, mock_open):
         # The application correctly handles working with local providers when
         # Juju requires an external "sudo" call to bootstrap the environment.
+        # Sudo privileges are required if the Juju version is < 1.17.2.
         options = self.make_options(env_type='local')
-        mock_app.bootstrap_requires_sudo.return_value = True
+        versions = [(0, 7, 9), (1, 0, 0), (1, 16, 42), (1, 17, 0), (1, 17, 1)]
+        mock_app.bootstrap.return_value = (True, 'precise')
+        for version in versions:
+            mock_app.ensure_dependencies.return_value = version
+            manage.run(options)
+            mock_app.bootstrap.assert_called_once_with(
+                options.env_name, requires_sudo=True, debug=options.debug)
+            mock_app.bootstrap.reset_mock()
+
+    def test_no_local_no_sudo(self, mock_app, mock_open):
+        # Sudo privileges are never required for non-local environments.
+        options = self.make_options(env_type='ec2')
+        mock_app.ensure_dependencies.return_value = (1, 14, 0)
         mock_app.bootstrap.return_value = (True, 'precise')
         manage.run(options)
         mock_app.bootstrap.assert_called_once_with(
-            options.env_name, requires_sudo=True, debug=options.debug)
+            options.env_name, requires_sudo=False, debug=options.debug)
 
     def test_no_browser(self, mock_app, mock_open):
         # It is possible to avoid opening the GUI in the browser.

=== modified file 'quickstart/tests/test_utils.py'
--- quickstart/tests/test_utils.py	2014-03-10 13:44:20 +0000
+++ quickstart/tests/test_utils.py	2014-03-11 13:05:34 +0000
@@ -93,7 +93,7 @@
         self.assertEqual(2, mock_print.call_count)
         mock_print.assert_has_calls([
             mock.call('adding the {} PPA repository'.format(self.repository)),
-            mock.call('sudo privileges are required for PPA installation'),
+            mock.call('sudo privileges will be required for PPA installation'),
         ])
 
     def test_command_error(self, mock_print):
@@ -668,16 +668,13 @@
         self.assertEqual((1, 17, 1), version)
 
     def test_juju_version_error(self):
-        # If Juju version returns an error this will throw.
-        with self.patch_call(1, 'foo', 'error'):
-            with self.assertRaises(ValueError) as context_manager:
+        # A ValueError is raised if "juju version" exits with an error.
+        with self.patch_call(1, 'foo', 'bad wolf'):
+            with self.assert_value_error('bad wolf'):
                 utils.get_juju_version()
-        self.assertEqual('error', bytes(context_manager.exception))
 
     def test_invalid_version_string(self):
-        # If Juju version returns an invalid id this will throw.
+        # A ValueError is raised if "juju version" outputs an invalid version.
         with self.patch_call(0, '1.17-precise-amd64', ''):
-            with self.assertRaises(ValueError) as context_manager:
+            with self.assert_value_error('invalid version string: 1.17'):
                 utils.get_juju_version()
-        msg = 'invalid version string: 1.17'
-        self.assertEqual(msg, bytes(context_manager.exception))

=== modified file 'quickstart/utils.py'
--- quickstart/utils.py	2014-03-10 13:44:20 +0000
+++ quickstart/utils.py	2014-03-11 13:05:34 +0000
@@ -51,7 +51,7 @@
     Raise an OSError if any error occur in the process.
     """
     print('adding the {} PPA repository'.format(repository))
-    print('sudo privileges are required for PPA installation')
+    print('sudo privileges will be required for PPA installation')
     # The package including add-apt-repository is python-software-properties
     # in precise and software-properties-common after precise.
     add_repository_package = 'software-properties-common'
@@ -314,13 +314,13 @@
 def get_juju_version():
     """Return the current juju-core version.
 
-    Return a (major:int, minor:int, patch:int) tuple, including
-    major, minor and patch version numbers.
+    Return a (major:int, minor:int, patch:int) tuple, including major, minor
+    and patch version numbers.
 
     Raise a ValueError if the "juju version" call exits with an error
     or the returned version is not well formed.
     """
-    retcode, output, error = call('juju', 'version')
+    retcode, output, error = call(settings.JUJU_CMD, 'version')
     if retcode:
         raise ValueError(error.encode('utf-8'))
     version_string = output.split('-')[0]