From a715e3dcf94af4cd94ba5a51b745d411bc473182 Mon Sep 17 00:00:00 2001 From: Zhenguo Niu Date: Fri, 21 Aug 2015 01:11:12 +0800 Subject: [PATCH] Check image size before provisioning for agent driver agent will download the image from glance into in-memory-folder first then run 'qemu-img' to 'copy' image to local harddisk, but if the RAM size is less than the (image size + reserved RAM size), will break the IPA deployments. Change-Id: I5ed62a8936e996ffed60c811bd01d3be97f95547 Closes-Bug: #1471081 --- etc/ironic/ironic.conf.sample | 7 ++++ ironic/drivers/modules/agent.py | 47 +++++++++++++++++++++- ironic/tests/drivers/test_agent.py | 62 ++++++++++++++++++++++++++++-- 3 files changed, 111 insertions(+), 5 deletions(-) diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index a60675de7d..c59128878d 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -379,6 +379,13 @@ # Deprecated group/name - [agent]/manage_tftp #manage_agent_boot=true +# The memory size in MiB consumed by agent when it is booted on a +# bare metal node. This is used for checking if the image can +# be downloaded and deployed on the bare metal node after booting +# agent ramdisk. This may be set according to the memory +# consumed by the agent ramdisk image. (integer value) +#memory_consumed_by_agent=0 + # # Options defined in ironic.drivers.modules.agent_base_vendor diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 7474459683..726d1e1fe8 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -17,6 +17,7 @@ import time from oslo_config import cfg from oslo_log import log from oslo_utils import excutils +from oslo_utils import units from ironic.common import dhcp_factory from ironic.common import exception @@ -24,7 +25,9 @@ from ironic.common.glance_service import service_utils from ironic.common.i18n import _ from ironic.common.i18n import _LE from ironic.common.i18n import _LI +from ironic.common.i18n import _LW from ironic.common import image_service +from ironic.common import images from ironic.common import keystone from ironic.common import paths from ironic.common import states @@ -67,6 +70,14 @@ agent_opts = [ 'ramdisk. If set to False, you will need to configure ' 'your mechanism to allow booting the agent ' 'ramdisk.')), + cfg.IntOpt('memory_consumed_by_agent', + default=0, + help=_('The memory size in MiB consumed by agent when it is ' + 'booted on a bare metal node. This is used for ' + 'checking if the image can be downloaded and deployed ' + 'on the bare metal node after booting agent ramdisk. ' + 'This may be set according to the memory consumed by ' + 'the agent ramdisk image.')), ] CONF = cfg.CONF @@ -156,6 +167,36 @@ def build_instance_info_for_deploy(task): return instance_info +def check_image_size(task, image_source): + """Check if the requested image is larger than the ram size. + + :param task: a TaskManager instance containing the node to act on. + :param image_source: href of the image. + :raises: InvalidParameterValue if size of the image is greater than + the available ram size. + """ + properties = task.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) + return + + memory_size = int(properties.get('memory_mb')) + image_size = int(images.download_size(task.context, image_source)) + 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) + + class AgentDeploy(base.DeployInterface): """Interface for deploy-related actions.""" @@ -174,7 +215,10 @@ class AgentDeploy(base.DeployInterface): the node. :param task: a TaskManager instance - :raises: MissingParameterValue + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. """ if CONF.agent.manage_agent_boot: task.driver.boot.validate(task) @@ -193,6 +237,7 @@ class AgentDeploy(base.DeployInterface): "image_source's image_checksum must be provided in " "instance_info for node %s") % node.uuid) + check_image_size(task, image_source) is_whole_disk_image = node.driver_internal_info.get( 'is_whole_disk_image') # TODO(sirushtim): Remove once IPA has support for partition images. diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index 2546d4f49f..3c5083b209 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -19,6 +19,7 @@ from oslo_config import cfg from ironic.common import exception from ironic.common import image_service +from ironic.common import images from ironic.common import keystone from ironic.common import states from ironic.conductor import task_manager @@ -137,6 +138,51 @@ 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 + 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') + + @mock.patch.object(images, 'download_size', autospec=True) + def test_check_image_size_without_memory_mb(self, size_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 + + 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 + self.assertRaises(exception.InvalidParameterValue, + agent.check_image_size, + task, 'fake-image') + size_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): + self.config(memory_consumed_by_agent=2, group='agent') + size_mock.return_value = 9 * 1024 * 1024 + 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 + self.assertRaises(exception.InvalidParameterValue, + agent.check_image_size, + task, 'fake-image') + size_mock.assert_called_once_with(self.context, 'fake-image') + class TestAgentDeploy(db_base.DbTestCase): def setUp(self): @@ -157,17 +203,20 @@ class TestAgentDeploy(db_base.DbTestCase): expected = agent.COMMON_PROPERTIES self.assertEqual(expected, self.driver.get_properties()) + @mock.patch.object(images, 'download_size', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) - def test_validate(self, pxe_boot_validate_mock): + def test_validate(self, pxe_boot_validate_mock, size_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') + @mock.patch.object(images, 'download_size', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_driver_info_manage_agent_boot_false( - self, pxe_boot_validate_mock): + self, pxe_boot_validate_mock, size_mock): self.config(manage_agent_boot=False, group='agent') self.node.driver_info = {} self.node.save() @@ -175,6 +224,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') @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_instance_info_missing_params( @@ -206,9 +256,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(pxe.PXEBoot, 'validate', autospec=True) def test_validate_agent_fail_partition_image( - self, pxe_boot_validate_mock): + self, pxe_boot_validate_mock, size_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False @@ -216,10 +267,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') + @mock.patch.object(images, 'download_size', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_invalid_root_device_hints( - self, pxe_boot_validate_mock): + self, pxe_boot_validate_mock, size_mock): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.properties['root_device'] = {'size': 'not-int'} @@ -227,6 +280,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') @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_deploy(self, power_mock):