Merge lp:~makyo/juju-quickstart/ssh-3-watch-keys into lp:juju-quickstart

[Madison Scott-Clary]

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://codereview.appspot.com/39610049/

[Madison Scott-Clary (makyo)]

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_ke...

[Gary Poster (gary)]

Wording suggestions. Didn't look other than that for now.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode99
quickstart/app.py:99: 'Would you like quickstart to create them for you?
Note that '
What do you think of something like this? Please feel free to polish or
counter-propose.

----

No SSH keys were found in ~/.ssh

To proceed, quickstart can automatically create keys for you [a], or it
can give you a command to run yourself [s] in another terminal or window
to create the keys.

Note that, if you want quickstart to create your key, the underlying
program (ssh-keygen) will ask for a passphrase, but quickstart will not
have access to it.

Automatically create keys [a], create the keys yourself [s], or cancel
[C]?

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode112
quickstart/app.py:112: raise ProgramExit('Please run this command and
follow the '
If you would like to create the keys yourself, please run this command
and then re-run quickstart:

ssh-keygen -b 4096 -t rsa

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode120
quickstart/app.py:120: return utils.call('/usr/bin/find', ssh_dir,
'-name', 'id_*.pub')[0]
We can wait for Windows compatibility, but it would be nice if
quickstart used built in Python tools rather than find. <shrug> for
now.

https://codereview.appspot.com/39610049/

[Gary Poster (gary)]

What you've added looks good to me, with the changes I've suggested, but
QA bad.

------------------------------------------------
Continue [y/N] or watch for new keys [w]? w
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.

ssh-keygen -b 4096 -t rsa

.
.
.
.
.
.
.
.
.
.
.
bootstrapping the ec2 environment (type: ec2)
juju-quickstart: error: ERROR error parsing environment "ec2": no public
ssh keys found
------------------------------------------------

Do we need to start (restart) an agent?

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode112
quickstart/app.py:112: raise ProgramExit('Please run this command and
follow the '
On 2013/12/13 01:49:45, gary.poster wrote:
> If you would like to create the keys yourself, please run this command

, follow its instructions,

> and then
> re-run quickstart:

> ssh-keygen -b 4096 -t rsa

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode120
quickstart/app.py:120: return utils.call('/usr/bin/find', ssh_dir,
'-name', 'id_*.pub')[0]
On 2013/12/13 01:49:45, gary.poster wrote:
> We can wait for Windows compatibility, but it would be nice if
quickstart used
> built in Python tools rather than find. <shrug> for now.
That is: please leave as is for now unless you find yourself really
motivated and it looks really fast and easy.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode130
quickstart/app.py:130: print('.')
I think this would be much better. OK with you?

print('.', end="")

https://codereview.appspot.com/39610049/diff/1/quickstart/tests/test_app.py
File quickstart/tests/test_app.py (right):

https://codereview.appspot.com/39610049/diff/1/quickstart/tests/test_app.py#newcode245
quickstart/tests/test_app.py:245: def test_watch(self, mock_print,
mock_sleep):
Nice test.

https://codereview.appspot.com/39610049/

[Madison Scott-Clary (makyo)]

Thanks for the comments. Will re-propose after fixing QA issue.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode99
quickstart/app.py:99: 'Would you like quickstart to create them for you?
Note that '
On 2013/12/13 01:49:45, gary.poster wrote:
> What do you think of something like this? Please feel free to polish
or
> counter-propose.

> ----

> No SSH keys were found in ~/.ssh

> To proceed, quickstart can automatically create keys for you [a], or
it can give
> you a command to run yourself [s] in another terminal or window to
create the
> keys.

> Note that, if you want quickstart to create your key, the underlying
program
> (ssh-keygen) will ask for a passphrase, but quickstart will not have
access to
> it.

> Automatically create keys [a], create the keys yourself [s], or cancel
[C]?

Done.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode112
quickstart/app.py:112: raise ProgramExit('Please run this command and
follow the '
On 2013/12/13 01:49:45, gary.poster wrote:
> If you would like to create the keys yourself, please run this command
and then
> re-run quickstart:

> ssh-keygen -b 4096 -t rsa

Done.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode120
quickstart/app.py:120: return utils.call('/usr/bin/find', ssh_dir,
'-name', 'id_*.pub')[0]
On 2013/12/13 13:51:45, gary.poster wrote:
> On 2013/12/13 01:49:45, gary.poster wrote:
> > We can wait for Windows compatibility, but it would be nice if
quickstart used
> > built in Python tools rather than find. <shrug> for now.
> That is: please leave as is for now unless you find yourself really
motivated
> and it looks really fast and easy.

This is encompassed by ssh keys 2.5 - this find is fragile and does not
a) verify that the keys are valid for ssh, or b) take into account that
keys can be stored elsewhere with an ssh config. Talked with Francesco,
leaving it as an increment.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode130
quickstart/app.py:130: print('.')
On 2013/12/13 13:51:45, gary.poster wrote:
> I think this would be much better. OK with you?

> print('.', end="")

Done.

https://codereview.appspot.com/39610049/

[lp:~makyo/juju-quickstart/ssh-3-watch-keys]

35. By Madison Scott-Clary on 2013-12-13

Wording changes per review.

36. By Madison Scott-Clary on 2013-12-13

tests agree with changes.

37. By Madison Scott-Clary on 2013-12-13

Tests pass, using agent now.

[Madison Scott-Clary (makyo)]

Please take a look.

https://codereview.appspot.com/39610049/

[Gary Poster (gary)]

Other than needing to add some docstrings, this looks good, but the QA
is still bad.

Note I am using a passphrase when I QA.

I was able to get it to work when I did the following.

(1) In the other terminal, also *add* the new key

ssh-keygen -b 4096 -t rsa && ssh-agent && ssh-add

(2) In the code, I did a time.sleep(10) after it saw the key in order to
wait for me to type the password in. This is obviously not ideal.

Note that I am pretty sure that create_ssh_keys should run ssh-add also.

Ideally, I think the script would run ssh-agent and ssh-add. In quick
hacks, I couldn't get this to work. I started debugging and it might be
that it needed some time in between the call to ssh-agent and ssh-add.
I'm not sure. Note that ssh-add needs to be interactive, so utils.call
can't work as is. We need stdin and stdout to be used.

Sorry it is still not working for me.

https://codereview.appspot.com/39610049/diff/20001/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/39610049/diff/20001/quickstart/app.py#newcode119
quickstart/app.py:119: def check_ssh_keys():
docstring please

https://codereview.appspot.com/39610049/diff/20001/quickstart/app.py#newcode124
quickstart/app.py:124: def watch_for_ssh_keys():
docstring please

https://codereview.appspot.com/39610049/

[Madison Scott-Clary (makyo)]

Putting this branch on hold, then. Thanks for the QA, Gary. We need to
find a check_ssh_keys method that actually checks if keys are available
to the agent, which will be the case once the user enters the password
(no matter how it works: automatic creation or manual). I'll add the
docstrings and push, then start researching tonight/this weekend on SSH
solutions.

Thanks again

https://codereview.appspot.com/39610049/

[Gary Poster (gary)]

Cool.

I could very well be wrong, but I think that the script should start the agent and add the key. Any thing else seems like it could easily go wrong and hang forever.

That's what I was trying to get to work, briefly. It's what the other code path needs, and it seems like it is not working yet either.

Gary

> On Dec 13, 2013, at 6:31 PM, <email address hidden> wrote:
>
> Putting this branch on hold, then. Thanks for the QA, Gary. We need to
> find a check_ssh_keys method that actually checks if keys are available
> to the agent, which will be the case once the user enters the password
> (no matter how it works: automatic creation or manual). I'll add the
> docstrings and push, then start researching tonight/this weekend on SSH
> solutions.
>
> Thanks again
>
> https://codereview.appspot.com/39610049/

[lp:~makyo/juju-quickstart/ssh-3-watch-keys]

38. By Madison Scott-Clary on 2013-12-16

Better check for keys, ssh-agent management.

39. By Madison Scott-Clary on 2013-12-17

tests pass

40. By Madison Scott-Clary on 2013-12-17

Was not able to communicate with existing ssh agents, starting one by default now.

[Madison Scott-Clary (makyo)]

Please take a look.

https://codereview.appspot.com/39610049/

[Francesco Banconi (frankban)]

This branch looks very good Matthew, thank you for this work and for
investigating this ssh stuff!
I wrote a lot of comments, but many of them are just nice to
have/minors/do-whit-them-what-you-want.
LGTM with (possible) changes.
QA ok with some comments (about the initial agent check and the SIGINT
handling).

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

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode98
quickstart/app.py:98: utils.start_ssh_agent()
It seems an OSError raised from this call would crash the application.
Also, this always starts the agent, even if the user has one already
running. Do we have a way to avoid that? This could also be tackled in
another card.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode108
quickstart/app.py:108: 'keys yourself [s], or cancel [C]? ')
Maybe ^C here too? <shrug>

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode114
quickstart/app.py:114: raise ProgramExit('If you would like to create
the keys yourself, '
This is displayed as an error, but I am not sure this is an error.
sys.exit(msg) could be used instead.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode120
quickstart/app.py:120: def check_ssh_keys():
Very nice commented function, thank you.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode124
quickstart/app.py:124: in some way. Raise OSError (via
utils.start_ssh_agent) if we couldn't
I don't see the start_ssh_agent call here. Is this still true?

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode131
quickstart/app.py:131: retcode, output, error =
utils.call('/usr/bin/ssh-add', '-l')
error does not seem to be used here and can be safely replaced with _

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode150
quickstart/app.py:150: raise
ProgramExit(msg.format(error).encode('utf-8'))
You can avoid encoding ProgramExit messages <shrug>.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode164
quickstart/app.py:164: while not check_ssh_keys():
^C produces a traceback. It's not that important, but we can easily
avoid it with something like:

     try:
         while not check_ssh_keys():
             print('.', end='')
             sys.stdout.flush()
             time.sleep(3)
     except KeyboardInterrupt:
         sys.exit('\nquitting')

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode166
quickstart/app.py:166: sys.stdout.flush()
Interesting, why is this required? A comment would be nice.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode185
quickstart/app.py:185: if retcode > 0:
Is it ok if retcode is < 0? Can this happen?
If not, we can just check "if retcode:".

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode188
quickstart/app.py:188: if retcode > 0:
Same as above.

https://codereview.appspot.com/39610049/diff/40001/quickstart/tests/test_app.py
File quickstart/tests/test_app.py (right):

https://codereview.appspot.com/39610049/diff/40001/quick...

[lp:~makyo/juju-quickstart/ssh-3-watch-keys]

41. By Madison Scott-Clary on 2013-12-17

responding to frankban's review comments.

[Madison Scott-Clary (makyo)]

Please take a look.

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

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode98
quickstart/app.py:98: utils.start_ssh_agent()
On 2013/12/17 17:22:39, frankban wrote:
> It seems an OSError raised from this call would crash the application.
Also,
> this always starts the agent, even if the user has one already
running. Do we
> have a way to avoid that? This could also be tackled in another card.

Oops, will catch the error.

Through testing, even if another agent is running, quickstart can only
use that agent for authentication, not for adding keys (and so ssh-add
will always fail with rc 2, "Cannot contact agent"). We can investigate
this part further in another card, though, if there's a way around it.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode108
quickstart/app.py:108: 'keys yourself [s], or cancel [C]? ')
On 2013/12/17 17:22:39, frankban wrote:
> Maybe ^C here too? <shrug>

