Merge "Move check_image_size to deploy_utils"

This commit is contained in:
Zuul 2025-12-14 08:11:28 +00:00 committed by Gerrit Code Review
commit aa8ae96017
5 changed files with 182 additions and 185 deletions

View file

@ -17,7 +17,6 @@ from urllib import parse as urlparse
from oslo_log import log
from oslo_utils import strutils
from oslo_utils import units
import tenacity
from ironic.common import async_steps
@ -102,46 +101,6 @@ PARTITION_IMAGE_LABELS = ('kernel', 'ramdisk', 'root_gb', 'root_mb', 'swap_mb',
'deploy_boot_mode')
@METRICS.timer('check_image_size')
def check_image_size(task):
"""Check if the requested image is larger than the ram size.
:param task: a TaskManager instance containing the node to act on.
:raises: InvalidParameterValue if size of the image is greater than
the available ram size.
"""
node = task.node
properties = node.properties
image_source = node.instance_info.get('image_source')
image_disk_format = node.instance_info.get('image_disk_format')
# skip check if 'memory_mb' is not defined
if 'memory_mb' not in properties:
LOG.debug('Skip the image size check as memory_mb is not '
'defined in properties on node %s.', node.uuid)
return
image_show = images.image_show(task.context, image_source)
if CONF.agent.stream_raw_images and (image_show.get('disk_format') == 'raw'
or image_disk_format == 'raw'):
LOG.debug('Skip the image size check since the image is going to be '
'streamed directly onto the disk for node %s', node.uuid)
return
memory_size = int(properties.get('memory_mb'))
image_size = int(image_show['size'])
reserved_size = CONF.agent.memory_consumed_by_agent
if (image_size + (reserved_size * units.Mi)) > (memory_size * units.Mi):
msg = (_('Memory size is too small for requested image, if it is '
'less than (image size + reserved RAM size), will break '
'the IPA deployments. Image size: %(image_size)d MiB, '
'Memory size: %(memory_size)d MiB, Reserved size: '
'%(reserved_size)d MiB.')
% {'image_size': image_size / units.Mi,
'memory_size': memory_size,
'reserved_size': reserved_size})
raise exception.InvalidParameterValue(msg)
@METRICS.timer('validate_image_proxies')
def validate_image_proxies(node):
"""Check that the provided proxy parameters are valid.
@ -610,7 +569,6 @@ class AgentDeploy(CustomAgentDeploy):
task.node.instance_info = (
deploy_utils.build_instance_info_for_deploy(task))
task.node.save()
check_image_size(task)
@METRICS.timer('AgentDeploy.validate')
def validate(self, task):

View file

@ -21,6 +21,7 @@ from oslo_log import log as logging
from oslo_utils import excutils
from oslo_utils import fileutils
from oslo_utils import strutils
from oslo_utils import units
from ironic.common import async_steps
from ironic.common import checksum_utils
@ -1426,6 +1427,52 @@ def _instance_info_for_glance(task, instance_info, image_download_source,
return instance_info, image_info
@METRICS.timer('check_image_size')
def check_image_size(node, image_info):
"""Check if the requested image is larger than the ram size.
:param node: a Node object containing the node to act on.
:param image_info: A dict containing image information.
:raises: InvalidParameterValue if size of the image is greater than
the available ram size.
"""
properties = node.properties
image_disk_format = node.instance_info.get('image_disk_format')
# Skip check if 'memory_mb' is not defined
if 'memory_mb' not in properties:
LOG.debug('Skip the image size check as memory_mb is not '
'defined in properties on node %s.', node.uuid)
return
if direct_deploy_should_convert_raw_image(node):
LOG.debug('Skip the image size check since the conductor is '
'converting the image to raw format for node %s.',
node.uuid)
return
image_format = image_info.get('disk_format')
if CONF.agent.stream_raw_images and (image_format == 'raw'
or image_disk_format == 'raw'):
LOG.debug('Skip the image size check since the image is going to be '
'streamed directly onto the disk for node %s', node.uuid)
return
memory_size = int(properties.get('memory_mb'))
image_size = int(image_info['size'])
reserved_size = CONF.agent.memory_consumed_by_agent
if (image_size + (reserved_size * units.Mi)) > (memory_size * units.Mi):
msg = (_('Memory size is too small for requested image, if it is '
'less than (image size + reserved RAM size), will break '
'the IPA deployments. Image size: %(image_size)d MiB, '
'Memory size: %(memory_size)d MiB, Reserved size: '
'%(reserved_size)d MiB.')
% {'image_size': image_size / units.Mi,
'memory_size': memory_size,
'reserved_size': reserved_size})
raise exception.InvalidParameterValue(msg)
@METRICS.timer('build_instance_info_for_deploy')
def build_instance_info_for_deploy(task):
"""Build instance_info necessary for deploying to a node.
@ -1488,6 +1535,8 @@ def build_instance_info_for_deploy(task):
task, instance_info, image_download_source, image_source,
iwdi)
check_image_size(node, image_info)
elif image_service.is_container_registry_url(image_source):
# Is an oci image, we need to figure out the particulars...
# but we *don't* need to also handle special casing with Swift.
@ -1524,6 +1573,9 @@ def build_instance_info_for_deploy(task):
# will work as expected.
di_info['image_source'] = image_source
node.driver_internal_info = di_info
check_image_size(node, image_info)
if not is_glance_image:
if (image_source.startswith('file://')
or image_download_source == 'local'):

