Merge lp:~frankban/lpsetup/create-scripts into lp:lpsetup

[Francesco Banconi]

== Changes ==

lpsetup is now capable to generate the scripts used by buildbot in parallel tests. A new step `create_scripts` is now part of the lxc-install sub command.
Since the script generation is probably only needed by buildbot, this step is only activated if --create-scripts argument is passed to lxc-install.
The script templates are present in the `templates` directory of the lpsetup package: theese templates are used to actually render the scripts (usually saving them in /usr/local/bin).

The Scrubber class is now separated from the cleanup script, and the tests Brad wrote are integrated in lpsetup, with some fixes:
- Scrubber now takes the lxc name to dynamically generate the directory patterns used to clean up the file system.
- I've found that, having containers running in my machine, Scrubber tests failed because I need to be root to run lxc-info. Vice versa, running the tests as root, they passed but my containers were killed. To work around this, I've added an optional argument `ignored_containers` to the Scrubber's __init__.
- PEP8 compliant naming for the Scrubber methods.

Updated the (naive) method used to retrieve the user home directory in a test.

== Tests ==

$ bin/test
Running zope.testrunner.layer.UnitTests tests:
  Set up zope.testrunner.layer.UnitTests in 0.000 seconds.
  Ran 46 tests with 0 failures and 0 errors in 0.469 seconds.
Tearing down left over layers:
  Tear down zope.testrunner.layer.UnitTests in 0.000 seconds.

[Brad Crittenden (bac)]

Hi Francesco,

This isn't a full review as Graham is doing it. I would like to comment on your decision to use PEP-8 naming for methods.

For Launchpad-related projects such as this one I think we should continue to follow the Launchpad style guide: https://dev.launchpad.net/PythonStyleGuide

Launchpad has agreed to use camelCase for method names as an exception to the PEP-8 guide.

I'd like for us to either continue following the LP style guide or agree not to.

[Francesco Banconi (frankban)]

Thanks for your comment Brad, but now I'm confused: of course lpsetup is a launchpad related project, but it's not lp itself, it's a separate branch/project, so I thought consistency with the launchpad code style was not a problem. Indeed right now lpsetup uses PEP-8 naming convention everywhere in the code.
However, there is a high possibility that I was wrong at the time (in my interpretation of what is "launchpad related") and there is no problem in replacing method names if we decide so.

[Graham Binns (gmb)]

Context for latecomers: After conversation with Gary, it's been decided to leave the PEP8 naming in place, since it's common throughout lpsetup.

review: Approve (code)

=== modified file 'lpsetup/settings.py'
--- lpsetup/settings.py	2012-03-30 10:47:23 +0000
+++ lpsetup/settings.py	2012-04-11 10:42:23 +0000
@@ -61,6 +61,12 @@
 LXC_GUEST_ARCH = 'i386'
 LXC_GUEST_CHOICES = ('lucid', 'oneiric', 'precise')
 LXC_GUEST_OS = LXC_GUEST_CHOICES[0]
