Adds config parameter kernel_append_param for iLO

This commit adds a new config parameter kernel_append_param for iLO. Change-Id: Ie962672ef81c58f651c2395439a3c69e98c1a4c0
2020-09-30 06:08:05 +00:00 · 2020-09-30 06:08:05 +00:00 · 755918343b
commit 755918343b
parent 178584ab99
6 changed files with 150 additions and 72 deletions
--- a/ironic/conf/ilo.py
+++ b/ironic/conf/ilo.py
@ -111,6 +111,15 @@ opts = [
                      'or as the octal number ``0o644`` in Python. '
                      'This setting must be set to the octal number '
                      'representation, meaning starting with ``0o``.')),
+    cfg.StrOpt('kernel_append_params',
+               default='nofb nomodeset vga=normal',
+               mutable=True,
+               help=_('Additional kernel parameters to pass down to the '
+                      'instance kernel. These parameters can be consumed by '
+                      'the kernel or by the applications by reading '
+                      '/proc/cmdline. Mind severe cmdline size limit! Can be '
+                      'overridden by `instance_info/kernel_append_params` '
+                      'property.')),
 ]


--- a/ironic/drivers/modules/ilo/boot.py
+++ b/ironic/drivers/modules/ilo/boot.py
@ -78,7 +78,17 @@ OPTIONAL_PROPERTIES = {
                              "certificates require to be added to the "
                              "iLO.")
 }
+KERNEL_PARAM_PROPERTIES = {
+    'ilo_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 "
+                                  "[ilo]/kernel_append_params ironic option.")
+}
 COMMON_PROPERTIES = REQUIRED_PROPERTIES
+VMEDIA_OPTIONAL_PROPERTIES = OPTIONAL_PROPERTIES.copy()
+VMEDIA_OPTIONAL_PROPERTIES.update(KERNEL_PARAM_PROPERTIES)

 KERNEL_RAMDISK_LABELS = {
    'deploy': REQUIRED_PROPERTIES_UEFI_HTTPS_BOOT,
@ -127,7 +137,7 @@ def parse_driver_info(node, mode='deploy'):

    d_info.update(
        {k: info.get(k, getattr(CONF.conductor, k.replace('ilo_', ''), None))
-         for k in OPTIONAL_PROPERTIES})
+         for k in VMEDIA_OPTIONAL_PROPERTIES})
    d_info.pop('ilo_add_certificates', None)

    return d_info
@ -926,6 +936,10 @@ class IloUefiHttpsBoot(base.BootInterface):
                       "parameters were missing in node's driver_info") % mode)
        deploy_utils.check_for_missing_params(deploy_info, error_msg)

+        deploy_info.update(
+            {k: info.get(k, getattr(CONF.ilo, k.replace('ilo_', ''), None))
+             for k in KERNEL_PARAM_PROPERTIES})
+
        deploy_info.update(ilo_common.parse_driver_info(node))

        return deploy_info
--- a/ironic/drivers/modules/image_utils.py
+++ b/ironic/drivers/modules/image_utils.py
@ -39,55 +39,53 @@ LOG = log.getLogger(__name__)

 class ImageHandler(object):

-    _SWIFT_MAP = {
-        "redfish": {
-            "swift_enabled": CONF.redfish.use_swift,
-            "container": CONF.redfish.swift_container,
-            "timeout": CONF.redfish.swift_object_expiry_timeout,
-            "image_subdir": "redfish",
-            "file_permission": CONF.redfish.file_permission,
-            "kernel_params": CONF.redfish.kernel_append_params
-        },
-        "idrac": {
-            "swift_enabled": CONF.redfish.use_swift,
-            "container": CONF.redfish.swift_container,
-            "timeout": CONF.redfish.swift_object_expiry_timeout,
-            "image_subdir": "redfish",
-            "file_permission": CONF.redfish.file_permission,
-            "kernel_params": CONF.redfish.kernel_append_params
-        },
-        "ilo5": {
-            "swift_enabled": not CONF.ilo.use_web_server_for_images,
-            "container": CONF.ilo.swift_ilo_container,
-            "timeout": CONF.ilo.swift_object_expiry_timeout,
-            "image_subdir": "ilo",
-            "file_permission": CONF.ilo.file_permission,
-            "kernel_params": CONF.pxe.pxe_append_params
-        },
-        "ilo": {
-            "swift_enabled": not CONF.ilo.use_web_server_for_images,
-            "container": CONF.ilo.swift_ilo_container,
-            "timeout": CONF.ilo.swift_object_expiry_timeout,
-            "image_subdir": "ilo",
-            "file_permission": CONF.ilo.file_permission,
-            "kernel_params": CONF.pxe.pxe_append_params
-        },
-    }
-
    def __init__(self, driver):
