openstack/ironic
Code Issues Proposed changes

Merge "Remove inspect_ports from ilo inspection"

Browse Source
This commit is contained in:
Jenkins 2015-04-01 16:35:01 +00:00 committed by Gerrit Code Review
commit ce4c677943
4 changed files with 11 additions and 303 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
ironic/drivers/modules/ilo/common.py
View File

@ -75,15 +75,6 @@ CONSOLE_PROPERTIES = {
'console_port': _("node's UDP port to connect to. Only required for "
"console access.")
}
INSPECT_PROPERTIES = {
'inspect_ports': _("Comma-separated values of ethernet ports "
"to be identified for creating node "
"ports. Valid values may be "
"inspect_ports = '1,2,...n' or "
"inspect_ports = 'all' or "
"inspect_ports = 'none'. "
"Required only for inspection.")
}
CLEAN_PROPERTIES = {
'ilo_change_password': _("new password for iLO. Required if the clean "
"step 'reset_ilo_credential' is enabled.")
@ -141,11 +132,6 @@ def parse_driver_info(node):
except ValueError:
not_integers.append(param)
for param in INSPECT_PROPERTIES:
value = info.get(param)
if value:
d_info[param] = value
if not_integers:
raise exception.InvalidParameterValue(_(
"The following iLO parameters from the node's driver_info "

117
ironic/drivers/modules/ilo/inspect.py
View File

@ -186,78 +186,6 @@ def _update_capabilities(node, new_capabilities):
for key, value in six.iteritems(cap_dict)])
def _get_macs_for_desired_ports(node, macs):
"""Get the dict of MACs which are desired by the operator.
Get the MACs for desired ports.
Returns a dictionary of MACs associated with the ports specified
in the node's driver_info/inspect_ports.
The driver_info field is expected to be populated with
comma-separated port numbers like driver_info/inspect_ports='1,2'.
In this case the inspection is expected to create ironic ports
only for these two ports.
The proliantutils is expected to return key value pair for each
MAC address like:
{'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
Possible scenarios:
'inspect_ports' == 'all' : creates ports for all inspected MACs
'inspect_ports' == <valid_port_numbers>: creates ports for
requested port numbers.
'inspect_ports' == <mix_of_valid_invalid> : raise error for
invalid inputs.
'inspect_ports' == 'none' : doesn't do any action with the
inspected mac addresses.
This method is not called if 'inspect_ports' == 'none', hence the
scenario is not covered under this method.
:param node: a node object.
:param macs: a dictionary of MAC addresses returned by the hardware
with inspection.
:returns: a dictionary of port numbers and MAC addresses with only
the MACs requested by operator in
node.driver_info['inspect_ports']
:raises: HardwareInspectionFailure for the non-existing ports
requested in node.driver_info['inspect_ports']
"""
driver_info = node.driver_info
desired_macs = str(driver_info.get('inspect_ports'))
# If the operator has given 'all' just return all the macs
# returned by inspection.
if desired_macs.lower() == 'all':
to_be_created_macs = macs
else:
to_be_created_macs = {}
# The list should look like ['Port 1', 'Port 2'] as
# iLO returns port numbers like this.
desired_macs_list = [
'Port %s' % port_number
for port_number in (desired_macs.split(','))]
# Check if the given input is valid or not. Return all the
# requested macs.
non_existing_ports = []
for port_number in desired_macs_list:
mac_address = macs.get(port_number)
if mac_address:
to_be_created_macs[port_number] = mac_address
else:
non_existing_ports.append(port_number)
# It is possible that operator has given a wrong input by mistake.
if non_existing_ports:
error = (_("Could not find requested ports %(ports)s on the "
"node %(node)s")
% {'ports': non_existing_ports, 'node': node.uuid})
raise exception.HardwareInspectionFailure(error=error)
return to_be_created_macs
def _get_capabilities(node, ilo_object):
"""inspects hardware and gets additional capabilities.
@ -281,9 +209,7 @@ def _get_capabilities(node, ilo_object):
class IloInspect(base.InspectInterface):
def get_properties(self):
d = ilo_common.REQUIRED_PROPERTIES.copy()
d.update(ilo_common.INSPECT_PROPERTIES)
return d
return ilo_common.REQUIRED_PROPERTIES
def validate(self, task):
"""Check that 'driver_info' contains required ILO credentials.
@ -295,33 +221,18 @@ class IloInspect(base.InspectInterface):
:raises: InvalidParameterValue if required iLO parameters
are not valid.
:raises: MissingParameterValue if a required parameter is missing.
:raises: InvalidParameterValue if invalid input provided.
"""
node = task.node
driver_info = ilo_common.parse_driver_info(node)
if 'inspect_ports' not in driver_info:
raise exception.MissingParameterValue(_(
"Missing 'inspect_ports' parameter in node's driver_info."))
value = driver_info['inspect_ports']
if (value.lower() != 'all' and value.lower() != 'none'
and not all(s.isdigit() for s in value.split(','))):
raise exception.InvalidParameterValue(_(
"inspect_ports can accept either comma separated "
"port numbers, or a single port number, or 'all' "
"or 'none'. %(value)s given for node %(node)s "
"driver_info['inspect_ports']")
% {'value': value, 'node': node})
ilo_common.parse_driver_info(node)
def inspect_hardware(self, task):
"""Inspect hardware to get the hardware properties.
Inspects hardware to get the essential and additional hardware
properties. It fails if any of the essential properties
are not received from the node or if 'inspect_ports' is
not provided in driver_info.
It doesn't fail if node fails to return any capabilities as
the capabilities differ from hardware to hardware mostly.
are not received from the node. It doesn't fail if node fails
to return any capabilities as the capabilities differ from hardware
to hardware mostly.
:param task: a TaskManager instance.
:raises: HardwareInspectionFailure if essential properties
@ -371,22 +282,8 @@ class IloInspect(base.InspectInterface):
task.node.save()
# Get the desired node inputs from the driver_info and create ports
# as requested. It doesn't delete the ports because there is
# no way for the operator to know which all MACs are associated
# with the node and which are not. The proliantutils can
# return only embedded NICs mac addresses and not the STANDUP NIC
# cards. The port creation code is not excercised if
# 'inspect_ports' == 'none'.
driver_info = task.node.driver_info
if (driver_info['inspect_ports']).lower() != 'none':
macs_input_given = (
_get_macs_for_desired_ports(task.node, result['macs']))
if macs_input_given:
# Create ports only for the requested ports.
_create_ports_if_not_exist(task.node, macs_input_given)
# Create ports for the nics detected.
_create_ports_if_not_exist(task.node, result['macs'])
LOG.debug(("Node properties for %(node)s are updated as "
"%(properties)s"),

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

@ -3338,20 +3338,19 @@ class ManagerTestProperties(tests_db_base.DbTestCase):
def test_driver_properties_fake_ilo(self):
expected = ['ilo_address', 'ilo_username', 'ilo_password',
'client_port', 'client_timeout', 'inspect_ports',
'ilo_change_password']
'client_port', 'client_timeout', 'ilo_change_password']
self._check_driver_properties("fake_ilo", expected)
def test_driver_properties_ilo_iscsi(self):
expected = ['ilo_address', 'ilo_username', 'ilo_password',
'client_port', 'client_timeout', 'ilo_deploy_iso',
'console_port', 'inspect_ports', 'ilo_change_password']
'console_port', 'ilo_change_password']
self._check_driver_properties("iscsi_ilo", expected)
def test_driver_properties_agent_ilo(self):
expected = ['ilo_address', 'ilo_username', 'ilo_password',
'client_port', 'client_timeout', 'ilo_deploy_iso',
'console_port', 'inspect_ports', 'ilo_change_password']
'console_port', 'ilo_change_password']
self._check_driver_properties("agent_ilo", expected)
def test_driver_properties_fail(self):

