← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~allenap/maas/move-tftp-related-commands into lp:maas

 

Gavin Panella has proposed merging lp:~allenap/maas/move-tftp-related-commands into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/move-tftp-related-commands/+merge/115514

This moves a couple of PXE/TFTP related commands from maasserver to
provisioningserver. It is in preparation for a subsequent branch where
TFTPROOT will be moved out of celeryconfig.py and into pserv.yaml.
-- 
https://code.launchpad.net/~allenap/maas/move-tftp-related-commands/+merge/115514
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/move-tftp-related-commands into lp:maas.
=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-06-29 06:52:07 +0000
+++ scripts/maas-import-pxe-files	2012-07-18 10:32:22 +0000
@@ -69,7 +69,7 @@
     if [ -f pxelinux.0 ]
     then
         # TODO: Pass sub-architecture once we support those.
-        maas install_pxe_bootloader \
+        maas-provision install-pxe-bootloader \
             --arch=$arch --loader='pxelinux.0' --tftproot=$TFTPROOT
     fi
 }
@@ -91,7 +91,7 @@
     done
     popd >/dev/null
 
-    maas install_pxe_image \
+    maas-provision install-pxe-image \
         --arch=$arch --release=$release --purpose="install" \
         --image="install" --tftproot=$TFTPROOT
 }

=== modified file 'src/provisioningserver/__main__.py'
--- src/provisioningserver/__main__.py	2012-06-12 15:08:32 +0000
+++ src/provisioningserver/__main__.py	2012-07-18 10:32:22 +0000
@@ -13,6 +13,8 @@
 __metaclass__ = type
 
 import provisioningserver.dhcp.writer
+import provisioningserver.pxe.install_bootloader
+import provisioningserver.pxe.install_image
 from provisioningserver.utils import ActionScript
 
 
@@ -20,4 +22,10 @@
 main.register(
     "generate-dhcp-config",
     provisioningserver.dhcp.writer)
+main.register(
+    "install-pxe-bootloader",
+    provisioningserver.pxe.install_bootloader)
+main.register(
+    "install-pxe-image",
+    provisioningserver.pxe.install_image)
 main()

=== renamed file 'src/maasserver/management/commands/install_pxe_bootloader.py' => 'src/provisioningserver/pxe/install_bootloader.py'
--- src/maasserver/management/commands/install_pxe_bootloader.py	2012-06-29 10:50:57 +0000
+++ src/provisioningserver/pxe/install_bootloader.py	2012-07-18 10:32:22 +0000
@@ -11,16 +11,15 @@
 
 __metaclass__ = type
 __all__ = [
-    'Command',
+    "add_arguments",
+    "run",
     ]
 
 import filecmp
-from optparse import make_option
 import os.path
 from shutil import copyfile
 
 from celeryconfig import TFTPROOT
