Handle non-key-value params in [inspector]extra_kernel_params

Since we use a dict for passing them internally, use None value for
single-value params.

Change-Id: I4e7d2a555eff48ee164d4ecce53156bacbc68da4
Story: #2008963
Task: #42599

6 files changed, 40 insertions, 17 deletions

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index 63fa1dca3..d425cffe4 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -840,8 +840,9 @@ def build_extra_pxe_options(task, ramdisk_params=None):
     if CONF.debug and 'ipa-debug' not in pxe_append_params:
         pxe_append_params += ' ipa-debug=1'
     if ramdisk_params:
-        pxe_append_params += ' ' + ' '.join('%s=%s' % tpl
-                                            for tpl in ramdisk_params.items())
+        pxe_append_params += ' ' + ' '.join(
+            ('%s=%s' % tpl) if tpl[1] is not None else tpl[0]
+            for tpl in ramdisk_params.items())
     if task and task.context.global_id:
         pxe_append_params += (
             ' ipa-global-request-id=%s' % task.context.global_id)
diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py
index 250449ea4..6f22c537a 100644
--- a/ironic/drivers/modules/image_utils.py
+++ b/ironic/drivers/modules/image_utils.py
@@ -453,7 +453,8 @@ def _prepare_iso_image(task, kernel_href, ramdisk_href,
     if params and not base_iso:
         kernel_params = ' '.join(
             (kernel_params, ' '.join(
-                '%s=%s' % kv for kv in params.items())))
+                ('%s=%s' % kv) if kv[1] is not None else kv[0]
+                for kv in params.items())))
 
     boot_mode = boot_mode_utils.get_boot_mode(task.node)
 
diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py
index 263f5f008..b344abb74 100644
--- a/ironic/drivers/modules/inspector.py
+++ b/ironic/drivers/modules/inspector.py
@@ -182,9 +182,9 @@ def _parse_kernel_params():
         try:
             key, value = s.split('=', 1)
         except ValueError:
-            raise exception.InvalidParameterValue(
-                _('Invalid key-value pair in extra_kernel_params: %s') % s)
-        result[key] = value
+            result[s] = None
+        else:
+            result[key] = value
     return result
 
 
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index c1b38b5c9..c191686cb 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -14,6 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import collections
 import os
 import tempfile
 from unittest import mock
@@ -1503,9 +1504,6 @@ class PXEBuildConfigOptionsTestCase(db_base.DbTestCase):
             expected_pxe_params = 'test_param'
         if debug:
             expected_pxe_params += ' ipa-debug=1'
-        if ramdisk_params:
-            expected_pxe_params += ' ' + ' '.join(
-                '%s=%s' % tpl for tpl in ramdisk_params.items())
         expected_pxe_params += (
             " ipa-global-request-id=%s" % self.context.global_id)
 
@@ -1573,8 +1571,11 @@ class PXEBuildConfigOptionsTestCase(db_base.DbTestCase):
                                                 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'})
+        self._test_build_pxe_config_options_pxe(
+            whle_dsk_img=True,
+            ramdisk_params=collections.OrderedDict([('foo', 'bar'),
+                                                    ('banana', None)]),
+            expected_pxe_params='test_param foo=bar banana')
 
     def test_build_pxe_config_options_pxe_no_kernel_no_ramdisk(self):
         del self.node.driver_internal_info['is_whole_disk_image']
diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py
index 91c0e514c..e3de0eed6 100644
--- a/ironic/tests/unit/drivers/modules/test_image_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_image_utils.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import collections
 import os
 import tempfile
 from unittest import mock
@@ -576,6 +577,29 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
                 root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
                 base_iso='/path/to/baseiso', inject_files=None)
 
+    @mock.patch.object(image_utils.ImageHandler, 'publish_image',
+                       autospec=True)
+    @mock.patch.object(images, 'create_boot_iso', autospec=True)
+    def test__prepare_iso_image_extra_params(
+            self, mock_create_boot_iso, mock_publish_image):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            kernel_params = 'network-config=base64-cloudinit-blob'
+            extra_params = collections.OrderedDict([('foo', 'bar'),
+                                                    ('banana', None)])
+            self.config(kernel_append_params=kernel_params, group='redfish')
+
+            image_utils._prepare_iso_image(
+                task, 'http://kernel/img', 'http://ramdisk/img',
+                root_uuid=task.node.uuid, params=extra_params)
+
+            mock_create_boot_iso.assert_called_once_with(
+                mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',
+                boot_mode='bios', esp_image_href=None,
+                kernel_params=kernel_params + ' foo=bar banana',
+                root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
+                base_iso=None, inject_files=None)
+
     def test__prepare_iso_image_bootable_iso(self):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py
index b318049b8..d0a529852 100644
--- a/ironic/tests/unit/drivers/modules/test_inspector.py
+++ b/ironic/tests/unit/drivers/modules/test_inspector.py
@@ -123,11 +123,6 @@ class InspectHardwareTestCase(BaseTestCase):
     def test_validate_ok(self, mock_client):
         self.iface.validate(self.task)
 
-    def test_validate_invalid_kernel_params(self, mock_client):
-        CONF.set_override('extra_kernel_params', 'abcdef', group='inspector')
-        self.assertRaises(exception.InvalidParameterValue,
-                          self.iface.validate, self.task)
-
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     @mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
                        autospec=True)
@@ -231,7 +226,7 @@ class InspectHardwareTestCase(BaseTestCase):
                                    mock_client):
         CONF.set_override('extra_kernel_params',
                           'ipa-inspection-collectors=default,logs '
-                          'ipa-collect-dhcp=1',
+                          'ipa-collect-dhcp=1 something',
                           group='inspector')
         endpoint = 'http://192.169.0.42:5050/v1'
         mock_client.return_value.get_endpoint.return_value = endpoint
@@ -246,6 +241,7 @@ class InspectHardwareTestCase(BaseTestCase):
                 'ipa-inspection-callback-url': endpoint + '/continue',
                 'ipa-inspection-collectors': 'default,logs',
                 'ipa-collect-dhcp': '1',
+                'something': None,
             })
         self.driver.network.add_inspection_network.assert_called_once_with(
             self.task)