Merge lp:~bac/lpsetup/muerte_finis_init_host into lp:lpsetup

[Brad Crittenden]

Move everything from finish-init-host to update or install-lxc and remove finish-init-host.

Move 'make schema' to 'update' subcommand. Move 'make install' and the ownership change of /srv/launchpad.dev to be a final step of 'install-lxc'.

The function 'this_command' was changed to take an optional arbitrary command rather than always using sys.argv[0] to allow other commands to be invoked inside the lxc.

[Gary Poster (gary)]

Thank you, Brad. This looks good. It is nice to see so much deleted code!

I'm slightly concerned that this code in the new finish function...

206 + # Change owner of /srv/launchpad.dev/.
207 + pwd_database = pwd.getpwnam(user)
208 + os.chown('/srv/launchpad.dev/', pwd_database.pw_uid, pwd_database.pw_gid)

...won't work in some edge cases because it should be using the password database in the container, and not in the host. I can imagine that this edge case won't happen because we need to have the same user in the host and the container, and they need to share passwords. If that's the case, please add a comment to that effect; otherwise, please consider using the container's password database.

In this help text...

231 This is basically the same of running `init-lxc` and then, inside the \
232 - newly created container, `init-repo`, `update` and `finish-init-target`.
233 + newly created container, `init-repo` and `update`.

...I suggest that we instead change the text to say, "This is basically the same as running `init-lxc` and then, inside the newly created container, `lp-setup init-repo`, `lp-setup update` and `make install`." To be clear, that is four changes: s/the same of running/the same as running/, s/`init-repo`/`lp-setup init-repo`, s/`update`/`lp-setup update`/, and s/`finish-init-target`/`make install`.

Thank you!

review: Approve