Done.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode114
quickstart/app.py:114: raise ProgramExit('If you would like to create
the keys yourself, '
On 2013/12/17 17:22:39, frankban wrote:
> This is displayed as an error, but I am not sure this is an error.
> sys.exit(msg) could be used instead.

Done.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode124
quickstart/app.py:124: in some way. Raise OSError (via
utils.start_ssh_agent) if we couldn't
On 2013/12/17 17:22:39, frankban wrote:
> I don't see the start_ssh_agent call here. Is this still true?

Nope, good catch.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode131
quickstart/app.py:131: retcode, output, error =
utils.call('/usr/bin/ssh-add', '-l')
On 2013/12/17 17:22:39, frankban wrote:
> error does not seem to be used here and can be safely replaced with _

Done.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode164
quickstart/app.py:164: while not check_ssh_keys():
On 2013/12/17 17:22:39, frankban wrote:
> ^C produces a traceback. It's not that important, but we can easily
avoid it
> with something like:

> try:
> while not check_ssh_keys():
> print('.', end='')
> sys.stdout.flush()
> time.sleep(3)
> except KeyboardInterrupt:
> sys.exit('\nquitting')

Done.

https://codereview.appspot.com/39610049/diff/40001/quickstart/app.py#newcode166
quickstart/app.py:166: sys.stdout.flush()
On 2013/12/17 17:22:39, frankban wrote:
> Interesting, why is this required? A comment would be nice.

Done.

With end being empty, stdout's buffer won't flush until after it reaches
a certain number of bytes.

https://codereview.appspot.com/39610049/

[Richard Harding (rharding)]

QA:

The text is too long and should wrap. I had the text wrapping mid-word
because of the length. I think a standard 78ish for terminal work would
be really nice.

http://uploads.mitechie.com/lp/quickstart.png

A list of options would read cleaner.

"Quickstart can

[a] Automatically create keys
[s] Provide you commands to create keys yourself
[C] Cancel Quickstart
"

There was a big lack of warning that it was going on into bootstrap and
such after the key creation. Maybe a pause/continue makes sense there?

Trying manual instructions:

What's the .? Is it doing something? I thought it was waiting for me to
enter a command?

Again, the instructions but up. It's not clear what's part of this step
and the last bit of info.

Once I did enter the command, the ...... wasn't cleared before the
'bootstraping' log line so I got.

........................bootstrapping the azure environment (type:
azure)

Trying to cancel:

I get a traceback during a ctrl-c to kill. It should be caught/exit
cleanly.

If I start to automatically create, but cancel when it asks me for a
passphrase it also dumps a traceback at me.

I'd propose something that looks to the user more like:

http://paste.ubuntu.com/6595673/

Note the use of more usual opposites of automatic/manual, indentation to
help distinguish commands to run manually as a callout, etc.

https://codereview.appspot.com/39610049/

[Richard Harding (rharding)]

The QA was LGTM with the comments and discussion points in my previous
message.

https://codereview.appspot.com/39610049/

[Madison Scott-Clary (makyo)]

*** Submitted:

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.

R=gary.poster, frankban, rharding
CC=
https://codereview.appspot.com/39610049

https://codereview.appspot.com/39610049/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2013-12-09 18:37:40 +0000
+++ quickstart/app.py	2013-12-17 21:07:10 +0000
@@ -25,6 +25,7 @@
 import json
 import logging
 import os
+import sys
 import time
 
 import jujuclient
@@ -94,23 +95,82 @@
     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:
+    utils.start_ssh_agent()
+    if not 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]? ')
-        if create_keys.lower() == 'y':
+              'To proceed, quickstart can automatically create keys for you '
+              '[a], or it can give you a command to run yourself [s] in '
+              'another terminal or window to create the keys.\n\n'
+              'Note that, if you want quickstart to create your key, the '
+              'underlying program (ssh-keygen) will ask for a passphrase, '
+              'but quickstart will not have access to it.\n\n')
+        create_keys = raw_input('Automatically create keys [a], create the '
+                                'keys yourself [s], or cancel [C or ^C]? ')
+        if create_keys.lower() == 'a':
             create_ssh_keys()
