Merge lp:~ltrager/maas/remove_di_from_kernel_opts into lp:~maas-committers/maas/trunk

[Lee Trager]

Remove deprecated d-i install code from kernel opts.

MAAS supports four netboot options. commissioning, xinstall, enlist, and install. xinstall is MASS's current, and only supported installation boot method which uses Curtin. install was used for deploying using the Debian installer. As that method is no longer supported we can always return the same kernel arguments.

[Gavin Panella (allenap)]

review: Approve

[Andres Rodriguez (andreserl)]

This doesn't really need fixing, but I want to keep this around for a bit longer provided that Caringo will make changes around this area.

review: Needs Fixing

[Mike Pontillo (mpontillo)]

Are we still planning on landing this?

[MAAS Lander (maas-lander)]

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5461. By Lee Trager on 2016-10-22

Remove deprecated d-i install code from kernel opts.

=== modified file 'src/provisioningserver/kernel_opts.py'
--- src/provisioningserver/kernel_opts.py	2016-10-22 06:28:12 +0000
+++ src/provisioningserver/kernel_opts.py	2016-10-22 06:28:12 +0000
@@ -48,21 +48,6 @@
     __call__ = KernelParametersBase._replace
 
 
-def compose_preseed_opt(preseed_url):
-    """Compose a kernel option for preseed URL.
-
-    :param preseed_url: The URL from which a preseed can be fetched.
-    """
-    # See https://help.ubuntu.com/12.04/installation-guide
-    #   /i386/preseed-using.html#preseed-auto
-    return "auto url=%s" % preseed_url
-
-
-def compose_locale_opt():
-    locale = 'en_US'
-    return "locale=%s" % locale
-
-
 def compose_logging_opts(log_host):
     return [
         'log_host=%s' % log_host,
@@ -97,63 +82,11 @@
         )
 
 
-def compose_hostname_opts(params):
-    """Return list of hostname/domain options based on `params`.
-
-    The domain is omitted if `params` does not include it.
-    """
-    options = [
-        'hostname=%s' % params.hostname,
-        ]
-    if params.domain is not None:
-        options.append('domain=%s' % params.domain)
-    return options
-
-
 def prefix_target_name(name):
     """Prefix an ISCSI target name with the standard target-name prefix."""
     return "%s:%s" % (ISCSI_TARGET_NAME_PREFIX, name)
 
 
-def compose_purpose_opts(params):
-    """Return the list of the purpose-specific kernel options."""
-    if params.purpose in ["commissioning", "xinstall", "enlist"]:
-        # These are kernel parameters read by the ephemeral environment.
-        tname = prefix_target_name(
-            get_ephemeral_name(
-                params.osystem, params.arch, params.subarch,
-                params.release, params.label))
-        kernel_params = [
-            # Read by the open-iscsi initramfs code.
-            "iscsi_target_name=%s" % tname,
-            "iscsi_target_ip=%s" % params.fs_host,
-            "iscsi_target_port=3260",
-            "iscsi_initiator=%s" % params.hostname,
-            # Read by cloud-initramfs-dyn-netconf and klibc's ipconfig
-            # in the initramfs.
-            "ip=::::%s:BOOTIF" % params.hostname,
-            # kernel / udev name iscsi devices with this path
-            "ro root=/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (
-                params.fs_host, "3260", tname),
-            # Read by overlayroot package.
-            "overlayroot=tmpfs",
-            # LP:1533822 - Disable reading overlay data from disk.
-            "overlayroot_cfgdisk=disabled",
-            # Read by cloud-init.
-            "cloud-config-url=%s" % params.preseed_url,
-            ]
-        return kernel_params
-    else:
-        # These are options used by the Debian Installer.
-        return [
-            "netcfg/choose_interface=auto",
-            # Use the text installer, display only critical messages.
-            "text priority=critical",
-            compose_preseed_opt(params.preseed_url),
-            compose_locale_opt(),
-            ] + compose_hostname_opts(params)
-
-
 def compose_arch_opts(params):
     """Return any architecture-specific options required"""
     arch_subarch = '%s/%s' % (params.arch, params.subarch)