-from django.core.management.base import BaseCommand
 from provisioningserver.pxe.tftppath import (
     compose_bootloader_path,
     locate_tftp_path,
@@ -88,34 +87,29 @@
     os.rename(temp_file, destination)
 
 
-class Command(BaseCommand):
+def add_arguments(parser):
+    parser.add_argument(
+        '--arch', dest='arch', default=None,
+        help="Main system architecture that the bootloader is for.")
+    parser.add_argument(
+        '--subarch', dest='subarch', default='generic',
+        help="Sub-architecture of the main architecture [%(default)s].")
+    parser.add_argument(
+        '--loader', dest='loader', default=None,
+        help="PXE pre-boot loader to install.")
+    parser.add_argument(
+        '--tftproot', dest='tftproot', default=TFTPROOT, help=(
+            "Store to this TFTP directory tree instead of the "
+            "default [%(default)s]."))
+
+
+def run(args):
     """Install a PXE pre-boot loader into the TFTP directory structure.
 
     This won't overwrite an existing loader if its contents are unchanged.
     However the new loader you give it will be deleted regardless.
     """
-
-    option_list = BaseCommand.option_list + (
-        make_option(
-            '--arch', dest='arch', default=None,
-            help="Main system architecture that the bootloader is for."),
-        make_option(
-            '--subarch', dest='subarch', default='generic',
-            help="Sub-architecture of the main architecture."),
-        make_option(
-            '--loader', dest='loader', default=None,
-            help="PXE pre-boot loader to install."),
-        make_option(
-            '--tftproot', dest='tftproot', default=TFTPROOT,
-            help="Store to this TFTP directory tree instead of the default."),
-        )
-
-    def handle(self, arch=None, subarch=None, loader=None, tftproot=None,
-               **kwargs):
-        if tftproot is None:
-            tftproot = TFTPROOT
-
-        install_bootloader(loader, make_destination(tftproot, arch, subarch))
-
-        if os.path.exists(loader):
-            os.remove(loader)
+    destination = make_destination(args.tftproot, args.arch, args.subarch)
+    install_bootloader(args.loader, destination)
+    if os.path.exists(args.loader):
+        os.remove(args.loader)

=== renamed file 'src/maasserver/management/commands/install_pxe_image.py' => 'src/provisioningserver/pxe/install_image.py'
--- src/maasserver/management/commands/install_pxe_image.py	2012-06-29 10:50:57 +0000
+++ src/provisioningserver/pxe/install_image.py	2012-07-18 10:32:22 +0000
@@ -11,11 +11,11 @@
 
 __metaclass__ = type
 __all__ = [
-    'Command',
+    "add_arguments",
+    "run",
     ]
 
 from filecmp import cmpfiles
-from optparse import make_option
 import os.path
 from shutil import (
     copytree,
@@ -23,7 +23,6 @@
     )
 
 from celeryconfig import TFTPROOT
-from django.core.management.base import BaseCommand
 from provisioningserver.pxe.tftppath import (
     compose_image_path,
     locate_tftp_path,
@@ -115,7 +114,29 @@
     rmtree('%s.old' % old, ignore_errors=True)
 
 
-class Command(BaseCommand):
+def add_arguments(parser):
+    parser.add_argument(
+        '--arch', dest='arch', default=None,
+        help="Main system architecture that the image is for.")
+    parser.add_argument(
+        '--subarch', dest='subarch', default='generic',
+        help="Sub-architecture of the main architecture [%(default)s].")
+    parser.add_argument(
+        '--release', dest='release', default=None,
+        help="Ubuntu release that the image is for.")
+    parser.add_argument(
+        '--purpose', dest='purpose', default=None,
+        help="Purpose of the image (e.g. 'install' or 'commissioning').")
+    parser.add_argument(
+        '--image', dest='image', default=None,
+        help="Netboot image directory, containing kernel & initrd.")
+    parser.add_argument(
+        '--tftproot', dest='tftproot', default=TFTPROOT, help=(
+            "Store to this TFTP directory tree instead of the "
+            "default [%(default)s]."))
+
+
+def run(args):
     """Move a netboot image into the TFTP directory structure.
 
     The image is a directory containing a kernel and an initrd.  If the
@@ -123,35 +144,9 @@
     containing identical files, the new image is deleted and the old one
     is left untouched.
     """
-
-    option_list = BaseCommand.option_list + (
-        make_option(
-            '--arch', dest='arch', default=None,
-            help="Main system architecture that the image is for."),
-        make_option(
-            '--subarch', dest='subarch', default='generic',
-            help="Sub-architecture of the main architecture."),
-        make_option(
-            '--release', dest='release', default=None,
-            help="Ubuntu release that the image is for."),
-        make_option(
-            '--purpose', dest='purpose', default=None,
-            help="Purpose of the image (e.g. 'install' or 'commissioning')."),
-        make_option(
-            '--image', dest='image', default=None,
-            help="Netboot image directory, containing kernel & initrd."),
-        make_option(
-            '--tftproot', dest='tftproot', default=TFTPROOT,
-            help="Store to this TFTP directory tree instead of the default."),
-        )
-
-    def handle(self, arch=None, subarch=None, release=None, purpose=None,
-               image=None, tftproot=None, **kwargs):
-        if tftproot is None:
-            tftproot = TFTPROOT
-
-        dest = make_destination(tftproot, arch, subarch, release, purpose)
-        if not are_identical_dirs(dest, image):
-            # Image has changed.  Move the new version into place.
-            install_dir(image, dest)
-        rmtree(image, ignore_errors=True)
+    destination = make_destination(
+        args.tftproot, args.arch, args.subarch, args.release, args.purpose)
+    if not are_identical_dirs(destination, args.image):
+        # Image has changed.  Move the new version into place.
+        install_dir(args.image, destination)
+    rmtree(args.image, ignore_errors=True)

=== added file 'src/provisioningserver/pxe/tests/__init__.py'
=== renamed file 'src/maasserver/tests/test_commands_install_pxe_bootloader.py' => 'src/provisioningserver/pxe/tests/test_install_bootloader.py'
--- src/maasserver/tests/test_commands_install_pxe_bootloader.py	2012-06-29 06:08:28 +0000
+++ src/provisioningserver/pxe/tests/test_install_bootloader.py	2012-07-18 10:32:22 +0000
@@ -14,21 +14,22 @@
 
 import os.path
 
-from django.core.management import call_command
-from maasserver.management.commands.install_pxe_bootloader import (
-    install_bootloader,
-    make_destination,
-    )
 from maastesting.factory import factory
 from maastesting.testcase import TestCase
 from maastesting.utils import (
     age_file,
     get_write_time,
     )
+import provisioningserver.pxe.install_bootloader
+from provisioningserver.pxe.install_bootloader import (
+    install_bootloader,
+    make_destination,
+    )
 from provisioningserver.pxe.tftppath import (
     compose_bootloader_path,
     locate_tftp_path,
     )
+from provisioningserver.utils import ActionScript
 from testtools.matchers import (
     DirExists,
     FileContains,
@@ -45,9 +46,12 @@
         arch = factory.make_name('arch')
         subarch = factory.make_name('subarch')
 
-        call_command(
-            'install_pxe_bootloader', arch=arch, subarch=subarch,
-            loader=loader, tftproot=tftproot)
+        action = factory.make_name("action")
+        script = ActionScript(action)
+        script.register(action, provisioningserver.pxe.install_bootloader)
+        script.execute(
+            (action, "--arch", arch, "--subarch", subarch,
+             "--loader", loader, "--tftproot", tftproot))
 
         self.assertThat(
             locate_tftp_path(

=== renamed file 'src/maasserver/tests/test_commands_install_pxe_image.py' => 'src/provisioningserver/pxe/tests/test_install_image.py'
--- src/maasserver/tests/test_commands_install_pxe_image.py	2012-06-26 07:18:10 +0000
+++ src/provisioningserver/pxe/tests/test_install_image.py	2012-07-18 10:32:22 +0000
@@ -14,18 +14,19 @@
 
 import os
 
-from django.core.management import call_command
-from maasserver.management.commands.install_pxe_image import (
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+import provisioningserver.pxe.install_image
+from provisioningserver.pxe.install_image import (
     are_identical_dirs,
     install_dir,
     make_destination,
     )
-from maasserver.testing.factory import factory
-from maasserver.testing.testcase import TestCase
 from provisioningserver.pxe.tftppath import (
     compose_image_path,
     locate_tftp_path,
     )
+from provisioningserver.utils import ActionScript
 from testtools.matchers import (
     DirExists,
     FileContains,
@@ -54,10 +55,13 @@
         tftproot = self.make_dir()
         arch, subarch, release, purpose = make_arch_subarch_release_purpose()
 
-        call_command(
-            'install_pxe_image', arch=arch, subarch=subarch, release=release,
-            purpose=purpose, image=image_dir,
-            tftproot=tftproot)
+        action = factory.make_name("action")
+        script = ActionScript(action)
+        script.register(action, provisioningserver.pxe.install_image)
+        script.execute(
+            (action, "--arch", arch, "--subarch", subarch, "--release",
+             release, "--purpose", purpose, "--image", image_dir,
+             "--tftproot", tftproot))
 
         self.assertThat(
             os.path.join(

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-06-29 06:59:00 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-07-18 10:32:22 +0000
@@ -109,7 +109,7 @@
         if release is not None:
             env['RELEASES'] = release
             env['CURRENT_RELEASE'] = release
-        with open('/dev/null', 'w') as dev_null:
+        with open(os.devnull, 'wb') as dev_null:
             check_call(script, env=env, stdout=dev_null)
 
     def test_downloads_pre_boot_loader(self):

=== modified file 'src/provisioningserver/tests/test_utils.py'
--- src/provisioningserver/tests/test_utils.py	2012-07-16 10:19:46 +0000
+++ src/provisioningserver/tests/test_utils.py	2012-07-18 10:32:22 +0000
@@ -185,7 +185,8 @@
         handler.run = handler_calls.append
         script = ActionScript("Description")
         script.register("amputate", handler)
-        script(["amputate"])
+        error = self.assertRaises(SystemExit, script, ["amputate"])
+        self.assertEqual(0, error.code)
         self.assertEqual(1, len(handler_calls))
         self.assertIsInstance(handler_calls[0], Namespace)
 

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-07-16 10:19:46 +0000
+++ src/provisioningserver/utils.py	2012-07-18 10:32:22 +0000
@@ -218,13 +218,24 @@
         handler.add_arguments(parser)
         return parser
 
+    def execute(self, argv=None):
+        """Execute this action.
+
+        This is intended for in-process invocation of an action, though it may
+        still raise L{SystemExit}. The L{__call__} method is intended for when
+        this object is executed as a script proper.
+        """
+        args = self.parser.parse_args(argv)
+        args.handler.run(args)
+
     def __call__(self, argv=None):
         try:
             self.setup()
-            args = self.parser.parse_args(argv)
-            args.handler.run(args)
+            self.execute(argv)
         except CalledProcessError, error:
             # Print error.cmd and error.output too?
             raise SystemExit(error.returncode)
         except KeyboardInterrupt:
             raise SystemExit(1)
+        else:
+            raise SystemExit(0)