From bb2942295a4f2b15f91e91b4b717bb39de228178 Mon Sep 17 00:00:00 2001 From: jiapei Date: Mon, 10 Dec 2018 06:15:08 +0000 Subject: [PATCH] Fix XClarity driver management defect Although previous XClarity driver passes the unit test, we find that getting boot order function test fails in the 3rd party CI, which will throw an exception of "Unsupported boot device". After checking with the code, we find that a boot device mapping from XClarity to Ironic recoginzed format is needed. This patch will fix this getting boot order defect. We have verified this function in our 3rd CI environment. Also it will fix a minor in power.py and enhance the unit test. Change-Id: Ia7ccf986cb6b1c332691c811d32cb41850d4796c Story: 2004576 Task: 28351 --- ironic/drivers/modules/xclarity/management.py | 72 ++++++++++++------- ironic/drivers/modules/xclarity/power.py | 2 +- .../modules/xclarity/test_management.py | 37 ++++++++++ .../drivers/modules/xclarity/test_power.py | 5 +- ...ty-management-defect-ec5af0cc6d1045d9.yaml | 8 +++ 5 files changed, 96 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/fix-xclarity-management-defect-ec5af0cc6d1045d9.yaml diff --git a/ironic/drivers/modules/xclarity/management.py b/ironic/drivers/modules/xclarity/management.py index 7a3b704259..73b8bc72c5 100644 --- a/ironic/drivers/modules/xclarity/management.py +++ b/ironic/drivers/modules/xclarity/management.py @@ -38,6 +38,9 @@ BOOT_DEVICE_MAPPING_TO_XCLARITY = { boot_devices.BIOS: 'Boot To F1' } +BOOT_DEVICE_MAPPING_FROM_XCLARITY = { + v: k for k, v in BOOT_DEVICE_MAPPING_TO_XCLARITY.items()} + SUPPORTED_BOOT_DEVICES = [ boot_devices.PXE, boot_devices.DISK, @@ -78,10 +81,11 @@ class XClarityManagement(base.ManagementInterface): """It validates if the boot device is supported by XClarity. :param task: a task from TaskManager. - :param boot_device: the boot device, one of [PXE, DISK, CDROM, BIOS] + :param boot_device: the boot device in XClarity format, one of + ['PXE Network', 'Hard Disk 0', 'CD/DVD Rom', 'Boot To F1'] :raises: InvalidParameterValue if the boot device is not supported. """ - if boot_device not in SUPPORTED_BOOT_DEVICES: + if boot_device not in BOOT_DEVICE_MAPPING_FROM_XCLARITY: raise exception.InvalidParameterValue( _("Unsupported boot device %(device)s for node: %(node)s ") % {"device": boot_device, "node": task.node.uuid} @@ -95,6 +99,7 @@ class XClarityManagement(base.ManagementInterface): :returns: a dictionary containing: :boot_device: the boot device, one of [PXE, DISK, CDROM, BIOS] :persistent: Whether the boot device will persist or not + It returns None if boot device is unknown. :raises: InvalidParameterValue if the boot device is unknown :raises: XClarityError if the communication with XClarity fails """ @@ -117,22 +122,38 @@ class XClarityManagement(base.ManagementInterface): primary = None boot_order = boot_info['bootOrder']['bootOrderList'] for item in boot_order: - current = item.get('currentBootOrderDevices', None) - boot_type = item.get('bootType', None) + current = item.get('currentBootOrderDevices') + if current is None: + LOG.warning( + 'Current boot order is None from XClarity for ' + 'node %(node)s. Please check the hardware and ' + 'XClarity connection', {'node': node.uuid, }) + return {'boot_device': None, 'persistent': None} + else: + primary = current[0] + boot_type = item.get('bootType') if boot_type == "SingleUse": persistent = False - primary = current[0] if primary != 'None': - boot_device = {'boot_device': primary, - 'persistent': persistent} - self._validate_whether_supported_boot_device(primary) + self._validate_supported_boot_device(task, primary) + boot_device = { + 'boot_device': + BOOT_DEVICE_MAPPING_FROM_XCLARITY.get(primary), + 'persistent': persistent + } return boot_device elif boot_type == "Permanent": persistent = True - boot_device = {'boot_device': current[0], - 'persistent': persistent} - self._validate_supported_boot_device(task, primary) - return boot_device + if primary != 'None': + self._validate_supported_boot_device(task, primary) + boot_device = { + 'boot_device': + BOOT_DEVICE_MAPPING_FROM_XCLARITY.get(primary), + 'persistent': persistent + } + return boot_device + else: + return {'boot_device': None, 'persistent': None} @METRICS.timer('XClarityManagement.set_boot_device') @task_manager.require_exclusive_lock @@ -149,12 +170,13 @@ class XClarityManagement(base.ManagementInterface): specified. :raises: XClarityError if the communication with XClarity fails """ - self._validate_supported_boot_device(task=task, boot_device=device) + node = task.node + xc_device = self._translate_ironic_to_xclarity(device) - server_hardware_id = task.node.driver_info.get('server_hardware_id') + server_hardware_id = common.get_server_hardware_id(node) LOG.debug("Setting boot device to %(device)s for node %(node)s", - {"device": device, "node": task.node.uuid}) - self._set_boot_device(task, server_hardware_id, device, + {"device": device, "node": node.uuid}) + self._set_boot_device(task, server_hardware_id, xc_device, singleuse=not persistent) @METRICS.timer('XClarityManagement.get_sensors_data') @@ -189,36 +211,34 @@ class XClarityManagement(base.ManagementInterface): client = common.get_xclarity_client(node) boot_info = client.get_node_all_boot_info( server_hardware_id) - xclarity_boot_device = self._translate_ironic_to_xclarity( - new_primary_boot_device) current = [] LOG.debug( ("Setting boot device to %(device)s for XClarity " "node %(node)s"), - {'device': xclarity_boot_device, 'node': node.uuid} + {'device': new_primary_boot_device, 'node': node.uuid} ) for item in boot_info['bootOrder']['bootOrderList']: if singleuse and item['bootType'] == 'SingleUse': - item['currentBootOrderDevices'][0] = xclarity_boot_device + item['currentBootOrderDevices'][0] = new_primary_boot_device elif not singleuse and item['bootType'] == 'Permanent': current = item['currentBootOrderDevices'] - if xclarity_boot_device == current[0]: + if new_primary_boot_device == current[0]: return - if xclarity_boot_device in current: - current.remove(xclarity_boot_device) - current.insert(0, xclarity_boot_device) + if new_primary_boot_device in current: + current.remove(new_primary_boot_device) + current.insert(0, new_primary_boot_device) item['currentBootOrderDevices'] = current try: client.set_node_boot_info(server_hardware_id, boot_info, - xclarity_boot_device, + new_primary_boot_device, singleuse) except xclarity_client_exceptions.XClarityError as xclarity_exc: LOG.error( ('Error setting boot device %(boot_device)s for the XClarity ' 'node %(node)s. Error: %(error)s'), - {'boot_device': xclarity_boot_device, 'node': node.uuid, + {'boot_device': new_primary_boot_device, 'node': node.uuid, 'error': xclarity_exc} ) raise exception.XClarityError(error=xclarity_exc) diff --git a/ironic/drivers/modules/xclarity/power.py b/ironic/drivers/modules/xclarity/power.py index 9eb6b110fc..36dc428c61 100644 --- a/ironic/drivers/modules/xclarity/power.py +++ b/ironic/drivers/modules/xclarity/power.py @@ -62,7 +62,7 @@ class XClarityPower(base.PowerInterface): server_hardware_id = common.get_server_hardware_id(node) try: power_state = client.get_node_power_status(server_hardware_id) - except xclarity_client_exceptions.XClarityException as xclarity_exc: + except xclarity_client_exceptions.XClarityError as xclarity_exc: LOG.error( ("Error getting power state for node %(node)s. Error: " "%(error)s"), diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_management.py b/ironic/tests/unit/drivers/modules/xclarity/test_management.py index aa05d2b16b..754e50a612 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_management.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_management.py @@ -118,3 +118,40 @@ class XClarityManagementDriverTestCase(db_base.DbTestCase): exception.XClarityError, task.driver.management.get_boot_device, task) + + def test_get_boot_device_current_none(self, mock_xc_client): + with task_manager.acquire(self.context, self.node.uuid) as task: + reference = {'boot_device': None, 'persistent': None} + mock_xc_client.return_value.get_node_all_boot_info.return_value = \ + { + 'bootOrder': { + 'bootOrderList': [{ + 'fakeBootOrderDevices': [] + }] + } + } + expected_boot_device = task.driver.management.get_boot_device( + task=task) + self.assertEqual(reference, expected_boot_device) + + def test_get_boot_device_primary_none(self, mock_xc_client): + with task_manager.acquire(self.context, self.node.uuid) as task: + reference = {'boot_device': None, 'persistent': None} + mock_xc_client.return_value.get_node_all_boot_info.return_value = \ + { + 'bootOrder': { + 'bootOrderList': [ + { + 'bootType': 'SingleUse', + 'CurrentBootOrderDevices': [] + }, + { + 'bootType': 'Permanent', + 'CurrentBootOrderDevices': [] + }, + ] + } + } + expected_boot_device = task.driver.management.get_boot_device( + task=task) + self.assertEqual(reference, expected_boot_device) diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_power.py b/ironic/tests/unit/drivers/modules/xclarity/test_power.py index fcda4e4722..d259a6b287 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_power.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_power.py @@ -72,7 +72,9 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): result = power.XClarityPower.get_power_state(task) self.assertEqual(STATE_POWER_ON, result) - def test_get_power_state_fail(self, mock_xc_client): + @mock.patch.object(common, 'translate_xclarity_power_state', + spec_set=True, autospec=True) + def test_get_power_state_fail(self, mock_translate_state, mock_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: xclarity_client_exceptions.XClarityError = Exception sys.modules['xclarity_client.exceptions'] = ( @@ -85,6 +87,7 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): self.assertRaises(exception.XClarityError, task.driver.power.get_power_state, task) + self.assertFalse(mock_translate_state.called) @mock.patch.object(power.LOG, 'warning') @mock.patch.object(power.XClarityPower, 'get_power_state', diff --git a/releasenotes/notes/fix-xclarity-management-defect-ec5af0cc6d1045d9.yaml b/releasenotes/notes/fix-xclarity-management-defect-ec5af0cc6d1045d9.yaml new file mode 100644 index 0000000000..7fecee2809 --- /dev/null +++ b/releasenotes/notes/fix-xclarity-management-defect-ec5af0cc6d1045d9.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes an issue where ``xclarity`` management interface fails to get + boot order. Now the driver correctly gets boot device and this has + been verified in the 3rd party CI. See story + `2004576 `_ for + details.