Merge lp:~bac/juju-quickstart/platform-settings-2 into lp:juju-quickstart

[Brad Crittenden]

Reorganize platform settings.

The first cut of the platform work was a bit unclean with respect to the
dividing lines between the quickstart app code and the parts that can be
re-used as a library. This branch moves things around to re-attain that
separation.

https://codereview.appspot.com/101980050/

[Brad Crittenden (bac)]

Reviewers: mp+221745_code.launchpad.net,

Message:
Please take a look.

Description:
Reorganize platform settings.

The first cut of the platform work was a bit unclean with respect to the
dividing lines between the quickstart app code and the parts that can be
re-used as a library. This branch moves things around to re-attain that
separation.

Francesco's original concerns are raised in the previous code review at:

https://codereview.appspot.com/102870043

https://code.launchpad.net/~bac/juju-quickstart/platform-settings-2/+merge/221745

(do not edit description out of merge proposal)

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

Affected files (+232, -176 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/platform_support.py
   M quickstart/settings.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_platform_support.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

[Brad Crittenden (bac)]

*** Submitted:

Reorganize platform settings.

The first cut of the platform work was a bit unclean with respect to the
dividing lines between the quickstart app code and the parts that can be
re-used as a library. This branch moves things around to re-attain that
separation.

R=redir
CC=
https://codereview.appspot.com/101980050

https://codereview.appspot.com/101980050/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2014-05-29 18:07:12 +0000
+++ quickstart/app.py	2014-06-02 14:34:25 +0000
@@ -58,7 +58,7 @@
         return b'juju-quickstart: error: {}'.format(self.message)
 
 
-def ensure_dependencies(distro_only):
+def ensure_dependencies(distro_only, platform, juju_command):
     """Ensure that Juju and LXC are installed.
 
     If the "juju" command is not found in the PATH, then install and
@@ -77,22 +77,19 @@
     the Juju version.
     """
     try:
-        platform, installer = platform_support.get_platform_installer()
+        installer = platform_support.get_juju_installer(platform)
     except platform_support.UnsupportedOS as err:
         raise ProgramExit(bytes(err))
 
-    if installer is None:
-        raise ProgramExit(b'No installer found for host platform.')
-
     required_packages = []
     # Check if juju is installed.
     try:
-        juju_version = utils.get_juju_version()
+        juju_version = utils.get_juju_version(juju_command)
     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(platform_support.JUJU_PACKAGES.get(platform))
+        required_packages.extend(settings.JUJU_PACKAGES.get(platform))
         juju_version = None
     else:
         if platform_support.supports_local(platform):
@@ -113,7 +110,7 @@
     if juju_version is None:
         # Juju has been just installed, retrieve its version.
         try:
-            juju_version = utils.get_juju_version()
+            juju_version = utils.get_juju_version(juju_command)
         except ValueError as err:
             raise ProgramExit(bytes(err))
     return juju_version
@@ -161,13 +158,12 @@
                 b'\nIf you would like to create the keys yourself,\n'
                 b'please run this command, follow its instructions,\n'
                 b'and then re-run quickstart:\n'
-                b'  ssh-keygen -b 4096 -t rsa'
-            )
+                b'  ssh-keygen -b 4096 -t rsa')
     except OSError as err:
         raise ProgramExit(bytes(err))
 
 
-def bootstrap(env_name, requires_sudo=False, debug=False):
+def bootstrap(env_name, juju_command, requires_sudo=False, debug=False):
     """Bootstrap the Juju environment with the given name.
 
     Do not bootstrap the environment if already bootstrapped.
@@ -187,7 +183,7 @@
     Raise a ProgramExit if any error occurs in the bootstrap process.
     """
     already_bootstrapped = False
-    cmd = [settings.JUJU_CMD, 'bootstrap', '-e', env_name]
+    cmd = [juju_command, 'bootstrap', '-e', env_name]
     if requires_sudo:
         cmd.insert(0, 'sudo')
     if debug:
@@ -224,7 +220,7 @@
     timeout = time.time() + (60*10)
     while time.time() < timeout:
         retcode, output, error = utils.call(
-            settings.JUJU_CMD, 'status', '-e', env_name, '--format', 'yaml')
+            juju_command, '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 ValueError(msg.encode('utf-8'))
 
 
-def get_api_url(env_name):
+def get_api_url(env_name, juju_command):
     """Return a Juju API URL for the given environment name.
 
     Use the Juju CLI in a subprocess in order to retrieve the API addresses.
@@ -274,7 +270,7 @@
     Raise a ProgramExit if any error occurs.
     """
     retcode, output, error = utils.call(
-        settings.JUJU_CMD, 'api-endpoints', '-e', env_name, '--format', 'json')
+        juju_command, '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-05-30 12:26:45 +0000
+++ quickstart/manage.py	2014-06-02 14:34:25 +0000
@@ -33,6 +33,7 @@
 from quickstart import (
     app,
     packaging,
+    platform_support,
     settings,
     utils,
 )
