Merge "Defer checking image size until instance info is built"

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'])