launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/lpsetup/missing_lxc_1045728 into lp:lpsetup.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1045728 in lpsetup: "lxcip.py looks for liblxc.so.0 in the wrong directory on 12.10"
  https://bugs.launchpad.net/lpsetup/+bug/1045728

For more details, see:
https://code.launchpad.net/~jameinel/lpsetup/missing_lxc_1045728/+merge/122663

This is meant to address bug #1045728. It looks like 'lxc.so' has moved between Ubuntu versions. So we need to probe for it in multiple locations.

The fix is pretty straightforward, most of the churn from the patch is to make it testable so we understand *why* we try multiple locations, and avoid regressing in the future. (Also allows us to rip out some of the code when P becomes obsolete many years from now.)
-- 
https://code.launchpad.net/~jameinel/lpsetup/missing_lxc_1045728/+merge/122663
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/lpsetup/missing_lxc_1045728 into lp:lpsetup.

=== modified file 'lplxcip/lxcip.py'
--- lplxcip/lxcip.py	2012-05-09 14:20:51 +0000
+++ lplxcip/lxcip.py	2012-09-04 11:59:38 +0000
@@ -169,10 +169,22 @@
 
     Raise OSError if LXC is not installed or the container is not found.
     """
+    return _get_pid(name)
+
+
+def _get_pid(name, loader=None):
+    """Return the pid of an LXC.
+
+    This is a private implementation detail, to allow for testing with a custom
+    loader.
+    """
     try:
-        liblxc = _load_library('lxc/liblxc.so.0')
+        liblxc = _load_library('lxc/liblxc.so.0', loader=loader)
     except OSError:
-        raise _error('not_installed')
+        try:
+            liblxc = _load_library('liblxc.so.0', loader=loader)
+        except OSError, e:
+            raise _error('not_installed')
     get_init_pid = _wrap(liblxc.get_init_pid, 'not_found')
     # Redirect the system stderr in order to get rid of the error raised by
     # the underlying C function call if the container is not found.

=== modified file 'lplxcip/tests/test_lxcip.py'
--- lplxcip/tests/test_lxcip.py	2012-05-03 11:26:37 +0000
+++ lplxcip/tests/test_lxcip.py	2012-09-04 11:59:38 +0000
@@ -67,6 +67,23 @@
     name = lxc.name
     interfaces = ['eth0']
 
+    def test__get_pid_tries_double_locations(self):
+        # See bug #1045728
+        # The location of 'lxc.so' is different on Quantal than Precise, so we
+        # make sure that the loading function checks both locations before
+        # giving up.
+        paths = []
+        def failing_loader(path):
+            paths.append(path)
+            raise OSError('not loading from path: %s' % (path,))
+        # The OSError we get at the end should be 'not_installed'
+        self.assertRaises(OSError,
+                          lxcip._get_pid, self.name, loader=failing_loader)
+        # The location on Precise
+        self.assertIn('/usr/lib/lxc/lxc.so.0', paths)
+        # The location on Quantal
+        self.assertIn('/usr/lib/lxc.so.0', paths)
+
     def test_ip_address(self):
         # Ensure the ip address of the container is correctly retrieved.
         pid = utils.get_pid(self.name)