@@ -54,7 +55,7 @@
         parser.exit()
 
 
-def _get_packaging_info(juju_source):
+def _get_packaging_info(juju_source, platform):
     """Return packaging info based on the given juju source.
 
     The juju_source argument can be either "ppa" or "distro".
@@ -65,16 +66,23 @@
         - distro_only_help: the help text for the --distro-only flag;
         - ppa_help: the help text for the --ppa flag.
     """
-    distro_only_help = ('Do not use external sources when installing and '
-                        'setting up Juju')
-    ppa_help = 'Use external sources when installing and setting up Juju'
-    disable_help = '\n(enabled by default, use {} to disable)'
-    if juju_source == 'distro':
+    if platform == settings.LINUX_APT:
+        distro_only_help = ('Do not use external sources when installing and '
+                            'setting up Juju')
+        ppa_help = 'Use external sources when installing and setting up Juju'
+        disable_help = '\n(enabled by default, use {} to disable)'
+        if juju_source == 'distro':
+            distro_only = True
+            distro_only_help += disable_help.format('--ppa')
+        else:
+            distro_only = False
+            ppa_help += disable_help.format('--distro-only')
+    else:
+        # If not LINUX_APT then distro_only and ppa make no sense.  Prevent
+        # the user from seeing them but allow reasonable values in options.
         distro_only = True
-        distro_only_help += disable_help.format('--ppa')
-    else:
-        distro_only = False
-        ppa_help += disable_help.format('--distro-only')
+        distro_only_help = argparse.SUPPRESS
+        ppa_help = argparse.SUPPRESS
     return distro_only, distro_only_help, ppa_help
 
 
@@ -329,6 +337,7 @@
         - env_type: the provider type of the selected Juju environment;
         - interactive: whether to start the interactive session;
         - open_browser: whether the GUI browser must be opened.
+        - platform: The host platform.
 
     The following attributes will also be included in the namespace if a bundle
     deployment is requested:
@@ -340,9 +349,17 @@
 
     Exit with an error if the provided arguments are not valid.
     """
+
+    # Determine the host platform.  This needs to be done early as it affects
+    # the options we present and allows an early failure.
+    try:
+        platform = platform_support.get_platform()
+    except platform_support.UnsupportedOS as err:
+        raise app.ProgramExit(bytes(err))
+
     default_env_name = envs.get_default_env_name()
     default_distro_only, distro_only_help, ppa_help = _get_packaging_info(
-        packaging.JUJU_SOURCE)
+        packaging.JUJU_SOURCE, platform)
     # Define the help message for the --environment option.
     env_help = 'The name of the Juju environment to use'
     if default_env_name is not None:
@@ -401,6 +418,7 @@
     parser.add_argument(
         '--no-browser', action='store_false', dest='open_browser',
         help='Avoid opening the browser to the GUI at the end of the\nprocess')
+
     parser.add_argument(
         '--distro-only', action='store_true', dest='distro_only',
         default=default_distro_only, help=distro_only_help)
@@ -420,6 +438,10 @@
         nargs=0, help="Show program's description and exit")
     # Parse the provided arguments.
     options = parser.parse_args()
+
+    # Add in the platform for convenience.
+    options.platform = platform
+
     # Convert the provided string arguments to unicode.
     _convert_options_to_unicode(options)
     # Validate and process the provided arguments.
@@ -440,8 +462,11 @@
         print('contents loaded for bundle {} (services: {})'.format(
             options.bundle_name, len(options.bundle_services)))
 
+    juju_command = platform_support.get_juju_command(options.platform)
+
     logging.debug('ensuring juju and dependencies are installed')
-    juju_version = app.ensure_dependencies(options.distro_only)
+    juju_version = app.ensure_dependencies(
+        options.distro_only, options.platform, juju_command)
 
     logging.debug('ensuring SSH keys are available')
     app.ensure_ssh_keys()
@@ -456,8 +481,10 @@
         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, bootstrap_node_series = app.bootstrap(
-        options.env_name, requires_sudo=requires_sudo, debug=options.debug)
+        options.env_name, juju_command,
+        requires_sudo=requires_sudo, debug=options.debug)
 
     # Retrieve the admin-secret for the current environment.
     try:
@@ -472,7 +499,7 @@
             raise app.ProgramExit(msg)
 
     print('retrieving the Juju API address')
-    api_url = app.get_api_url(options.env_name)
+    api_url = app.get_api_url(options.env_name, juju_command)
     print('connecting to {}'.format(api_url))
     env = app.connect(api_url, admin_secret)
 

=== modified file 'quickstart/platform_support.py'
--- quickstart/platform_support.py	2014-05-29 20:49:20 +0000
+++ quickstart/platform_support.py	2014-06-02 14:34:25 +0000
@@ -36,12 +36,6 @@
     pass
 
 
-OSX = object()
-LINUX_APT = object()
-LINUX_RPM = object()
-WINDOWS = object()
-
-
 def get_platform():
     """Return the platform of the host.
 
