openstack/ironic
Code Issues Proposed changes

Automatic zstd detection and decompression...

Browse Source 
... for conductor downloads.

The issue is we don't have the underlying library for requests to do
Zstandard decompression, but userspace tools are common in linux
distributions, and opportunistically we will try to detect, and
de-compresse artifacts.

Zstandard is popular for compression of artifacts in container
registries.

Change-Id: I0f6b3b7a8685bb2724505836c770e080bc0e0632
This commit is contained in:
Julia Kreger 2025-01-09 16:02:18 -08:00
parent db4412d570
commit d6b339ba34
5 changed files with 108 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
bindep.txt
View File

@ -94,3 +94,5 @@ unzip [imagebuild]
sudo [imagebuild]
gawk [imagebuild]
mtools [imagebuild]
# For automatic artifact decompression
zstd [devstack]

35
ironic/common/images.py
View File

@ -417,6 +417,34 @@ def fetch_into(context, image_href, image_file,
return None
def _handle_zstd_compression(path):
zstd_comp = False
with open(path, 'rb') as comp_check:
# Check for zstd compression. Zstd has a variable window for streaming
# clients with transparent connections, and 128 byte blocks.
# Ultimately, requests can't support handling of such content without
# the zstandard library (bsd), but that is not available in global
# requirements. As such, and likely best complexity wise, if we find it
# we can handle it directly.
# https://github.com/facebook/zstd/blob/dev/doc/zstd_compression_format.md
# Ensure we're at the start of the file
comp_check.seek(0)
read = comp_check.read(4)
if read.startswith(b"\x28\xb5\x2f\xfd"):
zstd_comp = True
if zstd_comp and not CONF.conductor.disable_zstandard_decompression:
temp_path = path + '.zstd'
shutil.move(path, temp_path)
try:
utils.execute('zstd', '-d', '--rm', temp_path)
except OSError as e:
LOG.error('Failed to decompress a zstd compressed file: %s', e)
# Restore the downloaded file... We might want to fail the
# entire process.
shutil.move(temp_path, path)
def fetch(context, image_href, path, force_raw=False,
checksum=None, checksum_algo=None,
image_auth_data=None):
@ -428,8 +456,11 @@ def fetch(context, image_href, path, force_raw=False,
and checksum):
checksum_utils.validate_checksum(path, checksum, checksum_algo)
# FIXME(TheJulia): need to check if we need to extract the file
# i.e. zstd... before forcing raw.
# Check and decompress zstd files, since python-requests realistically
# can't do it for us as-is. Also, some OCI container registry artifacts
# may generally just be zstd compressed, regardless if it is a raw file
# or a qcow2 file.
_handle_zstd_compression(path)
if force_raw:
image_to_raw(image_href, path, "%s.part" % path)

9
ironic/conf/conductor.py
View File

@ -522,6 +522,15 @@ opts = [
'functionality by setting this option to True will '
'create a more secure environment, however it may '
'break users in an unexpected fashion.')),
cfg.BoolOpt('disable_zstandard_decompression',
default=False,
mutable=False,
help=_('Option to enable disabling transparent decompression '
'of files which are compressed with Zstandard '
'compression. This option is provided should operators '
'wish to disable this functionality, otherwise it is '
'automaticly applied by the conductor should a '
'compressed artifact be detected.')),
]

60
ironic/tests/unit/common/test_images.py
View File