[Launchpad QA Bot (lpqabot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Launchpad QA Bot (lpqabot)]

The attempt to merge lp:~bac/lpsetup/muerte_finis_init_host into lp:lpsetup failed. Below is the output from the failed tests.

./lpsetup/subcommands/install_lxc.py
      16: 'pwd' imported but unused

+ set -o errexit
++ grep -v distribute_setup.py
++ find . -name build -prune -o -name '*.py'
+ pyfiles='./setup.py
./lplxcip/tests/utils.py
./lplxcip/tests/__init__.py
./lplxcip/tests/test_lxcip.py
./lplxcip/tests/test_helpers.py
./lplxcip/tests/test_utils.py
./lplxcip/lxcip.py
./lpsetup/utils.py
./lpsetup/__init__.py
./lpsetup/handlers.py
./lpsetup/tests/utils.py
./lpsetup/tests/__init__.py
./lpsetup/tests/test_argparser.py
./lpsetup/tests/test_cli.py
./lpsetup/tests/test_handlers.py
./lpsetup/tests/subcommands/__init__.py
./lpsetup/tests/subcommands/test_version.py
./lpsetup/tests/subcommands/test_initrepo.py
./lpsetup/tests/subcommands/test_init_target.py
./lpsetup/tests/subcommands/test_smoke.py
./lpsetup/tests/integration/common.py
./lpsetup/tests/integration/test_install_lxc.py
./lpsetup/tests/integration/test_init_target.py
./lpsetup/tests/test_utils.py
./lpsetup/subcommands/__init__.py
./lpsetup/subcommands/install_lxc.py
./lpsetup/subcommands/initlxc.py
./lpsetup/subcommands/init_target.py
./lpsetup/subcommands/update.py
./lpsetup/subcommands/initrepo.py
./lpsetup/subcommands/version.py
./lpsetup/exceptions.py
./lpsetup/argparser.py
./lpsetup/settings.py
./lpsetup/cli.py'
+ pocketlint ./setup.py ./lplxcip/tests/utils.py ./lplxcip/tests/__init__.py ./lplxcip/tests/test_lxcip.py ./lplxcip/tests/test_helpers.py ./lplxcip/tests/test_utils.py ./lplxcip/lxcip.py ./lpsetup/utils.py ./lpsetup/__init__.py ./lpsetup/handlers.py ./lpsetup/tests/utils.py ./lpsetup/tests/__init__.py ./lpsetup/tests/test_argparser.py ./lpsetup/tests/test_cli.py ./lpsetup/tests/test_handlers.py ./lpsetup/tests/subcommands/__init__.py ./lpsetup/tests/subcommands/test_version.py ./lpsetup/tests/subcommands/test_initrepo.py ./lpsetup/tests/subcommands/test_init_target.py ./lpsetup/tests/subcommands/test_smoke.py ./lpsetup/tests/integration/common.py ./lpsetup/tests/integration/test_install_lxc.py ./lpsetup/tests/integration/test_init_target.py ./lpsetup/tests/test_utils.py ./lpsetup/subcommands/__init__.py ./lpsetup/subcommands/install_lxc.py ./lpsetup/subcommands/initlxc.py ./lpsetup/subcommands/init_target.py ./lpsetup/subcommands/update.py ./lpsetup/subcommands/initrepo.py ./lpsetup/subcommands/version.py ./lpsetup/exceptions.py ./lpsetup/argparser.py ./lpsetup/settings.py ./lpsetup/cli.py

[Launchpad QA Bot (lpqabot)]

The attempt to merge lp:~bac/lpsetup/muerte_finis_init_host into lp:lpsetup failed. Below is the output from the failed tests.

lxc.aa_profile = lxc-container-with-nesting
WARNING: this command will destroy the 'lpsetup-testing-lxc' environment (type: local).
This includes all machines, services, data, and other resources. Continue [y/N]**********************************************************************
* Checking test environment. *
**********************************************************************
**********************************************************************
* Setting up the test environment for LXC container. *
**********************************************************************
----------------------------------------------------------------------
check_call:
juju bootstrap -e lpsetup-testing-lxc
----------------------------------------------------------------------
**********************************************************************
None
**********************************************************************
**********************************************************************
* Cleaning up. *
**********************************************************************

**********************************************************************
* Run time: 0:00:45.895267 *
**********************************************************************

+ set -o errexit
++ find . -name build -prune -o -name '*.py'
++ grep -v distribute_setup.py
+ pyfiles='./setup.py
./lplxcip/tests/utils.py
./lplxcip/tests/__init__.py
./lplxcip/tests/test_lxcip.py
./lplxcip/tests/test_helpers.py
./lplxcip/tests/test_utils.py
./lplxcip/lxcip.py
./lpsetup/utils.py
./lpsetup/__init__.py
./lpsetup/handlers.py
./lpsetup/tests/utils.py
./lpsetup/tests/__init__.py
./lpsetup/tests/test_argparser.py
./lpsetup/tests/test_cli.py
./lpsetup/tests/test_handlers.py
./lpsetup/tests/subcommands/__init__.py
./lpsetup/tests/subcommands/test_version.py
./lpsetup/tests/subcommands/test_initrepo.py
./lpsetup/tests/subcommands/test_init_target.py
./lpsetup/tests/subcommands/test_smoke.py
./lpsetup/tests/integration/common.py
./lpsetup/tests/integration/test_install_lxc.py
./lpsetup/tests/integration/test_init_target.py
./lpsetup/tests/test_utils.py
./lpsetup/subcommands/__init__.py
./lpsetup/subcommands/install_lxc.py
./lpsetup/subcommands/initlxc.py
./lpsetup/subcommands/init_target.py
./lpsetup/subcommands/update.py
./lpsetup/subcommands/initrepo.py
./lpsetup/subcommands/version.py
./lpsetup/exceptions.py
./lpsetup/argparser.py
./lpsetup/settings.py
./lpsetup/cli.py'
+ pocketlint ./setup.py ./lplxcip/tests/utils.py ./lplxcip/tests/__init__.py ./lplxcip/tests/test_lxcip.py ./lplxcip/tests/test_helpers.py ./lplxcip/tests/test_utils.py ./lplxcip/lxcip.py ./lpsetup/utils.py ./lpsetup/__init__.py ./lpsetup/handlers.py ./lpsetup/tests/utils.py ./lpsetup/tests/__init__.py ./lpsetup/tests/test_argparser.py ./lpsetup/tests/test_cli.py ./lpsetup/tests/test_handlers.py ./lpsetup/tests/subcommands/__init__.py ./lpsetup/tests/subcommands/t...

[Brad Crittenden (bac)]

The previous failure is curious. Retrying.

=== modified file 'commands.rst'
--- commands.rst	2012-08-15 20:50:45 +0000
+++ commands.rst	2012-08-24 11:57:42 +0000
@@ -6,7 +6,7 @@
 
 usage: lp-setup [-h]
 
-                {finish-init-target,init-target,init-lxc,init-repo,install-lxc,update,version,help}
+                {init-target,init-lxc,init-repo,install-lxc,update,version,help}
                 ...
 
 Create and update Launchpad development and testing environments.
@@ -15,10 +15,7 @@
   -h, --help            show this help message and exit
 
 subcommands:
-  {finish-init-target,init-target,init-lxc,init-repo,install-lxc,update,version,help}
                         Each subcommand accepts --h or --help to describe it.
-    finish-init-target  Finish the initialization of a Launchpad development
-                        target.
     init-target         Prepare a machine to run Launchpad. May be an LXC
                         container or not.
     init-lxc            Create an LXC container suitable for later installing
@@ -108,21 +105,6 @@
 Run as the user in the Launchpad target, either a host machine or LXC
 container.
 
-finish-init-target
-~~~~~~~~~~~~~~~~~~
-
-Finish the initialization of a Launchpad development host.
-
-Perform all tasks required to be run to initialize a Launchpad
-development host that need to be done after the Launchpad tree has
-been retrieved and run as root.
-
-Run as root in the Launchpad target, either a host machine or LXC
-container.  Must be done after `init-target` and `init-repo` have
-completed.
-
-TODO: rename to finish-init-target
-
 update
 ~~~~~~
 

=== modified file 'lpsetup/cli.py'
--- lpsetup/cli.py	2012-08-23 13:59:29 +0000
+++ lpsetup/cli.py	2012-08-24 11:57:42 +0000
@@ -20,7 +20,6 @@
     exceptions,
     )
 from lpsetup.subcommands import (
-    finish_init_target,
     init_target,
     initlxc,
     initrepo,
@@ -38,7 +37,6 @@
     ('init-target', init_target.SubCommand()),
     ('init-repo', initrepo.SubCommand()),
     ('update', update.SubCommand()),
-    ('finish-init-target', finish_init_target.SubCommand()),
     ('install-lxc', install_lxc.SubCommand()),
     ('version', version.SubCommand()),
     )