@@ -49,14 +43,14 @@
     """
     system = platform.system()
     if system == 'Darwin':
-        return OSX
+        return settings.OSX
     elif system == 'Windows':
-        return WINDOWS
+        return settings.WINDOWS
     elif system == 'Linux':
         if os.path.isfile('/usr/bin/apt-get'):
-            return LINUX_APT
+            return settings.LINUX_APT
         elif os.path.isfile('/usr/bin/rpm'):
-            return LINUX_RPM
+            return settings.LINUX_RPM
         raise UnsupportedOS(b'{} without apt-get nor rpm'.format(system))
     elif system == '':
         raise UnsupportedOS(b'Unable to determine the OS platform')
@@ -97,35 +91,34 @@
 
 
 INSTALLERS = {
-    LINUX_APT: _installer_apt,
-    OSX: _installer_osx,
-}
-
-
-JUJU_PACKAGES = {
-    LINUX_APT: ('juju-core', 'juju-local'),
-    OSX: ('juju', ),
-}
-
-
-ALTERNATE_JUJU_LOCATIONS = {
-    OSX: '/usr/local/bin/juju',
-}
-
-
-def get_platform_installer():
-    """Return the platorm and the installer for the host.
-
-    Raises UnsupportedOS if the OS is unknown.
-    Returns platform and installer callable.  The installer_callable is None
-    if no installer can be found for the platform.
-    """
-    platform = get_platform()
-    if platform in ALTERNATE_JUJU_LOCATIONS:
-        settings.JUJU_CMD = ALTERNATE_JUJU_LOCATIONS[platform]
-    return platform, INSTALLERS.get(platform)
+    settings.LINUX_APT: _installer_apt,
+    settings.OSX: _installer_osx,
+}
+
+
+def get_juju_command(platform):
+    """Return the path to the Juju command on the given platform.
+
+    If the platform does not have a novel location, the default will be
+    returned.
+    """
+    return settings.JUJU_CMD_PATHS.get(
+        platform,
+        settings.JUJU_CMD_PATHS['default'])
+
+
+def get_juju_installer(platform):
+    """Return the installer for the host platform.
+
+    Returns installer callable.
+    Raises UnsupportedOS if a platform we don't support is detected.
+    """
+    installer = INSTALLERS.get(platform)
+    if installer is None:
+        raise UnsupportedOS(b'No installer found for host platform.')
+    return installer
 
 
 def supports_local(platform):
     """Return True if the platform supports local (LXC) deploys."""
-    return platform in (LINUX_APT, LINUX_RPM)
+    return platform in (settings.LINUX_APT, settings.LINUX_RPM)

=== modified file 'quickstart/settings.py'
--- quickstart/settings.py	2014-04-23 15:53:32 +0000
+++ quickstart/settings.py	2014-06-02 14:34:25 +0000
@@ -21,6 +21,11 @@
 import collections
 import os
 
+# Platforms.
+OSX = object()
+LINUX_APT = object()
+LINUX_RPM = object()
+WINDOWS = object()
 
 # The base charmworld API URL containing information about charms.
 # This URL must be formatted with a series and a charm name.
@@ -41,8 +46,17 @@
 # The URL namespace for bundles in jujucharms.com.
 JUJUCHARMS_BUNDLE_URL = 'https://jujucharms.com/bundle/'
 
-# The path to the Juju command.
-JUJU_CMD = '/usr/bin/juju'
+# The path to the Juju command, based on platform.
+JUJU_CMD_PATHS = {
+    'default': '/usr/bin/juju',
+    OSX: '/usr/local/bin/juju',
+}
+
+# Juju packages to install per platform.
+JUJU_PACKAGES = {
+    LINUX_APT: ('juju-core', 'juju-local'),
+    OSX: ('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-05-29 18:07:12 +0000
+++ quickstart/tests/test_app.py	2014-06-02 14:34:25 +0000
@@ -68,10 +68,11 @@
     add_repository = '/usr/bin/add-apt-repository'
     apt_get = '/usr/bin/apt-get'
     brew = '/usr/local/bin/brew'
+    juju_command = settings.JUJU_CMD_PATHS['default']
 
     def call_ensure_dependencies(
             self, call_effects, distro_only=False,
-            platform=platform_support.LINUX_APT,
+            platform=settings.LINUX_APT,
             platform_installer=platform_support._installer_apt):
         """Execute the quickstart.app.ensure_dependencies call.
 
