Clean up kernel_append_params for PXE/iPXE

Currently handling of kernel_append_params is very inconsistent. This
change applies a straightforward process:
1. instance_info[kernel_append_params]
2. driver_info[kernel_append_params]
3. [pxe]kernel_append_params (renamed from pxe_append_params).

Also adds a helper for subsequent fixes in other drivers.

Change-Id: I79bcf4d8ef1f0f55a82e0991dd5bb1685b3f7957
Story: #2008902
Task: #42469

12 files changed, 86 insertions, 17 deletions

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index e6aa8d865..ebe77cc57 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -1717,7 +1717,7 @@ function configure_ironic_conductor {
     pxe_params+=" $IRONIC_EXTRA_PXE_PARAMS"
 
     if [[ -n "$pxe_params" ]]; then
-        iniset $IRONIC_CONF_FILE pxe pxe_append_params "$pxe_params"
+        iniset $IRONIC_CONF_FILE pxe kernel_append_params "$pxe_params"
     fi
 
     local kernel_append_params="nofb nomodeset console=${IRONIC_TTY_DEV}"
diff --git a/doc/source/install/include/kernel-boot-parameters.inc b/doc/source/install/include/kernel-boot-parameters.inc
index c8d684292..f0da59d60 100644
--- a/doc/source/install/include/kernel-boot-parameters.inc
+++ b/doc/source/install/include/kernel-boot-parameters.inc
@@ -13,11 +13,16 @@ Network boot
 Currently, the Bare Metal service supports assigning unified kernel parameters to PXE
 booted instances by:
 
-* Modifying the ``[pxe]/pxe_append_params`` configuration option, for example::
+* Modifying the ``[pxe]/kernel_append_params`` configuration option, for
+  example:
+
+  .. code-block:: ini
 
     [pxe]
+    kernel_append_params = quiet splash
 
-    pxe_append_params = quiet splash
+  .. note::
+     The option was called ``pxe_append_params`` before the Xena cycle.
 
 * Copying a template from shipped templates to another place, for example::
 
@@ -86,7 +91,7 @@ respectively.
         [pxe]
 
         # Additional append parameters for baremetal PXE boot.
-        pxe_append_params = nofb nomodeset vga=normal console=ttyS0,115200n8
+        kernel_append_params = nofb nomodeset vga=normal console=ttyS0,115200n8
 
 
 * For node web console configuration is similar with the addition of ``ttyX``
@@ -95,7 +100,7 @@ respectively.
         [pxe]
 
         # Additional append parameters for baremetal PXE boot.
-        pxe_append_params = nofb nomodeset vga=normal console=tty0 console=ttyS0,115200n8
+        kernel_append_params = nofb nomodeset vga=normal console=tty0 console=ttyS0,115200n8
 
 For detailed information on how to add consoles see the reference documents
 `kernel params`_ and `serial console`_.
diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index 5c5d771bb..63fa1dca3 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -38,6 +38,7 @@ from ironic.conf import CONF
 from ironic.drivers.modules import boot_mode_utils
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules import image_cache
+from ironic.drivers import utils as driver_utils
 from ironic import objects
 
 LOG = logging.getLogger(__name__)
@@ -831,10 +832,11 @@ def build_instance_pxe_options(task, pxe_info, ipxe_enabled=False):
     return pxe_opts
 
 
-def build_extra_pxe_options(task=None, ramdisk_params=None):
+def build_extra_pxe_options(task, ramdisk_params=None):
+    pxe_append_params = driver_utils.get_kernel_append_params(
+        task.node, default=CONF.pxe.kernel_append_params)
     # Enable debug in IPA according to CONF.debug if it was not
     # specified yet
-    pxe_append_params = CONF.pxe.pxe_append_params
     if CONF.debug and 'ipa-debug' not in pxe_append_params:
         pxe_append_params += ' ipa-debug=1'
     if ramdisk_params:
diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py
index 2ddf13e76..32933db75 100644
--- a/ironic/conf/pxe.py
+++ b/ironic/conf/pxe.py
@@ -21,7 +21,8 @@ from oslo_config import cfg
 from ironic.common.i18n import _
 
 opts = [
-    cfg.StrOpt('pxe_append_params',
+    cfg.StrOpt('kernel_append_params',
+               deprecated_name='pxe_append_params',
                default='nofb nomodeset vga=normal',
                mutable=True,
                help=_('Additional append parameters for baremetal PXE boot.')),
diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py
index 60ab2fa2e..77244bda7 100644
--- a/ironic/drivers/modules/irmc/boot.py
+++ b/ironic/drivers/modules/irmc/boot.py
@@ -308,7 +308,8 @@ def _prepare_boot_iso(task, root_uuid):
 
         deploy_iso_href = deploy_info['irmc_deploy_iso']
         boot_mode = boot_mode_utils.get_boot_mode(task.node)
-        kernel_params = CONF.pxe.pxe_append_params
+        # FIXME(dtantsur): why is iRMC virtual media using PXE options?
+        kernel_params = CONF.pxe.kernel_append_params
 
         boot_iso_filename = _get_iso_name(task.node, label='boot')
         boot_iso_fullpathname = os.path.join(
diff --git a/ironic/drivers/modules/pxe_base.py b/ironic/drivers/modules/pxe_base.py
index 8b8cc4f13..f71132d4c 100644
--- a/ironic/drivers/modules/pxe_base.py
+++ b/ironic/drivers/modules/pxe_base.py
@@ -51,9 +51,19 @@ RESCUE_PROPERTIES = {
                         'that is used at node rescue time. This value is '
                         'required for rescue mode.'),
 }
+OPTIONAL_PROPERTIES = {
+    'kernel_append_params': _("Additional kernel parameters to pass down to "
+                              "instance kernel. These parameters can be "
+                              "consumed by the kernel or by the applications "
+                              "by reading /proc/cmdline. Mind severe cmdline "
+                              "size limit. Overrides "
+                              "[pxe]/kernel_append_params ironic "
+                              "option."),
+}
 COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
 COMMON_PROPERTIES.update(driver_utils.OPTIONAL_PROPERTIES)
 COMMON_PROPERTIES.update(RESCUE_PROPERTIES)
+COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
 
 
 class PXEBaseMixin(object):
diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py
index 4aa2335d6..fa826c266 100644
--- a/ironic/drivers/utils.py
+++ b/ironic/drivers/utils.py
@@ -384,3 +384,23 @@ OPTIONAL_PROPERTIES = {
                                       "deprecated in favor of the new ones."
                                       "Defaults to 'Default'. Optional."),
 }
+
+
+def get_kernel_append_params(node, default):
+    """Get the applicable kernel params.
+
+    The locations are checked in this order:
+
+    1. The node's instance_info.
+    2. The node's driver_info.
+    3. Configuration.
+
+    :param node: Node object.
+    :param default: Default value.
+    """
+    for location in ('instance_info', 'driver_info'):
+        result = getattr(node, location).get('kernel_append_params')
+        if result is not None:
+            return result
+
+    return default
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index 1bd47192a..c1b38b5c9 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -1452,9 +1452,10 @@ class PXEBuildConfigOptionsTestCase(db_base.DbTestCase):
     def _test_build_pxe_config_options_pxe(self, render_mock,
                                            whle_dsk_img=False,
                                            debug=False, mode='deploy',
-                                           ramdisk_params=None):
+                                           ramdisk_params=None,
+                                           expected_pxe_params=None):
         self.config(debug=debug)
-        self.config(pxe_append_params='test_param', group='pxe')
+        self.config(kernel_append_params='test_param', group='pxe')
 
         driver_internal_info = self.node.driver_internal_info
         driver_internal_info['is_whole_disk_image'] = whle_dsk_img
@@ -1498,7 +1499,8 @@ class PXEBuildConfigOptionsTestCase(db_base.DbTestCase):
                                          'ramdisk'))
             })
 
