From 2e8db13e0917d127f107670a87453b9bdc308931 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 22 Jan 2024 19:39:22 +0100 Subject: [PATCH] Flip require_managed_boot to True for the new agent inspection This value is a way for an operator to signal Ironic whether they have an infrastructure for unmanaged inspection. Previously, unmanaged inspection was considered to be always supported. With this change, the inspector-based inspection works as previously, while the new built-in inspection defaults to only managed inspection. Change-Id: I4a9125881dc5822656efde1346807c3dd749973e --- doc/source/admin/inspection/managed.rst | 5 +++-- ironic/conf/inspector.py | 14 +++++++++----- ironic/drivers/modules/inspector/agent.py | 2 ++ ironic/drivers/modules/inspector/interface.py | 11 +++++++++-- .../unit/drivers/modules/inspector/test_agent.py | 7 +++++++ .../require-managed-boot-c33e8aa9cba1502c.yaml | 8 ++++++++ 6 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/require-managed-boot-c33e8aa9cba1502c.yaml diff --git a/doc/source/admin/inspection/managed.rst b/doc/source/admin/inspection/managed.rst index 93e17d227b..0fcbae00d7 100644 --- a/doc/source/admin/inspection/managed.rst +++ b/doc/source/admin/inspection/managed.rst @@ -57,8 +57,9 @@ Unmanaged inspection was the only inspection mode before the Ussuri release, and it is still used when the node's boot cannot be configured by the conductor. The options described above do not affect unmanaged inspection. -Unmanaged inspection is currently enabled by default but will be disabled -in the near future. To enable it, set ``require_managed_boot`` to ``False``: +Because of the complex installation and operation requirements, unmanaged +inspection is disabled by default. To enable it, set ``require_managed_boot`` +to ``False``: .. code-block:: ini diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py index db5e9b1549..c3ec562e24 100644 --- a/ironic/conf/inspector.py +++ b/ironic/conf/inspector.py @@ -59,14 +59,18 @@ opts = [ help=_('endpoint to use as a callback for posting back ' 'introspection data when boot is managed by ironic. ' 'Standard keystoneauth options are used by default.')), + # TODO(dtantsur): change the default to True when ironic-inspector is no + # longer supported (and update the help string). cfg.BoolOpt('require_managed_boot', default=None, help=_('require that the in-band inspection boot is fully ' 'managed by the node\'s boot interface. Set this to ' - 'True if your installation does not have a separate ' - '(i)PXE boot environment for node discovery. Set ' - 'to False if you need to inspect nodes that are not ' - 'supported by boot interfaces (e.g. because they ' - 'don\'t have ports).')), + 'False if your installation has a separate (i)PXE boot ' + 'environment for node discovery or unmanaged ' + 'inspection. You may need to set it to False to ' + 'inspect nodes that are not supported by boot ' + 'interfaces (e.g. because they don\'t have ports). ' + 'The default value depends on which inspect interface ' + 'is used: inspector uses False, agent - True.')), cfg.StrOpt('add_ports', default='pxe', help=_('Which MAC addresses to add as ports during ' diff --git a/ironic/drivers/modules/inspector/agent.py b/ironic/drivers/modules/inspector/agent.py index 671e873b70..baa6201d7f 100644 --- a/ironic/drivers/modules/inspector/agent.py +++ b/ironic/drivers/modules/inspector/agent.py @@ -38,6 +38,8 @@ CONF = cfg.CONF class AgentInspect(common.Common): """In-band inspection.""" + default_require_managed_boot = True + def __init__(self): super().__init__() self.hooks = hooks_base.validate_inspection_hooks() diff --git a/ironic/drivers/modules/inspector/interface.py b/ironic/drivers/modules/inspector/interface.py index c5911532f7..e0f142370e 100644 --- a/ironic/drivers/modules/inspector/interface.py +++ b/ironic/drivers/modules/inspector/interface.py @@ -153,6 +153,8 @@ def prepare_managed_inspection(task, endpoint): class Common(base.InspectInterface): + default_require_managed_boot = False + def __init__(self): super().__init__() if CONF.inspector.require_managed_boot is None: @@ -161,6 +163,11 @@ class Common(base.InspectInterface): "Set it to an explicit boolean value to avoid a " "potential breakage.") + def _require_managed_boot(self): + return (CONF.inspector.require_managed_boot + if CONF.inspector.require_managed_boot is not None + else self.default_require_managed_boot) + def get_properties(self): """Return the properties of the interface. @@ -177,7 +184,7 @@ class Common(base.InspectInterface): :raises: UnsupportedDriverExtension """ utils.parse_kernel_params(CONF.inspector.extra_kernel_params) - if CONF.inspector.require_managed_boot: + if self._require_managed_boot(): ironic_manages_boot(task, raise_exc=True) def inspect_hardware(self, task): @@ -197,7 +204,7 @@ class Common(base.InspectInterface): ' on node %s.', task.node.uuid) manage_boot = ironic_manages_boot( - task, raise_exc=CONF.inspector.require_managed_boot) + task, raise_exc=self._require_managed_boot()) utils.set_node_nested_field(task.node, 'driver_internal_info', _IRONIC_MANAGES_BOOT, manage_boot) diff --git a/ironic/tests/unit/drivers/modules/inspector/test_agent.py b/ironic/tests/unit/drivers/modules/inspector/test_agent.py index a16af75dae..452d2259ed 100644 --- a/ironic/tests/unit/drivers/modules/inspector/test_agent.py +++ b/ironic/tests/unit/drivers/modules/inspector/test_agent.py @@ -42,6 +42,7 @@ class InspectHardwareTestCase(db_base.DbTestCase): self.driver = self.task.driver def test_unmanaged_ok(self, mock_create_ports_if_not_exist): + CONF.set_override('require_managed_boot', False, group='inspector') self.driver.boot.validate_inspection.side_effect = ( exception.UnsupportedDriverExtension('')) self.assertEqual(states.INSPECTWAIT, @@ -58,6 +59,12 @@ class InspectHardwareTestCase(db_base.DbTestCase): self.assertFalse(self.driver.network.remove_inspection_network.called) self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + def test_unmanaged_disallowed(self, mock_create_ports_if_not_exist): + self.driver.boot.validate_inspection.side_effect = ( + exception.UnsupportedDriverExtension('')) + self.assertRaises(exception.UnsupportedDriverExtension, + self.iface.inspect_hardware, self.task) + @mock.patch.object(deploy_utils, 'get_ironic_api_url', autospec=True) def test_managed_ok(self, mock_get_url, mock_create_ports_if_not_exist): endpoint = 'http://192.169.0.42:6385/v1' diff --git a/releasenotes/notes/require-managed-boot-c33e8aa9cba1502c.yaml b/releasenotes/notes/require-managed-boot-c33e8aa9cba1502c.yaml new file mode 100644 index 0000000000..f27de3895e --- /dev/null +++ b/releasenotes/notes/require-managed-boot-c33e8aa9cba1502c.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The default value of the configuration option + ``[inspector]require_managed_boot`` is now ``True`` for the newer ``agent`` + inspect interface. The older ``inspector`` implementation is not affected. + Operators with deployments that support unmanaged inspection must set + this value to ``False`` explicitly.