@@ -81,10 +82,10 @@
         Return the mock call object and the ensure_dependencies return value.
         """
         with self.patch_multiple_calls(call_effects) as mock_call:
-            with mock.patch(
-                    'quickstart.app.platform_support.get_platform_installer',
-                    side_effect=[(platform, platform_installer)]):
-                juju_version = app.ensure_dependencies(distro_only)
+            path = 'quickstart.app.platform_support.get_juju_installer'
+            with mock.patch(path, side_effect=[platform_installer]):
+                juju_version = app.ensure_dependencies(
+                    distro_only, platform, self.juju_command)
         return mock_call, juju_version
 
     def test_success_install_apt(self, mock_print):
@@ -101,7 +102,7 @@
         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(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
             mock.call('lsb_release', '-cs'),
             mock.call('sudo', self.apt_get, 'install', '-y',
                       'software-properties-common'),
@@ -109,7 +110,7 @@
             mock.call('sudo', self.apt_get, 'update'),
             mock.call('sudo', self.apt_get, 'install', '-y',
                       'juju-core', 'juju-local'),
-            mock.call(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
         ])
         mock_print.assert_has_calls([
             mock.call('adding the ppa:juju/stable PPA repository'),
@@ -128,37 +129,27 @@
             (0, '1.18.0', ''),  # Retrieve the version again.
         )
         mock_call, juju_version = self.call_ensure_dependencies(
-            side_effects, platform=platform_support.OSX,
+            side_effects, platform=settings.OSX,
             platform_installer=platform_support._installer_osx)
         self.assertEqual(len(side_effects), mock_call.call_count)
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
             mock.call(self.brew, 'install', 'juju'),
-            mock.call(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
         ])
         mock_print.assert_has_calls([
             mock.call('Installing the following packages: juju\n'),
         ])
         self.assertEqual((1, 18, 0), juju_version)
 
-    def test_unsupported_os(self, mock_print):
-        # get_platform_installer raises UnsupportedOS, causing ProgramExit.
-        path = 'quickstart.app.platform_support.get_platform_installer'
-        err = platform_support.UnsupportedOS('Unknown TRS-80')
-        with mock.patch(path, side_effect=err):
-            with self.assertRaises(app.ProgramExit) as ctx:
-                app.ensure_dependencies(False)
-            expected = b'juju-quickstart: error: Unknown TRS-80'
-            self.assertEqual(expected, bytes(ctx.exception))
-
     def test_no_installer(self, mock_print):
         # If no installer is found a ProgramExit is raised.
-        path = 'quickstart.app.platform_support.get_platform_installer'
-        with mock.patch(path, side_effect=[('unknown', None)]):
+        path = 'quickstart.app.platform_support.get_juju_installer'
+        err = platform_support.UnsupportedOS('unknown')
+        with mock.patch(path, side_effect=err):
             with self.assertRaises(app.ProgramExit) as ctx:
-                app.ensure_dependencies(False)
-            expected = (b'juju-quickstart: error: '
-                        'No installer found for host platform.')
+                app.ensure_dependencies(False, None, None)
+            expected = (b'juju-quickstart: error: unknown')
             self.assertEqual(expected, bytes(ctx.exception))
 
     def test_distro_only_install(self, mock_print):
@@ -172,10 +163,10 @@
             side_effects, distro_only=True)
         self.assertEqual(len(side_effects), mock_call.call_count)
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
             mock.call('sudo', self.apt_get, 'install', '-y',
                       'juju-core', 'juju-local'),
-            mock.call(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
         ])
         mock_print.assert_called_once_with(
             'sudo privileges will be used for the installation of \n'
@@ -211,7 +202,7 @@
         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(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
             mock.call('/usr/bin/lxc-ls'),
             mock.call('lsb_release', '-cs'),
             mock.call('sudo', self.apt_get, 'install', '-y',
@@ -241,7 +232,7 @@
             side_effects, distro_only=True)
         self.assertEqual(len(side_effects), mock_call.call_count)
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'version'),
+            mock.call(self.juju_command, 'version'),
             mock.call('/usr/bin/lxc-ls'),
             mock.call('sudo', self.apt_get, 'install', '-y', 'juju-local'),
         ])
@@ -286,7 +277,7 @@
         with self.assert_program_exit('install error'):
             mock_call = self.call_ensure_dependencies(
                 side_effects,
-                platform=platform_support.OSX,
+                platform=settings.OSX,
                 platform_installer=platform_support._installer_osx)[0]
             self.assertEqual(2, mock_call.call_count)
 
@@ -447,6 +438,7 @@
 
     env_name = 'my-juju-env'
     status_message = 'retrieving the environment status'
+    juju_command = settings.JUJU_CMD_PATHS['default']
 
     def make_status_output(self, agent_state, series='hoary'):
         """Create and return a YAML status output."""
@@ -458,7 +450,7 @@
     def make_status_calls(self, number):
         """Return a list containing the given number of status calls."""
         call = mock.call(
-            settings.JUJU_CMD, 'status', '-e', self.env_name,
+            self.juju_command, 'status', '-e', self.env_name,
             '--format', 'yaml')
         return [call for _ in range(number)]
 
@@ -475,19 +467,20 @@
         Receive the list of side effects the mock status call will return.
         """
         with self.patch_multiple_calls(side_effects) as mock_call:
-            app.bootstrap(self.env_name)
+            app.bootstrap(self.env_name, self.juju_command)
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+            mock.call(self.juju_command, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(5))
 
     def test_success(self, mock_print):
         # The environment is successfully bootstrapped.
         with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
-            already_bootstrapped, series = app.bootstrap(self.env_name)
+            already_bootstrapped, series = app.bootstrap(
+                self.env_name, self.juju_command)
         self.assertFalse(already_bootstrapped)
         self.assertEqual(series, 'hoary')
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+            mock.call(self.juju_command, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
         mock_print.assert_called_once_with(self.status_message)
 
@@ -495,12 +488,12 @@
         # The environment is bootstrapped with sudo using the local provider.
         with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
             already_bootstrapped, series = app.bootstrap(
-                self.env_name, requires_sudo=True)
+                self.env_name, self.juju_command, requires_sudo=True)
         self.assertFalse(already_bootstrapped)
         self.assertEqual(series, 'hoary')
         mock_call.assert_has_calls([
             mock.call(
-                'sudo', settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+                'sudo', self.juju_command, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
         mock_print.assert_called_once_with(self.status_message)
 
@@ -508,12 +501,12 @@
         # The environment is successfully bootstrapped in debug mode.
         with self.patch_multiple_calls(self.make_side_effects()) as mock_call:
             already_bootstrapped, series = app.bootstrap(
-                self.env_name, debug=True)
+                self.env_name, self.juju_command, debug=True)
         self.assertFalse(already_bootstrapped)
         self.assertEqual(series, 'hoary')
         mock_call.assert_has_calls([
             mock.call(
-                settings.JUJU_CMD, 'bootstrap', '-e', self.env_name,
+                self.juju_command, 'bootstrap', '-e', self.env_name,
                 '--debug'),
         ] + self.make_status_calls(1))
 
@@ -525,11 +518,12 @@
             (0, self.make_status_output('started', 'precise'), ''),
         ]
         with self.patch_multiple_calls(side_effects) as mock_call:
-            already_bootstrapped, series = app.bootstrap(self.env_name)
+            already_bootstrapped, series = app.bootstrap(
+                self.env_name, self.juju_command)
         self.assertTrue(already_bootstrapped)
         self.assertEqual(series, 'precise')
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+            mock.call(self.juju_command, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
         existing_message = 'reusing the already bootstrapped {} environment'
         mock_print.assert_has_calls([
@@ -541,9 +535,9 @@
         # A ProgramExit is raised if an error occurs while bootstrapping.
         with self.patch_call(retcode=1, error='bad wolf') as mock_call:
             with self.assert_program_exit('bad wolf'):
-                app.bootstrap(self.env_name)
+                app.bootstrap(self.env_name, self.juju_command)
         mock_call.assert_called_once_with(
-            settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+            self.juju_command, 'bootstrap', '-e', self.env_name),
 
     def test_status_retry_error(self, mock_print):
         # Before raising a ProgramExit, the functions tries to call
@@ -602,9 +596,9 @@
         expected = 'state server failure:\n{}'.format(status_output)
         with self.patch_multiple_calls(side_effects) as mock_call:
             with self.assert_program_exit(expected):
-                app.bootstrap(self.env_name)
+                app.bootstrap(self.env_name, self.juju_command)
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+            mock.call(self.juju_command, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(1))
 
     def test_status_failure(self, mock_print):
@@ -628,9 +622,9 @@
             # the third for the second status check, the fourth should fail.
             with mock.patch('time.time', mock_time):
                 with self.assert_program_exit(expected):
-                    app.bootstrap(self.env_name)
+                    app.bootstrap(self.env_name, self.juju_command)
         mock_call.assert_has_calls([
-            mock.call(settings.JUJU_CMD, 'bootstrap', '-e', self.env_name),
+            mock.call(self.juju_command, 'bootstrap', '-e', self.env_name),
         ] + self.make_status_calls(2))
 
 
@@ -660,24 +654,25 @@
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
 
     env_name = 'ec2'
+    juju_command = settings.JUJU_CMD_PATHS['default']
 
     def test_success(self):
         # The API URL is correctly returned.
         api_addresses = json.dumps(['api.example.com:17070', 'not-today'])
         with self.patch_call(retcode=0, output=api_addresses) as mock_call:
-            api_url = app.get_api_url(self.env_name)
+            api_url = app.get_api_url(self.env_name, self.juju_command)
         self.assertEqual('wss://api.example.com:17070', api_url)
         mock_call.assert_called_once_with(
-            settings.JUJU_CMD, 'api-endpoints', '-e', self.env_name,
+            self.juju_command, 'api-endpoints', '-e', self.env_name,
             '--format', 'json')
 
     def test_failure(self):
         # A ProgramExit is raised if an error occurs retrieving the API URL.
         with self.patch_call(retcode=1, error='bad wolf') as mock_call:
             with self.assert_program_exit('bad wolf'):
-                app.get_api_url(self.env_name)
+                app.get_api_url(self.env_name, self.juju_command)
         mock_call.assert_called_once_with(
-            settings.JUJU_CMD, 'api-endpoints', '-e', self.env_name,
+            self.juju_command, 'api-endpoints', '-e', self.env_name,
             '--format', 'json')
 
 

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-05-23 13:26:56 +0000
+++ quickstart/tests/test_manage.py	2014-06-02 14:34:25 +0000
@@ -38,7 +38,10 @@
 from quickstart.cli import views
 from quickstart.models import envs
 from quickstart.tests import helpers
-from quickstart import app
+from quickstart import (
+    app,
+    platform_support,
+)
 
 
 class TestDescriptionAction(unittest.TestCase):
@@ -63,24 +66,34 @@
     distro_only_disable = '(enabled by default, use --ppa to disable)'
     ppa_disable = '(enabled by default, use --distro-only to disable)'
 
-    def test_ppa_source(self):
+    def test_ppa_source_apt(self):
         # The returned distro_only flag is set to False and the help texts are
         # formatted accordingly when the passed Juju source is "ppa".
         distro_only, distro_only_help, ppa_help = manage._get_packaging_info(
-            'ppa')
+            'ppa', platform=settings.LINUX_APT)
         self.assertFalse(distro_only)
         self.assertNotIn(self.distro_only_disable, distro_only_help)
         self.assertIn(self.ppa_disable, ppa_help)
 
-    def test_distro_source(self):
+    def test_distro_source_apt(self):
         # The returned distro_only flag is set to True and the help texts are
         # formatted accordingly when the passed Juju source is "distro".
         distro_only, distro_only_help, ppa_help = manage._get_packaging_info(
-            'distro')
+            'distro', platform=settings.LINUX_APT)
         self.assertTrue(distro_only)
         self.assertIn(self.distro_only_disable, distro_only_help)
         self.assertNotIn(self.ppa_disable, ppa_help)
 
+    def test_not_apt(self):
+        # If the platform is not LINUX_APT then distro_only should be False
+        # and the help is suppressed.
+        for platform in (settings.LINUX_RPM, settings.WINDOWS, settings.OSX):
+            distro_only, dist_only_help, ppa_help = manage._get_packaging_info(
+                'dontcare', platform=platform)
+            self.assertTrue(distro_only)
+            self.assertEqual(argparse.SUPPRESS, dist_only_help)
+            self.assertEqual(argparse.SUPPRESS, ppa_help)
+
 
 class TestValidateBundle(
         helpers.BundleFileTestsMixin, helpers.UrlReadTestsMixin,
@@ -610,6 +623,16 @@
         if exit_called:
             mock_exit.assert_called_once_with(0)
 
+    def test_unsupported_os(self):
+        # get_platform raises UnsupportedOS, causing early ProgramExit.
+        path = 'quickstart.manage.platform_support.get_platform'
+        err = platform_support.UnsupportedOS('Unknown TRS-80')
+        with mock.patch(path, side_effect=err):
+            with self.assertRaises(app.ProgramExit) as ctx:
+                manage.setup()
+            expected = b'juju-quickstart: error: Unknown TRS-80'
+            self.assertEqual(expected, bytes(ctx.exception))
+
     def test_help(self):
         # The program help message is properly formatted.
         with mock.patch('sys.stdout') as mock_stdout:
@@ -689,10 +712,12 @@
 @mock.patch('__builtin__.print', mock.Mock())
 class TestRun(unittest.TestCase):
 
+    juju_command = '/sbin/juju'
+
     def make_options(self, **kwargs):
         """Set up the options to be passed to the run function."""
         options = {
-            'admin_secret': 'Secret!',
+            'admin_secret': 'Secretz!',
             'bundle': None,
             'bundle_id': None,
             'charm_url': None,
@@ -734,14 +759,19 @@
         # Make mock_app.create_auth_token return a fake authentication token.
         mock_app.create_auth_token.return_value = 'AUTHTOKEN'
         options = self.make_options()
-        manage.run(options)
+        with mock.patch('quickstart.manage.platform_support.get_juju_command',
+                        side_effect=[self.juju_command]):
+            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=False, debug=options.debug)
-        mock_app.get_api_url.assert_called_once_with(options.env_name)
+            options.env_name, self.juju_command, requires_sudo=False,
+            debug=options.debug)
+        mock_app.get_api_url.assert_called_once_with(
+            options.env_name, self.juju_command)
         mock_app.connect.assert_has_calls([
-            mock.call(mock_app.get_api_url(), options.admin_secret),
+            mock.call(
+                mock_app.get_api_url(), options.admin_secret),
             mock.call().close(),
             mock.call('wss://1.2.3.4:443/ws', options.admin_secret),
             mock.call().close(),
@@ -811,9 +841,13 @@
             'cs:precise/juju-gui-42', '0', None, None)
         for version in versions:
             mock_app.ensure_dependencies.return_value = version
-            manage.run(options)
+            with mock.patch(
+                    'quickstart.manage.platform_support.get_juju_command',
+                    side_effect=[self.juju_command]):
+                manage.run(options)
             mock_app.bootstrap.assert_called_once_with(
-                options.env_name, requires_sudo=False, debug=options.debug)
+                options.env_name, self.juju_command, requires_sudo=False,
+                debug=options.debug)
             mock_app.bootstrap.reset_mock()
 
     def test_local_provider_requiring_sudo(self, mock_app, mock_open):
@@ -831,9 +865,13 @@
             'cs:trusty/juju-gui-42', '0', None, None)
         for version in versions:
             mock_app.ensure_dependencies.return_value = version
-            manage.run(options)
+            with mock.patch(
+                    'quickstart.manage.platform_support.get_juju_command',
+                    side_effect=[self.juju_command]):
+                manage.run(options)
             mock_app.bootstrap.assert_called_once_with(
-                options.env_name, requires_sudo=True, debug=options.debug)
+                options.env_name, self.juju_command, requires_sudo=True,
+                debug=options.debug)
             mock_app.bootstrap.reset_mock()
 
     def test_no_local_no_sudo(self, mock_app, mock_open):
