openstack/metalsmith
Code Issues Proposed changes

Set image_disk_format from file extension for HTTP images

Browse Source 
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
This commit is contained in:
Steve Baker
2021-06-25 10:52:06 +12:00
parent 2fe17330d6
commit 407e5051d4
3 changed files with 69 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
metalsmith/sources.py
View File

@@ -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,

42
metalsmith/test/test_sources.py
View File

@@ -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',

9
releasenotes/notes/raw_http_images-41007351896ff642.yaml Normal file
View File

@@ -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.