+LXC_LEASES = (
+    '/var/lib/dhcp3/dhclient.eth0.leases',
+    '/var/lib/dhcp/dhclient.eth0.leases',
+    )
+LXC_LP_DIR_PATTERN = '/tmp/lxc-lp-*'
+LXC_LP_TEST_DIR_PATTERN = '/var/lib/lxc/{lxc_name}-tmp-*'
 LXC_NAME = 'lptests'
 LXC_OPTIONS = """
 lxc.network.type = veth
@@ -70,4 +76,5 @@
 LXC_PACKAGES = ['lxc', 'libvirt-bin']
 LXC_PATH = '/var/lib/lxc/'
 RESOLV_FILE = '/etc/resolv.conf'
+SCRIPTS = ('lp-setup-lxc-build', 'lp-setup-lxc-cleanup', 'lp-setup-lxc-test')
 SSH_KEY_NAME = 'id_rsa'

=== modified file 'lpsetup/subcommands/lxcinstall.py'
--- lpsetup/subcommands/lxcinstall.py	2012-04-10 09:22:25 +0000
+++ lpsetup/subcommands/lxcinstall.py	2012-04-11 10:42:23 +0000
@@ -7,6 +7,7 @@
 __metaclass__ = type
 __all__ = [
     'create_lxc',
+    'create_scripts',
     'initialize_lxc',
     'SubCommand',
     'setup_launchpad_lxc',
@@ -36,10 +37,12 @@
     LXC_GUEST_ARCH,
     LXC_GUEST_CHOICES,
     LXC_GUEST_OS,
+    LXC_LEASES,
     LXC_NAME,
     LXC_OPTIONS,
     LXC_PACKAGES,
     RESOLV_FILE,
+    SCRIPTS,
     )
 from lpsetup.subcommands import install
 from lpsetup.utils import (
@@ -47,10 +50,46 @@
     get_container_path,
     get_lxc_gateway,
     lxc_stopped,
+    render_to_file,
     this_command,
     )
 
 
+def create_scripts(user, lxc_name, ssh_key_path):
+    """Create scripts to update the Launchpad environment and run tests."""
+    leases1, leases2 = [get_container_path(lxc_name, i) for i in LXC_LEASES]
+    context = {
+        'leases1': leases1,
+        'leases2': leases2,
+        'lxc_name': lxc_name,
+        'pattern':
+            r's/.* ([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}).*/\1/',
+        'ssh_key_path': ssh_key_path,
+        'user': user,
+        }
+    scripts = []
+    # Generate the scripts.
+    for template_name in SCRIPTS:
+        dest = os.path.join(os.path.sep, 'usr', 'local', 'bin', template_name)
+        render_to_file(template_name, context, dest)
+        os.chmod(dest, 0555)
+        scripts.append(dest)
+    # Add a file to sudoers.d that will let the buildbot user run the above.
+    sudoers_file = '/etc/sudoers.d/launchpad-' + user
+    sudoers_contents = '{user} ALL = (ALL) NOPASSWD: {scripts}\n'.format(
+        user=user, scripts=', '. join(scripts))
+    with open(sudoers_file, 'w') as sudoers:
+        sudoers.write(sudoers_contents)
+    # The sudoers must have this mode or it will be ignored.
+    os.chmod(sudoers_file, 0440)
+    # XXX 2012-03-13 frankban bug=944386:
+    #     Disable hardlink restriction. This workaround needs
+    #     to be removed once the kernel bug is resolved.
+    procfile = '/proc/sys/kernel/yama/protected_nonaccess_hardlinks'
+    with open(procfile, 'w') as f:
+        f.write('0\n')
+
+
 def create_lxc(user, lxc_name, lxc_arch, lxc_os):
     """Create the LXC named `lxc_name` sharing `user` home directory.
 
@@ -159,6 +198,8 @@
          'user', 'full_name', 'email', 'lpuser',
          'private_key', 'public_key', 'valid_ssh_keys', 'ssh_key_path',
          'use_urandom', 'dependencies_dir', 'directory'),
