From 7f98416fdba59c4305f88a30a457bbf6c68ef372 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Thu, 31 Mar 2011 09:20:47 -0400 Subject: Limit image metadata to the configured metadata quota for a project. --- nova/api/openstack/image_metadata.py | 12 ++++++++ nova/tests/api/openstack/test_image_metadata.py | 39 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+) (limited to 'nova') diff --git a/nova/api/openstack/image_metadata.py b/nova/api/openstack/image_metadata.py index c9d6ac532..76abdaa7e 100644 --- a/nova/api/openstack/image_metadata.py +++ b/nova/api/openstack/image_metadata.py @@ -18,6 +18,7 @@ from webob import exc from nova import flags +from nova import quota from nova import utils from nova import wsgi from nova.api.openstack import faults @@ -39,6 +40,15 @@ class Controller(wsgi.Controller): metadata = image.get('properties', {}) return metadata + def _check_quota_limit(self, context, metadata): + if metadata is None: + return + num_metadata = len(metadata) + quota_metadata = quota.allowed_metadata_items(context, num_metadata) + if quota_metadata < num_metadata: + expl = ("Image metadata limit exceeded") + raise exc.HTTPBadRequest(explanation=expl) + def index(self, req, image_id): """Returns the list of metadata for a given instance""" context = req.environ['nova.context'] @@ -61,6 +71,7 @@ class Controller(wsgi.Controller): if 'metadata' in body: for key, value in body['metadata'].iteritems(): metadata[key] = value + self._check_quota_limit(context, metadata) img['properties'] = metadata self.image_service.update(context, image_id, img, None) return dict(metadata=metadata) @@ -77,6 +88,7 @@ class Controller(wsgi.Controller): img = self.image_service.show(context, image_id) metadata = self._get_metadata(context, image_id, img) metadata[id] = body[id] + self._check_quota_limit(context, metadata) img['properties'] = metadata self.image_service.update(context, image_id, img, None) diff --git a/nova/tests/api/openstack/test_image_metadata.py b/nova/tests/api/openstack/test_image_metadata.py index 9be753f84..51f64680b 100644 --- a/nova/tests/api/openstack/test_image_metadata.py +++ b/nova/tests/api/openstack/test_image_metadata.py @@ -67,6 +67,19 @@ class ImageMetaDataTest(unittest.TestCase): 'key2': 'value2' }, 'size': 5882349}, + {'status': 'active', + 'name': 'image3', + 'deleted': False, + 'container_format': None, + 'created_at': '2011-03-22T17:40:15', + 'disk_format': None, + 'updated_at': '2011-03-22T17:40:15', + 'id': '3', + 'location': 'file:///var/lib/glance/images/2', + 'is_public': True, + 'deleted_at': None, + 'properties': {}, + 'size': 5882349}, ] def setUp(self): @@ -77,6 +90,10 @@ class ImageMetaDataTest(unittest.TestCase): fakes.FakeAuthManager.auth_data = {} fakes.FakeAuthDatabase.data = {} fakes.stub_out_auth(self.stubs) + # NOTE(dprince) max out properties/metadata in image 3 for testing + img3 = self.IMAGE_FIXTURES[2] + for num in range(FLAGS.quota_metadata_items): + img3['properties']['key%i' % num] = "blah" fakes.stub_out_glance(self.stubs, self.IMAGE_FIXTURES) def tearDown(self): @@ -164,3 +181,25 @@ class ImageMetaDataTest(unittest.TestCase): req.method = 'DELETE' res = req.get_response(fakes.wsgi_app()) self.assertEqual(404, res.status_int) + + def test_too_many_metadata_items_on_create(self): + data = {"metadata": {}} + for num in range(FLAGS.quota_metadata_items + 1): + data['metadata']['key%i' % num] = "blah" + json_string = str(data).replace("\'", "\"") + req = webob.Request.blank('/v1.1/images/2/meta') + req.environ['api.version'] = '1.1' + req.method = 'POST' + req.body = json_string + req.headers["content-type"] = "application/json" + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, res.status_int) + + def test_too_many_metadata_items_on_put(self): + req = webob.Request.blank('/v1.1/images/3/meta/blah') + req.environ['api.version'] = '1.1' + req.method = 'PUT' + req.body = '{"blah": "blah"}' + req.headers["content-type"] = "application/json" + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, res.status_int) -- cgit From 5c67809e4b9a1546c48316ea52676dfeba8f1a75 Mon Sep 17 00:00:00 2001 From: Ken Pepple Date: Mon, 4 Apr 2011 15:58:41 -0700 Subject: openstack api requires uppercase image format status responses --- nova/api/openstack/views/images.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'nova') diff --git a/nova/api/openstack/views/images.py b/nova/api/openstack/views/images.py index 16195b050..9dec8a355 100644 --- a/nova/api/openstack/views/images.py +++ b/nova/api/openstack/views/images.py @@ -34,11 +34,11 @@ class ViewBuilder(object): def _format_status(self, image): """Update the status field to standardize format.""" status_mapping = { - 'pending': 'queued', - 'decrypting': 'preparing', - 'untarring': 'saving', - 'available': 'active', - 'killed': 'failed', + 'pending': 'QUEUED', + 'decrypting': 'PREPARING', + 'untarring': 'SAVING', + 'available': 'ACTIVE', + 'killed': 'FAILED', } try: -- cgit From a6c283f00d67a5172d8271d7e48bd58484ac6b96 Mon Sep 17 00:00:00 2001 From: Ken Pepple Date: Mon, 4 Apr 2011 16:00:59 -0700 Subject: openstack api metadata responses must be strings --- nova/api/openstack/views/servers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index d24c025be..baa911590 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -82,7 +82,8 @@ class ViewBuilder(object): # Return the metadata as a dictionary metadata = {} for item in inst.get('metadata', []): - metadata[item['key']] = item['value'] + # metadata values must be strings per API + metadata[item['key']] = str(item['value']) inst_dict['metadata'] = metadata inst_dict['hostId'] = '' -- cgit From 868288b9705ea765b5515fd0ac9e0858f37239d0 Mon Sep 17 00:00:00 2001 From: Ken Pepple Date: Mon, 4 Apr 2011 16:07:39 -0700 Subject: correct test for numeric/string metadata value conversion --- nova/tests/api/openstack/test_servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 313676e72..08a3f5d5a 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -630,7 +630,7 @@ class ServersTest(test.TestCase): self.assertEqual(s['imageId'], '10') self.assertEqual(s['flavorId'], '1') self.assertEqual(s['status'], 'BUILD') - self.assertEqual(s['metadata']['seq'], i) + self.assertEqual(s['metadata']['seq'], str(i)) def test_get_all_server_details_v1_1(self): req = webob.Request.blank('/v1.1/servers/detail') @@ -644,7 +644,7 @@ class ServersTest(test.TestCase): self.assertEqual(s['imageRef'], 'http://localhost/v1.1/images/10') self.assertEqual(s['flavorRef'], 'http://localhost/v1.1/flavors/1') self.assertEqual(s['status'], 'BUILD') - self.assertEqual(s['metadata']['seq'], i) + self.assertEqual(s['metadata']['seq'], str(i)) def test_get_all_server_details_with_host(self): ''' -- cgit From ca2ce6dc4a96c7bded2c30c258b7becaee5a1ed7 Mon Sep 17 00:00:00 2001 From: Kei Masumoto Date: Tue, 5 Apr 2011 20:39:22 +0900 Subject: fix bug lp751242 --- nova/compute/api.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'nova') diff --git a/nova/compute/api.py b/nova/compute/api.py index 996955fe3..f67aa47e8 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -372,6 +372,10 @@ class API(base.Base): instance_id) return + if (instance['state_description'] == 'migrating'): + LOG.warning(_("Instance %s is being migrated"), instance_id) + return + self.update(context, instance['id'], state_description='terminating', -- cgit From d8ab404fd74facd181eaaee7cbc78b39a82afd1c Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 5 Apr 2011 08:26:16 -0400 Subject: Add missing underscore. --- nova/api/openstack/image_metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/api/openstack/image_metadata.py b/nova/api/openstack/image_metadata.py index 76abdaa7e..c733172ae 100644 --- a/nova/api/openstack/image_metadata.py +++ b/nova/api/openstack/image_metadata.py @@ -46,7 +46,7 @@ class Controller(wsgi.Controller): num_metadata = len(metadata) quota_metadata = quota.allowed_metadata_items(context, num_metadata) if quota_metadata < num_metadata: - expl = ("Image metadata limit exceeded") + expl = _("Image metadata limit exceeded") raise exc.HTTPBadRequest(explanation=expl) def index(self, req, image_id): -- cgit From 44eefb1eefda5e42a286ee1aa689c1c93e72aae4 Mon Sep 17 00:00:00 2001 From: Kei Masumoto Date: Wed, 6 Apr 2011 14:44:19 +0900 Subject: fixed based on reviwer's comment --- nova/compute/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/compute/api.py b/nova/compute/api.py index f67aa47e8..db04143c1 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -367,12 +367,12 @@ class API(base.Base): instance_id) raise - if (instance['state_description'] == 'terminating'): + if instance['state_description'] == 'terminating': LOG.warning(_("Instance %s is already being terminated"), instance_id) return - if (instance['state_description'] == 'migrating'): + if instance['state_description'] == 'migrating': LOG.warning(_("Instance %s is being migrated"), instance_id) return -- cgit From dbeab47bbdeceab0bef896c9d7646ae346c9dd3a Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Wed, 6 Apr 2011 09:05:12 -0700 Subject: Wait for device node to be created after mounting image VDI --- nova/virt/xenapi/vm_utils.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'nova') diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index d07d60800..d29456ec6 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -49,6 +49,8 @@ LOG = logging.getLogger("nova.virt.xenapi.vm_utils") FLAGS = flags.FLAGS flags.DEFINE_string('default_os_type', 'linux', 'Default OS type') +flags.DEFINE_integer('timeout_block', 10, + 'time to wait for a block device to be created') XENAPI_POWER_STATE = { 'Halted': power_state.SHUTDOWN, @@ -896,6 +898,16 @@ def remap_vbd_dev(dev): return remapped_dev +def _wait_for_device(dev): + """Wait for device node to appear""" + for i in xrange(0, FLAGS.timeout_block): + if os.path.exists('/dev/%s' % dev): + return + time.sleep(1) + + raise StorageError(_('Timeout waiting for device %s to be created') % dev) + + def with_vdi_attached_here(session, vdi_ref, read_only, f): this_vm_ref = get_this_vm_ref(session) vbd_rec = {} @@ -924,6 +936,7 @@ def with_vdi_attached_here(session, vdi_ref, read_only, f): if dev != orig_dev: LOG.debug(_('VBD %(vbd_ref)s plugged into wrong dev, ' 'remapping to %(dev)s') % locals()) + _wait_for_device(dev) return f(dev) finally: LOG.debug(_('Destroying VBD for VDI %s ... '), vdi_ref) -- cgit From c649c8d5febab7d0dfa329bc5d78f0147383c5ee Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Wed, 6 Apr 2011 11:36:08 -0700 Subject: Use a more descriptive name for the flag to make it easier to understand the purpose --- nova/virt/xenapi/vm_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index d29456ec6..cf2adb44c 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -49,7 +49,7 @@ LOG = logging.getLogger("nova.virt.xenapi.vm_utils") FLAGS = flags.FLAGS flags.DEFINE_string('default_os_type', 'linux', 'Default OS type') -flags.DEFINE_integer('timeout_block', 10, +flags.DEFINE_integer('block_device_creation_timeout', 10, 'time to wait for a block device to be created') XENAPI_POWER_STATE = { @@ -900,7 +900,7 @@ def remap_vbd_dev(dev): def _wait_for_device(dev): """Wait for device node to appear""" - for i in xrange(0, FLAGS.timeout_block): + for i in xrange(0, FLAGS.block_device_creation_timeout): if os.path.exists('/dev/%s' % dev): return time.sleep(1) -- cgit From dc8b3cb3fa124755ed3e5282b2d11a811f1db2d5 Mon Sep 17 00:00:00 2001 From: Ken Pepple Date: Wed, 6 Apr 2011 15:39:44 -0700 Subject: removed comment on API compliance --- nova/api/openstack/views/servers.py | 1 - 1 file changed, 1 deletion(-) (limited to 'nova') diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index baa911590..096bf70c6 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -82,7 +82,6 @@ class ViewBuilder(object): # Return the metadata as a dictionary metadata = {} for item in inst.get('metadata', []): - # metadata values must be strings per API metadata[item['key']] = str(item['value']) inst_dict['metadata'] = metadata -- cgit From 479c95b8e855c5f07b75883a1f55b4657e886a92 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Thu, 7 Apr 2011 21:44:41 -0400 Subject: Ignore errors when deleting the default route in the ensure_bridge function. --- nova/network/linux_net.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index ed6c943c7..ec5579dee 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -503,7 +503,7 @@ def ensure_bridge(bridge, interface, net_attrs=None): if fields and fields[0] == "0.0.0.0" and fields[-1] == interface: gateway = fields[1] _execute('sudo', 'route', 'del', 'default', 'gw', gateway, - 'dev', interface) + 'dev', interface, check_exit_code=False) out, err = _execute('sudo', 'ip', 'addr', 'show', 'dev', interface, 'scope', 'global') for line in out.split("\n"): -- cgit From 7badb6c0278c8cc51fc3e870fd3810ea3706f494 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Thu, 7 Apr 2011 23:39:29 -0400 Subject: Update the describe_image_attribute and modify_image_attribute functions in the ec2 API so they use the top level 'is_public' attribute of image objects. This brings these functions in line with the base image service. Added missing EC2 unit tests on describing and modifying image attributes. --- nova/api/ec2/cloud.py | 4 ++-- nova/tests/test_cloud.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 4ed8a9ecf..651ec47f9 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -984,7 +984,7 @@ class CloudController(object): except exception.NotFound: raise exception.NotFound(_('Image %s not found') % image_id) result = {'imageId': image_id, 'launchPermission': []} - if image['properties']['is_public']: + if image['is_public']: result['launchPermission'].append({'group': 'all'}) return result @@ -1009,7 +1009,7 @@ class CloudController(object): internal_id = image['id'] del(image['id']) - image['properties']['is_public'] = (operation_type == 'add') + image['is_public'] = (operation_type == 'add') return self.image_service.update(context, internal_id, image) def update_image(self, context, image_id, **kwargs): diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 5cb969979..5f76a9005 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -247,6 +247,37 @@ class CloudTestCase(test.TestCase): self.assertRaises(NotFound, describe_images, self.context, ['ami-fake']) + def test_describe_image_attribute(self): + describe_image_attribute = self.cloud.describe_image_attribute + + def fake_show(meh, context, id): + return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, + 'type': 'machine'}, 'is_public': True} + + self.stubs.Set(local.LocalImageService, 'show', fake_show) + self.stubs.Set(local.LocalImageService, 'show_by_name', fake_show) + result = describe_image_attribute(self.context, 'ami-00000001', + 'launchPermission') + self.assertEqual([{'group': 'all'}], result['launchPermission']) + + def test_modify_image_attribute(self): + modify_image_attribute = self.cloud.modify_image_attribute + + def fake_show(meh, context, id): + return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1, + 'type': 'machine'}, 'is_public': False} + + def fake_update(meh, context, image_id, metadata, data=None): + return metadata + + self.stubs.Set(local.LocalImageService, 'show', fake_show) + self.stubs.Set(local.LocalImageService, 'show_by_name', fake_show) + self.stubs.Set(local.LocalImageService, 'update', fake_update) + result = modify_image_attribute(self.context, 'ami-00000001', + 'launchPermission', 'add', + user_group=['all']) + self.assertEqual(True, result['is_public']) + def test_console_output(self): instance_type = FLAGS.default_instance_type max_count = 1 -- cgit From 5deb4796bc26d98eeea94065c5098f7ce30ac2af Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Fri, 8 Apr 2011 11:21:36 -0700 Subject: Short circuit non-existant device during unit tests. It won't ever be created because of the stubs used during the unit tests --- nova/virt/xenapi/vm_utils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 73a1e2a3a..d2045a557 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -936,7 +936,11 @@ def with_vdi_attached_here(session, vdi_ref, read_only, f): if dev != orig_dev: LOG.debug(_('VBD %(vbd_ref)s plugged into wrong dev, ' 'remapping to %(dev)s') % locals()) - _wait_for_device(dev) + if dev != 'autodetect': + # NOTE(johannes): Unit tests will end up with a device called + # 'autodetect' which obviously won't exist. It's not ideal, + # but the alternatives were much messier + _wait_for_device(dev) return f(dev) finally: LOG.debug(_('Destroying VBD for VDI %s ... '), vdi_ref) -- cgit