-        expected_pxe_params = 'test_param'
+        if expected_pxe_params is None:
+            expected_pxe_params = 'test_param'
         if debug:
             expected_pxe_params += ' ipa-debug=1'
         if ramdisk_params:
@@ -1554,6 +1556,22 @@ class PXEBuildConfigOptionsTestCase(db_base.DbTestCase):
         self.node.save()
         self._test_build_pxe_config_options_pxe(whle_dsk_img=False)
 
+    def test_build_pxe_config_options_kernel_params_from_driver_info(self):
+        info = self.node.driver_info
+        info['kernel_append_params'] = 'params2'
+        self.node.driver_info = info
+        self.node.save()
+        self._test_build_pxe_config_options_pxe(whle_dsk_img=True,
+                                                expected_pxe_params='params2')
+
+    def test_build_pxe_config_options_kernel_params_from_instance_info(self):
+        info = self.node.instance_info
+        info['kernel_append_params'] = 'params2'
+        self.node.instance_info = info
+        self.node.save()
+        self._test_build_pxe_config_options_pxe(whle_dsk_img=True,
+                                                expected_pxe_params='params2')
+
     def test_build_pxe_config_options_ramdisk_params(self):
         self._test_build_pxe_config_options_pxe(whle_dsk_img=True,
                                                 ramdisk_params={'foo': 'bar'})
