Merge lp:~makyo/juju-quickstart/ensure-juju-lxc into lp:juju-quickstart

[Madison Scott-Clary]

Ensure juju and lxc are installed

To test: make check

To QA:
- Create an lxc or vm without juju installed
- Clone this branch
- Run `make sysdeps`
- Run `make run`

It should install juju, lxc, and dependencies, then prompt you to run again after initializing your juju environments.

https://codereview.appspot.com/29980043/

[Madison Scott-Clary (makyo)]

Reviewers: mp+196011_code.launchpad.net,

Message:
Please take a look.

Description:
Ensure juju and lxc are installed

To test: make check

To QA:
- Create an lxc or vm without juju installed
- Clone this branch
- Run `make sysdeps`
- Run `make run`

It should install juju, lxc, and dependencies, then prompt you to run
again after initializing your juju environments.

https://code.launchpad.net/~makyo/juju-quickstart/ensure-juju-lxc/+merge/196011

(do not edit description out of merge proposal)

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

Affected files (+96, -1 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/tests/helpers.py
   M quickstart/tests/test_app.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: quickstart/app.py
=== modified file 'quickstart/app.py'
--- quickstart/app.py 2013-11-20 11:35:30 +0000
+++ quickstart/app.py 2013-11-20 20:36:24 +0000
@@ -45,6 +45,35 @@
          return 'juju-quickstart: error: {}'.format(self.message)

+def ensure_dependencies():
+ """Ensure that Juju and LXC are installed.
+
+ 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.
+ """
+ retcode = utils.call('juju', 'version')[0]
+ retcode += utils.call('lxc-ls')[0]
+ if retcode > 0:
+ cmds = [
+ ['apt-add-repository', '-y', 'ppa:juju/stable'],
+ ['apt-get', 'update'],
+ ['apt-get', 'install', '-y', 'juju-core', 'lxc']
+ ]
+ for cmd in cmds:
+ cmd.insert(0, 'sudo')
+ retcode, _, error = utils.call(*cmd)
+ if retcode:
+ raise ProgramExit(error)
+ # XXX Makyo #1247214:
+ # Here, we should guide the user through cloud credentials and
+ # juju init.
+ raise ProgramExit('Juju is now installed; please run the same
juju '
+ 'quickstart command again to proceed.')
+
+
  def bootstrap(env_name, is_local=False, debug=False):
      """Bootstrap the Juju environment with the given name.

Index: quickstart/manage.py
=== modified file 'quickstart/manage.py'
--- quickstart/manage.py 2013-11-20 11:35:30 +0000
+++ quickstart/manage.py 2013-11-20 20:30:08 +0000
@@ -218,6 +218,8 @@
  def run(options):
      """Run the application."""
      print('juju quickstart v{}'.format(version))
+ print('ensuring juju and lxc are installed')
+ app.ensure_dependencies()
      print('bootstrapping the {} environment (type: {})'.format(
          options.env_name, options.env_type))
      is_local = options.env_type == 'local'

Index: quickstart/tests/helpers.py
=== modified file 'quickstart/tests/helpers.py'
--- quickstart/tests/helpers.py 2013-11-18 11:42:15 +0000
+++ quickstart/tests/helpers.py 201...

[lp:~makyo/juju-quickstart/ensure-juju-lxc]

21. By Madison Scott-Clary on 2013-11-20

Clarified message

[Madison Scott-Clary (makyo)]

Please take a look.

https://codereview.appspot.com/29980043/

[Gary Poster (gary)]

LGTM. I'm afraid I won't have time for a QA today, but will do it
tomorrow if requested. Thank you!

https://codereview.appspot.com/29980043/diff/20001/quickstart/tests/helpers.py
File quickstart/tests/helpers.py (right):

https://codereview.appspot.com/29980043/diff/20001/quickstart/tests/helpers.py#newcode88
quickstart/tests/helpers.py:88: """Easily use the quickstart.utils.call
and
PEP8 style says that the first line should not break. Maybe this?

Easily use `call` and `sudo_call` from quickstart.utils.

https://codereview.appspot.com/29980043/

[lp:~makyo/juju-quickstart/ensure-juju-lxc]

22. By Madison Scott-Clary on 2013-11-20

Fix old docstring

[Madison Scott-Clary (makyo)]

Please take a look.

https://codereview.appspot.com/29980043/diff/20001/quickstart/tests/helpers.py
File quickstart/tests/helpers.py (right):

https://codereview.appspot.com/29980043/diff/20001/quickstart/tests/helpers.py#newcode88
quickstart/tests/helpers.py:88: """Easily use the quickstart.utils.call
and
On 2013/11/20 21:41:20, gary.poster wrote:
> PEP8 style says that the first line should not break. Maybe this?

> Easily use `call` and `sudo_call` from quickstart.utils.

Reverting, left over from my previous solution. Good to know, though!

https://codereview.appspot.com/29980043/

[Madison Scott-Clary (makyo)]

https://codereview.appspot.com/29980043/

[Madison Scott-Clary (makyo)]

On 2013/11/20 21:46:39, matthew.scott wrote:

Thanks, RV. That was supposed to be a reply to Gary - the comment was
old, reverted.

https://codereview.appspot.com/29980043/

[Francesco Banconi (frankban)]

Nice branch Matthew, thank you!
LGTM with changes (see below).
Will QA later.

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode60
quickstart/app.py:60: cmds = [
ISTM that this is a good place to inform the user about what's
happening, e.g.: 'juju is not installed', 'sudo privileges required to
install juju'. It could be nice to give an idea about why we need sudo.

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode66
quickstart/app.py:66: cmd.insert(0, 'sudo')
You can either add sudo in the cmds above (and make cmds a list of
tuples) or just "retcode, _, error = utils.call('sudo', *cmd)" below
(and make cmds a list of tuples :-). <shrug>

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode73
quickstart/app.py:73: raise ProgramExit('Juju is now installed; please
run `juju init` and '
I am not sure about exiting with an error here.
What if the user has a juju home properly set up but juju is not
installed? This is not uncommon, e.g. he removed the package, or he is
working in a lxc sharing his home directory. Moreover this code is never
reached if the envs.yaml is not properly set up, so we are just
preventing users with a configured juju home to proceed. I think we
should just continue with the normal execution here: in the future we
will guide the user setting up the environment.

https://codereview.appspot.com/29980043/diff/40001/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/29980043/diff/40001/quickstart/manage.py#newcode221
quickstart/manage.py:221: print('ensuring juju and lxc are installed')
What do you think about making this a logging.debug call? I am not sure
about how much this is informative after the first run.

https://codereview.appspot.com/29980043/

[Brad Crittenden (bac)]

Changes made. Having to re-propose to a new merge proposal before
landing.

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode60
quickstart/app.py:60: cmds = [
On 2013/11/21 12:02:03, frankban wrote:
> ISTM that this is a good place to inform the user about what's
happening, e.g.:
> 'juju is not installed', 'sudo privileges required to install juju'.
It could be
> nice to give an idea about why we need sudo.

Done.

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode66
quickstart/app.py:66: cmd.insert(0, 'sudo')
On 2013/11/21 12:02:03, frankban wrote:
> You can either add sudo in the cmds above (and make cmds a list of
tuples) or
> just "retcode, _, error = utils.call('sudo', *cmd)" below (and make
cmds a list
> of tuples :-). <shrug>

Done.

https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode73
quickstart/app.py:73: raise ProgramExit('Juju is now installed; please
run `juju init` and '
On 2013/11/21 12:02:03, frankban wrote:
> I am not sure about exiting with an error here.
> What if the user has a juju home properly set up but juju is not
installed? This
> is not uncommon, e.g. he removed the package, or he is working in a
lxc sharing
> his home directory. Moreover this code is never reached if the
envs.yaml is not
> properly set up, so we are just preventing users with a configured
juju home to
> proceed. I think we should just continue with the normal execution
here: in the
> future we will guide the user setting up the environment.

Done.

https://codereview.appspot.com/29980043/diff/40001/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/29980043/diff/40001/quickstart/manage.py#newcode221
quickstart/manage.py:221: print('ensuring juju and lxc are installed')
On 2013/11/21 12:02:03, frankban wrote:
> What do you think about making this a logging.debug call? I am not
sure about
> how much this is informative after the first run.

Done.

https://codereview.appspot.com/29980043/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2013-11-20 11:35:30 +0000
+++ quickstart/app.py	2013-11-20 21:43:48 +0000
@@ -45,6 +45,36 @@
         return 'juju-quickstart: error: {}'.format(self.message)
 
 
+def ensure_dependencies():
+    """Ensure that Juju and LXC are installed.
+
+    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.
+    """
+    retcode = utils.call('juju', 'version')[0]
+    retcode += utils.call('lxc-ls')[0]
+    if retcode > 0:
+        cmds = [
+            ['apt-add-repository', '-y', 'ppa:juju/stable'],
+            ['apt-get', 'update'],
+            ['apt-get', 'install', '-y', 'juju-core', 'lxc']
+        ]
+        for cmd in cmds:
+            cmd.insert(0, 'sudo')
+            retcode, _, error = utils.call(*cmd)
+            if retcode:
+                raise ProgramExit(error)
+        # XXX Makyo #1247214:
+            # Here, we should guide the user through cloud credentials and
+            # juju init.
+        raise ProgramExit('Juju is now installed; please run `juju init` and '
+                          'then the same juju quickstart command again to '
+                          'proceed.')
+
+
 def bootstrap(env_name, is_local=False, debug=False):
     """Bootstrap the Juju environment with the given name.
 

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2013-11-20 11:35:30 +0000
+++ quickstart/manage.py	2013-11-20 21:43:48 +0000
@@ -218,6 +218,8 @@
 def run(options):
     """Run the application."""
     print('juju quickstart v{}'.format(version))
+    print('ensuring juju and lxc are installed')
+    app.ensure_dependencies()
     print('bootstrapping the {} environment (type: {})'.format(
         options.env_name, options.env_type))
     is_local = options.env_type == 'local'

=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2013-11-20 11:35:30 +0000
+++ quickstart/tests/test_app.py	2013-11-20 21:43:48 +0000
@@ -61,6 +61,68 @@
         return jujuclient.EnvError({'Error': message})
 
 
+class TestEnsureDependencies(
+        helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
+
+    def call_ensure_dependencies(self, call_effects):
+        with self.patch_multiple_calls(call_effects) as mock_call:
+            app.ensure_dependencies()
+        return mock_call
+
+    def test_success_install(self):
+        # XXX Makyo #1247214:
+            # Here, we should guide the user through cloud credentials and
+            # juju init.
+        with self.assert_program_exit('Juju is now installed; please run '
+                                      '`juju init` and then the same juju '
+                                      'quickstart command again to '
+                                      'proceed.'):
+            call = self.call_ensure_dependencies(
+                [
+                    (127, '', 'no juju'),
+                    (127, '', 'no lxc'),
+                    (0, 'add repo', ''),
+                    (0, 'update', ''),
+                    (0, 'install', ''),
+                ]
+            )
+            self.assertEqual(call.call_count, 5)
+            call.assert_has_calls([
+                mock.call('juju', 'version'),
+                mock.call('lxc-ls'),
+                mock.call('sudo', 'apt-add-repository', '-y',
+                          'ppa:juju/stable'),
+                mock.call('sudo', 'apt-get', 'update'),
+                mock.call('sudo', 'apt-get', 'install', '-y', 'juju-core',
+                          'lxc'),
+            ])
+
+    def test_success_no_install(self):
+        call = self.call_ensure_dependencies(
+            [
+                (0, '1.16', ''),
+                (0, '', ''),
+                (0, 'add repo', ''),
+                (0, 'update', ''),
+                (0, 'install', ''),
+            ]
+        )
+        self.assertEqual(call.call_count, 2)
+
+    def test_failure(self):
+        with self.assert_program_exit('install failure'):
+            call = self.call_ensure_dependencies(
+                [
+                    (127, '', ''),
+                    (127, '', ''),
+                    (1, 'add repo', 'install failure'),
+                    (1, 'update', 'uh-oh'),
+                    (1, 'install', 'oh no'),
+                ]
+            )
+            self.assertEqual(call.call_count, 3)
+
+
 @mock_print
 class TestBootstrap(
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):