=== removed file 'lpsetup/subcommands/finish_init_target.py'
--- lpsetup/subcommands/finish_init_target.py	2012-08-23 09:05:38 +0000
+++ lpsetup/subcommands/finish_init_target.py	1970-01-01 00:00:00 +0000
@@ -1,99 +0,0 @@
-#!/usr/bin/env python
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Finish the initialization of a Launchpad development host.
-
-Perform all tasks required to be run to initialize a Launchpad development
-host that need to be done 1) after the Launchpad tree has been retrieved and
-2) run as root.
-
-This subcommand is slated for eventual removal.
-"""
-
-__metaclass__ = type
-__all__ = [
-    'setup_launchpad',
-    'SubCommand',
-    ]
-
-import os
-import pwd
-
-from shelltoolbox import (
-    cd,
-    get_su_command,
-    )
-
-from lpsetup import argparser
-from lpsetup.handlers import (
-    handle_user,
-    handle_target_dir,
-    )
-from lpsetup.utils import call
-
-
-def setup_launchpad(user, target_dir):
-    """Set up the Launchpad environment."""
-
-    # Make and install launchpad.
-    with cd(target_dir):
-        # Using real su because mailman make script uses uid.
-        call(*get_su_command(user, ['make', 'schema']))
-        call('make', 'install')
-
-    # Change owner of /srv/launchpad.dev/.
-    pwd_database = pwd.getpwnam(user)
-    os.chown('/srv/launchpad.dev/', pwd_database.pw_uid, pwd_database.pw_gid)
-
-setup_launchpad.description = """Set up the Launchpad database: this will \
-    destroy any other Postgres db!
-    Make and install Launchpad (buildout dependencies, Apache configuration).
-"""
-
-
-class SubCommand(argparser.StepsBasedSubCommand):
-    """Finish the initialization of a Launchpad development host.
-
-    Run the commands as root in the Launchpad target machine that must be done
-    after the Launchpad tree is available.
-
-    This subcommand is slated for eventual removal.
-    """
-
-    help = __doc__
-    root_required = True
-    steps = (
-        (setup_launchpad, ['user', 'target_dir']),
-        )
-    handlers = (handle_user, handle_target_dir)
-
-    epilog = """`%(prog)s` finalizes the Launchpad environment set up.
-        This subcommand must be run once the Launchpad code is retrieved.
-
-        \tcd mybranch
-        \t$ %(prog)s
-
-        This is completely equivalent to the following command:
-
-        \t$ %(prog)s mybranch
-
-        A user name is required to set up the Launchpad database, and by \
-        default the current system user name is used. If instead this \
-        subcommand is run as root, the user name needs to be explicitly \
-        specified:
-
-        \t$ %(prog)s -u myuser
-    """
-
-    def add_arguments(self, parser):
-        super(SubCommand, self).add_arguments(parser)
-        parser.add_argument(
-            'target_dir', nargs='?', default=os.getcwd(),
-            help='The directory of the Launchpad code checkout. '
-                 '[DEFAULT=current directory]')
-        parser.add_argument(
-            '-u', '--user',
-            help='The name of the system user.  '
-                 'The current user is used if this script is not run as '
-                 'root and this argument is omitted.')

=== modified file 'lpsetup/subcommands/install_lxc.py'
--- lpsetup/subcommands/install_lxc.py	2012-08-23 13:26:47 +0000
+++ lpsetup/subcommands/install_lxc.py	2012-08-24 11:57:42 +0000
@@ -7,7 +7,6 @@
 __metaclass__ = type
 __all__ = [
     'create_scripts',
-    'finish_init_target_in_lxc',
     'init_repo_in_lxc',
     'SubCommand',
     'update_in_lxc',
@@ -127,13 +126,23 @@
     update.update_tree.description)
 
 
-def finish_init_target_in_lxc(
-    lxc_name, ssh_key_path, home_dir, user, target_dir):
-    args = ['finish-init-target', target_dir, '--user', user, '--yes']
-    cmd_in_lxc(lxc_name, ssh_key_path, home_dir, args)
-
-finish_init_target_in_lxc.description = """Set up the database, make and \
-    install Launchpad inside the LXC instance $lxc_name.
+def finish_install_in_lxc(
+    lxc_name, ssh_key_path, user, target_dir):
+    args = ['install']
+    cmd = this_command(target_dir, args, cmd='make')
+    ssh(lxc_name, cmd, key=ssh_key_path)
+
+    # Change owner of /srv/launchpad.dev/ by running the following in the
+    # container as root:
+    # % chown user:user /srv/launchpad.dev
+    args = ['{0}:{0}'.format(user), '/srv/launchpad.dev']
+    cmd = this_command('/', args, cmd='chown')
+    ssh(lxc_name, cmd, key=ssh_key_path)
+
+finish_install_in_lxc.description = """Install Launchpad and \
+    configure Apache.
+
+    Also change ownership of /srv/launchpad.dev to the user.
 """
 
 
