diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index b3701fbed6..f2281c6715 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -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 " diff --git a/ironic/drivers/modules/ilo/inspect.py b/ironic/drivers/modules/ilo/inspect.py index 63dc2defc7..459e34bfef 100644 --- a/ironic/drivers/modules/ilo/inspect.py +++ b/ironic/drivers/modules/ilo/inspect.py @@ -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' == : creates ports for - requested port numbers. - 'inspect_ports' == : 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"), diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index 8817668968..d62be46750 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -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): diff --git a/ironic/tests/drivers/ilo/test_inspect.py b/ironic/tests/drivers/ilo/test_inspect.py index 05997cd1f3..9b7461a7b5 100644 --- a/ironic/tests/drivers/ilo/test_inspect.py +++ b/ironic/tests/drivers/ilo/test_inspect.py @@ -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)