Code review comment for lp:~makyo/juju-quickstart/ssh-3-watch-keys

Reviewers: mp+198851_code.launchpad.net,

Message:
Please take a look.

Description:
Watch for SSH key creation

Allow the user to create SSH keys in another term/window, watch for
creation and continue when available.

To test: make check
To QA:
1. Move ~/.ssh to a back up
2. Run quickstart - select w when prompted to watch
3. Create ssh keys as instructed in another term/window
4. Quickstart should continue
5. Move ~/.ssh back; other options should work as expected.

https://code.launchpad.net/~makyo/juju-quickstart/ssh-3-watch-keys/+merge/198851

(do not edit description out of merge proposal)

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

Affected files (+64, -10 lines):
   A [revision details]
   M quickstart/app.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-12-09 18:37:40 +0000
+++ quickstart/app.py 2013-12-13 00:18:27 +0000
@@ -94,25 +94,42 @@
      Allow the user to let quickstart create SSH keys, or quit by raising a
      ProgramExit if they would like to create the key themselves.
      """
- ssh_dir = os.path.join(os.path.expanduser('~'), '.ssh')
- retcode = utils.call('/usr/bin/find', ssh_dir, '-name', 'id_*.pub')[0]
- if retcode > 0:
+ if check_ssh_keys():
          print('No SSH keys were found in ~/.ssh\n\n'
                'Would you like quickstart to create them for you? Note
that '
                'this will create a new SSH key with the passphrase you
provide '
                'available to your SSH agent. ssh-keygen will ask for a '
                'passphrase, but juju quickstart will not have access to
it.\n'
- 'Alternatively, you may create the keys on your own.\n')
- create_keys = raw_input('Continue [y/N]? ')
+ 'Alternatively, you may create the keys on your own. '
+ 'Additionally, quickstart can watch for the creation of SSH '
+ 'keys that you create in another terminal or window.\n')
+ create_keys = raw_input('Continue [y/N] or watch for new keys
[w]? ')
          if create_keys.lower() == 'y':
              create_ssh_keys()
+ elif create_keys.lower() == 'w':
+ watch_for_ssh_keys()
          else:
- raise ProgramExit('Please run these commands and follow the '
- 'instructions they produce before running '
+ raise ProgramExit('Please run this command and follow the '
+ 'instructions it produces before running '
                                'quickstart again:\n\n'
                                'ssh-keygen -b 4096 -t rsa')

+def check_ssh_keys():
+ ssh_dir = os.path.join(os.path.expanduser('~'), '.ssh')
+ return utils.call('/usr/bin/find', ssh_dir, '-name', 'id_*.pub')[0]
+
+
+def watch_for_ssh_keys():
+ print('Please run this command in another terminal or window and '
+ 'follow the instructions it produces; quickstart will '
+ 'continue when keys are generated. ^C to quit.\n\n'
+ 'ssh-keygen -b 4096 -t rsa\n')
+ while check_ssh_keys() != 0:
+ time.sleep(3)
+ print('.')
+
+
  def create_ssh_keys():
      """Create SSH keys for the user

Index: quickstart/tests/test_app.py
=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py 2013-12-09 18:37:40 +0000
+++ quickstart/tests/test_app.py 2013-12-13 00:18:27 +0000
@@ -187,7 +187,9 @@
                  'you provide available to your SSH agent. ssh-keygen
will ' \
                  'ask for a passphrase, but juju quickstart will not have '
\
                  'access to it.\nAlternatively, you may create the keys
on ' \
- 'your own.\n'
+ 'your own. Additionally, quickstart can watch for the ' \
+ 'creation of SSH keys that you create in another
terminal ' \
+ 'or window.\n'
      ssh_key_find = ('/usr/bin/find',
                      os.path.join(os.path.expanduser('~'), '.ssh'),
                      '-name', 'id_*.pub')
@@ -202,8 +204,8 @@
          mock_call.assert_called_with(*self.ssh_key_find)

      def test_failure_no_keygen(self, mock_print):
- msg = 'Please run these commands and follow the instructions
they ' \
- 'produce before running quickstart again:\n\n' \
+ msg = 'Please run this command and follow the instructions it ' \
+ 'produces before running quickstart again:\n\n' \
                'ssh-keygen -b 4096 -t rsa'
          with self.assert_program_exit(msg):
              with self.patch_call(130) as mock_call:
@@ -224,6 +226,39 @@
          self.assertTrue(mock_raw_input.called)
          self.assertTrue(mock_create_ssh_keys.called)

+ def test_failure_with_watch(self, mock_print):
+ with self.patch_call(130) as mock_call:
+ with self.patch_raw_input(return_value='W') as mock_raw_input:
+ with mock.patch('quickstart.app.watch_for_ssh_keys') \
+ as mock_watch_for_ssh_keys:
+ app.ensure_ssh_keys()
+ mock_call.assert_called_with(*self.ssh_key_find)
+ mock_print.assert_has_calls([mock.call(self.print_msg)])
+ self.assertTrue(mock_raw_input.called)
+ self.assertTrue(mock_watch_for_ssh_keys.called)
+
+
+class TestWatchForSSHKeys(helpers.CallTestsMixin, unittest.TestCase):
+
+ @mock.patch('time.sleep')
+ @helpers.mock_print
+ def test_watch(self, mock_print, mock_sleep):
+ side_effects = (
+ (130, '', ''),
+ (0, '', ''),
+ )
+ with self.patch_multiple_calls(side_effects):
+ app.watch_for_ssh_keys()
+ mock_print.assert_has_calls([
+ mock.call('Please run this command in another terminal or
window '
+ 'and follow the instructions it produces;
quickstart '
+ 'will continue when keys are generated. ^C to
quit.\n\n'
+ 'ssh-keygen -b 4096 -t rsa\n'),
+ mock.call('.')
+ ])
+ mock_sleep.assert_called_once_with(3)
+ mock_print.assert_called_with('.')
+

  @helpers.mock_print
  class TestCreateSSHKeys(