@@ -151,8 +160,8 @@
         (update_in_lxc,
          ['lxc_name', 'ssh_key_path', 'home_dir', 'user', 'external_path',
           'target_dir', 'lp_source_deps', 'use_http']),
-        (finish_init_target_in_lxc,
-         ['lxc_name', 'ssh_key_path', 'home_dir', 'user', 'target_dir']),
+        (finish_install_in_lxc,
+         ['lxc_name', 'ssh_key_path', 'user', 'target_dir']),
         # Run on host:
         initlxc.SubCommand.stop_lxc_step,
         )
@@ -161,8 +170,9 @@
     epilog = """`%(prog)s` sets up a complete Launchpad environment in an LXC \
     container, creating the LXC, configuring the instance, retrieving the code.
 
-    This is basically the same of running `init-lxc` and then, inside the \
-    newly created container, `init-repo`, `update` and `finish-init-target`.
+    This is basically the same as running `init-lxc` and then, inside the \
+    newly created container, `lp-setup init-repo`, `lp-setup update`, and
+    `make install`.
 
     Set up Launchpad for the current user:
 

=== modified file 'lpsetup/subcommands/update.py'
--- lpsetup/subcommands/update.py	2012-08-23 08:45:40 +0000
+++ lpsetup/subcommands/update.py	2012-08-24 11:57:42 +0000
@@ -16,6 +16,7 @@
 
 from shelltoolbox import (
     cd,
+    get_su_command,
     mkdirs,
     )
 
@@ -78,6 +79,17 @@
 """
 
 