View file

@ -61,126 +61,6 @@ class TestAgentMethods(db_base.DbTestCase):
deploy_interface='direct')
dhcp_factory.DHCPFactory._dhcp_provider = None
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size(self, show_mock):
show_mock.return_value = {
'size': 10 * 1024 * 1024,
'disk_format': 'qcow2',
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
agent.check_image_size(task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_without_memory_mb(self, show_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties.pop('memory_mb', None)
task.node.instance_info['image_source'] = 'fake-image'
agent.check_image_size(task)
self.assertFalse(show_mock.called)
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_fail(self, show_mock):
show_mock.return_value = {
'size': 11 * 1024 * 1024,
'disk_format': 'qcow2',
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
self.assertRaises(exception.InvalidParameterValue,
agent.check_image_size,
task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_fail_by_agent_consumed_memory(self, show_mock):
self.config(memory_consumed_by_agent=2, group='agent')
show_mock.return_value = {
'size': 9 * 1024 * 1024,
'disk_format': 'qcow2',
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
self.assertRaises(exception.InvalidParameterValue,
agent.check_image_size,
task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_raw_stream_enabled(self, show_mock):
CONF.set_override('stream_raw_images', True, 'agent')
# Image is bigger than memory but it's raw and will be streamed
# so the test should pass
show_mock.return_value = {
'size': 15 * 1024 * 1024,
'disk_format': 'raw',
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
agent.check_image_size(task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_raw_stream_enabled_format_raw(self, show_mock):
CONF.set_override('stream_raw_images', True, 'agent')
# Image is bigger than memory but it's raw and will be streamed
# so the test should pass
show_mock.return_value = {
'size': 15 * 1024 * 1024,
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
task.node.instance_info['image_disk_format'] = 'raw'
agent.check_image_size(task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_raw_stream_enabled_format_qcow2(self, show_mock):
CONF.set_override('stream_raw_images', True, 'agent')
# Image is bigger than memory and won't be streamed
show_mock.return_value = {
'size': 15 * 1024 * 1024,
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
task.node.instance_info['image_disk_format'] = 'qcow2'
self.assertRaises(exception.InvalidParameterValue,
agent.check_image_size,
task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(images, 'image_show', autospec=True)
def test_check_image_size_raw_stream_disabled(self, show_mock):
CONF.set_override('stream_raw_images', False, 'agent')
show_mock.return_value = {
'size': 15 * 1024 * 1024,
'disk_format': 'raw',
}
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['memory_mb'] = 10
task.node.instance_info['image_source'] = 'fake-image'
# Image is raw but stream is disabled, so test should fail since
# the image is bigger than the RAM size
self.assertRaises(exception.InvalidParameterValue,
agent.check_image_size,
task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(deploy_utils, 'check_for_missing_params', autospec=True)
def test_validate_http_provisioning_http_image(self, utils_mock):
i_info = self.node.instance_info
@ -975,8 +855,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
self.context, self.node['uuid'], shared=False) as task:
self.assertEqual(0, len(task.volume_targets))
@mock.patch('ironic.drivers.modules.agent.check_image_size',
autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'attach_volumes',
autospec=True)
@mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info',
@ -997,7 +875,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
unconfigure_tenant_net_mock, add_provisioning_net_mock,
build_instance_info_mock, build_options_mock,
pxe_prepare_ramdisk_mock, storage_driver_info_mock,
storage_attach_volumes_mock, check_image_size_mock):
storage_attach_volumes_mock):
node = self.node
node.network_interface = 'flat'
node.save()
@ -1017,12 +895,9 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
build_options_mock.assert_called_once_with(task.node)
pxe_prepare_ramdisk_mock.assert_called_once_with(
task.driver.boot, task, {'a': 'b'})
check_image_size_mock.assert_called_once_with(task)
self.node.refresh()
self.assertEqual('bar', self.node.instance_info['foo'])
@mock.patch('ironic.drivers.modules.agent.check_image_size',
autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'attach_volumes',
autospec=True)
@mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info',
@ -1044,7 +919,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
unconfigure_tenant_net_mock, add_provisioning_net_mock,
build_instance_info_mock, build_options_mock,
pxe_prepare_ramdisk_mock, storage_driver_info_mock,
storage_attach_volumes_mock, check_image_size_mock):
storage_attach_volumes_mock):
node = self.node
node.network_interface = 'neutron'
node.save()
@ -1064,12 +939,9 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
build_options_mock.assert_called_once_with(task.node)
pxe_prepare_ramdisk_mock.assert_called_once_with(
task.driver.boot, task, {'a': 'b'})
check_image_size_mock.assert_called_once_with(task)
self.node.refresh()
self.assertEqual('bar', self.node.instance_info['foo'])
@mock.patch('ironic.drivers.modules.agent.check_image_size',
autospec=True)
@mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network',
spec_set=True, autospec=True)
@mock.patch.object(flat_network.FlatNetwork, 'validate',
@ -1081,8 +953,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
def test_prepare_manage_agent_boot_false(
self, build_instance_info_mock,
build_options_mock, pxe_prepare_ramdisk_mock,
validate_net_mock, add_provisioning_net_mock,
check_image_size_mock):
validate_net_mock, add_provisioning_net_mock):
self.config(group='agent', manage_agent_boot=False)
node = self.node
node.network_interface = 'flat'
@ -1097,7 +968,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
validate_net_mock.assert_called_once_with(mock.ANY, task)
build_instance_info_mock.assert_called_once_with(task)
add_provisioning_net_mock.assert_called_once_with(mock.ANY, task)
check_image_size_mock.assert_called_once_with(task)
self.assertFalse(build_options_mock.called)
self.assertFalse(pxe_prepare_ramdisk_mock.called)
@ -1262,8 +1132,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
build_options_mock.assert_not_called()
pxe_prepare_ramdisk_mock.assert_not_called()
@mock.patch('ironic.drivers.modules.agent.check_image_size',
autospec=True)
@mock.patch('ironic.conductor.utils.is_fast_track', autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'attach_volumes',
autospec=True)
@ -1285,8 +1153,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
unconfigure_tenant_net_mock, add_provisioning_net_mock,
build_instance_info_mock, build_options_mock,
pxe_prepare_ramdisk_mock, storage_driver_info_mock,
storage_attach_volumes_mock, is_fast_track_mock,
check_image_size_mock):
storage_attach_volumes_mock, is_fast_track_mock):
# TODO(TheJulia): We should revisit this test. Smartnic
# support didn't wire in tightly on testing for power in
# these tests, and largely fast_track impacts power operations.
@ -1303,7 +1170,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
validate_net_mock.assert_called_once_with(mock.ANY, task)
add_provisioning_net_mock.assert_called_once_with(mock.ANY, task)
unconfigure_tenant_net_mock.assert_called_once_with(mock.ANY, task)
check_image_size_mock.assert_called_once_with(task)
self.assertTrue(storage_attach_volumes_mock.called)
self.assertTrue(build_instance_info_mock.called)
# TODO(TheJulia): We should likely consider executing the
@ -1870,8 +1736,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
self.assertIsNone(driver_return)
self.assertTrue(mock_pxe_instance.called)
@mock.patch('ironic.drivers.modules.agent.check_image_size',
autospec=True)
@mock.patch.object(manager_utils, 'restore_power_state_if_needed',
autospec=True)
@mock.patch.object(manager_utils, 'power_on_node_if_needed',
@ -1898,7 +1762,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
build_instance_info_mock, build_options_mock,
pxe_prepare_ramdisk_mock, storage_driver_info_mock,
storage_attach_volumes_mock, power_on_node_if_needed_mock,
restore_power_state_mock, check_image_size_mock):
restore_power_state_mock):
node = self.node
node.network_interface = 'flat'
node.save()
@ -1923,7 +1787,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase):
power_on_node_if_needed_mock.assert_called_once_with(task)
restore_power_state_mock.assert_called_once_with(
task, states.POWER_OFF)
check_image_size_mock.assert_called_once_with(task)
self.node.refresh()
self.assertEqual('bar', self.node.instance_info['foo'])

View file

@ -1261,6 +1261,123 @@ class AgentMethodsTestCase(db_base.DbTestCase):
utils.direct_deploy_should_convert_raw_image(self.node))
class CheckImageSizeTestCase(db_base.DbTestCase):
"""Tests for check_image_size function."""
def setUp(self):
super().setUp()
self.node = obj_utils.create_test_node(
self.context, boot_interface='pxe',
instance_info=INST_INFO_DICT,
driver_info=DRV_INFO_DICT,
driver_internal_info=DRV_INTERNAL_INFO_DICT,
)
def test_check_image_size(self):
image_info = {
'size': 10 * 1024 * 1024,
'disk_format': 'qcow2',
}
self.node.properties['memory_mb'] = 20
utils.check_image_size(self.node, image_info)
def test_check_image_size_without_memory_mb(self):
image_info = {
'size': 10 * 1024 * 1024,
'disk_format': 'qcow2',
}
self.node.properties.pop('memory_mb', None)
# Should not raise any exception
utils.check_image_size(self.node, image_info)
def test_check_image_size_fail(self):
"""Test check fails when image is larger than available memory."""
cfg.CONF.set_override('force_raw_images', False)
image_info = {
'size': 11 * 1024 * 1024, # 11 MiB
'disk_format': 'qcow2',
}
self.node.properties['memory_mb'] = 10
self.assertRaises(exception.InvalidParameterValue,
utils.check_image_size,
self.node, image_info)
def test_check_image_size_fail_by_agent_consumed_memory(self):
"""Test check fails when accounting for agent memory consumption."""
cfg.CONF.set_override('force_raw_images', False)
cfg.CONF.set_override('memory_consumed_by_agent', 2, group='agent')
image_info = {
'size': 9 * 1024 * 1024, # 9 MiB
'disk_format': 'qcow2',
}
self.node.properties['memory_mb'] = 10
# 9 MiB image + 2 MiB agent > 10 MiB memory
self.assertRaises(exception.InvalidParameterValue,
utils.check_image_size,
self.node, image_info)
def test_check_image_size_raw_stream_enabled(self):
"""Test check is skipped for raw images when streaming is enabled."""
cfg.CONF.set_override('stream_raw_images', True, group='agent')
image_info = {
'size': 15 * 1024 * 1024, # 15 MiB - larger than memory
'disk_format': 'raw',
}
self.node.properties['memory_mb'] = 10
# Should not raise - raw image will be streamed
utils.check_image_size(self.node, image_info)
def test_check_image_size_raw_stream_enabled_format_raw(self):
"""Test check skipped when instance_info specifies raw format."""
cfg.CONF.set_override('stream_raw_images', True, group='agent')
image_info = {
'size': 15 * 1024 * 1024,
}
self.node.properties['memory_mb'] = 10
self.node.instance_info['image_disk_format'] = 'raw'
# Should not raise - raw image will be streamed
utils.check_image_size(self.node, image_info)
def test_check_image_size_raw_stream_enabled_format_qcow2(self):
"""Test check fails for qcow2 image even with streaming enabled."""
cfg.CONF.set_override('force_raw_images', False)
cfg.CONF.set_override('stream_raw_images', True, group='agent')
image_info = {
'size': 15 * 1024 * 1024,
}
self.node.properties['memory_mb'] = 10
self.node.instance_info['image_disk_format'] = 'qcow2'
# Should raise - qcow2 won't be streamed
self.assertRaises(exception.InvalidParameterValue,
utils.check_image_size,
self.node, image_info)
def test_check_image_size_raw_stream_disabled(self):
"""Test check fails for raw images when streaming is disabled."""
cfg.CONF.set_override('stream_raw_images', False, group='agent')
image_info = {
'size': 15 * 1024 * 1024,
'disk_format': 'raw',
}
self.node.properties['memory_mb'] = 10
# Should raise - streaming disabled, image too large
self.assertRaises(exception.InvalidParameterValue,
utils.check_image_size,
self.node, image_info)
def test_check_image_size_conductor_converts_raw(self):
"""Test check is skipped when conductor converts image to raw."""
cfg.CONF.set_override('force_raw_images', True)
cfg.CONF.set_override('stream_raw_images', True, group='agent')
image_info = {
'size': 15 * 1024 * 1024, # 15 MiB - larger than memory
'disk_format': 'qcow2',
}
self.node.properties['memory_mb'] = 10
# When conductor converts to raw, agent will stream it
utils.check_image_size(self.node, image_info)
class ValidateImagePropertiesTestCase(db_base.DbTestCase):
def setUp(self):
@ -2723,7 +2840,8 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
self.image_info = {'checksum': 'aa', 'disk_format': 'qcow2',
'os_hash_algo': 'sha512',
'os_hash_value': 'fake-sha512',
'container_format': 'bare', 'properties': {}}
'container_format': 'bare', 'properties': {},
'size': 1024 * 1024 * 1024}
@mock.patch.object(image_service.HttpImageService, 'validate_href',
autospec=True)

View file

@ -0,0 +1,6 @@
---
fixes:
- |
Moves the ``check_image_size`` to ``deploy_utils`` so it runs earlier in
the deploy flow and only when it matters, reusing already-fetched image
information.