Code review comment for lp:~yellow/launchpad/lxcsetup

The script looks great, don't let the wall of text below fool you. :)

All of the points below can be ignored if you so desire, except for the
last three paragraphs. All these things seemed important enough to
note, but not important enough to block on if you don't agree. The last
three paragraphs probably do need to be addressed though. I'm confident
you'll do that so I've approved the MP.

I lost my first draft of this, so I'm afraid this is more terse than I
would like.

Since this is a stand-alone script, we don't need to populate __all__.

The leading underscores in function names aren't needed (since we use
__all__ to document a module's exported names we don't have to ugly up
names that we don't want people to use).

We can use email.Utils.parseaddr instead of _parse_whoami.

I assume the "parser" argument for bzr_whois was intended as a
dependency injection point, but since it's not used it can be removed.

A small thing, but it seems to me that

    if not content.endswith('\n'):
        f.write('\n')
    f.write(line)

would be a bit easier to read than

    f.write(line if content.endswith('\n') else '\n{}'.format(line))

Since "ssh" doesn't perform any cleanup actions (i.e., there is no code
after the yield), it could just be a regular function.

There are a few instances of code like this:

    'Error running command: {}'.format(' '.join(sshcmd))

A slight simplification would be to use string concatenation instead:

    'Error running command: ' + ' '.join(sshcmd)

I don't understand the meaning of "clean" in _clean_users,
_clean_userdata, and _clean_ssh_keys. Maybe it's an assertion that they
are clean, in that case I'd expect the functions to be named
"validate...", but they do more than validation, so that doesn't sounnd
right either. Maybe "set" would be the right word ("set_directories",
etc.).

You can use email.Utils.formataddr instead of string operations in
initialize_host to construct the user's whois string.

The os.system call in initialize_host will do the wrong thing if
checkout_dir contains a space (or quotation mark, etc.).
subprocess.call might be a better choice there.

If a function_args_map action raises an exception in main, the exception
is returned (not reraised) and the return value of main is passed to
sys.exit which expects an integer. That doesn't seem quite right.
Perhaps main should just return 1 if an exception occurs.

The dance to restart as root deserves a couple lines of commentary
explaining that if we weren't started as root then we gather up all the
info we need about the user and then restart as root. (I really like
this feature, by the way).