Add validate_rescue() method to boot interface

Adds validate_rescue() method to boot interface to validate node's
boot properties related to rescue operation. This method is called
by the validate() method of rescue interface.

Closes-Bug: #1747467
Change-Id: Ib68d49a9cdb2ae4a5d43b90716c0a0c1166398c0

ironic/drivers/base.py

@@ -486,6 +486,17 @@ class BootInterface(BaseInterface):
         :returns: None
         """
 
+    def validate_rescue(self, task):
+        """Validate that the node has required properties for rescue.
+
+        :param task: a TaskManager instance with the node being checked
+        :raises: MissingParameterValue if node is missing one or more required
+            parameters
+        :raises: UnsupportedDriverExtension
+        """
+        raise exception.UnsupportedDriverExtension(
+            driver=task.node.driver, extension='validate_rescue')
+
 
 class PowerInterface(BaseInterface):
     """Interface for power-related actions."""

ironic/drivers/modules/agent.py

@@ -58,14 +58,6 @@ OPTIONAL_PROPERTIES = {
                         '``image_https_proxy`` are not specified. Optional.'),
 }
 
-RESCUE_PROPERTIES = {
-    'rescue_kernel': _('UUID (from Glance) of the rescue kernel. This value '
-                       'is required for rescue mode.'),
-    'rescue_ramdisk': _('UUID (from Glance) of the rescue ramdisk with agent '
-                        'that is used at node rescue time. This value is '
-                        'required for rescue mode.'),
-}
-
 COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
 COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
 COMMON_PROPERTIES.update(agent_base_vendor.VENDOR_PROPERTIES)
@@ -709,11 +701,8 @@ class AgentRescue(base.RescueInterface):
     """Implementation of RescueInterface which uses agent ramdisk."""
 
     def get_properties(self):
-        """Return the properties of the interface.
+        """Return the properties of the interface. """
-
+        return {}
-        :returns: dictionary of <property name>:<property description> entries.
-        """
-        return RESCUE_PROPERTIES.copy()
 
     @METRICS.timer('AgentRescue.rescue')
     @task_manager.require_exclusive_lock
@@ -770,35 +759,25 @@ class AgentRescue(base.RescueInterface):
         :param task: a TaskManager instance with the node being checked
         :raises: InvalidParameterValue if 'instance_info/rescue_password' has
             empty password or rescuing network UUID config option
-            has an invalid value when 'neutron' network is used.
+            has an invalid value.
         :raises: MissingParameterValue if node is missing one or more required
             parameters
         """
-        node = task.node
-        missing_params = []
-
         # Validate rescuing network
         task.driver.network.validate_rescue(task)
-
         if CONF.agent.manage_agent_boot:
-            # TODO(stendulker): boot.validate() performs validation of
+            # Validate boot properties
-            # provisioning related parameters which is not required during
-            # rescue operation.
             task.driver.boot.validate(task)
-            for req in RESCUE_PROPERTIES:
+            # Validate boot properties related to rescue
-                if node.driver_info.get(req) is None:
+            task.driver.boot.validate_rescue(task)
-                    missing_params.append('driver_info/' + req)
 
+        node = task.node
         rescue_pass = node.instance_info.get('rescue_password')
         if rescue_pass is None:
-            missing_params.append('instance_info/rescue_password')
+            msg = _("Node %(node)s is missing "
-
+                    "'instance_info/rescue_password'. "
-        if missing_params:
+                    "It is required for rescuing node.")
-            msg = _('Node %(node)s is missing parameter(s): '
+            raise exception.MissingParameterValue(msg % {'node': node.uuid})
-                    '%(params)s. These are required for rescuing node.')
-            raise exception.MissingParameterValue(
-                msg % {'node': node.uuid,
-                       'params': ', '.join(missing_params)})
 
         if not rescue_pass.strip():
             msg = (_("The 'instance_info/rescue_password' is an empty string "

ironic/drivers/modules/pxe.py

@@ -55,6 +55,13 @@ OPTIONAL_PROPERTIES = {
                                       "deploy and cleaning operations. "
                                       "Defaults to False. Optional."),
 }
+RESCUE_PROPERTIES = {
+    'rescue_kernel': _('UUID (from Glance) of the rescue kernel. This value '
+                       'is required for rescue mode.'),
+    'rescue_ramdisk': _('UUID (from Glance) of the rescue ramdisk with agent '
+                        'that is used at node rescue time. This value is '
+                        'required for rescue mode.'),
+}
 COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
 COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
 
@@ -414,6 +421,9 @@ class PXEBoot(base.BootInterface):
 
         :returns: dictionary of <property name>:<property description> entries.
         """
+        # TODO(stendulker): COMMON_PROPERTIES should also include rescue
+        # related properties (RESCUE_PROPERTIES). We can add them in Rocky,
+        # when classic drivers get removed.
         return COMMON_PROPERTIES
 
     @METRICS.timer('PXEBoot.validate')
@@ -668,3 +678,13 @@ class PXEBoot(base.BootInterface):
                         {'node': node.uuid, 'err': e})
         else:
             _clean_up_pxe_env(task, images_info)
+
+    @METRICS.timer('PXEBoot.validate_rescue')
+    def validate_rescue(self, task):
+        """Validate that the node has required properties for rescue.
+
+        :param task: a TaskManager instance with the node being checked
+        :raises: MissingParameterValue if node is missing one or more required
+            parameters
+        """
+        _parse_driver_info(task.node, mode='rescue')

ironic/tests/unit/drivers/modules/test_agent.py

@@ -1360,66 +1360,34 @@ class AgentRescueTestCase(db_base.DbTestCase):
     @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue',
                        autospec=True)
     @mock.patch.object(fake.FakeBoot, 'validate', autospec=True)
-    def test_agent_rescue_validate(self, mock_boot_validate,
+    @mock.patch.object(fake.FakeBoot, 'validate_rescue', autospec=True)
+    def test_agent_rescue_validate(self, mock_boot_validate_rescue,
+                                   mock_boot_validate,
                                    mock_validate_network):
         with task_manager.acquire(self.context, self.node.uuid) as task:
             task.driver.rescue.validate(task)
             mock_validate_network.assert_called_once_with(mock.ANY, task)
             mock_boot_validate.assert_called_once_with(mock.ANY, task)
+            mock_boot_validate_rescue.assert_called_once_with(mock.ANY, task)
 
     @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue',
                        autospec=True)
     @mock.patch.object(fake.FakeBoot, 'validate', autospec=True)
-    def test_agent_rescue_validate_no_manage_agent(self, mock_boot_validate,
+    @mock.patch.object(fake.FakeBoot, 'validate_rescue', autospec=True)
+    def test_agent_rescue_validate_no_manage_agent(self,
+                                                   mock_boot_validate_rescue,
+                                                   mock_boot_validate,
                                                    mock_rescuing_net):
-        # If ironic's not managing booting of ramdisks, we don't set up PXE for
-        # the ramdisk/kernel, so validation can pass without this info
         self.config(manage_agent_boot=False, group='agent')
-        driver_info = self.node.driver_info
-        del driver_info['rescue_ramdisk']
-        del driver_info['rescue_kernel']
-        self.node.driver_info = driver_info
-        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid) as task:
             task.driver.rescue.validate(task)
             mock_rescuing_net.assert_called_once_with(mock.ANY, task)
             self.assertFalse(mock_boot_validate.called)
+            self.assertFalse(mock_boot_validate_rescue.called)
 
     @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue',
                        autospec=True)
-    @mock.patch.object(fake.FakeBoot, 'validate', autospec=True)
+    @mock.patch.object(fake.FakeBoot, 'validate_rescue', autospec=True)
-    def test_agent_rescue_validate_fails_no_rescue_ramdisk(
-            self, mock_boot_validate, mock_rescuing_net):
-        driver_info = self.node.driver_info
-        del driver_info['rescue_ramdisk']
-        self.node.driver_info = driver_info
-        self.node.save()
-        with task_manager.acquire(self.context, self.node.uuid) as task:
-            self.assertRaisesRegex(exception.MissingParameterValue,
-                                   'Node.*missing.*rescue_ramdisk',
-                                   task.driver.rescue.validate, task)
-            mock_rescuing_net.assert_called_once_with(mock.ANY, task)
-            mock_boot_validate.assert_called_once_with(mock.ANY, task)
-
-    @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue',
-                       autospec=True)
-    @mock.patch.object(fake.FakeBoot, 'validate', autospec=True)
-    def test_agent_rescue_validate_fails_no_rescue_kernel(
-            self, mock_boot_validate, mock_rescuing_net):
-        driver_info = self.node.driver_info
-        del driver_info['rescue_kernel']
-        self.node.driver_info = driver_info
-        self.node.save()
-        with task_manager.acquire(self.context, self.node.uuid) as task:
-            self.assertRaisesRegex(exception.MissingParameterValue,
-                                   'Node.*missing.*rescue_kernel',
-                                   task.driver.rescue.validate, task)
-            mock_rescuing_net.assert_called_once_with(mock.ANY, task)
-            mock_boot_validate.assert_called_once_with(mock.ANY, task)
-
-    @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue',
-                       autospec=True)
-    @mock.patch.object(fake.FakeBoot, 'validate', autospec=True)
     def test_agent_rescue_validate_fails_no_rescue_password(
             self, mock_boot_validate, mock_rescuing_net):
         instance_info = self.node.instance_info
@@ -1435,7 +1403,7 @@ class AgentRescueTestCase(db_base.DbTestCase):
 
     @mock.patch.object(flat_network.FlatNetwork, 'validate_rescue',
                        autospec=True)
-    @mock.patch.object(fake.FakeBoot, 'validate', autospec=True)
+    @mock.patch.object(fake.FakeBoot, 'validate_rescue', autospec=True)
     def test_agent_rescue_validate_fails_empty_rescue_password(
             self, mock_boot_validate, mock_rescuing_net):
         instance_info = self.node.instance_info

ironic/tests/unit/drivers/modules/test_pxe.py

@@ -34,6 +34,7 @@ from ironic.common import states
 from ironic.common import utils as common_utils
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
+from ironic.drivers import base as drivers_base
 from ironic.drivers.modules import agent_base_vendor
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules import pxe
@@ -1295,3 +1296,57 @@ class PXEBootTestCase(db_base.DbTestCase):
             clean_up_pxe_env_mock.assert_called_once_with(task, image_info)
             get_image_info_mock.assert_called_once_with(
                 task.node, task.context)
+
+
+class PXEValidateRescueTestCase(db_base.DbTestCase):
+
+    def setUp(self):
+        super(PXEValidateRescueTestCase, self).setUp()
+        for iface in drivers_base.ALL_INTERFACES:
+            impl = 'fake'
+            if iface == 'network':
+                impl = 'flat'
+            if iface == 'rescue':
+                impl = 'agent'
+            if iface == 'boot':
+                impl = 'pxe'
+            config_kwarg = {'enabled_%s_interfaces' % iface: [impl],
+                            'default_%s_interface' % iface: impl}
+            self.config(**config_kwarg)
+        self.config(enabled_hardware_types=['fake-hardware'])
+        driver_info = DRV_INFO_DICT
+        driver_info.update({'rescue_ramdisk': 'my_ramdisk',
+                            'rescue_kernel': 'my_kernel'})
+        instance_info = INST_INFO_DICT
+        instance_info.update({'rescue_password': 'password'})
+        n = {
+            'driver': 'fake-hardware',
+            'instance_info': instance_info,
+            'driver_info': driver_info,
+            'driver_internal_info': DRV_INTERNAL_INFO_DICT,
+        }
+        self.node = obj_utils.create_test_node(self.context, **n)
+
+    def test_validate_rescue(self):
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.driver.boot.validate_rescue(task)
+
+    def test_validate_rescue_no_rescue_ramdisk(self):
+        driver_info = self.node.driver_info
+        del driver_info['rescue_ramdisk']
+        self.node.driver_info = driver_info
+        self.node.save()
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            self.assertRaisesRegex(exception.MissingParameterValue,
+                                   'Missing.*rescue_ramdisk',
+                                   task.driver.boot.validate_rescue, task)
+
+    def test_validate_rescue_fails_no_rescue_kernel(self):
+        driver_info = self.node.driver_info
+        del driver_info['rescue_kernel']
+        self.node.driver_info = driver_info
+        self.node.save()
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            self.assertRaisesRegex(exception.MissingParameterValue,
+                                   'Missing.*rescue_kernel',
+                                   task.driver.boot.validate_rescue, task)

ironic/tests/unit/drivers/test_base.py

@@ -412,6 +412,16 @@ class TestDeployInterface(base.TestCase):
         self.assertTrue(mock_log.called)
 
 
+class TestBootInterface(base.TestCase):
+
+    def test_validate_rescue_default_impl(self):
+        boot = fake.FakeBoot()
+        task_mock = mock.MagicMock(spec_set=['node'])
+
+        self.assertRaises(exception.UnsupportedDriverExtension,
+                          boot.validate_rescue, task_mock)
+
+
 class TestManagementInterface(base.TestCase):
 
     def test_inject_nmi_default_impl(self):

releasenotes/notes/add-validate-rescue-to-boot-interface-bd74aff9e250334b.yaml

@@ -0,0 +1,6 @@
+---
+other:
+  - |
+    Adds new method ``validate_rescue()`` to boot interface to validate
+    node's properties related to rescue operation. This method is called
+    by the validate() method of rescue interface.