yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/lpsetup/ssh-call into lp:lpsetup.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/lpsetup/ssh-call/+merge/99010

== Changes ==

Updated the way the script is called inside LXC containers: lpsetup is no longer present in the Launchpad checkout, it's a standalone project installed using apt. 
Added tests for this_command utility. 
Removed python-argparse installation in lucid containers: this should be handled by apt.


== Tests ==

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

-- 
https://code.launchpad.net/~frankban/lpsetup/ssh-call/+merge/99010
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/lpsetup/ssh-call into lp:lpsetup.

=== modified file 'lpsetup/subcommands/lxcinstall.py'
--- lpsetup/subcommands/lxcinstall.py	2012-03-22 10:48:50 +0000
+++ lpsetup/subcommands/lxcinstall.py	2012-03-23 11:54:36 +0000
@@ -116,12 +116,9 @@
             break
 
 
-def initialize_lxc(lxc_name, lxc_os, ssh_key_path):
+def initialize_lxc(lxc_name, ssh_key_path):
     """Initialize LXC container."""
-    base_packages = list(BASE_PACKAGES)
-    if lxc_os == 'lucid':
-        # Install argparse to be able to run this script inside a lucid lxc.
-        base_packages.append('python-argparse')
+    base_packages = list(BASE_PACKAGES) + ['lpsetup']
     sshcall = ssh(lxc_name, key=ssh_key_path)
     sshcall(
         'DEBIAN_FRONTEND=noninteractive '
@@ -133,7 +130,7 @@
     """Set up the Launchpad environment inside an LXC."""
     # Use ssh to call this script from inside the container.
     args = [
-        'install', '-u', user, '-a', 'setup_apt', 'setup_launchpad',
+        'install', '-u', user, '-s', 'setup_apt', 'setup_launchpad',
         '-d', dependencies_dir, '-c', directory
         ]
     cmd = this_command(directory, args)
@@ -161,7 +158,7 @@
         (wait_for_lxc,
          'lxc_name', 'ssh_key_path'),
         (initialize_lxc,
-         'lxc_name', 'lxc_os', 'ssh_key_path'),
+         'lxc_name', 'ssh_key_path'),
         (setup_launchpad_lxc,
          'user', 'dependencies_dir', 'directory',
          'valid_ssh_keys', 'ssh_key_path', 'lxc_name'),

=== modified file 'lpsetup/tests/test_utils.py'
--- lpsetup/tests/test_utils.py	2012-03-16 11:01:10 +0000
+++ lpsetup/tests/test_utils.py	2012-03-23 11:54:36 +0000
@@ -4,9 +4,13 @@
 
 """Tests for the utils module."""
 
+import sys
 import unittest
 
-from lpsetup.utils import get_container_path
+from lpsetup.utils import (
+    get_container_path,
+    this_command,
+    )
 
 
 class GetContainerPathTest(unittest.TestCase):
@@ -25,3 +29,24 @@
         self.assertEqual(
             '/var/lib/lxc/mycontainer/rootfs/home',
             get_container_path('mycontainer', 'home'))
+
+
+class ThisCommandTest(unittest.TestCase):
+
+    script_name = sys.argv[0]
+    directory = '/home/user/lp/branches'
+
+    def check_command(self, cmd, args):
+        expected = ['cd', self.directory, '&&', self.script_name, args]
+        self.assertEqual(' '.join(expected), cmd)
+
+    def test_directory(self):
+        # Ensure the returned command will execute this script from
+        # the given `directory`.
+        cmd = this_command(self.directory, ['install'])
+        self.check_command(cmd, 'install')
+
+    def test_args_are_quoted(self):
+        # Ensure arguments are correctly quoted.
+        cmd = this_command(self.directory, ['-arg', 'quote me'])
+        self.check_command(cmd, "-arg 'quote me'")

=== modified file 'lpsetup/utils.py'
--- lpsetup/utils.py	2012-03-09 10:14:32 +0000
+++ lpsetup/utils.py	2012-03-23 11:54:36 +0000
@@ -19,6 +19,7 @@
 import os
 import platform
 import subprocess
+import sys
 import time
 
 from shelltoolbox import (
@@ -26,10 +27,7 @@
     run,
     )
 
-from lpsetup.settings import (
-    LP_CHECKOUT,
-    LXC_PATH,
-    )
+from lpsetup.settings import LXC_PATH
 
 
 call = partial(run, stdout=None)
@@ -88,24 +86,19 @@
 def this_command(directory, args):
     """Return a command line to re-launch this script using given `args`.
 
-    The command returned will execute this script from `directory`::
+    The returned command will execute this script from `directory`::
 
-        >>> import os
-        >>> script_name = os.path.basename(__file__)
+        >>> script_name = sys.argv[0]
 
         >>> cmd = this_command('/home/user/lp/branches', ['install'])
-        >>> cmd == ('cd /home/user/lp/branches && devel/utilities/' +
-        ...         script_name + ' install')
+        >>> cmd == 'cd /home/user/lp/branches && ' + script_name + ' install'
         True
 
     Arguments are correctly quoted::
 
         >>> cmd = this_command('/path', ['-arg', 'quote me'])
-        >>> cmd == ('cd /path && devel/utilities/' +
-        ...         script_name + " -arg 'quote me'")
+        >>> cmd == ('cd /path && ' + script_name + " -arg 'quote me'")
         True
     """
-    script = join_command([
-        os.path.join(LP_CHECKOUT, 'utilities', os.path.basename(__file__)),
-        ] + list(args))
+    script = join_command(sys.argv[:1] + list(args))
     return join_command(['cd', directory]) + ' && ' + script