@ -42,9 +42,11 @@ class IronicImagesTestCase(base.TestCase):
class FakeImgInfo(object):
pass
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service(self, open_mock, image_service_mock):
def test_fetch_image_service(self, open_mock, image_service_mock,
mock_zstd):
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file'
open_mock.return_value = mock_file_handle
@ -56,12 +58,15 @@ class IronicImagesTestCase(base.TestCase):
context='context')
image_service_mock.return_value.download.assert_called_once_with(
'image_href', 'file')
mock_zstd.assert_called_once_with('path')
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@mock.patch.object(images, 'image_to_raw', autospec=True)
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service_force_raw(self, open_mock, image_to_raw_mock,
image_service_mock):
image_service_mock,
mock_zstd):
image_service_mock.return_value.transfer_verified_checksum = None
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file'
@ -74,7 +79,9 @@ class IronicImagesTestCase(base.TestCase):
'image_href', 'file')
image_to_raw_mock.assert_called_once_with(
'image_href', 'path', 'path.part')
mock_zstd.assert_called_once_with('path')
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(fileutils, 'compute_file_checksum',
autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@ -82,7 +89,7 @@ class IronicImagesTestCase(base.TestCase):
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service_force_raw_with_checksum(
self, open_mock, image_to_raw_mock,
image_service_mock, mock_checksum):
image_service_mock, mock_checksum, mock_zstd):
image_service_mock.return_value.transfer_verified_checksum = None
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file'
@ -98,7 +105,9 @@ class IronicImagesTestCase(base.TestCase):
'image_href', 'file')
image_to_raw_mock.assert_called_once_with(
'image_href', 'path', 'path.part')
mock_zstd.assert_called_once_with('path')
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(fileutils, 'compute_file_checksum',
autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@ -106,7 +115,8 @@ class IronicImagesTestCase(base.TestCase):
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service_with_checksum_mismatch(
self, open_mock, image_to_raw_mock,
image_service_mock, mock_checksum):
image_service_mock, mock_checksum,
mock_zstd):
image_service_mock.return_value.transfer_verified_checksum = None
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file'
@ -124,7 +134,9 @@ class IronicImagesTestCase(base.TestCase):
'image_href', 'file')
# If the checksum fails, then we don't attempt to convert the image.
image_to_raw_mock.assert_not_called()
mock_zstd.assert_not_called()
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(fileutils, 'compute_file_checksum',
autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@ -132,7 +144,8 @@ class IronicImagesTestCase(base.TestCase):
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service_force_raw_no_checksum_algo(
self, open_mock, image_to_raw_mock,
image_service_mock, mock_checksum):
image_service_mock, mock_checksum,
mock_zstd):
image_service_mock.return_value.transfer_verified_checksum = None
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file'
@ -148,7 +161,9 @@ class IronicImagesTestCase(base.TestCase):
'image_href', 'file')
image_to_raw_mock.assert_called_once_with(
'image_href', 'path', 'path.part')
mock_zstd.assert_called_once_with('path')
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(fileutils, 'compute_file_checksum',
autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@ -156,7 +171,8 @@ class IronicImagesTestCase(base.TestCase):
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service_force_raw_combined_algo(
self, open_mock, image_to_raw_mock,
image_service_mock, mock_checksum):
image_service_mock, mock_checksum,
mock_zstd):
image_service_mock.return_value.transfer_verified_checksum = None
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file'
@ -172,7 +188,9 @@ class IronicImagesTestCase(base.TestCase):
'image_href', 'file')
image_to_raw_mock.assert_called_once_with(
'image_href', 'path', 'path.part')
mock_zstd.assert_called_once_with('path')
@mock.patch.object(images, '_handle_zstd_compression', autospec=True)
@mock.patch.object(fileutils, 'compute_file_checksum',
autospec=True)
@mock.patch.object(image_service, 'get_image_service', autospec=True)
@ -180,7 +198,8 @@ class IronicImagesTestCase(base.TestCase):
@mock.patch.object(builtins, 'open', autospec=True)
def test_fetch_image_service_auth_data_checksum(
self, open_mock, image_to_raw_mock,
svc_mock, mock_checksum):
svc_mock, mock_checksum,
mock_zstd):
svc_mock.return_value.transfer_verified_checksum = 'f00'
svc_mock.return_value.is_auth_set_needed = True
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
@ -201,6 +220,7 @@ class IronicImagesTestCase(base.TestCase):
'image_href', 'path', 'path.part')
svc_mock.return_value.set_image_auth.assert_called_once_with(
'image_href', 'meow')
mock_zstd.assert_called_once_with('path')
@mock.patch.object(image_format_inspector, 'detect_file_format',
autospec=True)
@ -682,6 +702,32 @@ class IronicImagesTestCase(base.TestCase):
validate_mock.side_effect = OSError
self.assertIsNone(images.is_source_a_path('context', url))
@mock.patch.object(utils, 'execute', autospec=True)
@mock.patch.object(shutil, 'move', autospec=True)
@mock.patch.object(builtins, 'open', autospec=True)
def test__hanlde_zstd_compression(self, mock_open, mock_move,
mock_exec):
mock_file_handle = mock.Mock()
mock_file_handle.read.return_value = b"\x28\xb5\x2f\xfd"
mock_open.return_value.__enter__.open = mock_file_handle
images._handle_zstd_compression('path')
mock_move.assert_called_once_with('path', 'path.zstd')
mock_exec.assert_called_once_with('zstd', '-d', '--rm', 'path.zstd')
@mock.patch.object(utils, 'execute', autospec=True)
@mock.patch.object(shutil, 'move', autospec=True)
@mock.patch.object(builtins, 'open', autospec=True)
def test__hanlde_zstd_compression_disabled(self, mock_open, mock_move,
mock_exec):
mock_file_handle = mock.Mock()
mock_file_handle.read.return_value = b"\x28\xb5\x2f\xfd"
mock_open.return_value.__enter__.open = mock_file_handle
CONF.set_override('disable_zstandard_decompression', True,
group='conductor')
images._handle_zstd_compression('path')
mock_move.assert_not_called()
mock_exec.assert_not_called()
class FsImageTestCase(base.TestCase):

11
releasenotes/notes/automatic-zstd-decompression-bf30cb99ebbb07f3.yaml Normal file
View File

@ -0,0 +1,11 @@
---
features:
- |
Adds the capability for Ironic's conductor to detect Zstandard
compressed content and to automatically decompress the files
to enable image format detection and conversion.
This is due to use of Zstandard compression upon artifacts stored
in container registries is a popular practice, and can be disabled
using the ``[conductor]disable_zstandard_decompression``
configuration option.