@@ -847,9 +885,12 @@
         # where to deploy the charm, the service and unit data.
         mock_app.check_environment.return_value = (
             'cs:precise/juju-gui-42', '0', None, None)
-        manage.run(options)
+        with mock.patch('quickstart.manage.platform_support.get_juju_command',
+                        side_effect=[self.juju_command]):
+            manage.run(options)
         mock_app.bootstrap.assert_called_once_with(
-            options.env_name, requires_sudo=False, debug=options.debug)
+            options.env_name, self.juju_command,
+            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_platform_support.py'
--- quickstart/tests/test_platform_support.py	2014-05-29 20:49:20 +0000
+++ quickstart/tests/test_platform_support.py	2014-06-02 14:34:25 +0000
@@ -44,23 +44,23 @@
         with self.patch_platform_system('Linux'):
             with self.patch_isfile([True]):
                 result = platform_support.get_platform()
-        self.assertEqual(platform_support.LINUX_APT, result)
+        self.assertEqual(settings.LINUX_APT, result)
 
     def test_linux_rpm(self):
         with self.patch_platform_system('Linux'):
             with self.patch_isfile([False, True]):
                 result = platform_support.get_platform()
-        self.assertEqual(platform_support.LINUX_RPM, result)
+        self.assertEqual(settings.LINUX_RPM, result)
 
     def test_osx(self):
         with self.patch_platform_system('Darwin'):
             result = platform_support.get_platform()
