diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index 5c69c58748..2800c304b0 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -33,7 +33,7 @@ LOG = log.getLogger(__name__) EM_SEMAPHORE = 'extension_manager' -def build_driver_for_task(task, driver_name=None): +def build_driver_for_task(task): """Builds a composable driver for a given task. Starts with a `BareDriver` object, and attaches implementations of the @@ -42,8 +42,6 @@ def build_driver_for_task(task, driver_name=None): driver factories and are configurable via the database. :param task: The task containing the node to build a driver for. - :param driver_name: The name of the classic driver or hardware type to use - as a base, if different than task.node.driver. :returns: A driver object for the task. :raises: DriverNotFound if node.driver could not be found in either "ironic.drivers" or "ironic.hardware.types" namespaces. @@ -53,7 +51,7 @@ def build_driver_for_task(task, driver_name=None): the requested implementation is not compatible with it. """ node = task.node - driver_name = driver_name or node.driver + driver_name = node.driver driver_or_hw_type = get_driver_or_hardware_type(driver_name) try: diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 7e2c1273eb..8ad217dd71 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -189,9 +189,7 @@ class ConductorManager(base_manager.BaseConductorManager): driver_factory.check_and_update_node_interfaces(node_obj) - driver_name = node_obj.driver if 'driver' in delta else None with task_manager.acquire(context, node_id, shared=False, - driver_name=driver_name, purpose='node update') as task: # Prevent instance_uuid overwriting if ('instance_uuid' in delta and node_obj.instance_uuid and diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index f19a736170..19f7633d4f 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -149,23 +149,20 @@ def require_exclusive_lock(f): return wrapper -def acquire(context, node_id, shared=False, driver_name=None, - purpose='unspecified action'): +def acquire(context, node_id, shared=False, purpose='unspecified action'): """Shortcut for acquiring a lock on a Node. :param context: Request context. :param node_id: ID or UUID of node to lock. :param shared: Boolean indicating whether to take a shared or exclusive lock. Default: False. - :param driver_name: Name of Driver. Default: None. :param purpose: human-readable purpose to put to debug logs. :returns: An instance of :class:`TaskManager`. """ # NOTE(lintan): This is a workaround to set the context of periodic tasks. context.ensure_thread_contain_context() - return TaskManager(context, node_id, shared=shared, - driver_name=driver_name, purpose=purpose) + return TaskManager(context, node_id, shared=shared, purpose=purpose) class TaskManager(object): @@ -176,7 +173,7 @@ class TaskManager(object): """ - def __init__(self, context, node_id, shared=False, driver_name=None, + def __init__(self, context, node_id, shared=False, purpose='unspecified action'): """Create a new TaskManager. @@ -189,8 +186,6 @@ class TaskManager(object): :param node_id: ID or UUID of node to lock. :param shared: Boolean indicating whether to take a shared or exclusive lock. Default: False. - :param driver_name: The name of the driver to load, if different - from the Node's current driver. :param purpose: human-readable purpose to put to debug logs. :raises: DriverNotFound :raises: InterfaceNotFoundInEntrypoint @@ -236,8 +231,7 @@ class TaskManager(object): context, self.node.id) self.volume_targets = objects.VolumeTarget.list_by_node_id( context, self.node.id) - self.driver = driver_factory.build_driver_for_task( - self, driver_name=driver_name) + self.driver = driver_factory.build_driver_for_task(self) except Exception: with excutils.save_and_reraise_exception(): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 06662cf4a2..ca2a322c84 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -706,6 +706,21 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): states.ACTIVE, resource_class='old', new_resource_class=None, expect_error=True) + def test_update_node_hardware_type(self): + existing_hardware = 'fake-hardware' + existing_interface = 'fake' + new_hardware = 'manual-management' + new_interface = 'pxe' + node = obj_utils.create_test_node(self.context, + driver=existing_hardware, + boot_interface=existing_interface) + node.driver = new_hardware + node.boot_interface = new_interface + self.service.update_node(self.context, node) + node.refresh() + self.assertEqual(new_hardware, node.driver) + self.assertEqual(new_interface, node.boot_interface) + @mgr_utils.mock_record_keepalive class VendorPassthruTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): diff --git a/ironic/tests/unit/conductor/test_task_manager.py b/ironic/tests/unit/conductor/test_task_manager.py index 732db381ac..ced54195bd 100644 --- a/ironic/tests/unit/conductor/test_task_manager.py +++ b/ironic/tests/unit/conductor/test_task_manager.py @@ -67,36 +67,7 @@ class TaskManagerTestCase(db_base.DbTestCase): self.assertEqual(get_voltgt_mock.return_value, task.volume_targets) self.assertEqual(build_driver_mock.return_value, task.driver) self.assertFalse(task.shared) - build_driver_mock.assert_called_once_with(task, driver_name=None) - - node_get_mock.assert_called_once_with(self.context, 'fake-node-id') - reserve_mock.assert_called_once_with(self.context, self.host, - 'fake-node-id') - get_ports_mock.assert_called_once_with(self.context, self.node.id) - get_portgroups_mock.assert_called_once_with(self.context, self.node.id) - get_volconn_mock.assert_called_once_with(self.context, self.node.id) - get_voltgt_mock.assert_called_once_with(self.context, self.node.id) - release_mock.assert_called_once_with(self.context, self.host, - self.node.id) - - def test_excl_lock_with_driver( - self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, - get_ports_mock, build_driver_mock, - reserve_mock, release_mock, node_get_mock): - reserve_mock.return_value = self.node - with task_manager.TaskManager(self.context, 'fake-node-id', - driver_name='fake-driver') as task: - self.assertEqual(self.context, task.context) - self.assertEqual(self.node, task.node) - self.assertEqual(get_ports_mock.return_value, task.ports) - self.assertEqual(get_portgroups_mock.return_value, task.portgroups) - self.assertEqual(get_volconn_mock.return_value, - task.volume_connectors) - self.assertEqual(get_voltgt_mock.return_value, task.volume_targets) - self.assertEqual(build_driver_mock.return_value, task.driver) - self.assertFalse(task.shared) - build_driver_mock.assert_called_once_with( - task, driver_name='fake-driver') + build_driver_mock.assert_called_once_with(task) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') reserve_mock.assert_called_once_with(self.context, self.host, @@ -150,8 +121,7 @@ class TaskManagerTestCase(db_base.DbTestCase): self.assertEqual(mock.sentinel.driver2, task2.driver) self.assertFalse(task2.shared) - self.assertEqual([mock.call(task, driver_name=None), - mock.call(task2, driver_name=None)], + self.assertEqual([mock.call(task), mock.call(task2)], build_driver_mock.call_args_list) self.assertEqual([mock.call(self.context, 'node-id1'), @@ -312,7 +282,7 @@ class TaskManagerTestCase(db_base.DbTestCase): 'fake-node-id') get_ports_mock.assert_called_once_with(self.context, self.node.id) get_portgroups_mock.assert_called_once_with(self.context, self.node.id) - build_driver_mock.assert_called_once_with(mock.ANY, driver_name=None) + build_driver_mock.assert_called_once_with(mock.ANY) release_mock.assert_called_once_with(self.context, self.host, self.node.id) @@ -333,37 +303,7 @@ class TaskManagerTestCase(db_base.DbTestCase): self.assertEqual(build_driver_mock.return_value, task.driver) self.assertTrue(task.shared) - build_driver_mock.assert_called_once_with(task, driver_name=None) - - self.assertFalse(reserve_mock.called) - self.assertFalse(release_mock.called) - node_get_mock.assert_called_once_with(self.context, 'fake-node-id') - get_ports_mock.assert_called_once_with(self.context, self.node.id) - get_portgroups_mock.assert_called_once_with(self.context, self.node.id) - get_volconn_mock.assert_called_once_with(self.context, self.node.id) - get_voltgt_mock.assert_called_once_with(self.context, self.node.id) - - def test_shared_lock_with_driver( - self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, - get_ports_mock, build_driver_mock, - reserve_mock, release_mock, node_get_mock): - node_get_mock.return_value = self.node - with task_manager.TaskManager(self.context, - 'fake-node-id', - shared=True, - driver_name='fake-driver') as task: - self.assertEqual(self.context, task.context) - self.assertEqual(self.node, task.node) - self.assertEqual(get_ports_mock.return_value, task.ports) - self.assertEqual(get_portgroups_mock.return_value, task.portgroups) - self.assertEqual(get_volconn_mock.return_value, - task.volume_connectors) - self.assertEqual(get_voltgt_mock.return_value, task.volume_targets) - self.assertEqual(build_driver_mock.return_value, task.driver) - self.assertTrue(task.shared) - - build_driver_mock.assert_called_once_with( - task, driver_name='fake-driver') + build_driver_mock.assert_called_once_with(task) self.assertFalse(reserve_mock.called) self.assertFalse(release_mock.called) @@ -491,7 +431,7 @@ class TaskManagerTestCase(db_base.DbTestCase): get_portgroups_mock.assert_called_once_with(self.context, self.node.id) get_volconn_mock.assert_called_once_with(self.context, self.node.id) get_voltgt_mock.assert_called_once_with(self.context, self.node.id) - build_driver_mock.assert_called_once_with(mock.ANY, driver_name=None) + build_driver_mock.assert_called_once_with(mock.ANY) def test_upgrade_lock( self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, @@ -520,8 +460,7 @@ class TaskManagerTestCase(db_base.DbTestCase): self.assertFalse(task.shared) self.assertEqual('spam', task._purpose) - build_driver_mock.assert_called_once_with(mock.ANY, - driver_name=None) + build_driver_mock.assert_called_once_with(mock.ANY) # make sure reserve() was called only once reserve_mock.assert_called_once_with(self.context, self.host, diff --git a/releasenotes/notes/bug-2001832-62e244dc48c1f79e.yaml b/releasenotes/notes/bug-2001832-62e244dc48c1f79e.yaml new file mode 100644 index 0000000000..7f5eb75599 --- /dev/null +++ b/releasenotes/notes/bug-2001832-62e244dc48c1f79e.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes a bug where a node's hardware type cannot be changed to another + hardware type which doesn't support any hardware interface currently used. + See `bug 2001832 `_ + for details.