-        self._driver = driver
-        self._container = self._SWIFT_MAP[driver].get("container")
-        self._timeout = self._SWIFT_MAP[driver].get("timeout")
-        self._image_subdir = self._SWIFT_MAP[driver].get("image_subdir")
-        self._file_permission = self._SWIFT_MAP[driver].get("file_permission")
-        # To get the kernel parameters
-        self.kernel_params = self._SWIFT_MAP[driver].get("kernel_params")
+        self.update_driver_config(driver)

-    def _is_swift_enabled(self):
-        try:
-            return self._SWIFT_MAP[self._driver].get("swift_enabled")
-        except KeyError:
-            return False
+    def update_driver_config(self, driver):
+        _SWIFT_MAP = {
+            "redfish": {
+                "swift_enabled": CONF.redfish.use_swift,
+                "container": CONF.redfish.swift_container,
+                "timeout": CONF.redfish.swift_object_expiry_timeout,
+                "image_subdir": "redfish",
+                "file_permission": CONF.redfish.file_permission,
+                "kernel_params": CONF.redfish.kernel_append_params
+            },
+            "idrac": {
+                "swift_enabled": CONF.redfish.use_swift,
+                "container": CONF.redfish.swift_container,
+                "timeout": CONF.redfish.swift_object_expiry_timeout,
+                "image_subdir": "redfish",
+                "file_permission": CONF.redfish.file_permission,
+                "kernel_params": CONF.redfish.kernel_append_params
+            },
+            "ilo5": {
+                "swift_enabled": not CONF.ilo.use_web_server_for_images,
+                "container": CONF.ilo.swift_ilo_container,
+                "timeout": CONF.ilo.swift_object_expiry_timeout,
+                "image_subdir": "ilo",
+                "file_permission": CONF.ilo.file_permission,
+                "kernel_params": CONF.ilo.kernel_append_params
+            },
+            "ilo": {
+                "swift_enabled": not CONF.ilo.use_web_server_for_images,
+                "container": CONF.ilo.swift_ilo_container,
+                "timeout": CONF.ilo.swift_object_expiry_timeout,
+                "image_subdir": "ilo",
+                "file_permission": CONF.ilo.file_permission,
+                "kernel_params": CONF.ilo.kernel_append_params
+            },
+        }
+
+        self._driver = driver
+        self.swift_enabled = _SWIFT_MAP[driver].get("swift_enabled")
+        self._container = _SWIFT_MAP[driver].get("container")
+        self._timeout = _SWIFT_MAP[driver].get("timeout")
+        self._image_subdir = _SWIFT_MAP[driver].get("image_subdir")
+        self._file_permission = _SWIFT_MAP[driver].get("file_permission")
+        # To get the kernel parameters
+        self.kernel_params = _SWIFT_MAP[driver].get("kernel_params")

    def unpublish_image(self, object_name):
        """Withdraw the image previously made downloadable.
@ -98,7 +96,7 @@ class ImageHandler(object):

        :param object_name: name of the published file (optional)
        """
-        if self._is_swift_enabled():
+        if self.swift_enabled:
            container = self._container

            swift_api = swift.SwiftAPI()
