From 3d4229861931c42c9ad3bcfde551def6c725214e Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Tue, 24 Oct 2023 14:56:14 -0700 Subject: [PATCH] Remove standby.cache_image support Image caching was never fully supported in Ironic or IPA; this is vestigal code leftover from a partial implementation. Even if we implemetented it today, we'd likely use a completely different methodology. Change-Id: Id4ab7b3c4f106b209585dbd090cdcb229b1daa73 --- ironic_python_agent/extensions/standby.py | 39 ------ .../tests/unit/extensions/test_standby.py | 123 ------------------ .../cache-image-removal-3b5a80a6038a320b.yaml | 5 + 3 files changed, 5 insertions(+), 162 deletions(-) create mode 100644 releasenotes/notes/cache-image-removal-3b5a80a6038a320b.yaml diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index 6cc2be740..42aa9b868 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -887,45 +887,6 @@ class StandbyExtension(base.BaseAgentExtension): else: self.partition_uuids['root uuid'] = root_uuid - @base.async_command('cache_image', _validate_image_info) - def cache_image(self, image_info, force=False, configdrive=None): - """Asynchronously caches specified image to the local OS device. - - :param image_info: Image information dictionary. - :param force: Optional. If True forces cache_image to download and - cache image, even if the same image already exists on - the local OS install device. Defaults to False. - :param configdrive: A string containing the location of the config - drive as a URL OR the contents (as gzip/base64) - of the configdrive. Optional, defaults to None. - - :raises: ImageDownloadError if the image download fails for any reason. - :raises: ImageChecksumError if the downloaded image's checksum does not - match the one reported in image_info. - :raises: ImageWriteError if writing the image fails. - """ - LOG.debug('Caching image %s', image_info['id']) - device = hardware.dispatch_to_managers('get_os_install_device', - permit_refresh=True) - - msg = 'image ({}) already present on device {} ' - - if self.cached_image_id != image_info['id'] or force: - LOG.debug('Already had %s cached, overwriting', - self.cached_image_id) - # NOTE(dtantsur): backward compatibility - if configdrive is None: - configdrive = image_info.pop('configdrive', None) - self._cache_and_write_image(image_info, device, configdrive) - msg = 'image ({}) cached to device {} ' - - self._fix_up_partition_uuids(image_info, device) - result_msg = _message_format(msg, image_info, device, - self.partition_uuids) - - LOG.info(result_msg) - return result_msg - @base.async_command('prepare_image', _validate_image_info) def prepare_image(self, image_info, configdrive=None): """Asynchronously prepares specified image on local OS install device. diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index 53c303630..ea4d9a26c 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -191,11 +191,6 @@ class TestStandbyExtension(base.IronicAgentTest): standby._validate_image_info, invalid_info) - def test_cache_image_invalid_image_list(self): - self.assertRaises(errors.InvalidCommandParamsError, - self.agent_extension.cache_image, - image_info={'foo': 'bar'}) - def test_image_location(self): image_info = _build_fake_image_info() location = standby._image_location(image_info) @@ -842,124 +837,6 @@ class TestStandbyExtension(base.IronicAgentTest): standby.ImageDownload, image_info) - @mock.patch('ironic_lib.disk_utils.get_disk_identifier', - lambda dev: 'ROOT') - @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._write_image', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._download_image', - autospec=True) - def test_cache_image(self, download_mock, write_mock, - dispatch_mock): - image_info = _build_fake_image_info() - download_mock.return_value = None - write_mock.return_value = None - dispatch_mock.return_value = 'manager' - async_result = self.agent_extension.cache_image(image_info=image_info) - async_result.join() - download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager', None) - dispatch_mock.assert_called_once_with('get_os_install_device', - permit_refresh=True) - self.assertEqual(image_info['id'], - self.agent_extension.cached_image_id) - self.assertEqual('SUCCEEDED', async_result.command_status) - self.assertIn('result', async_result.command_result) - cmd_result = ('cache_image: image ({}) cached to device {} ' - 'root_uuid={}').format(image_info['id'], 'manager', - 'ROOT') - self.assertEqual(cmd_result, async_result.command_result['result']) - - @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._write_image', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._download_image', - autospec=True) - def test_cache_partition_image(self, download_mock, write_mock, - dispatch_mock): - image_info = _build_fake_partition_image_info() - download_mock.return_value = None - write_mock.return_value = {'root uuid': 'root_uuid'} - dispatch_mock.return_value = 'manager' - async_result = self.agent_extension.cache_image( - image_info=image_info, configdrive='configdrive_data') - async_result.join() - download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager', - 'configdrive_data') - dispatch_mock.assert_called_once_with('get_os_install_device', - permit_refresh=True) - self.assertEqual(image_info['id'], - self.agent_extension.cached_image_id) - self.assertEqual('SUCCEEDED', async_result.command_status) - self.assertIn('result', async_result.command_result) - cmd_result = ('cache_image: image ({}) cached to device {} ' - 'root_uuid={}').format(image_info['id'], 'manager', - 'root_uuid') - self.assertEqual(cmd_result, async_result.command_result['result']) - - @mock.patch('ironic_lib.disk_utils.get_disk_identifier', - lambda dev: 'ROOT') - @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._write_image', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._download_image', - autospec=True) - def test_cache_image_force(self, download_mock, write_mock, - dispatch_mock): - image_info = _build_fake_image_info() - self.agent_extension.cached_image_id = image_info['id'] - download_mock.return_value = None - write_mock.return_value = None - dispatch_mock.return_value = 'manager' - async_result = self.agent_extension.cache_image( - image_info=image_info, force=True - ) - async_result.join() - download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager', None) - dispatch_mock.assert_called_once_with('get_os_install_device', - permit_refresh=True) - self.assertEqual(image_info['id'], - self.agent_extension.cached_image_id) - self.assertEqual('SUCCEEDED', async_result.command_status) - self.assertIn('result', async_result.command_result) - cmd_result = ('cache_image: image ({}) cached to device {} ' - 'root_uuid=ROOT').format(image_info['id'], 'manager') - self.assertEqual(cmd_result, async_result.command_result['result']) - - @mock.patch('ironic_lib.disk_utils.get_disk_identifier', - lambda dev: 'ROOT') - @mock.patch('ironic_python_agent.hardware.dispatch_to_managers', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._write_image', - autospec=True) - @mock.patch('ironic_python_agent.extensions.standby._download_image', - autospec=True) - def test_cache_image_cached(self, download_mock, write_mock, - dispatch_mock): - image_info = _build_fake_image_info() - self.agent_extension.cached_image_id = image_info['id'] - download_mock.return_value = None - write_mock.return_value = None - dispatch_mock.return_value = 'manager' - async_result = self.agent_extension.cache_image(image_info=image_info) - async_result.join() - self.assertFalse(download_mock.called) - self.assertFalse(write_mock.called) - dispatch_mock.assert_called_once_with('get_os_install_device', - permit_refresh=True) - self.assertEqual(image_info['id'], - self.agent_extension.cached_image_id) - self.assertEqual('SUCCEEDED', async_result.command_status) - self.assertIn('result', async_result.command_result) - cmd_result = ('cache_image: image ({}) already present on device {} ' - 'root_uuid=ROOT').format(image_info['id'], 'manager') - self.assertEqual(cmd_result, async_result.command_result['result']) - @mock.patch('ironic_lib.disk_utils.get_disk_identifier', lambda dev: 'ROOT') @mock.patch('ironic_python_agent.utils.execute', diff --git a/releasenotes/notes/cache-image-removal-3b5a80a6038a320b.yaml b/releasenotes/notes/cache-image-removal-3b5a80a6038a320b.yaml new file mode 100644 index 000000000..7d9e4a6a2 --- /dev/null +++ b/releasenotes/notes/cache-image-removal-3b5a80a6038a320b.yaml @@ -0,0 +1,5 @@ +deprecations: + - Removed the partial-implementation of image caching. This method was never + used by Ironic, and was a vestige of a feature which was never completed. + Image caching, if it were to be implemented today, would not use this + method anyway. \ No newline at end of file