From 1fa3101fceab27c8ac70c091bac0d9e7bcc184b4 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Mon, 20 May 2013 20:01:08 +0000 Subject: Move ImageTooLarge check to Compute API The existing ImageTooLarge check is in the compute manager, meaning that a validation error puts the instance into an ERROR state without the ability to communicate the fault back to the user. This patch moves the check to the API where we can return the appropriate error to the user. Change-Id: I2183449351e3b70f98d0552ea247184a3641d85a --- nova/tests/api/openstack/compute/test_servers.py | 58 ++++++ nova/tests/compute/test_compute.py | 216 ++++++++++++----------- 2 files changed, 175 insertions(+), 99 deletions(-) (limited to 'nova/tests') diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py index 993451263..11eeff143 100644 --- a/nova/tests/api/openstack/compute/test_servers.py +++ b/nova/tests/api/openstack/compute/test_servers.py @@ -1451,6 +1451,35 @@ class ServersControllerTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller._action_rebuild, req, FAKE_UUID, body) + def test_rebuild_instance_image_too_large(self): + # make image size larger than our instance disk size + size = str(1000 * (1024 ** 3)) + + def fake_get_image(self, context, image_href): + return dict(id='76fa36fc-c930-4bf3-8c8a-ea2a2420deb6', + name='public image', is_public=True, + status='active', size=size) + + self.stubs.Set(fake._FakeImageService, 'show', fake_get_image) + + self.stubs.Set(db, 'instance_get_by_uuid', + fakes.fake_instance_get(vm_state=vm_states.ACTIVE)) + image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' + image_href = 'http://localhost/v2/fake/images/%s' % image_uuid + body = { + 'rebuild': { + 'name': 'new_name', + 'imageRef': image_href, + }, + } + + req = fakes.HTTPRequest.blank('/v2/fake/servers/a/action') + req.method = 'POST' + req.body = jsonutils.dumps(body) + req.headers["content-type"] = "application/json" + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._action_rebuild, req, FAKE_UUID, body) + def test_rebuild_instance_with_deleted_image(self): def fake_get_image(self, context, image_href): return dict(id='76fa36fc-c930-4bf3-8c8a-ea2a2420deb6', @@ -1908,6 +1937,35 @@ class ServersControllerCreateTest(test.TestCase): 'Image 76fa36fc-c930-4bf3-8c8a-ea2a2420deb6 is not active.'): self.controller.create(req, body) + def test_create_server_image_too_large(self): + image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' + + # Get the fake image service so we can set the status to deleted + (image_service, image_id) = glance.get_remote_image_service( + context, image_uuid) + + image = image_service.show(context, image_id) + + orig_size = image['size'] + new_size = str(1000 * (1024 ** 3)) + image_service.update(context, image_uuid, {'size': new_size}) + + self.addCleanup(image_service.update, context, image_uuid, + {'size': orig_size}) + + req = fakes.HTTPRequest.blank('/v2/fake/servers') + req.method = 'POST' + body = dict(server=dict(name='server_test', + imageRef=image_uuid, + flavorRef=2)) + req.body = jsonutils.dumps(body) + + req.headers["content-type"] = "application/json" + with testtools.ExpectedException( + webob.exc.HTTPBadRequest, + 'Image is larger than instance type allows'): + self.controller.create(req, body) + def test_create_instance_invalid_negative_min(self): self.ext_mgr.extensions = {'os-multiple-create': 'fake'} image_href = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 933d40d59..285261e18 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -4888,6 +4888,26 @@ class ComputeAPITestCase(BaseTestCase): inst_type, self.fake_image['id']) db.instance_destroy(self.context, refs[0]['uuid']) + def test_create_with_too_large_image(self): + # Test an instance type with too little disk space. + + inst_type = flavors.get_default_instance_type() + inst_type['root_gb'] = 1 + + self.fake_image['size'] = '1073741825' + + self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show) + + self.assertRaises(exception.ImageTooLarge, + self.compute_api.create, self.context, + inst_type, self.fake_image['id']) + + # Reduce image to 1 GB limit and ensure it works + self.fake_image['size'] = '1073741824' + (refs, resv_id) = self.compute_api.create(self.context, + inst_type, self.fake_image['id']) + db.instance_destroy(self.context, refs[0]['uuid']) + def test_create_just_enough_ram_and_disk(self): # Test an instance type with just enough ram and disk space. @@ -5642,6 +5662,29 @@ class ComputeAPITestCase(BaseTestCase): instance, self.fake_image['id'], 'new_password') db.instance_destroy(self.context, instance['uuid']) + def test_rebuild_with_too_large_image(self): + instance = jsonutils.to_primitive( + self._create_fake_instance(params={'image_ref': '1'})) + + def fake_extract_instance_type(_inst): + return dict(memory_mb=64, root_gb=1) + + self.stubs.Set(flavors, 'extract_instance_type', + fake_extract_instance_type) + + self.fake_image['size'] = '1073741825' + self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show) + + self.assertRaises(exception.ImageTooLarge, + self.compute_api.rebuild, self.context, + instance, self.fake_image['id'], 'new_password') + + # Reduce image to 1 GB limit and ensure it works + self.fake_image['size'] = '1073741824' + self.compute_api.rebuild(self.context, + instance, self.fake_image['id'], 'new_password') + db.instance_destroy(self.context, instance['uuid']) + def _stub_out_reboot(self, device_name): def fake_reboot_instance(rpcapi, context, instance, block_device_info, @@ -8910,105 +8953,6 @@ class ComputeInjectedFilesTestCase(BaseTestCase): injected_files=expected) -class GetAndCheckImageMetadataTest(test.TestCase): - NOT_PRESENT = object() - - def setUp(self): - super(GetAndCheckImageMetadataTest, self).setUp() - self.context = context.RequestContext('fakeuser', 'fakeproject') - self.compute = importutils.import_object(CONF.compute_manager) - - def assertCheckSucceedsWithWonkySize(self, size): - instance = {'uuid': 'fakeuuid', 'image_ref': 'fakeimage'} - image_meta = {'id': 'imageid'} - if size is not self.NOT_PRESENT: - image_meta['size'] = size - - self.mox.StubOutWithMock(compute_manager, '_get_image_meta') - compute_manager._get_image_meta(self.context, - instance['image_ref']).AndReturn(image_meta) - - self.mox.ReplayAll() - - image_meta2 = self.compute._get_and_check_image_metadata( - self.context, instance) - - self.assertEqual(image_meta, image_meta2) - - def test_size_not_present_on_image(self): - self.assertCheckSucceedsWithWonkySize(self.NOT_PRESENT) - - def test_invalid_image_size(self): - self.assertCheckSucceedsWithWonkySize('invalid') - - def test_image_size_present_but_empty(self): - self.assertCheckSucceedsWithWonkySize('') - - def test_image_size_is_none(self): - self.assertCheckSucceedsWithWonkySize(None) - - def test_root_gb_zero(self): - instance = {'uuid': 'fakeuuid', 'image_ref': 'fakeimage'} - self.mox.StubOutWithMock(flavors, 'extract_instance_type') - flavors.extract_instance_type(instance).AndReturn({'root_gb': 0}) - - self.mox.StubOutWithMock(compute_manager, '_get_image_meta') - - image_meta = {'id': 'imageid', 'size': '123'} - compute_manager._get_image_meta(self.context, - instance['image_ref']).AndReturn(image_meta) - - self.mox.ReplayAll() - - image_meta2 = self.compute._get_and_check_image_metadata( - self.context, instance) - - self.assertEqual(image_meta, image_meta2) - - def test_image_ref_is_none(self): - # Instance was started from volume - so no image ref - instance = {'uuid': 'fakeuuid', 'image_ref': None} - image_meta2 = self.compute._get_and_check_image_metadata( - self.context, instance) - - self.assertEqual({}, image_meta2) - - def test_image_not_too_large(self): - instance = {'uuid': 'fakeuuid', 'image_ref': 'fakeimage'} - - self.mox.StubOutWithMock(flavors, 'extract_instance_type') - flavors.extract_instance_type(instance).AndReturn({'root_gb': 1}) - - image_meta = {'id': 'imageid', 'size': '1073741824'} - self.mox.StubOutWithMock(compute_manager, '_get_image_meta') - compute_manager._get_image_meta(self.context, - instance['image_ref']).AndReturn(image_meta) - - self.mox.ReplayAll() - - image_meta2 = self.compute._get_and_check_image_metadata( - self.context, instance) - - self.assertEqual(image_meta, image_meta2) - - def test_image_too_large(self): - instance = {'uuid': 'fakeuuid', 'image_ref': 'fakeimage'} - - self.mox.StubOutWithMock(flavors, 'extract_instance_type') - flavors.extract_instance_type(instance).AndReturn({'root_gb': 1}) - - image_meta = {'id': 'imageid', 'size': '1073741825'} - self.mox.StubOutWithMock(compute_manager, '_get_image_meta') - compute_manager._get_image_meta(self.context, - instance['image_ref']).AndReturn(image_meta) - - self.mox.ReplayAll() - - self.assertRaises(exception.ImageTooLarge, - self.compute._get_and_check_image_metadata, - self.context, instance) - - class CheckConfigDriveTestCase(test.TestCase): # NOTE(sirp): `TestCase` is far too heavyweight for this test, this should # probably derive from a `test.FastTestCase` that omits DB and env @@ -9060,3 +9004,77 @@ class CheckConfigDriveTestCase(test.TestCase): def test_value_is_image_id(self): self.assertCheck((2, None), 2) + + +class CheckRequestedImageTestCase(test.TestCase): + def setUp(self): + super(CheckRequestedImageTestCase, self).setUp() + self.compute_api = compute.API() + self.context = context.RequestContext( + 'fake_user_id', 'fake_project_id') + + self.instance_type = flavors.get_default_instance_type() + self.instance_type['memory_mb'] = 64 + self.instance_type['root_gb'] = 1 + + def test_no_image_specified(self): + self.compute_api._check_requested_image(self.context, None, {}, + self.instance_type) + + def test_image_status_must_be_active(self): + image = dict(id='123', status='foo') + + self.assertRaises(exception.ImageNotActive, + self.compute_api._check_requested_image, self.context, + image['id'], image, self.instance_type) + + image['status'] = 'active' + self.compute_api._check_requested_image(self.context, image['id'], + image, self.instance_type) + + def test_image_min_ram_check(self): + image = dict(id='123', status='active', min_ram='65') + + self.assertRaises(exception.InstanceTypeMemoryTooSmall, + self.compute_api._check_requested_image, self.context, + image['id'], image, self.instance_type) + + image['min_ram'] = '64' + self.compute_api._check_requested_image(self.context, image['id'], + image, self.instance_type) + + def test_image_min_disk_check(self): + image = dict(id='123', status='active', min_disk='2') + + self.assertRaises(exception.InstanceTypeDiskTooSmall, + self.compute_api._check_requested_image, self.context, + image['id'], image, self.instance_type) + + image['min_disk'] = '1' + self.compute_api._check_requested_image(self.context, image['id'], + image, self.instance_type) + + def test_image_too_large(self): + image = dict(id='123', status='active', size='1073741825') + + self.assertRaises(exception.ImageTooLarge, + self.compute_api._check_requested_image, self.context, + image['id'], image, self.instance_type) + + image['size'] = '1073741824' + self.compute_api._check_requested_image(self.context, image['id'], + image, self.instance_type) + + def test_root_gb_zero_disables_size_check(self): + self.instance_type['root_gb'] = 0 + image = dict(id='123', status='active', size='1073741825') + + self.compute_api._check_requested_image(self.context, image['id'], + image, self.instance_type) + + def test_root_gb_zero_disables_min_disk(self): + self.instance_type['root_gb'] = 0 + image = dict(id='123', status='active', min_disk='2') + + self.compute_api._check_requested_image(self.context, image['id'], + image, self.instance_type) -- cgit