@@ -1562,7 +1580,7 @@ class PXEBuildConfigOptionsTestCase(db_base.DbTestCase):
         del self.node.driver_internal_info['is_whole_disk_image']
         self.node.save()
         pxe_params = 'my-pxe-append-params ipa-debug=0'
-        self.config(group='pxe', pxe_append_params=pxe_params)
+        self.config(group='pxe', kernel_append_params=pxe_params)
         self.config(group='pxe', tftp_server='my-tftp-server')
         self.config(group='pxe', tftp_root='/tftp-path/')
         image_info = {
@@ -1717,7 +1735,7 @@ class iPXEBuildConfigOptionsTestCase(db_base.DbTestCase):
                                             iso_boot=False,
                                             multipath=False):
         self.config(debug=debug)
-        self.config(pxe_append_params='test_param', group='pxe')
+        self.config(kernel_append_params='test_param', group='pxe')
         self.config(ipxe_timeout=ipxe_timeout, group='pxe')
         root_dir = CONF.deploy.http_root
 
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 8b60e7c64..7e6e4216e 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -5695,7 +5695,8 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
                              'image_http_proxy', 'image_https_proxy',
                              'image_no_proxy'])
         if pxe_common:
-            expected.extend(['rescue_kernel', 'rescue_ramdisk'])
+            expected.extend(['kernel_append_params',
+                             'rescue_kernel', 'rescue_ramdisk'])
         expected.append('force_persistent_boot_device')
         self.assertCountEqual(expected, properties)
 
diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py
index 3eab04672..c9e2907a4 100644
--- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py
@@ -492,7 +492,7 @@ class IRMCDeployPrivateMethodsTestCase(test_common.BaseIRMCTest):
                                          boot_mode_mock,
                                          create_boot_iso_mock,
                                          check_share_fs_mounted_mock):
-        CONF.pxe.pxe_append_params = 'kernel-params'
+        self.config(kernel_append_params='kernel-params', group='pxe')
 
         deploy_info_mock.return_value = \
             {'image_source': 'image-uuid',
diff --git a/ironic/tests/unit/drivers/test_generic.py b/ironic/tests/unit/drivers/test_generic.py
index cc0ecaa47..210706de3 100644
--- a/ironic/tests/unit/drivers/test_generic.py
+++ b/ironic/tests/unit/drivers/test_generic.py
@@ -72,7 +72,8 @@ class ManualManagementHardwareTestCase(db_base.DbTestCase):
             'deploy_kernel', 'deploy_ramdisk',
             'image_download_source', 'image_http_proxy',
             'image_https_proxy', 'image_no_proxy',
-            'force_persistent_boot_device', 'rescue_kernel', 'rescue_ramdisk']
+            'force_persistent_boot_device', 'kernel_append_params',
+            'rescue_kernel', 'rescue_ramdisk']
         hardware_type = driver_factory.get_hardware_type("manual-management")
         properties = hardware_type.get_properties()
         self.assertCountEqual(expected_prop_keys, properties)
diff --git a/releasenotes/notes/pxe-append-params-9cd5831959676371.yaml b/releasenotes/notes/pxe-append-params-9cd5831959676371.yaml
new file mode 100644
index 000000000..12cf46952
--- /dev/null
+++ b/releasenotes/notes/pxe-append-params-9cd5831959676371.yaml
@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    Adds support for setting kernel parameters for PXE and iPXE boot through
+    the new ``kernel_append_params`` setting in the node's ``driver_info`` or
+    ``instance_info``.
+deprecations:
+  - |
+    The configuration option ``[pxe]pxe_append_params`` has been renamed to
+    ``[pxe]kernel_append_params``. The old name is now deprecated.