From 5dc26eb95090d898697d7952bd3f3d4625bb09ef Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Mon, 6 Jul 2020 09:03:57 -0400 Subject: [PATCH] Adds steps 'apply_configuration' and 'flash_firmware_sum' Following inband deploy steps are added: 1. 'apply_configuration' is added for 'raid' interface. 2. 'flash_firmware_sum' is added for 'management' interface. Change-Id: I42e153aaa52befb13d0471a2a1f0a2401684aaea --- .../ipa_hw_manager/hardware_manager.py | 178 ++++++++++++++++-- proliantutils/sum/sum_controller.py | 23 +-- .../ipa_hw_manager/test_hardware_manager.py | 93 ++++++++- .../tests/sum/test_sum_controller.py | 63 +++++-- 4 files changed, 308 insertions(+), 49 deletions(-) diff --git a/proliantutils/ipa_hw_manager/hardware_manager.py b/proliantutils/ipa_hw_manager/hardware_manager.py index 758c06fe..5c30a72c 100644 --- a/proliantutils/ipa_hw_manager/hardware_manager.py +++ b/proliantutils/ipa_hw_manager/hardware_manager.py @@ -15,8 +15,48 @@ from ironic_python_agent import hardware from proliantutils.hpssa import manager as hpssa_manager +from proliantutils import log from proliantutils.sum import sum_controller +LOG = log.get_logger(__name__) + +_RAID_APPLY_CONFIGURATION_ARGSINFO = { + "raid_config": { + "description": "The RAID configuration to apply.", + "required": True, + }, + "delete_existing": { + "description": ( + "Setting this to 'True' indicates to delete existing RAID " + "configuration prior to creating the new configuration. " + "Default value is 'True'." + ), + "required": False, + } +} + +_FIRMWARE_UPDATE_SUM_ARGSINFO = { + 'url': { + 'description': ( + "The image location for SPP (Service Pack for Proliant) ISO." + ), + 'required': True, + }, + 'checksum': { + 'description': ( + "The md5 checksum of the SPP image file." + ), + 'required': True, + }, + 'components': { + 'description': ( + "The list of firmware component filenames. If not specified, " + "SUM updates all the firmware components." + ), + 'required': False, + } +} + class ProliantHardwareManager(hardware.GenericHardwareManager): @@ -36,22 +76,96 @@ class ProliantHardwareManager(hardware.GenericHardwareManager): :returns: A list of dictionaries, each item containing the step name, interface and priority for the clean step. """ - return [{'step': 'create_configuration', - 'interface': 'raid', - 'priority': 0}, - {'step': 'delete_configuration', - 'interface': 'raid', - 'priority': 0}, - {'step': 'erase_devices', - 'interface': 'deploy', - 'priority': 0}, - {'step': 'update_firmware_sum', - 'interface': 'management', - 'priority': 0}] + return [ + { + 'step': 'create_configuration', + 'interface': 'raid', + 'priority': 0, + 'reboot_requested': False, + }, + { + 'step': 'delete_configuration', + 'interface': 'raid', + 'priority': 0, + 'reboot_requested': False, + }, + { + 'step': 'erase_devices', + 'interface': 'deploy', + 'priority': 0, + 'reboot_requested': False, + }, + { + 'step': 'update_firmware_sum', + 'interface': 'management', + 'priority': 0, + 'reboot_requested': False, + }, + ] + + def get_deploy_steps(self, node, ports): + """Return the deploy steps supported by this hardware manager. + + This method returns the deploy steps that are supported by + proliant hardware manager. This method is invoked on every + hardware manager by Ironic Python Agent to give this information + back to Ironic. + + :param node: A dictionary of the node object + :param ports: A list of dictionaries containing information of ports + for the node + :returns: A list of dictionaries, each item containing the step name, + interface, priority, reboot_requested and + argsinfo for the deploy step. + """ + return [ + { + 'step': 'apply_configuration', + 'interface': 'raid', + 'priority': 0, + 'reboot_requested': False, + 'argsinfo': _RAID_APPLY_CONFIGURATION_ARGSINFO, + }, + { + 'step': 'flash_firmware_sum', + 'interface': 'management', + 'priority': 0, + 'reboot_requested': False, + 'argsinfo': _FIRMWARE_UPDATE_SUM_ARGSINFO, + }, + ] def evaluate_hardware_support(cls): return hardware.HardwareSupport.SERVICE_PROVIDER + def apply_configuration(self, node, ports, raid_config, + delete_existing=True): + """Apply RAID configuration. + + :param node: A dictionary of the node object. + :param ports: A list of dictionaries containing information + of ports for the node. + :param raid_config: The configuration to apply. + :param delete_existing: Whether to delete the existing configuration. + :returns: The current RAID configuration of the below format. + raid_config = { + 'logical_disks': [{ + 'size_gb': 100, + 'raid_level': 1, + 'physical_disks': [ + '5I:0:1', + '5I:0:2'], + 'controller': 'Smart array controller' + }, + ] + } + """ + if delete_existing: + self.delete_configuration(node, ports) + LOG.debug("Creating raid with configuration %(raid_config)s", + {'raid_config': raid_config}) + return hpssa_manager.create_configuration(raid_config=raid_config) + def create_configuration(self, node, ports): """Create RAID configuration on the bare metal. @@ -75,6 +189,12 @@ class ProliantHardwareManager(hardware.GenericHardwareManager): } """ target_raid_config = node.get('target_raid_config', {}).copy() + if not target_raid_config: + LOG.debug("No target_raid_config found") + return {} + + LOG.debug("Creating raid with configuration %(raid_config)s", + {'raid_config': target_raid_config}) return hpssa_manager.create_configuration( raid_config=target_raid_config) @@ -114,10 +234,40 @@ class ProliantHardwareManager(hardware.GenericHardwareManager): This method performs firmware update on all or some of the firmware components on the bare metal node. - + :param node: A dictionary of the node object. + :param port: A list of dictionaries containing information of ports + for the node. :returns: A string with return code and the statistics of updated/failed components. :raises: SUMOperationError, when the SUM based firmware update operation on the node fails. """ - return sum_controller.update_firmware(node) + url = node['clean_step']['args'].get('url') + checksum = node['clean_step']['args'].get('checksum') + components = node['clean_step']['args'].get('components') + return sum_controller.update_firmware(node, url, checksum, + components=components) + + def flash_firmware_sum(self, node, port, url, + checksum, components=None): + """Performs SUM based firmware update on the bare metal node. + + This method performs firmware update on all or some of the firmware + components on the bare metal node. + :param node: A dictionary of the node object. + :param port: A list of dictionaries containing information of ports + for the node. + :param url: URL of SPP (Service Pack for Proliant) ISO. + :param checksum: MD5 checksum of SPP ISO to verify the image. + :param components: List of filenames of the firmware components to be + flashed. If not provided, the firmware update is performed on all + the firmware components. + :returns: A string with return code and the statistics of + updated/failed components. + :raises: SUMOperationError, when the SUM based firmware update + operation on the node fails. + """ + LOG.debug("Flashing firmware from %(url)s for components %(comp)s", + {'url': url, 'comp': components}) + return sum_controller.update_firmware(node, url, checksum, + components=components) diff --git a/proliantutils/sum/sum_controller.py b/proliantutils/sum/sum_controller.py index 46f5d169..a8288fe3 100644 --- a/proliantutils/sum/sum_controller.py +++ b/proliantutils/sum/sum_controller.py @@ -155,25 +155,28 @@ def _parse_sum_ouput(exit_code): return "UPDATE STATUS: UNKNOWN" -def update_firmware(node): +def update_firmware(node, url, checksum, components=None): """Performs SUM based firmware update on the node. This method performs SUM firmware update by mounting the SPP ISO on the node. It performs firmware update on all or some of the firmware components. - :param node: A node object of type dict. + :param node: A dictionary of the node object. + :param url: URL of SPP (Service Pack for Proliant) ISO. + :param checksum: MD5 checksum of SPP ISO to verify the image. + :param components: List of filenames of the firmware components to be + flashed. If not provided, the firmware update is performed on all + the firmware components. :returns: Operation Status string. :raises: SUMOperationError, when the vmedia device is not found or when the mount operation fails or when the image validation fails. :raises: IloConnectionError, when the iLO connection fails. :raises: IloError, when vmedia eject or insert operation fails. """ - sum_update_iso = node['clean_step']['args'].get('url') - # Validates the http image reference for SUM update ISO. try: - utils.validate_href(sum_update_iso) + utils.validate_href(url) except exception.ImageRefValidationFailed as e: raise exception.SUMOperationError(reason=e) @@ -181,9 +184,9 @@ def update_firmware(node): # is identified by matching its label. time.sleep(WAIT_TIME_DISK_LABEL_TO_BE_VISIBLE) vmedia_device_dir = "/dev/disk/by-label/" - for file in os.listdir(vmedia_device_dir): - if fnmatch.fnmatch(file, 'SPP*'): - vmedia_device_file = os.path.join(vmedia_device_dir, file) + for fname in os.listdir(vmedia_device_dir): + if fnmatch.fnmatch(fname, 'SPP*'): + vmedia_device_file = os.path.join(vmedia_device_dir, fname) if not os.path.exists(vmedia_device_file): msg = "Unable to find the virtual media device for SUM" @@ -191,9 +194,8 @@ def update_firmware(node): # Validates the SPP ISO image for any file corruption using the checksum # of the ISO file. - expected_checksum = node['clean_step']['args'].get('checksum') try: - utils.verify_image_checksum(vmedia_device_file, expected_checksum) + utils.verify_image_checksum(vmedia_device_file, checksum) except exception.ImageRefValidationFailed as e: raise exception.SUMOperationError(reason=e) @@ -215,7 +217,6 @@ def update_firmware(node): if not os.path.exists(sum_file_path): sum_file_path = os.path.join(vmedia_mount_point, HPSUM_LOCATION) - components = node['clean_step']['args'].get('components') result = _execute_sum(sum_file_path, vmedia_mount_point, components=components) diff --git a/proliantutils/tests/ipa_hw_manager/test_hardware_manager.py b/proliantutils/tests/ipa_hw_manager/test_hardware_manager.py index 97206560..196652aa 100644 --- a/proliantutils/tests/ipa_hw_manager/test_hardware_manager.py +++ b/proliantutils/tests/ipa_hw_manager/test_hardware_manager.py @@ -28,24 +28,54 @@ class ProliantHardwareManagerTestCase(testtools.TestCase): def setUp(self): self.hardware_manager = hardware_manager.ProliantHardwareManager() + self.info = {'ilo_address': '1.2.3.4', + 'ilo_password': '12345678', + 'ilo_username': 'admin'} + clean_step = { + 'interface': 'management', + 'step': 'update_firmware_sum', + 'args': {'url': 'http://1.2.3.4/SPP.iso', + 'checksum': '1234567890'}} + self.node = {'driver_info': self.info, + 'clean_step': clean_step} super(ProliantHardwareManagerTestCase, self).setUp() def test_get_clean_steps(self): self.assertEqual( [{'step': 'create_configuration', 'interface': 'raid', - 'priority': 0}, + 'priority': 0, + 'reboot_requested': False}, {'step': 'delete_configuration', 'interface': 'raid', - 'priority': 0}, + 'priority': 0, + 'reboot_requested': False}, {'step': 'erase_devices', 'interface': 'deploy', - 'priority': 0}, + 'priority': 0, + 'reboot_requested': False}, {'step': 'update_firmware_sum', 'interface': 'management', - 'priority': 0}], + 'priority': 0, + 'reboot_requested': False}], self.hardware_manager.get_clean_steps("", "")) + def test_get_deploy_steps(self): + self.assertEqual( + [{'step': 'apply_configuration', + 'interface': 'raid', + 'reboot_requested': False, + 'priority': 0, + 'argsinfo': ( + hardware_manager._RAID_APPLY_CONFIGURATION_ARGSINFO)}, + {'step': 'flash_firmware_sum', + 'interface': 'management', + 'reboot_requested': False, + 'priority': 0, + 'argsinfo': ( + hardware_manager._FIRMWARE_UPDATE_SUM_ARGSINFO)}], + self.hardware_manager.get_deploy_steps("", [])) + @mock.patch.object(hpssa_manager, 'create_configuration') def test_create_configuration(self, create_mock): create_mock.return_value = 'current-config' @@ -55,6 +85,40 @@ class ProliantHardwareManagerTestCase(testtools.TestCase): create_mock.assert_called_once_with(raid_config={'foo': 'bar'}) self.assertEqual('current-config', ret) + @mock.patch.object(hpssa_manager, 'create_configuration') + def test_create_configuration_no_target_config(self, create_mock): + create_mock.return_value = 'current-config' + manager = self.hardware_manager + node = {'target_raid_config': {}} + manager.create_configuration(node, []) + create_mock.assert_not_called() + + @mock.patch.object(hardware_manager.ProliantHardwareManager, + 'delete_configuration') + @mock.patch.object(hpssa_manager, 'create_configuration') + def test_apply_configuration_with_delete(self, create_mock, delete_mock): + create_mock.return_value = 'current-config' + manager = self.hardware_manager + raid_config = {'foo': 'bar'} + ret = manager.apply_configuration("", [], raid_config, + delete_existing=True) + delete_mock.assert_called_once_with("", []) + create_mock.assert_called_once_with(raid_config={'foo': 'bar'}) + self.assertEqual('current-config', ret) + + @mock.patch.object(hardware_manager.ProliantHardwareManager, + 'delete_configuration') + @mock.patch.object(hpssa_manager, 'create_configuration') + def test_apply_configuration_no_delete(self, create_mock, delete_mock): + create_mock.return_value = 'current-config' + manager = self.hardware_manager + raid_config = {'foo': 'bar'} + ret = manager.apply_configuration("", [], raid_config, + delete_existing=False) + create_mock.assert_called_once_with(raid_config={'foo': 'bar'}) + delete_mock.assert_not_called() + self.assertEqual('current-config', ret) + @mock.patch.object(hpssa_manager, 'delete_configuration') def test_delete_configuration(self, delete_mock): delete_mock.return_value = 'current-config' @@ -96,7 +160,22 @@ class ProliantHardwareManagerTestCase(testtools.TestCase): @mock.patch.object(sum_controller, 'update_firmware') def test_update_firmware_sum(self, update_mock): update_mock.return_value = "log files" - node = {'foo': 'bar'} - ret = self.hardware_manager.update_firmware_sum(node, "") - update_mock.assert_called_once_with(node) + url = self.node['clean_step']['args'].get('url') + csum = self.node['clean_step']['args'].get('checksum') + comp = self.node['clean_step']['args'].get('components') + ret = self.hardware_manager.update_firmware_sum(self.node, "") + update_mock.assert_called_once_with(self.node, url, csum, + components=comp) + self.assertEqual('log files', ret) + + @mock.patch.object(sum_controller, 'update_firmware') + def test_flash_firmware_sum(self, update_mock): + update_mock.return_value = "log files" + url = self.node['clean_step']['args'].get('url') + csum = self.node['clean_step']['args'].get('checksum') + comp = self.node['clean_step']['args'].get('components') + ret = self.hardware_manager.flash_firmware_sum(self.node, "", url, + csum, components=comp) + update_mock.assert_called_once_with(self.node, url, csum, + components=comp) self.assertEqual('log files', ret) diff --git a/proliantutils/tests/sum/test_sum_controller.py b/proliantutils/tests/sum/test_sum_controller.py index 770b4d04..cbd3f477 100644 --- a/proliantutils/tests/sum/test_sum_controller.py +++ b/proliantutils/tests/sum/test_sum_controller.py @@ -16,6 +16,7 @@ import os import shutil import tarfile import tempfile +import time import mock from oslo_concurrency import processutils @@ -152,6 +153,7 @@ class SUMFirmwareUpdateTest(testtools.TestCase): os.remove(file_object.name) os.remove(tar_file.name) + @mock.patch.object(time, 'sleep') @mock.patch.object(utils, 'validate_href') @mock.patch.object(utils, 'verify_image_checksum') @mock.patch.object(sum_controller, '_execute_sum') @@ -164,21 +166,23 @@ class SUMFirmwareUpdateTest(testtools.TestCase): def test_update_firmware(self, execute_mock, mkdir_mock, exists_mock, mkdtemp_mock, rmtree_mock, listdir_mock, execute_sum_mock, - verify_image_mock, validate_mock): + verify_image_mock, validate_mock, sleep_mock): execute_sum_mock.return_value = 'SUCCESS' listdir_mock.return_value = ['SPP_LABEL'] mkdtemp_mock.return_value = "/tempdir" null_output = ["", ""] exists_mock.side_effect = [True, False] execute_mock.side_effect = [null_output, null_output] - - ret_val = sum_controller.update_firmware(self.node) + url = "http://a.b.c.d/spp.iso" + checksum = "12345678" + components = ['abc', 'pqr'] + ret_val = sum_controller.update_firmware(self.node, url, checksum, + components) execute_mock.assert_any_call('mount', "/dev/disk/by-label/SPP_LABEL", "/tempdir") execute_sum_mock.assert_any_call('/tempdir/hp/swpackages/hpsum', - '/tempdir', - components=None) + '/tempdir', components=components) calls = [mock.call("/dev/disk/by-label/SPP_LABEL"), mock.call("/tempdir/packages/smartupdate")] exists_mock.assert_has_calls(calls, any_order=False) @@ -187,6 +191,7 @@ class SUMFirmwareUpdateTest(testtools.TestCase): rmtree_mock.assert_called_once_with("/tempdir", ignore_errors=True) self.assertEqual('SUCCESS', ret_val) + @mock.patch.object(time, 'sleep') @mock.patch.object(utils, 'validate_href') @mock.patch.object(utils, 'verify_image_checksum') @mock.patch.object(sum_controller, '_execute_sum') @@ -199,17 +204,22 @@ class SUMFirmwareUpdateTest(testtools.TestCase): def test_update_firmware_sum(self, execute_mock, mkdir_mock, exists_mock, mkdtemp_mock, rmtree_mock, listdir_mock, execute_sum_mock, - verify_image_mock, validate_mock): + verify_image_mock, validate_mock, sleep_mock): execute_sum_mock.return_value = 'SUCCESS' listdir_mock.return_value = ['SPP_LABEL'] mkdtemp_mock.return_value = "/tempdir" null_output = ["", ""] - exists_mock.side_effect = [True, True] - execute_mock.side_effect = [null_output, null_output] + exists_mock.side_effect = [True, True, True, True] + execute_mock.side_effect = [null_output, null_output, + null_output, null_output] + url = "http://a.b.c.d/spp.iso" + checksum = "12345678" - ret_val = sum_controller.update_firmware(self.node) + ret_val = sum_controller.update_firmware( + self.node, url, checksum) - execute_mock.assert_any_call('mount', "/dev/disk/by-label/SPP_LABEL", + execute_mock.assert_any_call('mount', + "/dev/disk/by-label/SPP_LABEL", "/tempdir") execute_sum_mock.assert_any_call('/tempdir/packages/smartupdate', '/tempdir', @@ -226,41 +236,54 @@ class SUMFirmwareUpdateTest(testtools.TestCase): def test_update_firmware_throws_for_nonexistent_file(self, validate_href_mock): invalid_file_path = '/some/invalid/file/path' + url = "http://a.b.c.d/spp.iso" + checksum = "12345678" + components = ['abc', 'pqr'] value = ("Got HTTP code 503 instead of 200 in response to " "HEAD request.") validate_href_mock.side_effect = exception.ImageRefValidationFailed( reason=value, image_href=invalid_file_path) exc = self.assertRaises(exception.SUMOperationError, - sum_controller.update_firmware, self.node) + sum_controller.update_firmware, self.node, + url, checksum, components) self.assertIn(value, str(exc)) + @mock.patch.object(time, 'sleep') @mock.patch.object(utils, 'validate_href') @mock.patch.object(os.path, 'exists') @mock.patch.object(os, 'listdir') def test_update_firmware_device_file_not_found(self, listdir_mock, exists_mock, - validate_mock): + validate_mock, sleep_mock): listdir_mock.return_value = ['SPP_LABEL'] exists_mock.return_value = False + url = "http://a.b.c.d/spp.iso" + checksum = "12345678" + components = ['abc', 'pqr'] msg = ("An error occurred while performing SUM based firmware " "update, reason: Unable to find the virtual media device " "for SUM") exc = self.assertRaises(exception.SUMOperationError, - sum_controller.update_firmware, self.node) + sum_controller.update_firmware, self.node, + url, checksum, components=components) self.assertEqual(msg, str(exc)) exists_mock.assert_called_once_with("/dev/disk/by-label/SPP_LABEL") + @mock.patch.object(time, 'sleep') @mock.patch.object(utils, 'validate_href') @mock.patch.object(utils, 'verify_image_checksum') @mock.patch.object(os, 'listdir') @mock.patch.object(os.path, 'exists') def test_update_firmware_invalid_checksum(self, exists_mock, listdir_mock, verify_image_mock, - validate_mock): + validate_mock, sleep_mock): listdir_mock.return_value = ['SPP_LABEL'] exists_mock.side_effect = [True, False] + url = "http://1.2.3.4/SPP.iso" + checksum = "1234567890" + components = ['abc', 'pqr'] value = ("Error verifying image checksum. Image " "http://1.2.3.4/SPP.iso failed to verify against checksum " @@ -270,12 +293,14 @@ class SUMFirmwareUpdateTest(testtools.TestCase): reason=value, image_href='http://1.2.3.4/SPP.iso') self.assertRaisesRegex(exception.SUMOperationError, value, - sum_controller.update_firmware, self.node) + sum_controller.update_firmware, self.node, + url, checksum, components=components) verify_image_mock.assert_called_once_with( '/dev/disk/by-label/SPP_LABEL', '1234567890') exists_mock.assert_called_once_with("/dev/disk/by-label/SPP_LABEL") + @mock.patch.object(time, 'sleep') @mock.patch.object(utils, 'validate_href') @mock.patch.object(utils, 'verify_image_checksum') @mock.patch.object(processutils, 'execute') @@ -286,16 +311,20 @@ class SUMFirmwareUpdateTest(testtools.TestCase): def test_update_firmware_mount_fails(self, listdir_mock, exists_mock, mkdir_mock, mkdtemp_mock, execute_mock, - verify_image_mock, validate_mock): + verify_image_mock, validate_mock, + sleep_mock): listdir_mock.return_value = ['SPP_LABEL'] exists_mock.return_value = True mkdtemp_mock.return_value = "/tempdir" execute_mock.side_effect = processutils.ProcessExecutionError + url = "http://a.b.c.d/spp.iso" + checksum = "12345678" msg = ("Unable to mount virtual media device " "/dev/disk/by-label/SPP_LABEL") exc = self.assertRaises(exception.SUMOperationError, - sum_controller.update_firmware, self.node) + sum_controller.update_firmware, self.node, + url, checksum) self.assertIn(msg, str(exc)) exists_mock.assert_called_once_with("/dev/disk/by-label/SPP_LABEL")