diff options
author | Zuul <zuul@review.opendev.org> | 2021-07-06 13:06:50 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2021-07-06 13:06:50 +0000 |
commit | 7dd26522b0eb3275d890d0771886c41d0d1aa7e6 (patch) | |
tree | c9bc9bb6bcd3c6aaa8a1c3a95d49c4f497e7709a | |
parent | 9defccbaea84b0e2e18f72dc9ef20a92777ca0c3 (diff) | |
parent | ad3ce797cfbd0093fcb1bafb8aaa38c39f08926a (diff) | |
download | ironic-7dd26522b0eb3275d890d0771886c41d0d1aa7e6.tar.gz |
Merge "Defer checking image size until instance info is built"
-rw-r--r-- | ironic/drivers/modules/agent.py | 10 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/test_agent.py | 96 |
2 files changed, 63 insertions, 43 deletions
diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index c38b0cba6..5f2185b65 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -93,17 +93,17 @@ PARTITION_IMAGE_LABELS = ('kernel', 'ramdisk', 'root_gb', 'root_mb', 'swap_mb', @METRICS.timer('check_image_size') -def check_image_size(task, image_source, image_disk_format=None): +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. - :param image_source: href of the image. - :param image_disk_format: The disk format of the image if provided :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.warning('Skip the image size check as memory_mb is not ' @@ -434,6 +434,7 @@ 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): @@ -464,7 +465,6 @@ class AgentDeploy(CustomAgentDeploy): params = {} image_source = node.instance_info.get('image_source') image_checksum = node.instance_info.get('image_checksum') - image_disk_format = node.instance_info.get('image_disk_format') os_hash_algo = node.instance_info.get('image_os_hash_algo') os_hash_value = node.instance_info.get('image_os_hash_value') @@ -499,8 +499,6 @@ class AgentDeploy(CustomAgentDeploy): _raise_missing_checksum_exception(node) validate_http_provisioning_configuration(node) - - check_image_size(task, image_source, image_disk_format) validate_image_proxies(node) @METRICS.timer('AgentDeployMixin.write_image') diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 8cc3ef809..586bdc07f 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -64,7 +64,8 @@ class TestAgentMethods(db_base.DbTestCase): 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') + 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) @@ -72,7 +73,8 @@ class TestAgentMethods(db_base.DbTestCase): 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') + 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) @@ -84,9 +86,10 @@ class TestAgentMethods(db_base.DbTestCase): 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, 'fake-image') + task) show_mock.assert_called_once_with(self.context, 'fake-image') @mock.patch.object(images, 'image_show', autospec=True) @@ -99,9 +102,10 @@ class TestAgentMethods(db_base.DbTestCase): 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, 'fake-image') + task) show_mock.assert_called_once_with(self.context, 'fake-image') @mock.patch.object(images, 'image_show', autospec=True) @@ -116,7 +120,8 @@ class TestAgentMethods(db_base.DbTestCase): 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') + 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) @@ -130,7 +135,9 @@ class TestAgentMethods(db_base.DbTestCase): 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', 'raw') + 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) @@ -143,9 +150,11 @@ class TestAgentMethods(db_base.DbTestCase): 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, 'fake-image', 'qcow2') + task) show_mock.assert_called_once_with(self.context, 'fake-image') @mock.patch.object(images, 'image_show', autospec=True) @@ -158,11 +167,12 @@ class TestAgentMethods(db_base.DbTestCase): 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, 'fake-image') + task) show_mock.assert_called_once_with(self.context, 'fake-image') @mock.patch.object(deploy_utils, 'check_for_missing_params', autospec=True) @@ -492,16 +502,14 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): autospec=True) @mock.patch.object(deploy_utils, 'validate_capabilities', spec_set=True, 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, show_mock, + def test_validate(self, pxe_boot_validate_mock, validate_capability_mock, validate_http_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) - show_mock.assert_called_once_with(self.context, 'fake-image') validate_capability_mock.assert_called_once_with(task.node) validate_http_mock.assert_called_once_with(task.node) @@ -509,11 +517,10 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): autospec=True) @mock.patch.object(deploy_utils, 'validate_capabilities', spec_set=True, 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, show_mock, - validate_capability_mock, validate_http_mock): + self, pxe_boot_validate_mock, validate_capability_mock, + validate_http_mock): self.config(manage_agent_boot=False, group='agent') self.node.driver_info = {} @@ -522,7 +529,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): self.context, self.node['uuid'], shared=False) as task: self.driver.validate(task) self.assertFalse(pxe_boot_validate_mock.called) - show_mock.assert_called_once_with(self.context, 'fake-image') validate_capability_mock.assert_called_once_with(task.node) validate_http_mock.assert_called_once_with(task.node) @@ -605,10 +611,9 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - @mock.patch.object(images, 'image_show', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_nonglance_image_no_os_checksum( - self, pxe_boot_validate_mock, show_mock): + self, pxe_boot_validate_mock): i_info = self.node.instance_info i_info['image_source'] = 'http://image-ref' del i_info['image_checksum'] @@ -622,14 +627,10 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): self.driver.validate(task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - show_mock.assert_called_once_with(self.context, - 'http://image-ref') - @mock.patch.object(image_service.FileImageService, 'validate_href', - autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) def test_validate_file_image_no_checksum( - self, pxe_boot_validate_mock, validate_mock): + self, pxe_boot_validate_mock): i_info = self.node.instance_info i_info['image_source'] = 'file://image-ref' del i_info['image_checksum'] @@ -641,14 +642,12 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): self.driver.validate(task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - validate_mock.assert_called_once_with(mock.ANY, 'file://image-ref') @mock.patch.object(agent, 'validate_http_provisioning_configuration', 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, show_mock, validate_http_mock): + self, pxe_boot_validate_mock, validate_http_mock): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.properties['root_device'] = {'size': 'not-int'} @@ -656,7 +655,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): task.driver.deploy.validate, task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - show_mock.assert_not_called() validate_http_mock.assert_not_called() @mock.patch.object(agent, 'validate_http_provisioning_configuration', @@ -678,9 +676,8 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): @mock.patch.object(agent, 'validate_http_provisioning_configuration', autospec=True) - @mock.patch.object(images, 'image_show', autospec=True) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) - def test_validate_invalid_proxies(self, pxe_boot_validate_mock, show_mock, + def test_validate_invalid_proxies(self, pxe_boot_validate_mock, validate_http_mock): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -693,7 +690,6 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): task.driver.deploy.validate, task) pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) - show_mock.assert_called_once_with(self.context, 'fake-image') validate_http_mock.assert_called_once_with(task.node) @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) @@ -748,6 +744,8 @@ 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', @@ -768,7 +766,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): + storage_attach_volumes_mock, check_image_size_mock): node = self.node node.network_interface = 'flat' node.save() @@ -788,9 +786,12 @@ 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', @@ -812,7 +813,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): + storage_attach_volumes_mock, check_image_size_mock): node = self.node node.network_interface = 'neutron' node.save() @@ -832,9 +833,12 @@ 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', @@ -856,7 +860,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): unconfigure_tenant_net_mock, add_provisioning_net_mock, validate_href_mock, build_options_mock, pxe_prepare_ramdisk_mock, storage_driver_info_mock, - storage_attach_volumes_mock): + storage_attach_volumes_mock, check_image_size_mock): node = self.node node.network_interface = 'neutron' instance_info = node.instance_info @@ -881,11 +885,14 @@ 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() capabilities = self.node.instance_info['capabilities'] self.assertEqual('local', capabilities['boot_option']) self.assertEqual('roar', capabilities['lion']) + @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', @@ -907,7 +914,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): unconfigure_tenant_net_mock, add_provisioning_net_mock, validate_href_mock, build_options_mock, pxe_prepare_ramdisk_mock, storage_driver_info_mock, - storage_attach_volumes_mock): + storage_attach_volumes_mock, check_image_size_mock): node = self.node node.network_interface = 'neutron' node.save() @@ -929,10 +936,13 @@ 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() capabilities = self.node.instance_info['capabilities'] self.assertEqual('local', capabilities['boot_option']) + @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', @@ -954,7 +964,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): unconfigure_tenant_net_mock, add_provisioning_net_mock, validate_href_mock, build_options_mock, pxe_prepare_ramdisk_mock, storage_driver_info_mock, - storage_attach_volumes_mock): + storage_attach_volumes_mock, check_image_size_mock): node = self.node node.network_interface = 'neutron' instance_info = node.instance_info @@ -979,6 +989,7 @@ 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() capabilities = self.node.instance_info['capabilities'] self.assertEqual('local', capabilities['boot_option']) @@ -1032,6 +1043,8 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): capabilities = self.node.instance_info['capabilities'] self.assertEqual('netboot', capabilities['boot_option']) + @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', @@ -1043,7 +1056,8 @@ 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): + validate_net_mock, add_provisioning_net_mock, + check_image_size_mock): self.config(group='agent', manage_agent_boot=False) node = self.node node.network_interface = 'flat' @@ -1058,6 +1072,7 @@ 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) @@ -1219,6 +1234,8 @@ 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) @@ -1240,7 +1257,8 @@ 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): + storage_attach_volumes_mock, is_fast_track_mock, + check_image_size_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. @@ -1257,6 +1275,7 @@ 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 @@ -1744,6 +1763,8 @@ 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', @@ -1770,7 +1791,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): + restore_power_state_mock, check_image_size_mock): node = self.node node.network_interface = 'flat' node.save() @@ -1795,6 +1816,7 @@ 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']) |