-        else:
-            raise ProgramExit('Please run these commands and follow the '
-                              'instructions they produce before running '
-                              'quickstart again:\n\n'
-                              'ssh-keygen -b 4096 -t rsa')
+        elif create_keys.lower() == 's':
+            watch_for_ssh_keys()
+        else:
+            msg = 'If you would like to create the keys yourself, ' \
+                  'please run this command, follow its ' \
+                  'instructions, and then re-run quickstart:\n\n' \
+                  'ssh-keygen -b 4096 -t rsa && ssh-agent'
+            sys.exit(msg)
+
+
+def check_ssh_keys():
+    """Check whether or not ssh keys exist and are loaded by the agent.
+
+    Raise a program exit if we can't load the keys because they're broken
+    in some way.
+
+    Return true if there are ssh identities loaded and available in an agent,
+    false otherwise.
+    """
+    no_keys_msg = 'The agent has no identities.\n'
+    retcode, output, _ = utils.call('/usr/bin/ssh-add', '-l')
+    if retcode == 0:
+        # We have keys and an agent.
+        return True
+    elif retcode == 1 and output == no_keys_msg:
+        # We have an agent, but no keys currently loaded.
+        retcode, output, error = utils.call('/usr/bin/ssh-add')
+        if retcode == 0:
+            # We were able to load an identity
+            return True
+        elif retcode == 1:
+            # If ssh-add is called without -l and there are no identities
+            # available, there will be no output or error, but retcode will
+            # still be 1.
+            return False
+        else:
+            # We weren't able to load keys for some other reason, such as being
+            # readable by group or world, or malformed.
+            msg = 'error attempting to add ssh keys! {}'
+            raise ProgramExit(msg.format(error).encode('utf-8'))
+    return False
+
+
+def watch_for_ssh_keys():
+    """Watch for generation of ssh keys from another terminal or window.
+
+    This will run until keys become visible to quickstart or killed by
+    the user.
+    """
+    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 && ssh-agent\n')
+    try:
+        while not check_ssh_keys():
+            # Print and flush the buffer immediately; an empty end kwarg will
+            # not cause the buffer to flush until after a certain number of
+            # bytes.
+            print('.', end='')
+            sys.stdout.flush()
+            time.sleep(3)
+    except KeyboardInterrupt:
+        sys.exit('\nquitting')
 
 
 def create_ssh_keys():
