openstack/ironic
Code Issues Proposed changes

Do not recalculate checksum if disk_format is not changed

Browse Source 
Even if a glance image is raw, we still recalculate the checksum after
"converting" it to raw. This process may take exceptionally long.

Change-Id: Id93d518b8d2b8064ff901f1a0452abd825e366c0
This commit is contained in:
Dmitry Tantsur 2023-03-06 18:14:31 +01:00
parent 17d97d51ad
commit f00da959ea
6 changed files with 150 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
doc/source/admin/fast-track.rst
View File

@ -15,6 +15,19 @@ provisioned within a short period of time.
the ``noop`` networking. The case where inspection, cleaning and
provisioning networks are different is not supported.
.. note::
Fast track mode is very sensitive to long-running processes on the conductor
side that may prevent agent heartbeats from being registered.
For example, converting a large image to the raw format may take long enough
to reach the fast track timeout. In this case, you can either :ref:`use raw
images <stream_raw_images>` or move the conversion to the agent side with:
.. code-block:: ini
[DEFAULT]
force_raw_images = False
Enabling
========

40
doc/source/admin/interfaces/deploy.rst
View File

@ -81,6 +81,46 @@ accessible from HTTP service. Please refer to configuration option
``FollowSymLinks`` if you are using Apache HTTP server, or
``disable_symlinks`` if Nginx HTTP server is in use.
.. _stream_raw_images:
Streaming raw images
--------------------
The Bare Metal service is capable of streaming raw images directly to the
target disk of a node, without caching them in the node's RAM. When the source
image is not already raw, the conductor will convert the image and calculate
the new checksum.
.. note::
If no algorithm is specified via the ``image_os_hash_algo`` field, or if
this field is set to ``md5``, SHA256 is used for the updated checksum.
For HTTP or local file images that are already raw, you need to explicitly set
the disk format to prevent the checksum from being unnecessarily re-calculated.
For example:
.. code-block:: shell
baremetal node set <node> \
--instance-info image_source=http://server/myimage.img \
--instance-info image_os_hash_algo=sha512 \
--instance-info image_os_hash_value=<SHA512 of the raw image> \
--instance-info image_disk_format=raw
To disable this feature and cache images in the node's RAM, set
.. code-block:: ini
[agent]
stream_raw_images = False
To disable the conductor-side conversion completely, set
.. code-block:: ini
[DEFAULT]
force_raw_images = False
.. _ansible-deploy:
Ansible deploy

5
doc/source/install/refarch/common.rst
View File

@ -277,9 +277,8 @@ the space requirements are different:
In both cases a cached image is converted to raw if ``force_raw_images``
is ``True`` (the default).
.. note::
``image_download_source`` can also be provided in the node's
``driver_info`` or ``instance_info``. See :ref:`image_download_source`.
See :ref:`image_download_source` and :ref:`stream_raw_images` for more
details.
* When network boot is used, the instance image kernel and ramdisk are cached
locally while the instance is active.

15
ironic/drivers/modules/deploy_utils.py
View File

@ -1093,6 +1093,11 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
_, image_path = cache_instance_image(task.context, task.node,
force_raw=force_raw)
if force_raw or image_info is None:
if image_info is None:
initial_format = instance_info.get('image_disk_format')
else:
initial_format = image_info.get('disk_format')
if force_raw:
instance_info['image_disk_format'] = 'raw'
else:
@ -1108,12 +1113,17 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
# sha256.
if image_info is None:
os_hash_algo = instance_info.get('image_os_hash_algo')
hash_value = instance_info.get('image_os_hash_value')
old_checksum = instance_info.get('image_checksum')
else:
os_hash_algo = image_info.get('os_hash_algo')
hash_value = image_info.get('os_hash_value')
old_checksum = image_info.get('checksum')
if initial_format != instance_info['image_disk_format']:
if not os_hash_algo or os_hash_algo == 'md5':
LOG.debug("Checksum algorithm for image %(image)s for node "
"%(node)s is set to '%(algo)s', changing to 'sha256'",
"%(node)s is set to '%(algo)s', changing to sha256",
{'algo': os_hash_algo, 'node': task.node.uuid,
'image': image_path})
os_hash_algo = 'sha256'
@ -1123,6 +1133,9 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
{'image': image_path, 'node': task.node.uuid})
instance_info['image_checksum'] = None
hash_value = compute_image_checksum(image_path, os_hash_algo)
else:
instance_info['image_checksum'] = old_checksum
instance_info['image_os_hash_algo'] = os_hash_algo
instance_info['image_os_hash_value'] = hash_value
else:

73
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -1940,7 +1940,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
self.node.save()
self.checksum_mock = self.useFixture(fixtures.MockPatchObject(
fileutils, 'compute_file_checksum')).mock
fileutils, 'compute_file_checksum', autospec=True)).mock
self.checksum_mock.return_value = 'fake-checksum'
self.cache_image_mock = self.useFixture(fixtures.MockPatchObject(
utils, 'cache_instance_image', autospec=True)).mock
@ -2012,9 +2012,25 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
image_info=self.image_info, expect_raw=True)
self.assertIsNone(instance_info['image_checksum'])
self.assertEqual(instance_info['image_os_hash_algo'], 'sha512')
self.assertEqual(instance_info['image_os_hash_value'],
'fake-checksum')
self.assertEqual(instance_info['image_disk_format'], 'raw')
calls = [mock.call(image_path, algorithm='sha512')]
self.checksum_mock.assert_has_calls(calls)
self.checksum_mock.assert_called_once_with(image_path,
algorithm='sha512')
def test_build_instance_info_already_raw(self):
cfg.CONF.set_override('force_raw_images', True)
self.image_info['disk_format'] = 'raw'
image_path, instance_info = self._test_build_instance_info(
image_info=self.image_info, expect_raw=True)
self.assertEqual(instance_info['image_checksum'], 'aa')
self.assertEqual(instance_info['image_os_hash_algo'], 'sha512')
self.assertEqual(instance_info['image_os_hash_value'],
'fake-sha512')
self.assertEqual(instance_info['image_disk_format'], 'raw')
self.checksum_mock.assert_not_called()
def test_build_instance_info_force_raw_drops_md5(self):
cfg.CONF.set_override('force_raw_images', True)
@ -2027,6 +2043,17 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
calls = [mock.call(image_path, algorithm='sha256')]
self.checksum_mock.assert_has_calls(calls)
def test_build_instance_info_already_raw_keeps_md5(self):
cfg.CONF.set_override('force_raw_images', True)
self.image_info['os_hash_algo'] = 'md5'
self.image_info['disk_format'] = 'raw'
image_path, instance_info = self._test_build_instance_info(
image_info=self.image_info, expect_raw=True)
self.assertEqual(instance_info['image_checksum'], 'aa')
self.assertEqual(instance_info['image_disk_format'], 'raw')
self.checksum_mock.assert_not_called()
@mock.patch.object(image_service.HttpImageService, 'validate_href',
autospec=True)
def test_build_instance_info_file_image(self, validate_href_mock):
@ -2035,7 +2062,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
i_info['image_source'] = 'file://image-ref'
i_info['image_checksum'] = 'aa'
i_info['root_gb'] = 10
i_info['image_checksum'] = 'aa'
driver_internal_info['is_whole_disk_image'] = True
self.node.instance_info = i_info
self.node.driver_internal_info = driver_internal_info
@ -2052,6 +2078,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
self.assertEqual(expected_url, info['image_url'])
self.assertEqual('sha256', info['image_os_hash_algo'])
self.assertEqual('fake-checksum', info['image_os_hash_value'])
self.assertEqual('raw', info['image_disk_format'])
self.cache_image_mock.assert_called_once_with(
task.context, task.node, force_raw=True)
self.checksum_mock.assert_called_once_with(
@ -2068,7 +2095,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
i_info['image_source'] = 'http://image-ref'
i_info['image_checksum'] = 'aa'
i_info['root_gb'] = 10
i_info['image_checksum'] = 'aa'
driver_internal_info['is_whole_disk_image'] = True
self.node.instance_info = i_info
self.node.driver_internal_info = driver_internal_info
@ -2102,7 +2128,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
i_info['image_source'] = 'http://image-ref'
i_info['image_checksum'] = 'aa'
i_info['root_gb'] = 10
i_info['image_checksum'] = 'aa'
i_info['image_download_source'] = 'local'
driver_internal_info['is_whole_disk_image'] = True
self.node.instance_info = i_info
@ -2138,7 +2163,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
i_info['image_source'] = 'http://image-ref'
i_info['image_checksum'] = 'aa'
i_info['root_gb'] = 10
i_info['image_checksum'] = 'aa'
d_info['image_download_source'] = 'local'
driver_internal_info['is_whole_disk_image'] = True
self.node.instance_info = i_info
@ -2164,6 +2188,41 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
validate_href_mock.assert_called_once_with(
mock.ANY, expected_url, False)
@mock.patch.object(image_service.HttpImageService, 'validate_href',
autospec=True)
def test_build_instance_info_local_image_already_raw(self,
validate_href_mock):
cfg.CONF.set_override('image_download_source', 'local', group='agent')
i_info = self.node.instance_info
driver_internal_info = self.node.driver_internal_info
i_info['image_source'] = 'http://image-ref'
i_info['image_checksum'] = 'aa'
i_info['root_gb'] = 10
i_info['image_disk_format'] = 'raw'
driver_internal_info['is_whole_disk_image'] = True
self.node.instance_info = i_info
self.node.driver_internal_info = driver_internal_info
self.node.save()
expected_url = (
'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid)
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
info = utils.build_instance_info_for_deploy(task)
self.assertEqual(expected_url, info['image_url'])
self.assertEqual('aa', info['image_checksum'])
self.assertEqual('raw', info['image_disk_format'])
self.assertIsNone(info['image_os_hash_algo'])
self.assertIsNone(info['image_os_hash_value'])
self.cache_image_mock.assert_called_once_with(
task.context, task.node, force_raw=True)
self.checksum_mock.assert_not_called()
validate_href_mock.assert_called_once_with(
mock.ANY, expected_url, False)
class TestStorageInterfaceUtils(db_base.DbTestCase):
def setUp(self):

5
releasenotes/notes/no-recalculate-653e524fd6160e72.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
No longer re-calculates checksums for images that are already raw.
Previously, it would cause significant delays in deploying raw images.