+        (create_scripts,
+         'user', 'lxc_name', 'ssh_key_path'),
         (create_lxc,
          'user', 'lxc_name', 'lxc_arch', 'lxc_os'),
         (start_lxc,
@@ -177,6 +218,11 @@
     ssh_key_name_help = ('The ssh key name used to connect to Launchpad '
                          'and to to the LXC container.')
 
+    def call_create_scripts(self, namespace, step, args):
+        """Run the `create_scripts` step only if the related flag is set."""
+        if namespace.create_scripts:
+            return step(*args)
+
     def add_arguments(self, parser):
         super(SubCommand, self).add_arguments(parser)
         parser.add_argument(
@@ -192,3 +238,6 @@
             choices=LXC_GUEST_CHOICES,
             help='The LXC container distro codename. '
                  '[DEFAULT={0}]'.format(LXC_GUEST_OS))
+        parser.add_argument(
+            '-C', '--create-scripts', action='store_true',
+            help='Create the scripts used by buildbot for parallel testing.')

=== added directory 'lpsetup/templates'
=== added file 'lpsetup/templates/lp-setup-lxc-build'
--- lpsetup/templates/lp-setup-lxc-build	1970-01-01 00:00:00 +0000
+++ lpsetup/templates/lp-setup-lxc-build	2012-04-11 10:42:23 +0000
@@ -0,0 +1,48 @@
+#!/bin/sh
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Run the Launchpad build inside LXC.
+
+# Assumptions:
+#  * This script is run as root.
+
+set -ux
+truncate -c -s0 {leases1}
+truncate -c -s0 {leases2}
+
+lxc-start -n {lxc_name} -d
+lxc-wait -n {lxc_name} -s RUNNING
+
+delay=30
+while [ "$delay" -gt 0 -a ! -s {leases1} -a ! -s {leases2} ]
+do
+    delay=$(( $delay - 1 ))
+    sleep 1
+done
+
+[ -s {leases1} ] && LEASES={leases1} || LEASES={leases2}
+IP_ADDRESS=`grep fixed-address $LEASES | tail -n 1 | sed -r '{pattern}'`
+
+if [ 0 -eq $? -a -n "$IP_ADDRESS" ]; then
+    for i in $(seq 1 30); do
+        su {user} -c "/usr/bin/ssh -o StrictHostKeyChecking=no \
+            -i '{ssh_key_path}' $IP_ADDRESS make -C $PWD schema"
+        if [ ! 255 -eq $? ]; then
+            # If ssh returns 255 then its connection failed.
+            # Anything else is either success (status 0) or a
+            # failure from whatever we ran over the SSH connection.
+            # In those cases we want to stop looping, so we break
+            # here.
+            break;
+        fi
+        sleep 1
+    done
+else
+    echo "could not get IP address - aborting." >&2
+    echo "content of $LEASES:" >&2
+    cat $LEASES >&2
+fi
+
+lxc-stop -n {lxc_name}
+lxc-wait -n {lxc_name} -s STOPPED

=== added file 'lpsetup/templates/lp-setup-lxc-cleanup'
--- lpsetup/templates/lp-setup-lxc-cleanup	1970-01-01 00:00:00 +0000
+++ lpsetup/templates/lp-setup-lxc-cleanup	2012-04-11 10:42:23 +0000
@@ -0,0 +1,33 @@
+#!/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).
+
+# Cleanup remnants of LXC containers from previous runs.
+
+# Runs of LXC may leave cruft laying around that interferes with
+# the next run.  These items need to be cleaned up.
+
+# 1) Shut down all running containers
+
+# 2) for every /var/lib/lxc/{lxc_name}-tmp-* directory:
+#      umount [directory]/ephemeralbind
+#      umount [directory]
+#      rm -rf [directory]
+
+# 3) for every /tmp/lxc-lp-* (or something like that?) directory:
+#      umount [directory]
+#      rm -rf [directory]
+
+# Assumptions:
+#  * This script is run as root.
+
+from lpsetup.utils import Scrubber
+
+
+USER = '{user}'
+LXC_NAME = '{lxc_name}'
+
+
+if __name__ == '__main__':
+    scrubber = Scrubber(USER, LXC_NAME)
+    scrubber.run()

=== added file 'lpsetup/templates/lp-setup-lxc-test'
--- lpsetup/templates/lp-setup-lxc-test	1970-01-01 00:00:00 +0000
+++ lpsetup/templates/lp-setup-lxc-test	2012-04-11 10:42:23 +0000
@@ -0,0 +1,20 @@
+#!/bin/sh
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Test Launchpad using LXC ephemeral instances.
+
+# Assumptions:
+#  * This script is run as root.
+
+# We intentionally generate a very long line for the
+# lxc-start-ephemeral command below because ssh does not propagate
+# quotes the way we want.  E.g.,
+#     touch a; touch b; ssh localhost -- ls "a b"
+# succeeds, when it should say that the file "a b" does not exist.
+
+set -uex
+lxc-start-ephemeral -u {user} -S '{ssh_key_path}' -o {lxc_name} -- \
+    "xvfb-run --error-file=/var/tmp/xvfb-errors.log \
+    --server-args='-screen 0 1024x768x24' \
+    -a $PWD/bin/test --shuffle --subunit $@"

=== modified file 'lpsetup/tests/test_handlers.py'
--- lpsetup/tests/test_handlers.py	2012-03-16 16:49:23 +0000
+++ lpsetup/tests/test_handlers.py	2012-04-11 10:42:23 +0000
@@ -7,6 +7,7 @@
 import argparse
 from contextlib import contextmanager
 import getpass
+import pwd
 import unittest
 
 from lpsetup.exceptions import ValidationError
@@ -157,7 +158,7 @@
         username = getpass.getuser()
         namespace = argparse.Namespace(user=username)
         handle_user(namespace)
-        self.assertEqual('/home/' + username, namespace.home_dir)
+        self.assertEqual(pwd.getpwnam(username).pw_dir, namespace.home_dir)
 
     def test_failure(self):
         # Ensure the validation fails if the current user is root and

=== modified file 'lpsetup/tests/test_utils.py'
--- lpsetup/tests/test_utils.py	2012-03-23 11:50:11 +0000
+++ lpsetup/tests/test_utils.py	2012-04-11 10:42:23 +0000
@@ -4,11 +4,25 @@
 
 """Tests for the utils module."""
 
+import getpass
+import os
+import shutil
 import sys
+import tempfile
 import unittest
 
+from shelltoolbox import run
+
+from lpsetup.settings import (
+    LXC_LP_DIR_PATTERN,
+    LXC_LP_TEST_DIR_PATTERN,
+    LXC_NAME,
+    )
 from lpsetup.utils import (
     get_container_path,
+    get_running_containers,
+    render_to_file,
+    Scrubber,
     this_command,
     )
 
@@ -31,6 +45,99 @@
             get_container_path('mycontainer', 'home'))
 
 
