From 216969427b964d91079c0f781b8579a6aaa8290c Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 29 Apr 2014 18:12:15 +0100 Subject: [PATCH] Remove 'node' parameter from the validate() methods The node parameter is redundant since the node object is an attribute of the task object. Closes-Bug: #1312632 Change-Id: Ia4193ea561a31e54754049d9964c6c9021474caf --- ironic/conductor/manager.py | 16 ++++++------- ironic/drivers/base.py | 31 +++++--------------------- ironic/drivers/modules/fake.py | 8 +++---- ironic/drivers/modules/ipminative.py | 5 ++--- ironic/drivers/modules/ipmitool.py | 13 +++++------ ironic/drivers/modules/pxe.py | 2 +- ironic/drivers/modules/seamicro.py | 3 +-- ironic/drivers/modules/ssh.py | 7 +++--- ironic/tests/conductor/test_manager.py | 6 ++--- ironic/tests/drivers/test_fake.py | 6 ++--- ironic/tests/drivers/test_pxe.py | 24 ++++++++------------ ironic/tests/drivers/test_seamicro.py | 5 ++--- ironic/tests/drivers/test_ssh.py | 7 +++--- 13 files changed, 50 insertions(+), 83 deletions(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 6f05db90f0..a693582615 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -208,7 +208,7 @@ class ConductorManager(periodic_task.PeriodicTasks): # TODO(deva): Determine what value will be passed by API when # instance_uuid needs to be unset, and handle it. if 'instance_uuid' in delta: - task.driver.power.validate(task, node_obj) + task.driver.power.validate(task) node_obj['power_state'] = \ task.driver.power.get_power_state(task) @@ -246,7 +246,7 @@ class ConductorManager(periodic_task.PeriodicTasks): % {'node': node_id, 'state': new_state}) with task_manager.acquire(context, node_id, shared=False) as task: - task.driver.power.validate(task, task.node) + task.driver.power.validate(task) task.spawn_after(self._spawn_worker, utils.node_power_action, task, new_state) @@ -376,7 +376,7 @@ class ConductorManager(periodic_task.PeriodicTasks): node=node.uuid) try: - task.driver.deploy.validate(task, node) + task.driver.deploy.validate(task) except exception.InvalidParameterValue as e: raise exception.InstanceDeployFailure(_( "RPC do_node_deploy failed to validate deploy info. " @@ -444,7 +444,7 @@ class ConductorManager(periodic_task.PeriodicTasks): % {'node': node_id, 'state': node.provision_state}) try: - task.driver.deploy.validate(task, node) + task.driver.deploy.validate(task) except exception.InvalidParameterValue as e: raise exception.InstanceDeployFailure(_( "RPC do_node_tear_down failed to validate deploy info. " @@ -510,7 +510,7 @@ class ConductorManager(periodic_task.PeriodicTasks): # prevent node from switching to maintenance mode. if node.power_state is None: try: - task.driver.power.validate(task, node) + task.driver.power.validate(task) except exception.InvalidParameterValue: return @@ -720,7 +720,7 @@ class ConductorManager(periodic_task.PeriodicTasks): result = reason = None if iface: try: - iface.validate(task, task.node) + iface.validate(task) result = True except (exception.InvalidParameterValue, exception.UnsupportedDriverExtension) as e: @@ -832,7 +832,7 @@ class ConductorManager(periodic_task.PeriodicTasks): if not node.console_enabled: raise exception.NodeConsoleNotEnabled(node=node_id) - task.driver.console.validate(task, node) + task.driver.console.validate(task) return task.driver.console.get_console(task) @messaging.expected_exceptions(exception.NoFreeConductorWorker, @@ -865,7 +865,7 @@ class ConductorManager(periodic_task.PeriodicTasks): raise exception.UnsupportedDriverExtension(driver=node.driver, extension='console') - task.driver.console.validate(task, node) + task.driver.console.validate(task) if enabled == node.console_enabled: op = _('enabled') if enabled else _('disabled') diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 13fc13754c..707c73cb87 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -93,11 +93,8 @@ class BaseDriver(object): class DeployInterface(object): """Interface for deploy-related actions.""" - # TODO(lucasagomes): The 'node' parameter needs to be passed to validate() - # because of the ConductorManager.validate_driver_interfaces(). - # Remove it after all cleaning all the interfaces @abc.abstractmethod - def validate(self, task, node): + def validate(self, task): """Validate the driver-specific Node deployment info. This method validates whether the 'driver_info' property of the @@ -106,7 +103,6 @@ class DeployInterface(object): returns None. :param task: a TaskManager instance containing the node to act on. - :param node: a single Node to validate. :raises: InvalidParameterValue """ @@ -193,11 +189,8 @@ class DeployInterface(object): class PowerInterface(object): """Interface for power-related actions.""" - # TODO(lucasagomes): The 'node' parameter needs to be passed to validate() - # because of the ConductorManager.validate_driver_interfaces(). - # Remove it after all cleaning all the interfaces @abc.abstractmethod - def validate(self, task, node): + def validate(self, task): """Validate the driver-specific Node power info. This method validates whether the 'driver_info' property of the @@ -206,7 +199,6 @@ class PowerInterface(object): otherwise, returns None. :param task: a TaskManager instance containing the node to act on. - :param node: a single Node to validate. :raises: InvalidParameterValue """ @@ -238,11 +230,8 @@ class PowerInterface(object): class ConsoleInterface(object): """Interface for console-related actions.""" - # TODO(lucasagomes): The 'node' parameter needs to be passed to validate() - # because of the ConductorManager.validate_driver_interfaces(). - # Remove it after all cleaning all the interfaces @abc.abstractmethod - def validate(self, task, node): + def validate(self, task): """Validate the driver-specific Node console info. This method validates whether the 'driver_info' property of the @@ -251,7 +240,6 @@ class ConsoleInterface(object): otherwise returns None. :param task: a TaskManager instance containing the node to act on. - :param node: a single Node to validate. :raises: InvalidParameterValue """ @@ -285,17 +273,13 @@ class ConsoleInterface(object): class RescueInterface(object): """Interface for rescue-related actions.""" - # TODO(lucasagomes): The 'node' parameter needs to be passed to validate() - # because of the ConductorManager.validate_driver_interfaces(). - # Remove it after all cleaning all the interfaces @abc.abstractmethod - def validate(self, task, node): + def validate(self, task): """Validate the rescue info stored in the node' properties. If invalid, raises an exception; otherwise returns None. :param task: a TaskManager instance containing the node to act on. - :param node: a single Node to validate. :raises: InvalidParameterValue """ @@ -373,18 +357,13 @@ class VendorInterface(object): class ManagementInterface(object): """Interface for management related actions.""" - # TODO(lucasagomes): The 'node' parameter - # needs to be passed to validate() because of the - # ConductorManager.validate_driver_interfaces(). Remove it as part of - # https://bugs.launchpad.net/ironic/+bug/1312632. @abc.abstractmethod - def validate(self, task, node): + def validate(self, task): """Validate the driver-specific management information. If invalid, raises an exception; otherwise returns None. :param task: a task from TaskManager. - :param node: a single Node to validate. :raises: InvalidParameterValue """ diff --git a/ironic/drivers/modules/fake.py b/ironic/drivers/modules/fake.py index b69ef9c015..a6632f3870 100644 --- a/ironic/drivers/modules/fake.py +++ b/ironic/drivers/modules/fake.py @@ -42,7 +42,7 @@ def _raise_unsupported_error(method=None): class FakePower(base.PowerInterface): """Example implementation of a simple power interface.""" - def validate(self, task, node): + def validate(self, task): pass def get_power_state(self, task): @@ -63,7 +63,7 @@ class FakeDeploy(base.DeployInterface): separate power interface. """ - def validate(self, task, node): + def validate(self, task): pass def deploy(self, task): @@ -133,7 +133,7 @@ class FakeVendorB(base.VendorInterface): class FakeConsole(base.ConsoleInterface): """Example implementation of a simple console interface.""" - def validate(self, task, node): + def validate(self, task): pass def start_console(self, task): @@ -149,7 +149,7 @@ class FakeConsole(base.ConsoleInterface): class FakeManagement(base.ManagementInterface): """Example implementation of a simple management interface.""" - def validate(self, task, node): + def validate(self, task): pass def get_supported_boot_devices(self): diff --git a/ironic/drivers/modules/ipminative.py b/ironic/drivers/modules/ipminative.py index 7a2adb9fbf..c1196d488d 100644 --- a/ironic/drivers/modules/ipminative.py +++ b/ironic/drivers/modules/ipminative.py @@ -203,11 +203,10 @@ def _power_status(driver_info): class NativeIPMIPower(base.PowerInterface): """The power driver using native python-ipmi library.""" - def validate(self, task, node): + def validate(self, task): """Check that node['driver_info'] contains IPMI credentials. - :param task: a task from TaskManager. - :param node: a single node to validate. + :param task: a TaskManager instance containing the node to act on. :raises: InvalidParameterValue when required ipmi credentials are missing. """ diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index d679b62ea3..a6a2a0adce 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -353,17 +353,16 @@ class IPMIPower(base.PowerInterface): # is not present on the system. pass - def validate(self, task, node): + def validate(self, task): """Validate driver_info for ipmitool driver. Check that node['driver_info'] contains IPMI credentials. - :param task: a task from TaskManager. - :param node: Single node object. + :param task: a TaskManager instance containing the node to act on. :raises: InvalidParameterValue if required ipmi parameters are missing. """ - _parse_driver_info(node) + _parse_driver_info(task.node) # NOTE(deva): don't actually touch the BMC in validate because it is # called too often, and BMCs are too fragile. # This is a temporary measure to mitigate problems while @@ -485,13 +484,13 @@ class IPMIShellinaboxConsole(base.ConsoleInterface): # is not present on the system. pass - def validate(self, task, node): + def validate(self, task): """Validate the Node console info. - :param node: a single Node to validate. + :param task: a task from TaskManager. :raises: InvalidParameterValue """ - driver_info = _parse_driver_info(node) + driver_info = _parse_driver_info(task.node) if not driver_info['port']: raise exception.InvalidParameterValue(_( "IPMI terminal port not supplied to IPMI driver.")) diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 15192ce2bf..b98e0c9174 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -542,7 +542,7 @@ def _validate_glance_image(ctx, driver_info): class PXEDeploy(base.DeployInterface): """PXE Deploy Interface: just a stub until the real driver is ported.""" - def validate(self, task, node): + def validate(self, task): """Validate the driver-specific Node deployment info. :param task: a TaskManager instance containing the node to act on. diff --git a/ironic/drivers/modules/seamicro.py b/ironic/drivers/modules/seamicro.py index a7ad2b275f..92990346f4 100644 --- a/ironic/drivers/modules/seamicro.py +++ b/ironic/drivers/modules/seamicro.py @@ -316,13 +316,12 @@ class Power(base.PowerInterface): state of servers in a seamicro chassis. """ - def validate(self, task, node): + def validate(self, task): """Check that node 'driver_info' is valid. Check that node 'driver_info' contains the required fields. :param task: a TaskManager instance containing the node to act on. - :param node: Single node object. :raises: InvalidParameterValue if required seamicro parameters are missing. """ diff --git a/ironic/drivers/modules/ssh.py b/ironic/drivers/modules/ssh.py index 4f1a1e148f..2d8d470f89 100644 --- a/ironic/drivers/modules/ssh.py +++ b/ironic/drivers/modules/ssh.py @@ -354,20 +354,19 @@ class SSHPower(base.PowerInterface): NOTE: This driver does not currently support multi-node operations. """ - def validate(self, task, node): + def validate(self, task): """Check that the node's 'driver_info' is valid. Check that the node's 'driver_info' contains the requisite fields and that an SSH connection to the node can be established. - :param task: a task from TaskManager. - :param node: Single node object. + :param task: a TaskManager instance containing the node to act on. :raises: InvalidParameterValue if any connection parameters are incorrect or if ssh failed to connect to the node. """ if not driver_utils.get_node_mac_addresses(task): raise exception.InvalidParameterValue(_("Node %s does not have " - "any port associated with it.") % node.uuid) + "any port associated with it.") % task.node.uuid) try: _get_connection(task.node) except exception.SSHConnectFailed as e: diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index e6183d8e9c..000b2b3e06 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -288,7 +288,7 @@ class ManagerTestCase(tests_db_base.DbTestCase): self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) node.refresh() - validate_mock.assert_called_once_with(mock.ANY, mock.ANY) + validate_mock.assert_called_once_with(mock.ANY) self.assertEqual(states.POWER_ON, node.power_state) self.assertIsNone(node.target_power_state) self.assertIsNone(node.last_error) @@ -1255,7 +1255,7 @@ class ManagerDoSyncPowerStateTestCase(tests_base.TestCase): def test_state_not_set(self, node_power_action): self._do_sync_power_state(None, states.POWER_ON) - self.power.validate.assert_called_once_with(self.task, self.node) + self.power.validate.assert_called_once_with(self.task) self.power.get_power_state.assert_called_once_with(self.task) self.node.save.assert_called_once_with(self.context) self.assertFalse(node_power_action.called) @@ -1265,7 +1265,7 @@ class ManagerDoSyncPowerStateTestCase(tests_base.TestCase): self._do_sync_power_state(None, states.POWER_ON, fail_validate=True) - self.power.validate.assert_called_once_with(self.task, self.task.node) + self.power.validate.assert_called_once_with(self.task) self.assertFalse(self.power.get_power_state.called) self.assertFalse(self.node.save.called) self.assertFalse(node_power_action.called) diff --git a/ironic/tests/drivers/test_fake.py b/ironic/tests/drivers/test_fake.py index 6ffade5112..ca05120421 100644 --- a/ironic/tests/drivers/test_fake.py +++ b/ironic/tests/drivers/test_fake.py @@ -54,7 +54,7 @@ class FakeDriverTestCase(base.TestCase): self.assertIsNone(self.driver.rescue) def test_power_interface(self): - self.driver.power.validate(self.task, self.node) + self.driver.power.validate(self.task) self.driver.power.get_power_state(self.task) self.assertRaises(exception.InvalidParameterValue, self.driver.power.set_power_state, @@ -63,7 +63,7 @@ class FakeDriverTestCase(base.TestCase): self.driver.power.reboot(self.task) def test_deploy_interface(self): - self.driver.deploy.validate(None, self.node) + self.driver.deploy.validate(None) self.driver.deploy.prepare(None) self.driver.deploy.deploy(None) @@ -74,7 +74,7 @@ class FakeDriverTestCase(base.TestCase): self.driver.deploy.tear_down(None) def test_management_interface_validate(self): - self.driver.management.validate(self.task, self.node) + self.driver.management.validate(self.task) def test_management_interface_set_boot_device_good(self): self.driver.management.set_boot_device(self.task, boot_devices.PXE) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index 288c72889c..c73d5e8560 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -577,7 +577,7 @@ class PXEDriverTestCase(db_base.DbTestCase): 'ramdisk_id': 'fake-initr'}} with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - task.driver.deploy.validate(task, self.node) + task.driver.deploy.validate(task) def test_validate_fail(self): info = dict(INFO_DICT) @@ -587,8 +587,7 @@ class PXEDriverTestCase(db_base.DbTestCase): shared=True) as task: task.node['driver_info'] = json.dumps(info) self.assertRaises(exception.InvalidParameterValue, - task.driver.deploy.validate, - task, task.node) + task.driver.deploy.validate, task) def test_validate_fail_no_port(self): new_node = obj_utils.create_test_node( @@ -598,8 +597,7 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, new_node.uuid, shared=True) as task: self.assertRaises(exception.InvalidParameterValue, - task.driver.deploy.validate, - task, new_node) + task.driver.deploy.validate, task) @mock.patch.object(base_image_service.BaseImageService, '_show') @mock.patch.object(keystone, 'get_service_url') @@ -612,7 +610,7 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - task.driver.deploy.validate(task, self.node) + task.driver.deploy.validate(task) self.assertFalse(mock_ks.called) @mock.patch.object(base_image_service.BaseImageService, '_show') @@ -627,7 +625,7 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - task.driver.deploy.validate(task, self.node) + task.driver.deploy.validate(task) mock_ks.assert_called_once_with() @mock.patch.object(keystone, 'get_service_url') @@ -640,8 +638,7 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: self.assertRaises(exception.InvalidParameterValue, - task.driver.deploy.validate, - task, self.node) + task.driver.deploy.validate, task) mock_ks.assert_called_once_with() @mock.patch.object(base_image_service.BaseImageService, '_show') @@ -651,7 +648,7 @@ class PXEDriverTestCase(db_base.DbTestCase): shared=True) as task: self.assertRaises(exception.InvalidParameterValue, task.driver.deploy.validate, - task, self.node) + task) @mock.patch.object(base_image_service.BaseImageService, '_show') def test_validate_fail_glance_image_doesnt_exists(self, mock_glance): @@ -659,8 +656,7 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: self.assertRaises(exception.InvalidParameterValue, - task.driver.deploy.validate, - task, self.node) + task.driver.deploy.validate, task) @mock.patch.object(base_image_service.BaseImageService, '_show') def test_validate_fail_glance_conn_problem(self, mock_glance): @@ -672,8 +668,7 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: self.assertRaises(exception.InvalidParameterValue, - task.driver.deploy.validate, - task, self.node) + task.driver.deploy.validate, task) def test_vendor_passthru_validate_good(self): with task_manager.acquire(self.context, self.node.uuid, @@ -758,7 +753,6 @@ class PXEDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - # state = task.driver.deploy.deploy(task, self.node) self.assertRaises(exception.InstanceDeployFailure, task.driver.deploy.deploy, task) mock_cache_instance_image.assert_called_once_with( diff --git a/ironic/tests/drivers/test_seamicro.py b/ironic/tests/drivers/test_seamicro.py index 1d406161b2..a936bef76e 100644 --- a/ironic/tests/drivers/test_seamicro.py +++ b/ironic/tests/drivers/test_seamicro.py @@ -276,7 +276,7 @@ class SeaMicroPowerDriverTestCase(db_base.DbTestCase): def test_power_interface_validate_good(self, parse_drv_info_mock): with task_manager.acquire(self.context, self.node['uuid'], shared=True) as task: - task.driver.power.validate(task, self.node) + task.driver.power.validate(task) self.assertEqual(1, parse_drv_info_mock.call_count) @mock.patch.object(seamicro, '_parse_driver_info') @@ -286,8 +286,7 @@ class SeaMicroPowerDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node['uuid'], shared=True) as task: self.assertRaises(exception.InvalidParameterValue, - task.driver.power.validate, - task, self.node) + task.driver.power.validate, task) self.assertEqual(1, parse_drv_info_mock.call_count) @mock.patch.object(seamicro, '_reboot') diff --git a/ironic/tests/drivers/test_ssh.py b/ironic/tests/drivers/test_ssh.py index 2e2e66ec99..6f09f0dc4a 100644 --- a/ironic/tests/drivers/test_ssh.py +++ b/ironic/tests/drivers/test_ssh.py @@ -576,9 +576,8 @@ class SSHDriverTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, info['uuid'], shared=False) as task: self.assertRaises(exception.InvalidParameterValue, - task.driver.power.validate, - task, self.node) - driver_info = ssh._parse_driver_info(self.node) + task.driver.power.validate, task) + driver_info = ssh._parse_driver_info(task.node) ssh_connect_mock.assert_called_once_with(driver_info) def test_validate_fail_no_port(self): @@ -592,7 +591,7 @@ class SSHDriverTestCase(db_base.DbTestCase): shared=True) as task: self.assertRaises(exception.InvalidParameterValue, task.driver.power.validate, - task, new_node) + task) @mock.patch.object(driver_utils, 'get_node_mac_addresses') @mock.patch.object(ssh, '_get_connection')