+def make_schema(user, target_dir):
+    """Set up the Launchpad environment."""
+
+    # Make and install launchpad.
+    with cd(target_dir):
+        # Using real su because mailman make script uses uid.
+        call(*get_su_command(user, ['make', 'schema']))
+
+make_schema.description = """Run 'make schema' within the Launchpad tree."""
+
+
 class SubCommand(argparser.StepsBasedSubCommand):
     """Updates an existing Launchpad development environment.
 
@@ -90,6 +102,7 @@
         (update_dependencies,
            ['target_dir', 'external_path', 'use_http', 'lp_source_deps']),
         (update_tree, ['target_dir']),
+        (make_schema, ['user', 'target_dir']),
         )
     help = __doc__
     handlers = (

=== modified file 'lpsetup/tests/subcommands/test_smoke.py'
--- lpsetup/tests/subcommands/test_smoke.py	2012-08-15 20:50:45 +0000
+++ lpsetup/tests/subcommands/test_smoke.py	2012-08-24 11:57:42 +0000
@@ -27,7 +27,6 @@
         # and verify that a non-error exit code is returned.
         required_args = ['-f', 'Example User', '-E', '[e-mail]']
         name_args_map = {
-            'finish-init-target': [],
             'init-target': required_args,
             'init-lxc': required_args,
             'init-repo': [],

=== modified file 'lpsetup/tests/test_utils.py'
--- lpsetup/tests/test_utils.py	2012-08-10 09:16:56 +0000
+++ lpsetup/tests/test_utils.py	2012-08-24 11:57:42 +0000
@@ -348,3 +348,9 @@
         # Ensure arguments are correctly quoted.
         cmd = this_command(self.directory, ['-arg', 'quote me'])
         self.check_command(cmd, "-arg 'quote me'")
+
+    def test_arbitrary_command(self):
+        # Ensure the returned command can reference any arbitrary command.
+        self.script_name = 'ls'
+        cmd = this_command(self.directory, ['/tmp'], self.script_name)
+        self.check_command(cmd, '/tmp')

=== modified file 'lpsetup/utils.py'
--- lpsetup/utils.py	2012-08-13 14:27:59 +0000
+++ lpsetup/utils.py	2012-08-24 11:57:42 +0000
@@ -428,22 +428,33 @@
     return sshcall(cmd)
 
 
-def this_command(directory, args):
-    """Return a command line to re-launch this script using given `args`.
+def this_command(directory, args, cmd=None):
+    """Return a command line to run a command using given `args`.
 
-    The returned command will execute this script from `directory`::
+    The returned command will execute the command from `directory`.
+    If cmd is not specified, the invoking script is used::
 
         >>> script_name = sys.argv[0]
-
         >>> cmd = this_command('/home/user/lp/branches', ['install'])
         >>> cmd == 'cd /home/user/lp/branches && ' + script_name + ' install'
         True
 
+        >>> script_name = 'ls'
+        >>> cmd = this_command('/home/user/lp/branches',
+        ...                    ['install'], script_name)
+        >>> cmd == 'cd /home/user/lp/branches && ' + script_name + ' install'
+        True
+
     Arguments are correctly quoted::
 
+        >>> script_name = sys.argv[0]
         >>> cmd = this_command('/path', ['-arg', 'quote me'])
         >>> cmd == ('cd /path && ' + script_name + " -arg 'quote me'")
         True
     """
-    script = join_command(sys.argv[:1] + list(args))
+    if cmd is None:
+        cmd = sys.argv[:1]
+    if isinstance(cmd, basestring):
+        cmd = [cmd]
+    script = join_command(cmd + list(args))
     return join_command(['cd', directory]) + ' && ' + script