From 878e6f8e0a97a4710660768aff615d43814a6073 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 17 Nov 2015 12:19:15 +0000 Subject: [PATCH] Add switch to enable/disable streaming raw images for IPA This patch is adding a configuration option called "stream_raw_images" under the "agent" group. By default this option is set to True, meaning that the IPA ramdisk will be instructed to stream raw images directly onto the disk instead of copying it to a tmpfs partition prior of writing it to the disk. By streaming raw images IPA will consume less memory to run and the deployment will be faster (unless the disk being deployed is really slow). Depends-On: Iddf67907bc9b54bbd3065a97064cb5a3602cfe18 Related-Bug: #1505685 Change-Id: I7e7ed7e7aeebbf7c936605a72aa974db2123a43e --- etc/ironic/ironic.conf.sample | 9 ++ ironic/common/images.py | 10 +- ironic/drivers/modules/agent.py | 26 ++++- ironic/tests/unit/common/test_images.py | 16 ++- .../tests/unit/drivers/modules/test_agent.py | 102 +++++++++++++----- 5 files changed, 124 insertions(+), 39 deletions(-) diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 435b802e3b..2f35c550de 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -422,6 +422,15 @@ # memory consumed by the agent ramdisk image. (integer value) #memory_consumed_by_agent=0 +# Whether the agent ramdisk should stream raw images directly +# onto the disk or not. By streaming raw images directly onto +# the disk the agent ramdisk will not spend time copying the +# image to a tmpfs partition (therefore consuming less memory) +# prior to writing it to the disk. Unless the disk where the +# image will be copied to is really slow, this option should +# be set to True. Defaults to True. (boolean value) +#stream_raw_images=true + # # Options defined in ironic.drivers.modules.agent_base_vendor diff --git a/ironic/common/images.py b/ironic/common/images.py index 2898f0913b..06e0a73a1f 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -390,10 +390,14 @@ def image_to_raw(image_href, path, path_tmp): os.rename(path_tmp, path) -def download_size(context, image_href, image_service=None): - if not image_service: +def image_show(context, image_href, image_service=None): + if image_service is None: image_service = service.get_image_service(image_href, context=context) - return image_service.show(image_href)['size'] + return image_service.show(image_href) + + +def download_size(context, image_href, image_service=None): + return image_show(context, image_href, image_service)['size'] def converted_size(path): diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 10b1b93f3b..16c5956656 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -66,6 +66,16 @@ agent_opts = [ 'on the bare metal node after booting agent ramdisk. ' 'This may be set according to the memory consumed by ' 'the agent ramdisk image.')), + cfg.BoolOpt('stream_raw_images', + default=True, + help=_('Whether the agent ramdisk should stream raw images ' + 'directly onto the disk or not. By streaming raw ' + 'images directly onto the disk the agent ramdisk will ' + 'not spend time copying the image to a tmpfs partition ' + '(therefore consuming less memory) prior to writing it ' + 'to the disk. Unless the disk where the image will be ' + 'copied to is really slow, this option should be set ' + 'to True. Defaults to True.')), ] CONF = cfg.CONF @@ -133,15 +143,22 @@ def check_image_size(task, image_source): :raises: InvalidParameterValue if size of the image is greater than the available ram size. """ - properties = task.node.properties + node = task.node + properties = node.properties # skip check if 'memory_mb' is not defined if 'memory_mb' not in properties: LOG.warning(_LW('Skip the image size check as memory_mb is not ' - 'defined in properties on node %s.'), task.node.uuid) + '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': + 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(images.download_size(task.context, image_source)) + 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 ' @@ -377,7 +394,8 @@ class AgentVendorInterface(agent_base_vendor.BaseAgentVendor): # upgraded in the middle of a build request. 'disk_format': node.instance_info.get('image_disk_format'), 'container_format': node.instance_info.get( - 'image_container_format') + 'image_container_format'), + 'stream_raw_images': CONF.agent.stream_raw_images, } # Tell the client to download and write the image with the given args diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index 868fe3c86f..defe81e7f6 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -187,18 +187,26 @@ class IronicImagesTestCase(base.TestCase): rename_mock.assert_called_once_with('path_tmp', 'path') @mock.patch.object(image_service, 'get_image_service', autospec=True) - def test_download_size_no_image_service(self, image_service_mock): - images.download_size('context', 'image_href') + def test_image_show_no_image_service(self, image_service_mock): + images.image_show('context', 'image_href') image_service_mock.assert_called_once_with('image_href', context='context') image_service_mock.return_value.show.assert_called_once_with( 'image_href') - def test_download_size_image_service(self): + def test_image_show_image_service(self): image_service_mock = mock.MagicMock() - images.download_size('context', 'image_href', image_service_mock) + images.image_show('context', 'image_href', image_service_mock) image_service_mock.show.assert_called_once_with('image_href') + @mock.patch.object(images, 'image_show', autospec=True) + def test_download_size(self, show_mock): + show_mock.return_value = {'size': 123456} + size = images.download_size('context', 'image_href', 'image_service') + self.assertEqual(123456, size) + show_mock.assert_called_once_with('context', 'image_href', + 'image_service') + @mock.patch.object(images, 'qemu_img_info', autospec=True) def test_converted_size(self, qemu_img_info_mock): info = self.FakeImgInfo() diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 0cd518f744..cc3c8e9d08 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -117,29 +117,34 @@ class TestAgentMethods(db_base.DbTestCase): self.assertRaises(exception.ImageRefValidationFailed, agent.build_instance_info_for_deploy, task) - @mock.patch.object(images, 'download_size', autospec=True) - def test_check_image_size(self, size_mock): - size_mock.return_value = 10 * 1024 * 1024 + @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', + } mgr_utils.mock_the_extension_manager(driver='fake_agent') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.properties['memory_mb'] = 10 agent.check_image_size(task, 'fake-image') - size_mock.assert_called_once_with(self.context, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') - @mock.patch.object(images, 'download_size', autospec=True) - def test_check_image_size_without_memory_mb(self, size_mock): + @mock.patch.object(images, 'image_show', autospec=True) + def test_check_image_size_without_memory_mb(self, show_mock): mgr_utils.mock_the_extension_manager(driver='fake_agent') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.properties.pop('memory_mb', None) agent.check_image_size(task, 'fake-image') - self.assertFalse(size_mock.called) - - @mock.patch.object(images, 'download_size', autospec=True) - def test_check_image_size_fail(self, size_mock): - size_mock.return_value = 11 * 1024 * 1024 + 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', + } mgr_utils.mock_the_extension_manager(driver='fake_agent') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: @@ -147,12 +152,15 @@ class TestAgentMethods(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, agent.check_image_size, task, 'fake-image') - size_mock.assert_called_once_with(self.context, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') - @mock.patch.object(images, 'download_size', autospec=True) - def test_check_image_size_fail_by_agent_consumed_memory(self, size_mock): + @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') - size_mock.return_value = 9 * 1024 * 1024 + show_mock.return_value = { + 'size': 9 * 1024 * 1024, + 'disk_format': 'qcow2', + } mgr_utils.mock_the_extension_manager(driver='fake_agent') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: @@ -160,7 +168,41 @@ class TestAgentMethods(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, agent.check_image_size, task, 'fake-image') - size_mock.assert_called_once_with(self.context, 'fake-image') + 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', + } + mgr_utils.mock_the_extension_manager(driver='fake_agent') + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.properties['memory_mb'] = 10 + agent.check_image_size(task, 'fake-image') + 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', + } + mgr_utils.mock_the_extension_manager(driver='fake_agent') + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.properties['memory_mb'] = 10 + # 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, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') class TestAgentDeploy(db_base.DbTestCase): @@ -185,25 +227,26 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(deploy_utils, 'validate_capabilities', spec_set=True, autospec=True) - @mock.patch.object(images, 'download_size', autospec=True) + @mock.patch.object(images, 'image_show', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) - def test_validate(self, pxe_boot_validate_mock, size_mock, + def test_validate(self, pxe_boot_validate_mock, show_mock, validate_capability_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.driver.validate(task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - size_mock.assert_called_once_with(self.context, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') validate_capability_mock.assert_called_once_with(task.node) @mock.patch.object(deploy_utils, 'validate_capabilities', spec_set=True, autospec=True) - @mock.patch.object(images, 'download_size', autospec=True) + @mock.patch.object(images, 'image_show', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_driver_info_manage_agent_boot_false( - self, pxe_boot_validate_mock, size_mock, + self, pxe_boot_validate_mock, show_mock, validate_capability_mock): + self.config(manage_agent_boot=False, group='agent') self.node.driver_info = {} self.node.save() @@ -211,7 +254,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.context, self.node['uuid'], shared=False) as task: self.driver.validate(task) self.assertFalse(pxe_boot_validate_mock.called) - size_mock.assert_called_once_with(self.context, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') validate_capability_mock.assert_called_once_with(task.node) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) @@ -244,10 +287,10 @@ class TestAgentDeploy(db_base.DbTestCase): pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - @mock.patch.object(images, 'download_size', autospec=True) + @mock.patch.object(images, 'image_show', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_agent_fail_partition_image( - self, pxe_boot_validate_mock, size_mock): + self, pxe_boot_validate_mock, show_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False @@ -255,12 +298,12 @@ class TestAgentDeploy(db_base.DbTestCase): self.driver.validate, task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - size_mock.assert_called_once_with(self.context, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') - @mock.patch.object(images, 'download_size', autospec=True) + @mock.patch.object(images, 'image_show', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_invalid_root_device_hints( - self, pxe_boot_validate_mock, size_mock): + self, pxe_boot_validate_mock, show_mock): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.properties['root_device'] = {'size': 'not-int'} @@ -268,7 +311,7 @@ class TestAgentDeploy(db_base.DbTestCase): task.driver.deploy.validate, task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - size_mock.assert_called_once_with(self.context, 'fake-image') + show_mock.assert_called_once_with(self.context, 'fake-image') @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_deploy(self, power_mock): @@ -453,6 +496,7 @@ class TestAgentVendor(db_base.DbTestCase): self.node = object_utils.create_test_node(self.context, **n) def test_continue_deploy(self): + CONF.set_override('stream_raw_images', False, 'agent') self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE self.node.save() @@ -463,6 +507,7 @@ class TestAgentVendor(db_base.DbTestCase): 'checksum': 'checksum', 'disk_format': 'qcow2', 'container_format': 'bare', + 'stream_raw_images': False, } client_mock = mock.MagicMock(spec_set=['prepare_image']) @@ -489,6 +534,7 @@ class TestAgentVendor(db_base.DbTestCase): 'checksum': 'checksum', 'disk_format': 'qcow2', 'container_format': 'bare', + 'stream_raw_images': True, } client_mock = mock.MagicMock(spec_set=['prepare_image'])