From 1c162058e3f20c89716290360b3c54024429ee64 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Mon, 5 Feb 2018 12:28:49 -0500 Subject: [PATCH] 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 | 11 ++++ ironic/drivers/modules/agent.py | 43 ++++----------- ironic/drivers/modules/pxe.py | 20 +++++++ .../tests/unit/drivers/modules/test_agent.py | 54 ++++-------------- ironic/tests/unit/drivers/modules/test_pxe.py | 55 +++++++++++++++++++ ironic/tests/unit/drivers/test_base.py | 10 ++++ ...ue-to-boot-interface-bd74aff9e250334b.yaml | 6 ++ 7 files changed, 124 insertions(+), 75 deletions(-) create mode 100644 releasenotes/notes/add-validate-rescue-to-boot-interface-bd74aff9e250334b.yaml diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 7412f88c2b..13e5b2ea28 100644 --- a/ironic/drivers/base.py +++ b/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.""" diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 12b72f3817..0a47ab3a9b 100644 --- a/ironic/drivers/modules/agent.py +++ b/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. - - :returns: dictionary of : entries. - """ - return RESCUE_PROPERTIES.copy() + """Return the properties of the interface. """ + return {} @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 - # provisioning related parameters which is not required during - # rescue operation. + # Validate boot properties task.driver.boot.validate(task) - for req in RESCUE_PROPERTIES: - if node.driver_info.get(req) is None: - missing_params.append('driver_info/' + req) + # Validate boot properties related to rescue + task.driver.boot.validate_rescue(task) + node = task.node rescue_pass = node.instance_info.get('rescue_password') if rescue_pass is None: - missing_params.append('instance_info/rescue_password') - - if missing_params: - msg = _('Node %(node)s is missing parameter(s): ' - '%(params)s. These are required for rescuing node.') - raise exception.MissingParameterValue( - msg % {'node': node.uuid, - 'params': ', '.join(missing_params)}) + msg = _("Node %(node)s is missing " + "'instance_info/rescue_password'. " + "It is required for rescuing node.") + raise exception.MissingParameterValue(msg % {'node': node.uuid}) if not rescue_pass.strip(): msg = (_("The 'instance_info/rescue_password' is an empty string " diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index c43558377f..dea8e2f974 100644 --- a/ironic/drivers/modules/pxe.py +++ b/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 : 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') diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 3303e0b349..bc9ff9c3bf 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/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) - 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) + @mock.patch.object(fake.FakeBoot, 'validate_rescue', 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 diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 454d508c5d..0af37c88e6 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/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) diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index d0185c3c8b..892da1c8f9 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/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): diff --git a/releasenotes/notes/add-validate-rescue-to-boot-interface-bd74aff9e250334b.yaml b/releasenotes/notes/add-validate-rescue-to-boot-interface-bd74aff9e250334b.yaml new file mode 100644 index 0000000000..5b74e30559 --- /dev/null +++ b/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.