diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index 0878b08bfc..d2d93d9ffb 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -385,6 +385,8 @@ The ``update_firmware`` cleaning step accepts JSON in the following format:: "firmware_images":[ { "url": "", + "checksum": "", + "source": "", "wait": }, { @@ -410,16 +412,21 @@ Each firmware image dictionary, is of the form:: { "url": "", + "checksum": "", + "source": "", "wait": } -The ``url`` argument in the firmware image dictionary is mandatory, while the -``wait`` argument is optional. +The ``url``and ``checksum`` arguments in the firmware image dictionary are +mandatory, while the ``source`` and ``wait`` arguments are optional. +For ``url`` currently ``http``, ``https``, ``swift`` and ``file`` schemes are +supported. + +``source`` corresponds to ``[redfish]firmware_source`` and by setting it here, +it is possible to override global setting per firmware image in clean step +arguments. -.. note:: - Only ``http`` and ``https`` URLs are currently supported in the ``url`` - argument. .. note:: At the present time, targets for the firmware update cannot be specified. @@ -427,19 +434,20 @@ The ``url`` argument in the firmware image dictionary is mandatory, while the node. It is assumed that the BMC knows what components a given firmware image is applicable to. -To perform a firmware update, first download the firmware to a web server that -the BMC has network access to. This could be the ironic conductor web server -or another web server on the BMC network. Using a web browser, curl, or similar -tool on a server that has network access to the BMC, try downloading -the firmware to verify that the URLs are correct and that the web server is -configured properly. +To perform a firmware update, first download the firmware to a web server, +Swift or filesystem that the Ironic conductor or BMC has network access to. +This could be the ironic conductor web server or another web server on the BMC +network. Using a web browser, curl, or similar tool on a server that has +network access to the BMC or Ironic conductor, try downloading the firmware to +verify that the URLs are correct and that the web server is configured +properly. Next, construct the JSON for the firmware update cleaning step to be executed. When launching the firmware update, the JSON may be specified on the command -line directly or in a file. The following -example shows one cleaning step that installs two firmware updates. The first -updates the BMC firmware followed by a five minute wait to allow the BMC time -to start back up. The second updates the firmware on all applicable NICs.:: +line directly or in a file. The following example shows one cleaning step that +installs four firmware updates. All except 3rd entry that has explicit +``source`` added, uses setting from ``[redfish]firmware_source`` to determine +if and where to stage the files:: [{ "interface": "management", @@ -448,10 +456,21 @@ to start back up. The second updates the firmware on all applicable NICs.:: "firmware_images":[ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "", "wait": 300 }, { - "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE" + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "checksum": "" + }, + { + "url": "file:///firmware_images/idrac/9/PERC_WN64_6.65.65.65_A00.EXE", + "checksum": "", + "source": "http" + }, + { + "url": "swift://firmware_container/BIOS_W8Y0W_WN64_2.1.7.EXE", + "checksum": "" } ] } diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py index eddf3e0134..3cc9fe0159 100644 --- a/ironic/conf/redfish.py +++ b/ironic/conf/redfish.py @@ -90,6 +90,21 @@ opts = [ default=60, help=_('Number of seconds to wait between checking for ' 'failed firmware update tasks')), + cfg.StrOpt('firmware_source', + choices=[('http', _('If firmware source URL is also HTTP, then ' + 'serve from original location, otherwise ' + 'copy to ironic\'s HTTP server. Default.')), + ('local', _('Download from original location and ' + 'server from ironic\'s HTTP server.')), + ('swift', _('If firmware source URL is also Swift, ' + 'serve from original location, otherwise ' + 'copy to ironic\'s Swift server.'))], + default='http', + mutable=True, + help=_('Specifies how firmware image should be served. Whether ' + 'from its original location using the firmware source ' + 'URL directly, or should serve it from ironic\'s Swift ' + 'or HTTP server.')), cfg.IntOpt('raid_config_status_interval', min=0, default=60, diff --git a/ironic/drivers/modules/redfish/firmware_utils.py b/ironic/drivers/modules/redfish/firmware_utils.py index 35e4bb1f29..c73cb80dd6 100644 --- a/ironic/drivers/modules/redfish/firmware_utils.py +++ b/ironic/drivers/modules/redfish/firmware_utils.py @@ -11,11 +11,20 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import shutil +import tempfile +from urllib import parse as urlparse + import jsonschema from oslo_log import log +from oslo_utils import fileutils from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import image_service +from ironic.common import swift +from ironic.conf import CONF LOG = log.getLogger(__name__) @@ -26,22 +35,35 @@ _UPDATE_FIRMWARE_SCHEMA = { # list of firmware update images "items": { "type": "object", - "required": ["url"], + "required": ["url", "checksum"], "properties": { "url": { "description": "URL for firmware file", "type": "string", "minLength": 1 }, + "checksum": { + "description": "SHA1 checksum for firmware file", + "type": "string", + "minLength": 1 + }, "wait": { "description": "optional wait time for firmware update", "type": "integer", "minimum": 1 + }, + "source": + { + "description": "optional firmware_source to override global " + "setting for firmware file", + "type": "string", + "enum": ["http", "local", "swift"] } }, "additionalProperties": False } } +_FIRMWARE_SUBDIR = 'firmware' def validate_update_firmware_args(firmware_images): @@ -56,3 +78,180 @@ def validate_update_firmware_args(firmware_images): raise exception.InvalidParameterValue( _('Invalid firmware update %(firmware_images)s. Errors: %(err)s') % {'firmware_images': firmware_images, 'err': err}) + + +def get_swift_temp_url(parsed_url): + """Gets Swift temporary URL + + :param parsed_url: Parsed URL from URL in format + swift://container/[sub-folder/]file + :returns: Swift temporary URL + """ + return swift.SwiftAPI().get_temp_url( + parsed_url.netloc, parsed_url.path.lstrip('/'), + CONF.redfish.swift_object_expiry_timeout) + + +def download_to_temp(node, url): + """Downloads to temporary location from given URL + + :param node: Node for which to download to temporary location + :param url: URL to download from + :returns: File path of temporary location file is downloaded to + """ + parsed_url = urlparse.urlparse(url) + scheme = parsed_url.scheme.lower() + if scheme not in ('http', 'swift', 'file'): + raise exception.InvalidParameterValue( + _('%(scheme)s is not supported for %(url)s.') + % {'scheme': scheme, 'url': parsed_url.geturl()}) + + tempdir = os.path.join(tempfile.gettempdir(), node.uuid) + os.makedirs(tempdir, exist_ok=True) + temp_file = os.path.join( + tempdir, + os.path.basename(parsed_url.path)) + LOG.debug('For node %(node)s firmware at %(url)s will be downloaded to ' + 'temporary location at %(temp_file)s', + {'node': node.uuid, 'url': url, 'temp_file': temp_file}) + if scheme == 'http': + with open(temp_file, 'wb') as tf: + image_service.HttpImageService().download(url, tf) + elif scheme == 'swift': + swift_url = get_swift_temp_url(parsed_url) + with open(temp_file, 'wb') as tf: + image_service.HttpImageService().download(swift_url, tf) + elif scheme == 'file': + with open(temp_file, 'wb') as tf: + image_service.FileImageService().download( + parsed_url.path, tf) + + return temp_file + + +def verify_checksum(node, checksum, file_path): + """Verify checksum. + + :param node: Node for which file to verify checksum + :param checksum: Expected checksum value + :param file_path: File path for which to verify checksum + :raises RedfishError: When checksum does not match + """ + calculated_checksum = fileutils.compute_file_checksum( + file_path, algorithm='sha1') + if checksum != calculated_checksum: + raise exception.RedfishError( + _('For node %(node)s firmware file %(temp_file)s checksums do not ' + 'match. Expected: %(checksum)s, calculated: ' + '%(calculated_checksum)s.') + % {'node': node.uuid, 'temp_file': file_path, 'checksum': checksum, + 'calculated_checksum': calculated_checksum}) + + +def stage(node, source, temp_file): + """Stage temporary file to configured location + + :param node: Node for which to stage the file + :param source: Where to stage the file. Corresponds to + CONF.redfish.firmware_source. + :param temp_file: File path of temporary file to stage + :returns: Tuple of staged URL and source (http or swift) that needs + cleanup of staged files afterwards. + :raises RedfishError: If staging to HTTP server has failed. + """ + staged_url = None + filename = os.path.basename(temp_file) + if source in ('http', 'local'): + http_url = CONF.deploy.external_http_url or CONF.deploy.http_url + staged_url = urlparse.urljoin( + http_url, "/".join([_FIRMWARE_SUBDIR, node.uuid, filename])) + staged_folder = os.path.join( + CONF.deploy.http_root, _FIRMWARE_SUBDIR, node.uuid) + staged_path = os.path.join(staged_folder, filename) + LOG.debug('For node %(node)s temporary file %(temp_file)s will be ' + 'hard-linked or copied to %(staged_path)s and served over ' + '%(staged_url)s', + {'node': node.uuid, 'temp_file': temp_file, + 'staged_path': staged_path, 'staged_url': staged_url}) + os.makedirs(staged_folder, exist_ok=True) + try: + os.link(temp_file, staged_path) + os.chmod(temp_file, CONF.redfish.file_permission) + except OSError as oserror: + LOG.debug("Could not hardlink file %(temp_file)s to location " + "%(staged_path)s. Will try to copy it. Error: %(error)s", + {'temp_file': temp_file, 'staged_path': staged_path, + 'error': oserror}) + try: + shutil.copyfile(temp_file, staged_path) + os.chmod(staged_path, CONF.redfish.file_permission) + except IOError as ioerror: + raise exception.RedfishError( + _('For %(node)s failed to copy firmware file ' + '%(temp_file)s to HTTP server root. Error %(error)s') + % {'node': node.uuid, 'temp_file': temp_file, + 'error': ioerror}) + + elif source == 'swift': + container = CONF.redfish.swift_container + timeout = CONF.redfish.swift_object_expiry_timeout + swift_api = swift.SwiftAPI() + object_name = "/".join([node.uuid, filename]) + swift_api.create_object( + container, + object_name, + temp_file, + object_headers={'X-Delete-After': str(timeout)}) + staged_url = swift_api.get_temp_url( + container, object_name, timeout) + LOG.debug('For node %(node)s temporary file at %(temp_file)s will be ' + 'served from Swift temporary URL %(staged_url)s', + {'node': node.uuid, 'temp_file': temp_file, + 'staged_url': staged_url}) + + need_cleanup = 'swift' if source == 'swift' else 'http' + return staged_url, need_cleanup + + +def cleanup(node): + """Clean up staged files + + :param node: Node for which to clean up. Should contain + 'firmware_cleanup' entry in `driver_internal_info` to indicate + source(s) to be cleaned up. + """ + # Cleaning up temporary just in case there is something when staging + # to http or swift has failed. + temp_dir = os.path.join(tempfile.gettempdir(), node.uuid) + LOG.debug('For node %(node)s cleaning up temporary files, if any, from ' + '%(temp_dir)s.', {'node': node.uuid, 'temp_dir': temp_dir}) + shutil.rmtree(temp_dir, ignore_errors=True) + + cleanup = node.driver_internal_info.get('firmware_cleanup') + if not cleanup: + return + + if 'http' in cleanup: + http_dir = os.path.join( + CONF.deploy.http_root, _FIRMWARE_SUBDIR, node.uuid) + LOG.debug('For node %(node)s cleaning up files from %(http_dir)s.', + {'node': node.uuid, 'http_dir': http_dir}) + shutil.rmtree(http_dir, ignore_errors=True) + + if 'swift' in cleanup: + swift_api = swift.SwiftAPI() + container = CONF.redfish.swift_container + LOG.debug('For node %(node)s cleaning up files from Swift container ' + '%(container)s.', + {'node': node.uuid, 'container': container}) + _, objects = swift_api.connection.get_container(container) + for o in objects: + name = o.get('name') + if name and name.startswith(node.uuid): + try: + swift_api.delete_object(container, name) + except exception.SwiftOperationError as error: + LOG.warning('For node %(node)s failed to clean up ' + '%(object)s. Error: %(error)s', + {'node': node.uuid, 'object': name, + 'error': error}) diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index cb56a821b3..a669d09bc8 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -14,6 +14,7 @@ # under the License. import collections +from urllib.parse import urlparse from ironic_lib import metrics_utils from oslo_log import log @@ -799,7 +800,8 @@ class RedfishManagement(base.ManagementInterface): """ firmware_update = firmware_updates[0] - firmware_url = firmware_update['url'] + firmware_url, need_cleanup = self._stage_firmware_file( + node, firmware_update) LOG.debug('Applying firmware %(firmware_image)s to node ' '%(node_uuid)s', @@ -809,8 +811,15 @@ class RedfishManagement(base.ManagementInterface): task_monitor = update_service.simple_update(firmware_url) firmware_update['task_monitor'] = task_monitor.task_monitor_uri - node.set_driver_internal_info('firmware_updates', - firmware_updates) + node.set_driver_internal_info('firmware_updates', firmware_updates) + + if need_cleanup: + fw_cleanup = node.driver_internal_info.get('firmware_cleanup') + if not fw_cleanup: + fw_cleanup = [need_cleanup] + elif need_cleanup not in fw_cleanup: + fw_cleanup.append(need_cleanup) + node.set_driver_internal_info('firmware_cleanup', fw_cleanup) def _continue_firmware_updates(self, task, update_service, firmware_updates): @@ -860,13 +869,18 @@ class RedfishManagement(base.ManagementInterface): manager_utils.node_power_action(task, states.REBOOT) def _clear_firmware_updates(self, node): - """Clears firmware updates from driver_internal_info + """Clears firmware updates artifacts + + Clears firmware updates from driver_internal_info and any files + that were staged. Note that the caller must have an exclusive lock on the node. :param node: the node to clear the firmware updates from """ + firmware_utils.cleanup(node) node.del_driver_internal_info('firmware_updates') + node.del_driver_internal_info('firmware_cleanup') node.save() @METRICS.timer('RedfishManagement._query_firmware_update_failed') @@ -1012,6 +1026,56 @@ class RedfishManagement(base.ManagementInterface): {'node': node.uuid, 'firmware_image': current_update['url']}) + def _stage_firmware_file(self, node, firmware_update): + """Stage firmware update according to configuration. + + :param node: Node for which to stage the firmware file + :param firmware_update: Firmware update to stage + :returns: Tuple of staged URL and source that needs cleanup of + staged files afterwards. If not staging, then return + original URL and None for source that needs cleanup. + :raises IronicException: If something goes wrong with staging. + """ + try: + url = firmware_update['url'] + parsed_url = urlparse(url) + scheme = parsed_url.scheme.lower() + source = (firmware_update.get('source') + or CONF.redfish.firmware_source).lower() + + # Keep it simple, in further processing TLS does not matter + if scheme == 'https': + scheme = 'http' + + # If source and scheme is HTTP, then no staging, + # returning original location + if scheme == 'http' and source == scheme: + LOG.debug('For node %(node)s serving firmware from original ' + 'location %(url)s', {'node': node.uuid, 'url': url}) + return url, None + + # If source and scheme is Swift, then not moving, but + # returning Swift temp URL + if scheme == 'swift' and source == scheme: + temp_url = firmware_utils.get_swift_temp_url(parsed_url) + LOG.debug('For node %(node)s serving original firmware at ' + '%(url)s via Swift temporary url %(temp_url)s', + {'node': node.uuid, 'url': url, + 'temp_url': temp_url}) + return temp_url, None + + # For remaining, download the image to temporary location + temp_file = firmware_utils.download_to_temp(node, url) + + firmware_utils.verify_checksum( + node, firmware_update.get('checksum'), temp_file) + + return firmware_utils.stage(node, source, temp_file) + + except exception.IronicException as error: + firmware_utils.cleanup(node) + raise error + def get_secure_boot_state(self, task): """Get the current secure boot state for the node. diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py index 60c66c0249..e2c6e75b2d 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py @@ -11,7 +11,18 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import shutil +import tempfile +from unittest import mock +from urllib.parse import urlparse + +from oslo_utils import fileutils + from ironic.common import exception +from ironic.common import image_service +from ironic.common import swift +from ironic.conf import CONF from ironic.drivers.modules.redfish import firmware_utils from ironic.tests import base @@ -22,10 +33,12 @@ class FirmwareUtilsTestCase(base.TestCase): firmware_images = [ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300 }, { - "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE" + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "checksum": "9f6227549221920e312fed2cfc6586ee832cc546" } ] firmware_utils.validate_update_firmware_args(firmware_images) @@ -33,6 +46,7 @@ class FirmwareUtilsTestCase(base.TestCase): def test_validate_update_firmware_args_not_list(self): firmware_images = { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300 } self.assertRaisesRegex( @@ -43,10 +57,12 @@ class FirmwareUtilsTestCase(base.TestCase): firmware_images = [ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300, }, { "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "checksum": "9f6227549221920e312fed2cfc6586ee832cc546", "something": "unknown" } ] @@ -58,9 +74,11 @@ class FirmwareUtilsTestCase(base.TestCase): firmware_images = [ { "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 300, }, { + "checksum": "9f6227549221920e312fed2cfc6586ee832cc546", "wait": 300 } ] @@ -72,6 +90,34 @@ class FirmwareUtilsTestCase(base.TestCase): def test_validate_update_firmware_args_url_not_string(self): firmware_images = [{ "url": 123, + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", + "wait": 300 + }] + self.assertRaisesRegex( + exception.InvalidParameterValue, "123 is not of type 'string'", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_checksum_missing(self): + firmware_images = [ + { + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", + "wait": 300, + }, + { + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "wait": 300 + } + ] + self.assertRaisesRegex( + exception.InvalidParameterValue, + "'checksum' is a required property", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_checksum_not_string(self): + firmware_images = [{ + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": 123, "wait": 300 }] self.assertRaisesRegex( @@ -81,8 +127,335 @@ class FirmwareUtilsTestCase(base.TestCase): def test_validate_update_firmware_args_wait_not_int(self): firmware_images = [{ "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", "wait": 'abc' }] self.assertRaisesRegex( exception.InvalidParameterValue, "'abc' is not of type 'integer'", firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_source_not_known(self): + firmware_images = [{ + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d", + "source": "abc" + }] + self.assertRaisesRegex( + exception.InvalidParameterValue, "'abc' is not one of", + firmware_utils.validate_update_firmware_args, firmware_images) + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_get_swift_temp_url(self, mock_swift_api): + mock_swift_api.return_value.get_temp_url.return_value = 'http://temp' + parsed_url = urlparse("swift://firmware/sub/bios.exe") + + result = firmware_utils.get_swift_temp_url(parsed_url) + + self.assertEqual(result, 'http://temp') + mock_swift_api.return_value.get_temp_url.assert_called_with( + 'firmware', 'sub/bios.exe', + CONF.redfish.swift_object_expiry_timeout) + + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image_service, 'HttpImageService', autospec=True) + def test_download_to_temp_http( + self, mock_http_image_service, mock_makedirs, mock_gettempdir): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + mock_gettempdir.return_value = '/tmp' + http_url = 'http://example.com/bios.exe' + + with mock.patch.object(firmware_utils, 'open', mock.mock_open(), + create=True) as mock_open: + result = firmware_utils.download_to_temp(node, http_url) + + exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe' + exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435' + mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True) + self.assertEqual(result, exp_result) + mock_http_image_service.return_value.download.assert_called_with( + http_url, mock_open.return_value) + mock_open.assert_has_calls([mock.call(exp_result, 'wb')]) + + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image_service, 'HttpImageService', autospec=True) + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_download_to_temp_swift( + self, mock_swift_api, mock_http_image_service, mock_makedirs, + mock_gettempdir): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + mock_gettempdir.return_value = '/tmp' + swift_url = 'swift://firmware/sub/bios.exe' + temp_swift_url = 'http://swift_temp' + mock_swift_api.return_value.get_temp_url.return_value = temp_swift_url + + with mock.patch.object(firmware_utils, 'open', mock.mock_open(), + create=True) as mock_open: + result = firmware_utils.download_to_temp(node, swift_url) + + exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe' + exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435' + mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True) + self.assertEqual(result, exp_result) + mock_http_image_service.return_value.download.assert_called_with( + temp_swift_url, mock_open.return_value) + mock_open.assert_has_calls([mock.call(exp_result, 'wb')]) + + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image_service, 'FileImageService', autospec=True) + def test_download_to_temp_file( + self, mock_file_image_service, mock_makedirs, + mock_gettempdir): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + mock_gettempdir.return_value = '/tmp' + file_url = 'file:///firmware/bios.exe' + + with mock.patch.object(firmware_utils, 'open', mock.mock_open(), + create=True) as mock_open: + result = firmware_utils.download_to_temp(node, file_url) + + exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe' + exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435' + mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True) + self.assertEqual(result, exp_result) + mock_file_image_service.return_value.download.assert_called_with( + '/firmware/bios.exe', mock_open.return_value) + mock_open.assert_has_calls([mock.call(exp_result, 'wb')]) + + def test_download_to_temp_invalid(self): + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + self.assertRaises( + exception.InvalidParameterValue, + firmware_utils.download_to_temp, node, 'ftp://firmware/bios.exe') + + @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) + def test_verify_checksum(self, mock_compute_file_checksum): + checksum = 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d' + file_path = '/tmp/bios.exe' + mock_compute_file_checksum.return_value = checksum + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + + firmware_utils.verify_checksum(node, checksum, file_path) + + mock_compute_file_checksum.assert_called_with( + file_path, algorithm='sha1') + + @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) + def test_verify_checksum_mismatch(self, mock_compute_file_checksum): + checksum1 = 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d' + checksum2 = '9f6227549221920e312fed2cfc6586ee832cc546' + file_path = '/tmp/bios.exe' + mock_compute_file_checksum.return_value = checksum1 + node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435') + + self.assertRaises( + exception.RedfishError, firmware_utils.verify_checksum, node, + checksum2, file_path) + mock_compute_file_checksum.assert_called_with( + file_path, algorithm='sha1') + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_http(self, mock_chmod, mock_link, mock_copyfile, + mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = None + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + + staged_url, need_cleanup = firmware_utils.stage( + node, 'http', '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + self.assertEqual(staged_url, + 'http://10.0.0.2/firmware/' + '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + self.assertEqual(need_cleanup, 'http') + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + CONF.redfish.file_permission) + mock_copyfile.assert_not_called() + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_http_copyfile(self, mock_chmod, mock_link, mock_copyfile, + mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = None + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + mock_link.side_effect = OSError + + staged_url, need_cleanup = firmware_utils.stage( + node, 'http', '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + self.assertEqual(staged_url, + 'http://10.0.0.2/firmware/' + '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + self.assertEqual(need_cleanup, 'http') + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_copyfile.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + CONF.redfish.file_permission) + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_http_copyfile_fails(self, mock_chmod, mock_link, + mock_copyfile, mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = None + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + mock_link.side_effect = OSError + mock_copyfile.side_effect = IOError + + self.assertRaises(exception.RedfishError, firmware_utils.stage, + node, 'http', + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_copyfile.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_not_called() + + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + def test_stage_local_external(self, mock_chmod, mock_link, mock_rmtree, + mock_copyfile, mock_makedirs): + CONF.deploy.http_url = 'http://10.0.0.2' + CONF.deploy.external_http_url = 'http://90.0.0.9' + CONF.deploy.http_root = '/httproot' + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + + staged_url, need_cleanup = firmware_utils.stage( + node, 'local', + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + + self.assertEqual(staged_url, + 'http://90.0.0.9/firmware/' + '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + self.assertEqual(need_cleanup, 'http') + mock_makedirs.assert_called_with( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + exist_ok=True) + mock_link.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe') + mock_chmod.assert_called_with( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe', + CONF.redfish.file_permission) + mock_copyfile.assert_not_called() + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_stage_swift(self, mock_swift_api): + node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3') + mock_swift_api.return_value.get_temp_url.return_value = 'http://temp' + temp_file = '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + + staged_url, need_cleanup = firmware_utils.stage( + node, 'swift', temp_file) + + self.assertEqual(staged_url, 'http://temp') + self.assertEqual(need_cleanup, 'swift') + exp_object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + mock_swift_api.return_value.create_object.assert_called_with( + CONF.redfish.swift_container, + exp_object_name, temp_file, + object_headers={'X-Delete-After': + str(CONF.redfish.swift_object_expiry_timeout)}) + mock_swift_api.return_value.get_temp_url.assert_called_with( + CONF.redfish.swift_container, exp_object_name, + CONF.redfish.swift_object_expiry_timeout) + + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_cleanup(self, mock_swift_api, mock_gettempdir, mock_rmtree): + mock_gettempdir.return_value = '/tmp' + CONF.deploy.http_root = '/httproot' + node = mock.Mock( + uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3', + driver_internal_info={'firmware_cleanup': ['http', 'swift']}) + object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + get_container = mock_swift_api.return_value.connection.get_container + get_container.return_value = (mock.Mock(), [{'name': object_name}]) + + firmware_utils.cleanup(node) + + mock_rmtree.assert_any_call( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + mock_rmtree.assert_any_call( + '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + mock_swift_api.return_value.delete_object.assert_called_with( + CONF.redfish.swift_container, object_name) + + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + def test_cleanup_notstaged(self, mock_gettempdir, mock_rmtree): + mock_gettempdir.return_value = '/tmp' + node = mock.Mock( + uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3', + driver_internal_info={'something': 'else'}) + + firmware_utils.cleanup(node) + + mock_rmtree.assert_any_call( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(tempfile, 'gettempdir', autospec=True) + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + @mock.patch.object(firmware_utils.LOG, 'warning', autospec=True) + def test_cleanup_swift_fails(self, mock_warning, mock_swift_api, + mock_gettempdir, mock_rmtree): + mock_gettempdir.return_value = '/tmp' + node = mock.Mock( + uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3', + driver_internal_info={'firmware_cleanup': ['swift']}) + object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe' + get_container = mock_swift_api.return_value.connection.get_container + get_container.return_value = (mock.Mock(), [{'name': object_name}]) + mock_swift_api.return_value.delete_object.side_effect =\ + exception.SwiftOperationError + + firmware_utils.cleanup(node) + + mock_rmtree.assert_any_call( + '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3', + ignore_errors=True) + mock_swift_api.return_value.delete_object.assert_called_with( + CONF.redfish.swift_container, object_name) + mock_warning.assert_called_once() diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index b467006648..93aae5de84 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -27,8 +27,10 @@ from ironic.common import indicator_states from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils +from ironic.conf import CONF from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.redfish import boot as redfish_boot +from ironic.drivers.modules.redfish import firmware_utils from ironic.drivers.modules.redfish import management as redfish_mgmt from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.db import base as db_base @@ -834,22 +836,145 @@ class RedfishManagementTestCase(db_base.DbTestCase): mock_update_service = mock.Mock() mock_update_service.simple_update.return_value = mock_task_monitor mock_get_update_service.return_value = mock_update_service + CONF.redfish.firmware_source = 'http' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.save = mock.Mock() - task.driver.management.update_firmware(task, - [{'url': 'test1'}, - {'url': 'test2'}]) + task.driver.management.update_firmware( + task, + [{'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}]) mock_get_update_service.assert_called_once_with(task.node) - mock_update_service.simple_update.assert_called_once_with('test1') + mock_update_service.simple_update.assert_called_once_with( + 'http://test1') self.assertIsNotNone(task.node .driver_internal_info['firmware_updates']) self.assertEqual( - [{'task_monitor': '/task/123', 'url': 'test1'}, - {'url': 'test2'}], + [{'task_monitor': '/task/123', 'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}], task.node.driver_internal_info['firmware_updates']) + self.assertIsNone( + task.node.driver_internal_info.get('firmware_cleanup')) + mock_set_async_step_flags.assert_called_once_with( + task.node, reboot=True, skip_current_step=True, polling=True) + mock_get_async_step_return_state.assert_called_once_with( + task.node) + mock_node_power_action.assert_called_once_with(task, states.REBOOT) + + @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file', + autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', + spec_set=True, autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', + spec_set=True, autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(deploy_utils, 'get_async_step_return_state', + autospec=True) + @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True) + @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) + def test_update_firmware_stage( + self, mock_get_update_service, mock_set_async_step_flags, + mock_get_async_step_return_state, mock_node_power_action, + mock_prepare, build_mock, mock_stage): + build_mock.return_value = {'a': 'b'} + mock_task_monitor = mock.Mock() + mock_task_monitor.task_monitor_uri = '/task/123' + mock_update_service = mock.Mock() + mock_update_service.simple_update.return_value = mock_task_monitor + mock_get_update_service.return_value = mock_update_service + mock_stage.return_value = ('http://staged/test1', 'http') + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.save = mock.Mock() + + task.driver.management.update_firmware( + task, + [{'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}]) + + mock_get_update_service.assert_called_once_with(task.node) + mock_update_service.simple_update.assert_called_once_with( + 'http://staged/test1') + self.assertIsNotNone(task.node + .driver_internal_info['firmware_updates']) + self.assertEqual( + [{'task_monitor': '/task/123', 'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}], + task.node.driver_internal_info['firmware_updates']) + self.assertIsNotNone( + task.node.driver_internal_info['firmware_cleanup']) + self.assertEqual( + ['http'], task.node.driver_internal_info['firmware_cleanup']) + mock_set_async_step_flags.assert_called_once_with( + task.node, reboot=True, skip_current_step=True, polling=True) + mock_get_async_step_return_state.assert_called_once_with( + task.node) + mock_node_power_action.assert_called_once_with(task, states.REBOOT) + + @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file', + autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', + spec_set=True, autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', + spec_set=True, autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(deploy_utils, 'get_async_step_return_state', + autospec=True) + @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True) + @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) + def test_update_firmware_stage_both( + self, mock_get_update_service, mock_set_async_step_flags, + mock_get_async_step_return_state, mock_node_power_action, + mock_prepare, build_mock, mock_stage): + build_mock.return_value = {'a': 'b'} + mock_task_monitor = mock.Mock() + mock_task_monitor.task_monitor_uri = '/task/123' + mock_update_service = mock.Mock() + mock_update_service.simple_update.return_value = mock_task_monitor + mock_get_update_service.return_value = mock_update_service + mock_stage.return_value = ('http://staged/test1', 'http') + info = self.node.driver_internal_info + info['firmware_cleanup'] = ['swift'] + self.node.driver_internal_info = info + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.save = mock.Mock() + + task.driver.management.update_firmware( + task, + [{'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}]) + + mock_get_update_service.assert_called_once_with(task.node) + mock_update_service.simple_update.assert_called_once_with( + 'http://staged/test1') + self.assertIsNotNone(task.node + .driver_internal_info['firmware_updates']) + self.assertEqual( + [{'task_monitor': '/task/123', 'url': 'http://test1', + 'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'}, + {'url': 'http://test2', + 'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}], + task.node.driver_internal_info['firmware_updates']) + self.assertIsNotNone( + task.node.driver_internal_info['firmware_cleanup']) + self.assertEqual( + ['swift', 'http'], + task.node.driver_internal_info['firmware_cleanup']) mock_set_async_step_flags.assert_called_once_with( task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( @@ -1218,9 +1343,10 @@ class RedfishManagementTestCase(db_base.DbTestCase): driver_internal_info = { 'something': 'else', 'firmware_updates': [ - {'task_monitor': '/task/123', 'url': 'test1'}, - {'url': 'test2'}]} + {'task_monitor': '/task/123', 'url': 'http://test1'}, + {'url': 'http://test2'}]} self.node.driver_internal_info = driver_internal_info + CONF.redfish.firmware_source = 'http' management = redfish_mgmt.RedfishManagement() with task_manager.acquire(self.context, self.node.uuid, @@ -1230,19 +1356,88 @@ class RedfishManagementTestCase(db_base.DbTestCase): management._continue_firmware_updates( task, mock_update_service, - [{'task_monitor': '/task/123', 'url': 'test1'}, - {'url': 'test2'}]) + [{'task_monitor': '/task/123', 'url': 'http://test1'}, + {'url': 'http://test2'}]) self.assertTrue(mock_log.called) - mock_update_service.simple_update.assert_called_once_with('test2') + mock_update_service.simple_update.assert_called_once_with( + 'http://test2') self.assertIsNotNone( task.node.driver_internal_info['firmware_updates']) self.assertEqual( - [{'url': 'test2', 'task_monitor': '/task/987'}], + [{'url': 'http://test2', 'task_monitor': '/task/987'}], task.node.driver_internal_info['firmware_updates']) task.node.save.assert_called_once_with() mock_node_power_action.assert_called_once_with(task, states.REBOOT) + @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True) + @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True) + @mock.patch.object(firmware_utils, 'stage', autospec=True) + def test__stage_firmware_file_https(self, mock_stage, mock_verify_checksum, + mock_download_to_temp): + CONF.redfish.firmware_source = 'local' + firmware_update = {'url': 'https://test1', 'checksum': 'abc'} + node = mock.Mock() + mock_download_to_temp.return_value = '/tmp/test1' + mock_stage.return_value = ('http://staged/test1', 'http') + + management = redfish_mgmt.RedfishManagement() + + staged_url, needs_cleanup = management._stage_firmware_file( + node, firmware_update) + + self.assertEqual(staged_url, 'http://staged/test1') + self.assertEqual(needs_cleanup, 'http') + mock_download_to_temp.assert_called_with(node, 'https://test1') + mock_verify_checksum.assert_called_with(node, 'abc', '/tmp/test1') + mock_stage.assert_called_with(node, 'local', '/tmp/test1') + + @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True) + @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True) + @mock.patch.object(firmware_utils, 'stage', autospec=True) + @mock.patch.object(firmware_utils, 'get_swift_temp_url', autospec=True) + def test__stage_firmware_file_swift( + self, mock_get_swift_temp_url, mock_stage, mock_verify_checksum, + mock_download_to_temp): + CONF.redfish.firmware_source = 'swift' + firmware_update = {'url': 'swift://container/bios.exe'} + node = mock.Mock() + mock_get_swift_temp_url.return_value = 'http://temp' + + management = redfish_mgmt.RedfishManagement() + + staged_url, needs_cleanup = management._stage_firmware_file( + node, firmware_update) + + self.assertEqual(staged_url, 'http://temp') + self.assertIsNone(needs_cleanup) + mock_download_to_temp.assert_not_called() + mock_verify_checksum.assert_not_called() + mock_stage.assert_not_called() + + @mock.patch.object(firmware_utils, 'cleanup', autospec=True) + @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True) + @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True) + @mock.patch.object(firmware_utils, 'stage', autospec=True) + def test__stage_firmware_file_error(self, mock_stage, mock_verify_checksum, + mock_download_to_temp, mock_cleanup): + node = mock.Mock() + firmware_update = {'url': 'https://test1'} + CONF.redfish.firmware_source = 'local' + firmware_update = {'url': 'https://test1'} + node = mock.Mock() + mock_download_to_temp.return_value = '/tmp/test1' + mock_stage.side_effect = exception.IronicException + + management = redfish_mgmt.RedfishManagement() + self.assertRaises(exception.IronicException, + management._stage_firmware_file, node, + firmware_update) + mock_download_to_temp.assert_called_with(node, 'https://test1') + mock_verify_checksum.assert_called_with(node, None, '/tmp/test1') + mock_stage.assert_called_with(node, 'local', '/tmp/test1') + mock_cleanup.assert_called_with(node) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_get_secure_boot_state(self, mock_get_system): fake_system = mock_get_system.return_value diff --git a/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml b/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml new file mode 100644 index 0000000000..559ae22710 --- /dev/null +++ b/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + For ``redfish`` and ``idrac-redfish`` management interface + ``firmware_update`` clean step adds Swift, HTTP service and file system + support to serve and Ironic's HTTP and Swift service to stage files. Also + adds mandatory parameter ``checksum`` for file checksum verification. + +upgrade: + - | + For ``redfish`` and ``idrac-redfish`` management interface + ``firmware_update`` clean step there is now mandatory ``checksum`` + parameter necessary. Update existing clean steps to include it, otherwise + clean step will fail with error "'checksum' is a required property".