@@ -121,15 +181,18 @@
     NB: this involves user interaction for entering the passphrase; this may
     have to change if creating SSH keys takes place in the urwid interface.
     """
-    key_file = os.path.join(os.path.expanduser('~'), '.ssh', 'id_juju_rsa')
+    key_file = os.path.join(os.path.expanduser('~'), '.ssh', 'id_rsa')
     retcode, _, error = utils.call('/usr/bin/ssh-keygen',
                                    '-q',  # silent
                                    '-b', '4096',  # 4096 bytes
                                    '-t', 'rsa',  # RSA type
                                    '-C', 'Generated with Juju Quickstart',
                                    '-f', key_file)
-    if retcode > 0:
+    if retcode:
         raise ProgramExit('error generating ssh key!  {}'.format(error))
+    retcode, _, error = utils.call('/usr/bin/ssh-agent')
+    if retcode:
+        raise ProgramExit('error starting ssh-agent!  {}'.format(error))
     print('a new ssh key was generated in {}'.format(key_file))
 
 

=== 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-17 21:07:10 +0000
@@ -181,63 +181,165 @@
 class TestEnsureSSHKeys(
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
 
-    print_msg = '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.\nAlternatively, you may create the keys on ' \
-                'your own.\n'
-    ssh_key_find = ('/usr/bin/find',
-                    os.path.join(os.path.expanduser('~'), '.ssh'),
-                    '-name', 'id_*.pub')
-
-    def patch_raw_input(self, return_value='N'):
+    print_msg = 'No SSH keys were found in ~/.ssh\n\nTo proceed, quickstart ' \
+                'can automatically create keys for you [a], or it can give ' \
+                'you a command to run yourself [s] in another terminal or ' \
+                'window to create the keys.\n\nNote that, if you want ' \
+                'quickstart to create your key, the underlying program ' \
+                '(ssh-keygen) will ask for a passphrase, but quickstart ' \
+                'will not have access to it.\n\n'
+
+    def patch_check_ssh_keys(self, return_value=False):
+        mock_check_ssh_keys = mock.Mock(return_value=return_value)
+        return mock.patch('quickstart.app.check_ssh_keys', mock_check_ssh_keys)
+
+    def patch_raw_input(self, return_value='C'):
         mock_raw_input = mock.Mock(return_value=return_value)
         return mock.patch('__builtin__.raw_input', mock_raw_input)
 
     def test_success(self, mock_print):
-        with self.patch_call(0) as mock_call:
+        with self.patch_check_ssh_keys(return_value=True) as mock_check:
             app.ensure_ssh_keys()
-        mock_call.assert_called_with(*self.ssh_key_find)
+        self.assertTrue(mock_check.called)
 
     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' \
-              'ssh-keygen -b 4096 -t rsa'
-        with self.assert_program_exit(msg):
-            with self.patch_call(130) as mock_call:
+        msg = 'If you would like to create the keys yourself, please run ' \
+              'this command, follow its instructions, and then re-run ' \
+              'quickstart:\n\nssh-keygen -b 4096 -t rsa && ssh-agent'
+        with mock.patch('sys.exit') as mock_exit:
+            with self.patch_check_ssh_keys() as mock_check:
                 with self.patch_raw_input() as mock_raw_input:
                     app.ensure_ssh_keys()
-        mock_call.assert_called_with(*self.ssh_key_find)
+        self.assertTrue(mock_check.called)
         mock_print.assert_has_calls([mock.call(self.print_msg)])
         self.assertTrue(mock_raw_input.called)
+        mock_exit.assert_called_once_with(msg)
 
     def test_failure_with_keygen(self, mock_print):
-        with self.patch_call(130) as mock_call:
-            with self.patch_raw_input(return_value='Y') as mock_raw_input:
+        with self.patch_check_ssh_keys() as mock_check:
+            with self.patch_raw_input(return_value='A') as mock_raw_input:
                 with mock.patch('quickstart.app.create_ssh_keys') \
                         as mock_create_ssh_keys:
                     app.ensure_ssh_keys()
-        mock_call.assert_called_with(*self.ssh_key_find)
+        self.assertTrue(mock_check.called)
         mock_print.assert_has_calls([mock.call(self.print_msg)])
         self.assertTrue(mock_raw_input.called)
         self.assertTrue(mock_create_ssh_keys.called)
 
+    def test_failure_with_watch(self, mock_print):
+        with self.patch_check_ssh_keys() as mock_check:
+            with self.patch_raw_input(return_value='S') as mock_raw_input:
+                with mock.patch('quickstart.app.watch_for_ssh_keys') \
+                        as mock_watch_for_ssh_keys:
+                    app.ensure_ssh_keys()
+        self.assertTrue(mock_check.called)
+        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 TestCheckSSHKeys(
+        helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
+
+    def test_keys_and_agent(self):
+        with self.patch_call(0) as mock_call:
+            have_keys = app.check_ssh_keys()
+        mock_call.assert_called_once_with('/usr/bin/ssh-add', '-l')
+        self.assertTrue(have_keys)
+
+    def test_agent_no_keys_success(self):
+        side_effects = (
+            (1, 'The agent has no identities.\n', ''),
+            (0, '', ''),
+        )
+        with self.patch_multiple_calls(side_effects) as mock_call:
+            have_keys = app.check_ssh_keys()
+        mock_call.assert_has_calls([
+            mock.call('/usr/bin/ssh-add', '-l'),
+            mock.call('/usr/bin/ssh-add'),
+        ])
+        self.assertTrue(have_keys)
+
+    def test_agent_no_keys_failure(self):
+        side_effects = (
+            (1, 'The agent has no identities.\n', ''),
+            (1, 'Still no identities...', ''),
+        )
+        with self.patch_multiple_calls(side_effects) as mock_call:
+            have_keys = app.check_ssh_keys()
+        mock_call.assert_has_calls([
+            mock.call('/usr/bin/ssh-add', '-l'),
+            mock.call('/usr/bin/ssh-add'),
+        ])
+        self.assertFalse(have_keys)
+
+    def test_agent_bad_keys(self):
+        side_effects = (
+            (1, 'The agent has no identities.\n', ''),
+            (2, '', 'Oh no!'),
+        )
+        msg = 'error attempting to add ssh keys! Oh no!'
+        with self.assert_program_exit(msg):
+            with self.patch_multiple_calls(side_effects) as mock_call:
+                app.check_ssh_keys()
+        mock_call.assert_has_calls([
+            mock.call('/usr/bin/ssh-add', '-l'),
+            mock.call('/usr/bin/ssh-add'),
+        ])
+
+    def test_no_agent(self):
+        with self.patch_call(2) as mock_call:
+            have_keys = app.check_ssh_keys()
+        mock_call.assert_called_once_with('/usr/bin/ssh-add', '-l')
+        self.assertFalse(have_keys)
+
+
[e-mail]('time.sleep')
[e-mail]_print
+class TestWatchForSSHKeys(helpers.CallTestsMixin, unittest.TestCase):
+
+    def test_watch(self, mock_print, mock_sleep):
+        with mock.patch('quickstart.app.check_ssh_keys',
+                        mock.Mock(side_effect=(False, True))):
+            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 && ssh-agent\n'),
+            mock.call('.', end='')
+        ])
+        mock_sleep.assert_called_once_with(3)
+
+    def test_cancel(self, mock_print, mock_sleep):
+        with mock.patch('quickstart.app.check_ssh_keys',
+                        mock.Mock(side_effect=KeyboardInterrupt)):
+            with mock.patch('sys.exit') as mock_exit:
+                app.watch_for_ssh_keys()
+        mock_print.assert_called_once_with(
+            '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 && ssh-agent\n')
+        mock_exit.assert_called_once_with('\nquitting')
+
 
 @helpers.mock_print
 class TestCreateSSHKeys(
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
 
     def test_success(self, mock_print):
-        key_file = os.path.join(os.path.expanduser('~'), '.ssh', 'id_juju_rsa')
+        key_file = os.path.join(os.path.expanduser('~'), '.ssh', 'id_rsa')
         with self.patch_call(0) as mock_call:
             app.create_ssh_keys()
-        mock_call.assert_called_once_with('/usr/bin/ssh-keygen',
-                                          '-q', '-b', '4096', '-t', 'rsa',
-                                          '-C',
-                                          'Generated with Juju Quickstart',
-                                          '-f', key_file)
+        mock_call.assert_has_calls([
+            mock.call('/usr/bin/ssh-keygen',
+                      '-q', '-b', '4096', '-t', 'rsa',
+                      '-C',
+                      'Generated with Juju Quickstart',
+                      '-f', key_file),
+            mock.call('/usr/bin/ssh-agent')
+        ])
         mock_print.assert_called_with('a new ssh key was generated in {}'
                                       .format(key_file))
 
@@ -246,6 +348,14 @@
             with self.patch_call(1, error='Oh no!') as mock_call:
                 app.create_ssh_keys()
         self.assertTrue(mock_call.called)
+        with self.assert_program_exit('error starting ssh-agent!  Oh no!'):
+            side_effects = (
+                (0, '', ''),
+                (1, '', 'Oh no!'),
+            )
+            with self.patch_multiple_calls(side_effects) as mock_call:
+                app.create_ssh_keys()
+        self.assertTrue(mock_call.called)
 
 
 @helpers.mock_print

=== modified file 'quickstart/tests/test_utils.py'
--- quickstart/tests/test_utils.py	2013-12-16 23:52:37 +0000
+++ quickstart/tests/test_utils.py	2013-12-17 21:07:10 +0000
@@ -154,6 +154,37 @@
             utils.call('echo', 'we are the borg!')
 
 
[e-mail]_print
+class TestStartSSHAgent(helpers.CallTestsMixin, unittest.TestCase):
+
+    def test_success(self, mock_print):
+        out = 'SSH_AUTH_SOCK=/tmp/ssh-authsock/agent.21000; ' \
+              'export SSH_AUTH_SOCK;\n' \
+              'SSH_AGENT_PID=21001; export SSH_AGENT_PID;\n' \
+              'echo Agent pid 21001;'
+        with self.patch_call(0, out, '') as mock_call:
+            with mock.patch('os.putenv') as mock_putenv:
+                utils.start_ssh_agent()
+        mock_call.assert_called_once_with('/usr/bin/ssh-agent')
+        mock_putenv.assert_has_calls([
+            mock.call('SSH_AUTH_SOCK', '/tmp/ssh-authsock/agent.21000'),
+            mock.call('SSH_AGENT_PID', '21001'),
+        ])
+        mock_print.assert_called_once_with(
+            'ssh-agent has been started; it will run for the duration of '
+            'quickstart, but will be unavailable once quickstart finishes. '
+            'To interact with Juju or quickstart again after quickstart '
+            'finishes, please run the following in a terminal to start '
+            'ssh-agent:\n\n'
+            'eval `ssh-agent`')
+
+    def test_failure(self, mock_print):
+        with self.patch_call(1, 'Cannot start agent!', '') as mock_call:
+            with self.assertRaises(OSError):
+                utils.start_ssh_agent()
+        self.assertTrue(mock_call.called)
+
+
 @mock.patch('__builtin__.print', mock.Mock())
 class TestCheckGuiCharmUrl(unittest.TestCase):
 

=== modified file 'quickstart/utils.py'
--- quickstart/utils.py	2013-12-17 15:23:28 +0000
+++ quickstart/utils.py	2013-12-17 21:07:10 +0000
@@ -42,7 +42,9 @@
 import httplib
 import json
 import logging
+import os
 import pipes
+import re
 import socket
 import subprocess
 import urllib2
@@ -105,6 +107,23 @@
     return retcode, output.decode('utf-8'), error.decode('utf-8')
 
 
+def start_ssh_agent():
+    """Start an ssh-agent and propagate its environment variables"""
+    retcode, output, error = call('/usr/bin/ssh-agent')
+    if retcode != 0:
+        raise OSError(error.encode('utf-8'))
+    os.putenv('SSH_AUTH_SOCK',
+              re.search('SSH_AUTH_SOCK=([^;]+);', output).group(1))
+    os.putenv('SSH_AGENT_PID',
+              re.search('SSH_AGENT_PID=([^;]+);', output).group(1))
+    print('ssh-agent has been started; it will run for the duration of '
+          'quickstart, but will be unavailable once quickstart finishes. '
+          'To interact with Juju or quickstart again after quickstart '
+          'finishes, please run the following in a terminal to start '
+          'ssh-agent:\n\n'
+          'eval `ssh-agent`')
+
+
 def check_gui_charm_url(charm_url):
     """Print (to stdout or to logs) info and warnings about the charm URL."""
     print('charm URL: {}'.format(charm_url))