@ -179,7 +177,7 @@ class ImageHandler(object):
        :return: a URL to download published file
        """

-        if self._is_swift_enabled():
+        if self.swift_enabled:
            container = self._container
            timeout = self._timeout

--- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py
@ -54,7 +54,9 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):

    def test_parse_driver_info_deploy_iso(self):
        self.node.driver_info['ilo_deploy_iso'] = 'deploy-iso'
+        self.node.driver_info['ilo_kernel_append_params'] = 'kernel-param'
        expected_driver_info = {'ilo_bootloader': None,
+                                'ilo_kernel_append_params': 'kernel-param',
                                'ilo_deploy_iso': 'deploy-iso'}

        actual_driver_info = ilo_boot.parse_driver_info(self.node)
@ -63,6 +65,7 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):
    def test_parse_driver_info_rescue_iso(self):
        self.node.driver_info['ilo_rescue_iso'] = 'rescue-iso'
        expected_driver_info = {'ilo_bootloader': None,
+                                'ilo_kernel_append_params': None,
                                'ilo_rescue_iso': 'rescue-iso'}

        actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue')
@ -72,9 +75,11 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):
        self.node.driver_info['ilo_deploy_kernel'] = 'kernel'
        self.node.driver_info['ilo_deploy_ramdisk'] = 'ramdisk'
        self.node.driver_info['ilo_bootloader'] = 'bootloader'
+        self.node.driver_info['ilo_kernel_append_params'] = 'kernel-param'
        expected_driver_info = {'ilo_deploy_kernel': 'kernel',
                                'ilo_deploy_ramdisk': 'ramdisk',
-                                'ilo_bootloader': 'bootloader'}
+                                'ilo_bootloader': 'bootloader',
+                                'ilo_kernel_append_params': 'kernel-param'}

        actual_driver_info = ilo_boot.parse_driver_info(self.node)
        self.assertEqual(expected_driver_info, actual_driver_info)
@ -85,7 +90,8 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):
        self.node.driver_info['ilo_bootloader'] = 'bootloader'
        expected_driver_info = {'ilo_rescue_kernel': 'kernel',
                                'ilo_rescue_ramdisk': 'ramdisk',
-                                'ilo_bootloader': 'bootloader'}
+                                'ilo_bootloader': 'bootloader',
+                                'ilo_kernel_append_params': None}

        actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue')
        self.assertEqual(expected_driver_info, actual_driver_info)
@ -96,7 +102,8 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):
        CONF.conductor.bootloader = 'bootloader'
        expected_driver_info = {'ilo_deploy_kernel': 'kernel',
                                'ilo_deploy_ramdisk': 'ramdisk',
-                                'ilo_bootloader': 'bootloader'}
+                                'ilo_bootloader': 'bootloader',
+                                'ilo_kernel_append_params': None}

        actual_driver_info = ilo_boot.parse_driver_info(self.node)
        self.assertEqual(expected_driver_info, actual_driver_info)
@ -108,7 +115,8 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):

        expected_driver_info = {'ilo_rescue_kernel': 'kernel',
                                'ilo_rescue_ramdisk': 'ramdisk',
-                                'ilo_bootloader': 'bootloader'}
+                                'ilo_bootloader': 'bootloader',
+                                'ilo_kernel_append_params': None}

        actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue')
        self.assertEqual(expected_driver_info, actual_driver_info)
@ -119,7 +127,8 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest):

        expected_driver_info = {'ilo_deploy_kernel': 'kernel',
                                'ilo_deploy_ramdisk': 'ramdisk',
-                                'ilo_bootloader': None}
+                                'ilo_bootloader': None,
+                                'ilo_kernel_append_params': None}

        actual_driver_info = ilo_boot.parse_driver_info(self.node)
        self.assertEqual(expected_driver_info, actual_driver_info)
@ -1535,7 +1544,8 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
            deploy_info = {
                'ilo_deploy_kernel': 'deploy-kernel',
                'ilo_deploy_ramdisk': 'deploy-ramdisk',
-                'ilo_bootloader': 'bootloader'
+                'ilo_bootloader': 'bootloader',
+                'ilo_kernel_append_params': 'nofb nomodeset vga=normal'
            }

            deploy_info.update({'ilo_username': 'admin',
@ -1566,6 +1576,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
        driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk'
        driver_info['ilo_bootloader'] = 'bootloader'
        driver_info['ilo_add_certificates'] = 'false'
+        driver_info['ilo_kernel_append_params'] = 'kernel-param'
        driver_info['dummy_key'] = 'dummy-value'
        self.node.driver_info = driver_info
        self.node.save()
@ -1575,7 +1586,8 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
            deploy_info = {
                'ilo_rescue_kernel': 'rescue-kernel',
                'ilo_rescue_ramdisk': 'rescue-ramdisk',
-                'ilo_bootloader': 'bootloader'
+                'ilo_bootloader': 'bootloader',
+                'ilo_kernel_append_params': 'kernel-param'
            }

            deploy_info.update({'ilo_username': 'admin',
--- a/ironic/tests/unit/drivers/modules/test_image_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_image_utils.py
@ -30,6 +30,7 @@ from ironic.tests.unit.objects import utils as obj_utils
 sushy = importutils.try_import('sushy')

 INFO_DICT = db_utils.get_test_redfish_info()
+INFO_DICT_ILO = db_utils.get_test_ilo_info()


 class RedfishImageHandlerTestCase(db_base.DbTestCase):
@ -100,19 +101,15 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase):
            mock_swift_api.delete_object.assert_called_once_with(
                'ironic_redfish_container', object_name)

-    @mock.patch.object(image_utils.ImageHandler, '_is_swift_enabled',
-                       autospec=True)
    @mock.patch.object(os, 'chmod', autospec=True)
    @mock.patch.object(image_utils, 'shutil', autospec=True)
    @mock.patch.object(os, 'link', autospec=True)
    @mock.patch.object(os, 'mkdir', autospec=True)
    def test_publish_image_local_link(
-            self, mock_mkdir, mock_link, mock_shutil, mock_chmod,
-            mock__is_swift):
-        img_handler_obj = image_utils.ImageHandler(self.node.driver)
-        mock__is_swift.return_value = False
+            self, mock_mkdir, mock_link, mock_shutil, mock_chmod):
        self.config(use_swift=False, group='redfish')
        self.config(http_url='http://localhost', group='deploy')
+        img_handler_obj = image_utils.ImageHandler(self.node.driver)

        url = img_handler_obj.publish_image('file.iso', 'boot.iso')

@ -124,16 +121,12 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase):
            'file.iso', '/httpboot/redfish/boot.iso')
        mock_chmod.assert_called_once_with('file.iso', 0o644)

-    @mock.patch.object(image_utils.ImageHandler, '_is_swift_enabled',
-                       autospec=True)
    @mock.patch.object(os, 'chmod', autospec=True)
    @mock.patch.object(image_utils, 'shutil', autospec=True)
    @mock.patch.object(os, 'link', autospec=True)
    @mock.patch.object(os, 'mkdir', autospec=True)
    def test_publish_image_local_copy(self, mock_mkdir, mock_link,
-                                      mock_shutil, mock_chmod,
-                                      mock__is_swift):
-        mock__is_swift.return_value = False
+                                      mock_shutil, mock_chmod):
        self.config(use_swift=False, group='redfish')
        self.config(http_url='http://localhost', group='deploy')
        img_handler_obj = image_utils.ImageHandler(self.node.driver)
@ -152,12 +145,9 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase):
        mock_chmod.assert_called_once_with('/httpboot/redfish/boot.iso',
                                           0o644)

-    @mock.patch.object(image_utils.ImageHandler, '_is_swift_enabled',
-                       autospec=True)
    @mock.patch.object(image_utils, 'ironic_utils', autospec=True)
-    def test_unpublish_image_local(self, mock_ironic_utils, mock__is_swift):
+    def test_unpublish_image_local(self, mock_ironic_utils):
        self.config(use_swift=False, group='redfish')
-        mock__is_swift.return_value = False
        with task_manager.acquire(self.context, self.node.uuid,
                                  shared=True) as task:
            img_handler_obj = image_utils.ImageHandler(self.node.driver)
@ -171,6 +161,50 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase):
                expected_file)


+class IloImageHandlerTestCase(db_base.DbTestCase):
+
+    def setUp(self):
+        super(IloImageHandlerTestCase, self).setUp()
+        self.config(enabled_hardware_types=['ilo'],
+                    enabled_power_interfaces=['ilo'],
+                    enabled_boot_interfaces=['ilo-virtual-media'],
+                    enabled_management_interfaces=['ilo'],
+                    enabled_inspect_interfaces=['ilo'],
+                    enabled_bios_interfaces=['ilo'])
+        self.node = obj_utils.create_test_node(
+            self.context, driver='ilo', driver_info=INFO_DICT_ILO)
+
+    def test_ilo_kernel_param_config(self):
+        self.config(kernel_append_params="console=ttyS1", group='ilo')
+        img_handler_obj = image_utils.ImageHandler(self.node.driver)
+        actual_k_param = img_handler_obj.kernel_params
+        expected_k_param = "console=ttyS1"
+
+        self.assertEqual(expected_k_param, actual_k_param)
+
+
+class Ilo5ImageHandlerTestCase(db_base.DbTestCase):
+
+    def setUp(self):
+        super(Ilo5ImageHandlerTestCase, self).setUp()
+        self.config(enabled_hardware_types=['ilo5'],
+                    enabled_power_interfaces=['ilo'],
+                    enabled_boot_interfaces=['ilo-virtual-media'],
+                    enabled_management_interfaces=['ilo5'],
+                    enabled_inspect_interfaces=['ilo'],
+                    enabled_bios_interfaces=['ilo'])
+        self.node = obj_utils.create_test_node(
+            self.context, driver='ilo5', driver_info=INFO_DICT_ILO)
+
+    def test_ilo5_kernel_param_config(self):
+        self.config(kernel_append_params="console=ttyS1", group='ilo')
+        img_handler_obj = image_utils.ImageHandler(self.node.driver)
+        actual_k_param = img_handler_obj.kernel_params
+        expected_k_param = "console=ttyS1"
+
+        self.assertEqual(expected_k_param, actual_k_param)
+
+
 class RedfishImageUtilsTestCase(db_base.DbTestCase):

    def setUp(self):
--- a/releasenotes/notes/add-kernel-param-config-ilo-9b2cee8b0447f82e.yaml
+++ b/releasenotes/notes/add-kernel-param-config-ilo-9b2cee8b0447f82e.yaml
@ -0,0 +1,11 @@
+---
+features:
+  - |
+    Adding a new config parameter ``[ilo]/kernel_append_param `` to
+    ``ilo-virtual-media`` and ``ilo-uefi-https`` boot interfaces of
+    ``ilo`` and ``ilo5`` hardware types.
+upgrade:
+  - |
+    The ``ilo-virtual-media`` and ``ilo-uefi-https`` boot interfaces does
+    not use ``[pxe]pxe_append_params`` anymore. To pass kernel parameters
+    use new configuration parameter ``[ilo]/kernel_append_param``.