176
ironic/tests/drivers/ilo/test_inspect.py
View File

@ -48,78 +48,30 @@ class IloInspectTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
properties = ilo_common.REQUIRED_PROPERTIES.copy()
properties.update(ilo_common.INSPECT_PROPERTIES)
self.assertEqual(properties,
task.driver.inspect.get_properties())
@mock.patch.object(ilo_common, 'parse_driver_info')
def test_validate_inspect_ports_valid_with_comma(self, driver_info_mock):
def test_validate(self, driver_info_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
driver_info_mock.return_value = {'inspect_ports': '1,2'}
task.driver.inspect.validate(task)
driver_info_mock.assert_called_once_with(task.node)
@mock.patch.object(ilo_common, 'parse_driver_info')
def test_validate_inspect_ports_valid_None(self, driver_info_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
driver_info_mock.return_value = {'inspect_ports': 'None'}
task.driver.inspect.validate(task)
driver_info_mock.assert_called_once_with(task.node)
@mock.patch.object(ilo_common, 'parse_driver_info')
def test_validate_inspect_ports_valid_all(self, driver_info_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
driver_info_mock.return_value = {'inspect_ports': 'all'}
task.driver.inspect.validate(task)
driver_info_mock.assert_called_once_with(task.node)
@mock.patch.object(ilo_common, 'parse_driver_info')
def test_validate_inspect_ports_valid_single(self, driver_info_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
driver_info_mock.return_value = {'inspect_ports': '1'}
task.driver.inspect.validate(task)
driver_info_mock.assert_called_once_with(task.node)
@mock.patch.object(ilo_common, 'parse_driver_info')
def test_validate_inspect_ports_invalid(self, driver_info_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
driver_info_mock.return_value = {'inspect_ports': 'abc'}
self.assertRaises(exception.InvalidParameterValue,
task.driver.inspect.validate, task)
driver_info_mock.assert_called_once_with(task.node)
@mock.patch.object(ilo_common, 'parse_driver_info')
def test_validate_inspect_ports_missing(self, driver_info_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
driver_info_mock.return_value = {'xyz': 'abc'}
self.assertRaises(exception.MissingParameterValue,
task.driver.inspect.validate, task)
driver_info_mock.assert_called_once_with(task.node)
@mock.patch.object(ilo_inspect, '_get_capabilities')
@mock.patch.object(ilo_inspect, '_create_ports_if_not_exist')
@mock.patch.object(ilo_inspect, '_get_macs_for_desired_ports')
@mock.patch.object(ilo_inspect, '_get_essential_properties')
@mock.patch.object(ilo_power.IloPower, 'get_power_state')
@mock.patch.object(ilo_common, 'get_ilo_object')
def test_inspect_essential_ok(self, get_ilo_object_mock,
power_mock,
get_essential_mock,
desired_macs_mock,
create_port_mock,
get_capabilities_mock):
ilo_object_mock = get_ilo_object_mock.return_value
properties = {'memory_mb': '512', 'local_gb': '10',
'cpus': '1', 'cpu_arch': 'x86_64'}
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
desired_macs_mock.return_value = {'Port 1': 'aa:aa:aa:aa:aa:aa',
'Port 2': 'bb:bb:bb:bb:bb:bb'}
capabilities = ''
result = {'properties': properties, 'macs': macs}
get_essential_mock.return_value = result
@ -127,7 +79,6 @@ class IloInspectTestCase(db_base.DbTestCase):
power_mock.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.driver_info = {'inspect_ports': 'all'}
task.driver.inspect.inspect_hardware(task)
self.assertEqual(properties, task.node.properties)
power_mock.assert_called_once_with(task)
@ -139,7 +90,6 @@ class IloInspectTestCase(db_base.DbTestCase):
@mock.patch.object(ilo_inspect, '_get_capabilities')
@mock.patch.object(ilo_inspect, '_create_ports_if_not_exist')
@mock.patch.object(ilo_inspect, '_get_macs_for_desired_ports')
@mock.patch.object(ilo_inspect, '_get_essential_properties')
@mock.patch.object(conductor_utils, 'node_power_action')
@mock.patch.object(ilo_power.IloPower, 'get_power_state')
@ -148,15 +98,12 @@ class IloInspectTestCase(db_base.DbTestCase):
power_mock,
set_power_mock,
get_essential_mock,
desired_macs_mock,
create_port_mock,
get_capabilities_mock):
ilo_object_mock = get_ilo_object_mock.return_value
properties = {'memory_mb': '512', 'local_gb': '10',
'cpus': '1', 'cpu_arch': 'x86_64'}
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
desired_macs_mock.return_value = {'Port 1': 'aa:aa:aa:aa:aa:aa',
'Port 2': 'bb:bb:bb:bb:bb:bb'}
capabilities = ''
result = {'properties': properties, 'macs': macs}
get_essential_mock.return_value = result
@ -164,7 +111,6 @@ class IloInspectTestCase(db_base.DbTestCase):
power_mock.return_value = states.POWER_OFF
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.driver_info = {'inspect_ports': 'all'}
task.driver.inspect.inspect_hardware(task)
self.assertEqual(properties, task.node.properties)
power_mock.assert_called_once_with(task)
@ -177,22 +123,18 @@ class IloInspectTestCase(db_base.DbTestCase):
@mock.patch.object(ilo_inspect, '_get_capabilities')
@mock.patch.object(ilo_inspect, '_create_ports_if_not_exist')
@mock.patch.object(ilo_inspect, '_get_macs_for_desired_ports')
@mock.patch.object(ilo_inspect, '_get_essential_properties')
@mock.patch.object(ilo_power.IloPower, 'get_power_state')
@mock.patch.object(ilo_common, 'get_ilo_object')
def test_inspect_essential_capabilities_ok(self, get_ilo_object_mock,
power_mock,
get_essential_mock,
desired_macs_mock,
create_port_mock,
get_capabilities_mock):
ilo_object_mock = get_ilo_object_mock.return_value
properties = {'memory_mb': '512', 'local_gb': '10',
'cpus': '1', 'cpu_arch': 'x86_64'}
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
desired_macs_mock.return_value = {'Port 1': 'aa:aa:aa:aa:aa:aa',
'Port 2': 'bb:bb:bb:bb:bb:bb'}
capability_str = 'BootMode:uefi'
capabilities = {'BootMode': 'uefi'}
result = {'properties': properties, 'macs': macs}
@ -201,7 +143,6 @@ class IloInspectTestCase(db_base.DbTestCase):
power_mock.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.driver_info = {'inspect_ports': 'all'}
task.driver.inspect.inspect_hardware(task)
expected_properties = {'memory_mb': '512', 'local_gb': '10',
'cpus': '1', 'cpu_arch': 'x86_64',
@ -216,14 +157,12 @@ class IloInspectTestCase(db_base.DbTestCase):
@mock.patch.object(ilo_inspect, '_get_capabilities')
@mock.patch.object(ilo_inspect, '_create_ports_if_not_exist')
@mock.patch.object(ilo_inspect, '_get_macs_for_desired_ports')
@mock.patch.object(ilo_inspect, '_get_essential_properties')
@mock.patch.object(ilo_power.IloPower, 'get_power_state')
@mock.patch.object(ilo_common, 'get_ilo_object')
def test_inspect_essential_capabilities_exist_ok(self, get_ilo_object_mock,
power_mock,
get_essential_mock,
desired_macs_mock,
create_port_mock,
get_capabilities_mock):
ilo_object_mock = get_ilo_object_mock.return_value
@ -231,8 +170,6 @@ class IloInspectTestCase(db_base.DbTestCase):
'cpus': '1', 'cpu_arch': 'x86_64',
'somekey': 'somevalue'}
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
desired_macs_mock.return_value = {'Port 1': 'aa:aa:aa:aa:aa:aa',
'Port 2': 'bb:bb:bb:bb:bb:bb'}
result = {'properties': properties, 'macs': macs}
capabilities = {'BootMode': 'uefi'}
get_essential_mock.return_value = result
@ -240,7 +177,6 @@ class IloInspectTestCase(db_base.DbTestCase):
power_mock.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.driver_info = {'inspect_ports': 'all'}
task.node.properties = {'capabilities': 'foo:bar'}
expected_capabilities = ('BootMode:uefi,'
'foo:bar')
@ -260,75 +196,6 @@ class IloInspectTestCase(db_base.DbTestCase):
ilo_object_mock)
create_port_mock.assert_called_once_with(task.node, macs)
@mock.patch.object(ilo_inspect, '_get_capabilities')
@mock.patch.object(ilo_inspect, '_create_ports_if_not_exist')
@mock.patch.object(ilo_inspect, '_get_macs_for_desired_ports')
@mock.patch.object(ilo_inspect, '_get_essential_properties')
@mock.patch.object(ilo_power.IloPower, 'get_power_state')
@mock.patch.object(ilo_common, 'get_ilo_object')
def test_inspect_hardware_port_desired(self, get_ilo_object_mock,
power_mock,
get_essential_mock,
desired_macs_mock,
create_port_mock,
get_capabilities_mock):
ilo_object_mock = get_ilo_object_mock.return_value
properties = {'memory_mb': '512', 'local_gb': '10',
'cpus': '1', 'cpu_arch': 'x86_64'}
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
result = {'properties': properties, 'macs': macs}
macs_input_given = {'Port 1': 'aa:aa:aa:aa:aa:aa'}
desired_macs_mock.return_value = macs_input_given
capabilities = ''
get_essential_mock.return_value = result
get_capabilities_mock.return_value = capabilities
power_mock.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.driver_info = {'inspect_ports': '1'}
task.driver.inspect.inspect_hardware(task)
power_mock.assert_called_once_with(task)
get_essential_mock.assert_called_once_with(task.node,
ilo_object_mock)
self.assertEqual(task.node.properties, result['properties'])
get_capabilities_mock.assert_called_once_with(task.node,
ilo_object_mock)
create_port_mock.assert_called_once_with(task.node,
macs_input_given)
@mock.patch.object(ilo_inspect, '_get_capabilities')
@mock.patch.object(ilo_inspect, '_create_ports_if_not_exist')
@mock.patch.object(ilo_inspect, '_get_macs_for_desired_ports')
@mock.patch.object(ilo_inspect, '_get_essential_properties')
@mock.patch.object(ilo_power.IloPower, 'get_power_state')
@mock.patch.object(ilo_common, 'get_ilo_object')
def test_inspect_hardware_port_desired_none(self, get_ilo_object_mock,
power_mock,
get_essential_mock,
desired_macs_mock,
create_port_mock,
get_capabilities_mock):
ilo_object_mock = get_ilo_object_mock.return_value
properties = {'memory_mb': '512', 'local_gb': '10',
'cpus': '1', 'cpu_arch': 'x86_64'}
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
result = {'properties': properties, 'macs': macs}
macs_input_given = {'Port 1': 'aa:aa:aa:aa:aa:aa'}
capabilities = ''
get_capabilities_mock.return_value = capabilities
desired_macs_mock.return_value = macs_input_given
get_essential_mock.return_value = result
power_mock.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.driver_info = {'inspect_ports': 'none'}
task.driver.inspect.inspect_hardware(task)
power_mock.assert_called_once_with(task)
get_essential_mock.assert_called_once_with(task.node,
ilo_object_mock)
self.assertEqual(task.node.properties, result['properties'])
self.assertFalse(create_port_mock.called)
class TestInspectPrivateMethods(db_base.DbTestCase):
@ -523,44 +390,3 @@ class TestInspectPrivateMethods(db_base.DbTestCase):
set2 = set(cap_returned.split(','))
self.assertEqual(set1, set2)
self.assertIsInstance(cap_returned, str)
def test__get_macs_for_desired_ports(self):
driver_info_mock = {'inspect_ports': '1,2'}
self.node.driver_info = driver_info_mock
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
expected_macs = {'Port 1': 'aa:aa:aa:aa:aa:aa',
'Port 2': 'bb:bb:bb:bb:bb:bb'}
macs_out = (
ilo_inspect._get_macs_for_desired_ports(self.node,
macs))
self.assertEqual(expected_macs, macs_out)
def test__get_macs_for_desired_ports_few(self):
driver_info_mock = {'inspect_ports': '1,2'}
self.node.driver_info = driver_info_mock
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb',
'Port 3': 'cc:cc:cc:cc:cc:cc', 'Port 4': 'dd:dd:dd:dd:dd:dd'}
expected_macs = {'Port 1': 'aa:aa:aa:aa:aa:aa',
'Port 2': 'bb:bb:bb:bb:bb:bb'}
macs_out = (
ilo_inspect._get_macs_for_desired_ports(self.node,
macs))
self.assertEqual(expected_macs, macs_out)
def test__get_macs_for_desired_ports_one(self):
driver_info_mock = {'inspect_ports': '1'}
self.node.driver_info = driver_info_mock
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
expected_macs = {'Port 1': 'aa:aa:aa:aa:aa:aa'}
macs_out = (
ilo_inspect._get_macs_for_desired_ports(self.node,
macs))
self.assertEqual(expected_macs, macs_out)
def test__get_macs_for_desired_ports_none(self):
driver_info_mock = {}
self.node.driver_info = driver_info_mock
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
self.assertRaises(exception.HardwareInspectionFailure,
ilo_inspect._get_macs_for_desired_ports,
self.node, macs)