-        self.assertEqual(platform_support.OSX, result)
+        self.assertEqual(settings.OSX, result)
 
     def test_windows(self):
         with self.patch_platform_system('Windows'):
             result = platform_support.get_platform()
-        self.assertEqual(platform_support.WINDOWS, result)
+        self.assertEqual(settings.WINDOWS, result)
 
     def test_unsupported_raises_exception(self):
         with self.patch_platform_system('CP/M'):
@@ -91,50 +91,38 @@
 
     def test_support_local(self):
         expected = {
-            platform_support.LINUX_APT: True,
-            platform_support.LINUX_RPM: True,
-            platform_support.OSX: False,
-            platform_support.WINDOWS: False,
+            settings.LINUX_APT: True,
+            settings.LINUX_RPM: True,
+            settings.OSX: False,
+            settings.WINDOWS: False,
             object(): False,
         }
         for key, value in expected.items():
             self.assertEqual(value, platform_support.supports_local(key))
 
 
-class TestGetPlatformInstaller(unittest.TestCase):
+class TestGetJujuInstaller(unittest.TestCase):
 
     def test_linux_apt(self):
-        expected = platform_support.LINUX_APT
-        with mock.patch('quickstart.platform_support.get_platform',
-                        side_effect=[expected]):
-            platform, installer = platform_support.get_platform_installer()
-        self.assertEqual(expected, platform)
+        platform = settings.LINUX_APT
+        installer = platform_support.get_juju_installer(platform)
         self.assertEqual(platform_support._installer_apt, installer)
 
     def test_osx(self):