@@ -178,10 +111,33 @@
 
     :type params: `KernelParameters`.
     """
-    options = []
-    # nomodeset prevents video mode switching.
-    options += ["nomodeset"]
-    options += compose_purpose_opts(params)
+    # These are kernel parameters read by the ephemeral environment.
+    tname = prefix_target_name(
+        get_ephemeral_name(
+            params.osystem, params.arch, params.subarch,
+            params.release, params.label))
+    options = [
+        # nomodeset prevents video mode switching.
+        "nomodeset",
+        # Read by the open-iscsi initramfs code.
+        "iscsi_target_name=%s" % tname,
+        "iscsi_target_ip=%s" % params.fs_host,
+        "iscsi_target_port=3260",
+        "iscsi_initiator=%s" % params.hostname,
+        # Read by cloud-initramfs-dyn-netconf and klibc's ipconfig
+        # in the initramfs.
+        "ip=::::%s:BOOTIF" % params.hostname,
+        # kernel / udev name iscsi devices with this path
+        "ro root=/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (
+            params.fs_host, "3260", tname),
+        # Read by overlayroot package.
+        "overlayroot=tmpfs",
+        # LP:1533822 - Disable reading overlay data from disk.
+        "overlayroot_cfgdisk=disabled",
+        # Read by cloud-init.
+        "cloud-config-url=%s" % params.preseed_url,
+        ]
+    
     # Note: logging opts are not respected by ephemeral images, so
     #       these are actually "purpose_opts" but were left generic
     #       as it would be nice to have.

=== modified file 'src/provisioningserver/tests/test_kernel_opts.py'
--- src/provisioningserver/tests/test_kernel_opts.py	2016-10-22 06:28:12 +0000
+++ src/provisioningserver/tests/test_kernel_opts.py	2016-10-22 06:28:12 +0000
@@ -21,7 +21,6 @@
 from provisioningserver.kernel_opts import (
     compose_arch_opts,
     compose_kernel_command_line,
-    compose_preseed_opt,
     CURTIN_KERNEL_CMDLINE_NAME,
     get_curtin_kernel_cmdline_sep,
     get_ephemeral_name,
@@ -135,34 +134,6 @@
         cmdline = compose_kernel_command_line(params)
         self.assertThat(cmdline, Contains("overlayroot_cfgdisk=disabled"))
 
-    def test_compose_kernel_command_line_includes_preseed_url(self):
-        params = self.make_kernel_parameters()
-        self.assertIn(
-            "auto url=%s" % params.preseed_url,
-            compose_kernel_command_line(params))
-
-    def test_install_compose_kernel_command_line_includes_name_domain(self):
-        params = self.make_kernel_parameters(purpose="install")
-        self.assertThat(
-            compose_kernel_command_line(params),
-            ContainsAll([
-                "hostname=%s" % params.hostname,
-                "domain=%s" % params.domain,
-                ]))
-
-    def test_install_compose_kernel_command_line_omits_domain_if_omitted(self):
-        params = self.make_kernel_parameters(purpose="install", domain=None)
-        kernel_command_line = compose_kernel_command_line(params)
-        self.assertIn("hostname=%s" % params.hostname, kernel_command_line)
-        self.assertNotIn('domain=', kernel_command_line)
-
-    def test_install_compose_kernel_command_line_includes_locale(self):
-        params = self.make_kernel_parameters(purpose="install")
-        locale = "en_US"
-        self.assertIn(
-            "locale=%s" % locale,
-            compose_kernel_command_line(params))
-
     def test_install_compose_kernel_command_line_includes_log_settings(self):
         params = self.make_kernel_parameters(purpose="install")
         # Port 514 (UDP) is syslog.
@@ -174,20 +145,6 @@
                 "log_port=%s" % log_port,
                 ]))
 
-    def test_install_compose_kernel_command_line_includes_di_settings(self):
-        params = self.make_kernel_parameters(purpose="install")
-        self.assertThat(
-            compose_kernel_command_line(params),
-            Contains("text priority=critical"))
-
-    def test_install_compose_kernel_command_line_inc_purpose_opts(self):
-        # The result of compose_kernel_command_line includes the purpose
-        # options for a non "commissioning" node.
-        params = self.make_kernel_parameters(purpose="install")
-        self.assertIn(
-            "netcfg/choose_interface=auto",
-            compose_kernel_command_line(params))
-
     def test_xinstall_compose_kernel_command_line_inc_purpose_opts(self):
         # The result of compose_kernel_command_line includes the purpose
         # options for a non "xinstall" node.
@@ -296,12 +253,6 @@
                 "iscsi_target_ip=%s" % params.fs_host,
                 ]))
 
-    def test_compose_preseed_kernel_opt_returns_kernel_option(self):
-        dummy_preseed_url = factory.make_name("url")
-        self.assertEqual(
-            "auto url=%s" % dummy_preseed_url,
-            compose_preseed_opt(dummy_preseed_url))
-
     def test_compose_kernel_command_line_inc_arm_specific_option(self):
         params = self.make_kernel_parameters(arch="armhf", subarch="highbank")
         self.assertThat(