openstack
/
ironic
Code Issues Proposed changes

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
This commit is contained in:
Lucas Alvares Gomes 2014-04-29 18:12:15 +01:00
parent 28b1dc364c
commit 216969427b
13 changed files with 50 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic/conductor/manager.py
View File

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

31
ironic/drivers/base.py
View File

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

8
ironic/drivers/modules/fake.py
View File

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

5
ironic/drivers/modules/ipminative.py
View File

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

13
ironic/drivers/modules/ipmitool.py
View File

@ -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."))

2
ironic/drivers/modules/pxe.py
View File

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

3
ironic/drivers/modules/seamicro.py
View File

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

7
ironic/drivers/modules/ssh.py
View File

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

6
ironic/tests/conductor/test_manager.py
View File

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

6
ironic/tests/drivers/test_fake.py
View File

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

24
ironic/tests/drivers/test_pxe.py
View File

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

5
ironic/tests/drivers/test_seamicro.py
View File

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

7
ironic/tests/drivers/test_ssh.py
View File

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