+class GetRunningContainersTest(unittest.TestCase):
+
+    def assertRunning(self, expected, containers):
+        running = get_running_containers(containers)
+        self.assertItemsEqual(expected, list(running))
+
+    def test_running_container(self):
+        # Ensure one container is returned if running.
+        self.assertRunning(['c1'], ['c1', 'c2', 'c1'])
+
+    def test_multiple_running_containers(self):
+        # Ensure multple running containers are correctly reported.
+        self.assertRunning(['c1', 'c3'], ['c1', 'c2', 'c3', 'c3', 'c1'])
+
+    def test_no_containers_running(self):
+        # Ensure an empty iterable is returned if no containers are running.
+        self.assertRunning([], ['c1', 'c2', 'c3'])
+
+
+class RenderToFileTest(unittest.TestCase):
+
+    def setUp(self):
+        self.dest = tempfile.mktemp()
+        template_path = tempfile.mktemp()
+        self.templates_dir, self.template_name = os.path.split(template_path)
+        self.template = '{var1}, {var2}'
+        with open(template_path, 'w') as f:
+            f.write(self.template)
+
+    def test_render_to_file(self):
+        # Ensure the template is correctly rendered using the given context.
+        context = {'var1': 'foo', 'var2': 'bar'}
+        contents = render_to_file(
+            self.template_name, context, self.dest,
+            templates_dir=self.templates_dir)
+        expected = self.template.format(**context)
+        self.assertEqual(expected, contents)
+        with open(self.dest) as f:
+            self.assertEqual(expected, f.read())
+
+
+class TestScrubber(unittest.TestCase):
+
+    def make_dirs(self, tmp_path, pattern, num=10, extra=None):
+        dir_ = os.path.split(pattern)[0]
+        path = tmp_path + dir_
+        os.makedirs(path)
+        for i in xrange(num):
+            tmp_dirname = pattern.replace('*', str(i))
+            dirname = tmp_path + tmp_dirname
+            os.makedirs(dirname)
+            if extra is not None:
+                os.makedirs(os.path.join(dirname, extra))
+        return path
+
+    def setUp(self):
+        self.tmp = tempfile.mkdtemp()
+        test_dir_pattern = LXC_LP_TEST_DIR_PATTERN.format(lxc_name=LXC_NAME)
+        self.lp_pattern = self.tmp + test_dir_pattern
+        self.lxc_pattern = self.tmp + LXC_LP_DIR_PATTERN
+        self.lp_dir = self.make_dirs(
+            self.tmp, test_dir_pattern)
+        self.lxc_dir = self.make_dirs(
+            self.tmp, LXC_LP_DIR_PATTERN)
+        self.user = getpass.getuser()
+
+    def tearDown(self):
+        if os.path.exists(self.tmp):
+            shutil.rmtree(self.tmp)
+
+    def test_scrub(self):
+        scrubber = Scrubber(self.user, LXC_NAME, self.lp_pattern,
+            self.lxc_pattern, get_running_containers())
+        scrubber.run()
+        self.assertEqual([], os.listdir(self.lp_dir))
+        self.assertEqual([], os.listdir(self.lxc_dir))
+
+    def test_scrub_no_dir(self):
+        # Running the scrubber when no directories to be scrubbed exist does
+        # not cause an error.
+        shutil.rmtree(self.tmp)
+        scrubber = Scrubber(self.user, LXC_NAME, self.lp_pattern,
+            self.lxc_pattern, get_running_containers())
+        scrubber.run()
+
+    def test_idempotent(self):
+        # Running the scrubber multiple times does not cause an error.
+        scrubber = Scrubber(self.user, LXC_NAME, self.lp_pattern,
+            self.lxc_pattern, get_running_containers())
+        scrubber.run()
+        scrubber.run()
+
+
 class ThisCommandTest(unittest.TestCase):
 
     script_name = sys.argv[0]

