launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/remove-obsolete-generate-enlistment-pxe into lp:maas with lp:~allenap/maas/tftp-path-in-pserv as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/remove-obsolete-generate-enlistment-pxe/+merge/116363

Removes the obsolete generate_enlistment_pxe command. This has been replaced by dynamic PXE generation. I think it's safe to remove this, because it was part of the Cobbler replacement work (and won't, therefore, break the existing Cobbler integration).
-- 
https://code.launchpad.net/~allenap/maas/remove-obsolete-generate-enlistment-pxe/+merge/116363
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/remove-obsolete-generate-enlistment-pxe into lp:maas.

=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-07-23 20:00:32 +0000
+++ scripts/maas-import-pxe-files	2012-07-23 20:00:32 +0000
@@ -34,11 +34,6 @@
 # Supported architectures.
 ARCHES=${ARCHES:-amd64 i386}
 
-# TFTP root directory. Mandatory.
-# TODO: Remove this; it's here to support the obsolete
-# generate_enlistment_pxe command.
-TFTPROOT=${TFTPROOT?}
-
 # Path to the provisioning configuration. Mandatory.
 MAAS_PROVISIONING_SETTINGS=${MAAS_PROVISIONING_SETTINGS?}
 
@@ -121,12 +116,6 @@
         do
             update_install_files $arch $release
         done
-
-        # TODO: Pass sub-architecture once we support those.
-        # TODO: Remove this; it's obsolete.
-        maas generate_enlistment_pxe \
-            --arch=$arch --release=$CURRENT_RELEASE \
-            --tftproot=$TFTPROOT
     done
 
     rm -rf -- $DOWNLOAD_DIR

=== removed file 'src/maasserver/management/commands/generate_enlistment_pxe.py'
--- src/maasserver/management/commands/generate_enlistment_pxe.py	2012-07-23 20:00:32 +0000
+++ src/maasserver/management/commands/generate_enlistment_pxe.py	1970-01-01 00:00:00 +0000
@@ -1,75 +0,0 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Django command: generate a PXE configuration file for node enlistment.
-
-Produces the "default" PXE configuration that we provide to nodes that
-MAAS is not yet aware of.  A node that netboots using this configuration
-will then register itself with the MAAS.
-"""
-
-from __future__ import (
-    absolute_import,
-    print_function,
-    unicode_literals,
-    )
-
-__metaclass__ = type
-__all__ = [
-    'Command',
-    ]
-
-
-from optparse import make_option
-
-from django.core.management.base import BaseCommand
-from provisioningserver.pxe.pxeconfig import PXEConfig
-from provisioningserver.pxe.tftppath import compose_image_path
-
-
-class Command(BaseCommand):
-    """Print out enlistment PXE config."""
-    # TODO: Remove this; it's obsolete.
-
-    option_list = BaseCommand.option_list + (
-        make_option(
-            '--arch', dest='arch', default=None,
-            help="Main system architecture to generate config 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 to run when enlisting nodes."),
-        make_option(
-            '--tftproot', dest='tftproot', default=None,
-            help="Root of TFTP directory hierarchy to place config into."),
-        )
-
-    def handle(self, arch=None, subarch='generic', release=None,
-               tftproot=None, **kwargs):
-        image_path = compose_image_path(arch, subarch, release, 'install')
-        # TODO: This needs to go somewhere more appropriate, and
-        # probably contain more appropriate options.
-        kernel_opts = ' '.join([
-            # Default kernel options (similar to those used by Cobbler):
-            'initrd=%s' % '/'.join([image_path, 'initrd.gz']),
-            'ksdevice=bootif',
-            'lang=  text ',
-            'hostname=%s-%s' % (release, arch),
-            'domain=local.lan',
-            'suite=%s' % release,
-
-            # MAAS-specific options:
-            'priority=critical',
-            'local=en_US',
-            'netcfg/choose_interface=auto',
-            ])
-        template_args = {
-            'menutitle': "Enlisting with MAAS",
-            # Enlistment uses the same kernel as installation.
-            'kernelimage': '/'.join([image_path, 'linux']),
-            'append': kernel_opts,
-        }
-        writer = PXEConfig(arch, subarch, tftproot=tftproot)
-        writer.write_config(**template_args)

=== removed file 'src/maasserver/tests/test_commands_generate_enlistment_pxe.py'
--- src/maasserver/tests/test_commands_generate_enlistment_pxe.py	2012-06-26 07:27:06 +0000
+++ src/maasserver/tests/test_commands_generate_enlistment_pxe.py	1970-01-01 00:00:00 +0000
@@ -1,52 +0,0 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for the generate-enlistment-pxe command."""
-
-from __future__ import (
-    absolute_import,
-    print_function,
-    unicode_literals,
-    )
-
-__metaclass__ = type
-__all__ = []
-
-from django.core.management import call_command
-from maasserver.enum import ARCHITECTURE_CHOICES
-from maasserver.testing.factory import factory
-from maasserver.testing.testcase import TestCase
-from provisioningserver.pxe.pxeconfig import PXEConfig
-from provisioningserver.pxe.tftppath import (
-    compose_config_path,
-    compose_image_path,
-    locate_tftp_path,
-    )
-from testtools.matchers import (
-    Contains,
-    FileContains,
-    )
-
-
-class TestGenerateEnlistmentPXE(TestCase):
-
-    def test_generates_default_pxe_config(self):
-        arch = factory.getRandomChoice(ARCHITECTURE_CHOICES)
-        subarch = 'generic'
-        release = 'precise'
-        tftproot = self.make_dir()
-        self.patch(PXEConfig, 'target_basedir', tftproot)
-        call_command(
-            'generate_enlistment_pxe', arch=arch, release=release,
-            tftproot=tftproot)
-        # This produces a "default" PXE config file in the right place.
-        # It refers to the kernel and initrd for the requested
-        # architecture and release.
-        result_path = locate_tftp_path(
-            compose_config_path(arch, subarch, 'default'),
-            tftproot=tftproot)
-        self.assertThat(
-            result_path,
-            FileContains(matcher=Contains(
-                compose_image_path(arch, subarch, release, 'install') +
-                    '/linux')))

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-07-23 20:00:32 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-07-23 20:00:32 +0000
@@ -110,9 +110,6 @@
             # Substitute curl for wget; it accepts file:// URLs.
             'DOWNLOAD': 'curl -O --silent',
             'PATH': os.pathsep.join(path),
-            # TODO: Remove TFTPROOT; it's here to support the obsolete
-            # generate_enlistment_pxe command.
-            'TFTPROOT': self.tftproot,
         }
         env.update(self.config_fixture.environ)
         if arch is not None:
@@ -192,14 +189,3 @@
         original_timestamp = get_write_time(tftp_path)
         self.call_script(archive, self.tftproot, arch=arch, release=release)
         self.assertEqual(original_timestamp, get_write_time(tftp_path))
-
-    def test_generates_default_pxe_config(self):
-        arch = factory.make_name('arch')
-        release = 'precise'
-        archive = self.make_downloads(arch=arch, release=release)
-        self.call_script(archive, self.tftproot, arch=arch, release=release)
-        self.assertThat(
-            os.path.join(
-                self.tftproot, 'maas', arch, 'generic',
-                'pxelinux.cfg', 'default'),
-            FileContains(matcher=Contains("MENU TITLE")))