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

@@ -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
 

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 '

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()

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)

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'

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.