=== modified file 'lpsetup/utils.py'
--- lpsetup/utils.py	2012-03-23 11:50:11 +0000
+++ lpsetup/utils.py	2012-04-11 10:42:23 +0000
@@ -12,13 +12,18 @@
     'lxc_in_state',
     'lxc_running',
     'lxc_stopped',
+    'render_to_file',
+    'Scrubber',
     'this_command',
     ]
 
 from functools import partial
+import glob
 import os
 import platform
+import re
 import subprocess
+import shutil
 import sys
 import time
 
@@ -27,9 +32,14 @@
     run,
     )
 
-from lpsetup.settings import LXC_PATH
-
-
+from lpsetup.settings import (
+    LXC_LP_DIR_PATTERN,
+    LXC_LP_TEST_DIR_PATTERN,
+    LXC_PATH,
+    )
+
+
+PID_RE = re.compile("pid:\s+(\d+)")
 call = partial(run, stdout=None)
 
 
@@ -61,6 +71,18 @@
         return 'lxcbr0', '10.0.3.1'
 
 
+def get_running_containers(containers=None):
+    """Return an iterable of currently running LXC container's names."""
+    if containers is None:
+        containers = run('lxc-ls').split()
+    visited = {}
+    for container in containers:
+        if container in visited:
+            yield container
+        else:
+            visited[container] = 1
+
+
 def lxc_in_state(state, lxc_name, timeout=30):
     """Return True if the LXC named `lxc_name` is in state `state`.
 
@@ -83,6 +105,108 @@
 lxc_stopped = partial(lxc_in_state, 'STOPPED')
 
 
+def render_to_file(template_name, context, dest, templates_dir=None):
+    """Render `template_name` using `context`. Write the result inside `dest`.
+
+    The argument `template_dir` is the directory containing the template:
+    if None, the `templates` directory inside the lpsetup package is used.
+
+    Return the rendered contents.
+    """
+    if templates_dir is None:
+        templates_dir = os.path.join(os.path.dirname(__file__), 'templates')
+    with open(os.path.join(templates_dir, template_name)) as source:
+        template = source.read()
+    contents = template.format(**context)
+    with open(dest, 'w') as destination:
+        destination.write(contents)
+    return contents
+
+
+class Scrubber(object):
+    """Scrubber will cleanup after lxc ephemeral uncleanliness.
+
+    All running containers will be killed, except for the ones listed
+    in the `ignored_containers` iterable.
+
+    Those directories corresponding the lp_test_dir_pattern will
+    be unmounted and removed.  The 'ephemeralbind' subdirectories
+    will be unmounted.
+
+    The directories corresponding to the lxc_lp_dir_pattern will
+    be unmounted and removed.  No subdirectories will need
+    unmounting.
+    """
+    def __init__(self, user, lxc_name,
+                 lp_test_dir_pattern=LXC_LP_TEST_DIR_PATTERN,
+                 lxc_lp_dir_pattern=LXC_LP_DIR_PATTERN,
+                 ignored_containers=None):
+        if ignored_containers is None:
+            self.ignored_containers = []
+        else:
+            self.ignored_containers = list(ignored_containers)
+        self.lp_test_dir_pattern = lp_test_dir_pattern.format(
+            lxc_name=lxc_name)
+        self.lxc_lp_dir_pattern = lxc_lp_dir_pattern
+        self.user = user
+
+    def umount(self, dir_):
+        if os.path.ismount(dir_):
+            run("umount", dir_)
+
+    def scrubdir(self, dir_, extra):
+        dirs = [dir_]
+        if extra is not None:
+            dirs.insert(0, os.path.join(dir_, extra))
+        for d in dirs:
+            self.umount(d)
+        shutil.rmtree(dir_)
+
+    def scrub(self, pattern, extra=None):
+        for dir_ in glob.glob(pattern):
+            if os.path.isdir(dir_):
+                self.scrubdir(dir_, extra)
+
+    def get_pid(self, container):
+        info = run("lxc-info", "-n", container)
+        # lxc-info returns a string containing 'RUNNING' for those
+        # containers that are running followed by 'pid: <pid>', so
+        # that must be parsed.
+        if 'RUNNING' in info:
+            match = PID_RE.search(info)
+            if match:
+                return match.group(1)
+        return None
+
+    def get_running_containers(self):
+        """Get the running containers.
+
+        Returns a list of (name, pid) tuples.
+        """
+        output = run("lxc-ls")
+        containers = set(output.split()).difference(self.ignored_containers)
+        pidlist = [(c, self.get_pid(c)) for c in containers]
+        return [(c,p) for c,p in pidlist if p is not None]
+
+    def killer(self):
+        """Kill all running ephemeral containers."""
+        pids = self.get_running_containers()
+        if len(pids) > 0:
+            # We can do this the easy way...
+            for name, pid in pids:
+                run("/usr/bin/lxc-stop", "-n", name)
+            time.sleep(2)
+            pids = self.get_running_containers()
+            # ...or, the hard way.
+            for name, pid in pids:
+                run("kill", "-9", pid)
+
+    def run(self):
+        self.killer()
+        self.scrub(self.lp_test_dir_pattern, "ephemeralbind")
+        self.scrub(self.lxc_lp_dir_pattern)
+
+
 def this_command(directory, args):
     """Return a command line to re-launch this script using given `args`.