-        expected = platform_support.OSX
-        # The JUJU_CMD should be the default location.
-        self.assertEqual('/usr/bin/juju', settings.JUJU_CMD)
-        with mock.patch('quickstart.platform_support.get_platform',
-                        side_effect=[expected]):
-            platform, installer = platform_support.get_platform_installer()
-        self.assertEqual(expected, platform)
+        platform = settings.OSX
+        installer = platform_support.get_juju_installer(platform)
         self.assertEqual(platform_support._installer_osx, installer)
-        # The JUJU_CMD should be changed as a side-effect.
-        self.assertEqual('/usr/local/bin/juju', settings.JUJU_CMD)
 
     def test_linux_rpm(self):
-        expected = platform_support.LINUX_RPM
-        with mock.patch('quickstart.platform_support.get_platform',
-                        side_effect=[expected]):
-            platform, installer = platform_support.get_platform_installer()
-        self.assertEqual(expected, platform)
-        self.assertIsNone(installer)
+        platform = settings.LINUX_RPM
+        with self.assertRaises(platform_support.UnsupportedOS) as ctx:
+            platform_support.get_juju_installer(platform)
+        self.assertEqual(
+            'No installer found for host platform.', ctx.exception.message)
 
     def test_windows(self):
-        expected = platform_support.WINDOWS
-        with mock.patch('quickstart.platform_support.get_platform',
-                        side_effect=[expected]):
-            platform, installer = platform_support.get_platform_installer()
-        self.assertEqual(expected, platform)
-        self.assertIsNone(installer)
+        platform = settings.WINDOWS
+        with self.assertRaises(platform_support.UnsupportedOS) as ctx:
+            platform_support.get_juju_installer(platform)
+        self.assertEqual(
+            'No installer found for host platform.', ctx.exception.message)

=== modified file 'quickstart/tests/test_utils.py'
--- quickstart/tests/test_utils.py	2014-05-30 09:23:00 +0000
+++ quickstart/tests/test_utils.py	2014-06-02 14:34:25 +0000
@@ -759,17 +759,17 @@
     def test_return_deconstructed_version(self):
         # Should return a deconstructed juju version.
         with self.patch_call(0, '1.17.1-precise-amd64\n', ''):
-            version = utils.get_juju_version()
+            version = utils.get_juju_version('juju')
         self.assertEqual((1, 17, 1), version)
 
     def test_juju_version_error(self):
         # 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()
+                utils.get_juju_version('juju')
 
     def test_invalid_version_string(self):
         # A ValueError is raised if "juju version" outputs an invalid version.
         with self.patch_call(0, '1.17-precise-amd64', ''):
             with self.assert_value_error('invalid version string: 1.17'):
-                utils.get_juju_version()
+                utils.get_juju_version('juju')

=== modified file 'quickstart/utils.py'
--- quickstart/utils.py	2014-05-29 18:07:12 +0000
+++ quickstart/utils.py	2014-06-02 14:34:25 +0000
@@ -338,7 +338,7 @@
     return parse_status_output(output, ['machines', '0', 'series'])
 
 
-def get_juju_version():
+def get_juju_version(juju_command):
     """Return the current juju-core version.
 
     Return a (major:int, minor:int, patch:int) tuple, including major, minor
@@ -347,7 +347,7 @@
     Raise a ValueError if the "juju version" call exits with an error
     or the returned version is not well formed.
     """
-    retcode, output, error = call(settings.JUJU_CMD, 'version')
+    retcode, output, error = call(juju_command, 'version')
     if retcode:
         raise ValueError(error.encode('utf-8'))
     version_string = output.split('-')[0]