From 407e5051d4b644196ba4fc58d61f874fed75dc2b Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 25 Jun 2021 10:52:06 +1200 Subject: [PATCH] Set image_disk_format from file extension for HTTP images Node instance_info `image_disk_format` needs to be set to `raw` for large raw images or the deployment may fail with a memory check error or a full node tmpfs. Even when there is no error, without image_disk_format=raw the image will not be streamed directly to disk. image_disk_format is auto-detected and set by ironic for glance and file sourced images, but this does not occur for direct HTTP based images. This change will set image_disk_format=raw when the URL file extension ends with .raw, which is enough to support TripleO's conventions for the overcloud raw image. Change-Id: I6a9c225fc2d14b2d07cd0bf2379cd2c8c548f312 --- metalsmith/sources.py | 22 ++++++++-- metalsmith/test/test_sources.py | 42 +++++++++++++++++++ .../raw_http_images-41007351896ff642.yaml | 9 ++++ 3 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/raw_http_images-41007351896ff642.yaml diff --git a/metalsmith/sources.py b/metalsmith/sources.py index 930c555..8c9d87f 100644 --- a/metalsmith/sources.py +++ b/metalsmith/sources.py @@ -95,7 +95,8 @@ class HttpWholeDiskImage(_Source): specifically, by **ironic-conductor** processes). """ - def __init__(self, url, checksum=None, checksum_url=None): + def __init__(self, url, checksum=None, checksum_url=None, + disk_format=None): """Create an HTTP source. :param url: URL of the image. @@ -104,6 +105,8 @@ class HttpWholeDiskImage(_Source): :param checksum_url: URL of the checksum file for the image. Has to be in the standard format of the ``md5sum`` tool. Mutually exclusive with ``checksum``. + :param disk_format: Optional value to set for ``instance_info`` + ``image_disk_format``. """ if (checksum and checksum_url) or (not checksum and not checksum_url): raise TypeError('Exactly one of checksum and checksum_url has ' @@ -112,6 +115,7 @@ class HttpWholeDiskImage(_Source): self.url = url self.checksum = checksum self.checksum_url = checksum_url + self.disk_format = disk_format def _validate(self, connection, root_size_gb): # TODO(dtantsur): should we validate image URLs here? Ironic will do it @@ -147,17 +151,20 @@ class HttpWholeDiskImage(_Source): def _node_updates(self, connection): LOG.debug('Image: %(image)s, checksum %(checksum)s', {'image': self.url, 'checksum': self.checksum}) - return { + updates = { 'image_source': self.url, 'image_checksum': self.checksum, } + if self.disk_format: + updates['image_disk_format'] = self.disk_format + return updates class HttpPartitionImage(HttpWholeDiskImage): """A partition image from an HTTP(s) location.""" def __init__(self, url, kernel_url, ramdisk_url, checksum=None, - checksum_url=None): + checksum_url=None, disk_format=None): """Create an HTTP source. :param url: URL of the root disk image. @@ -168,9 +175,12 @@ class HttpPartitionImage(HttpWholeDiskImage): :param checksum_url: URL of the checksum file for the root disk image. Has to be in the standard format of the ``md5sum`` tool. Mutually exclusive with ``checksum``. + :param disk_format: Optional value to set for ``instance_info`` + ``image_disk_format``. """ super(HttpPartitionImage, self).__init__(url, checksum=checksum, - checksum_url=checksum_url) + checksum_url=checksum_url, + disk_format=disk_format) self.kernel_url = kernel_url self.ramdisk_url = ramdisk_url @@ -322,6 +332,10 @@ def detect(image, kernel=None, ramdisk=None, checksum=None): else: kwargs = {'checksum': checksum} + # Assume raw image based on file extension + if image.endswith('.raw'): + kwargs['disk_format'] = 'raw' + if kernel or ramdisk: return HttpPartitionImage(image, kernel_url=kernel, diff --git a/metalsmith/test/test_sources.py b/metalsmith/test/test_sources.py index 7b0645f..5a0b508 100644 --- a/metalsmith/test/test_sources.py +++ b/metalsmith/test/test_sources.py @@ -111,6 +111,23 @@ class TestDetect(unittest.TestCase): self.assertEqual(source.checksum, 'abcd') source._validate(mock.Mock(), None) + self.assertEqual({ + 'image_checksum': 'abcd', + 'image_source': 'http:///image' + }, source._node_updates(None)) + + def test_http_whole_disk_raw(self): + source = sources.detect('http:///image.raw', checksum='abcd') + self.assertIs(source.__class__, sources.HttpWholeDiskImage) + self.assertEqual(source.url, 'http:///image.raw') + self.assertEqual(source.checksum, 'abcd') + + source._validate(mock.Mock(), None) + self.assertEqual({ + 'image_checksum': 'abcd', + 'image_source': 'http:///image.raw', + 'image_disk_format': 'raw' + }, source._node_updates(None)) def test_https_whole_disk(self): source = sources.detect('https:///image', checksum='abcd') @@ -138,6 +155,31 @@ class TestDetect(unittest.TestCase): self.assertEqual(source.ramdisk_url, 'http:///ramdisk') source._validate(mock.Mock(), 9) + self.assertEqual({ + 'image_checksum': 'abcd', + 'image_source': 'http:///image', + 'kernel': 'http:///kernel', + 'ramdisk': 'http:///ramdisk' + }, source._node_updates(None)) + + def test_http_partition_disk_raw(self): + source = sources.detect('http:///image.raw', checksum='abcd', + kernel='http:///kernel', + ramdisk='http:///ramdisk') + self.assertIs(source.__class__, sources.HttpPartitionImage) + self.assertEqual(source.url, 'http:///image.raw') + self.assertEqual(source.checksum, 'abcd') + self.assertEqual(source.kernel_url, 'http:///kernel') + self.assertEqual(source.ramdisk_url, 'http:///ramdisk') + + source._validate(mock.Mock(), 9) + self.assertEqual({ + 'image_checksum': 'abcd', + 'image_source': 'http:///image.raw', + 'kernel': 'http:///kernel', + 'ramdisk': 'http:///ramdisk', + 'image_disk_format': 'raw' + }, source._node_updates(None)) def test_http_partition_disk_missing_root(self): source = sources.detect('http:///image', checksum='abcd', diff --git a/releasenotes/notes/raw_http_images-41007351896ff642.yaml b/releasenotes/notes/raw_http_images-41007351896ff642.yaml new file mode 100644 index 0000000..637ee62 --- /dev/null +++ b/releasenotes/notes/raw_http_images-41007351896ff642.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Images sourced by HTTP would never have the node instance_info + `image_disk_format` set to `raw` because the image file is not processed by + ironic. This would result in errors for large images, or + ironic-python-agent never using streaming to copy the image to disk. To + work around this, `image_disk_format` is set to `raw` when the image URL + ends with a `.raw` file extension.