From 323eb60884cf8736448d997068d32f252d22e7f3 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Mar 2011 12:05:25 -0700 Subject: Key type values in ec2_api off of container format --- nova/api/ec2/cloud.py | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 7ba8dfbea..1e6c4d1db 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -146,7 +146,7 @@ class CloudController(object): floating_ip = db.instance_get_floating_address(ctxt, instance_ref['id']) ec2_id = ec2utils.id_to_ec2_id(instance_ref['id']) - image_ec2_id = self._image_ec2_id(instance_ref['image_id'], 'machine') + image_ec2_id = self._image_ec2_id(instance_ref['image_id'], 'ami') data = { 'user-data': base64.b64decode(instance_ref['user_data']), 'meta-data': { @@ -176,7 +176,7 @@ class CloudController(object): for image_type in ['kernel', 'ramdisk']: if '%s_id' % image_type in instance_ref: ec2_id = self._image_ec2_id(instance_ref['%s_id' % image_type], - image_type) + self._image_type(image_type)) data['meta-data']['%s-id' % image_type] = ec2_id if False: # TODO(vish): store ancestor ids @@ -862,13 +862,27 @@ class CloudController(object): self.compute_api.update(context, instance_id=instance_id, **kwargs) return True - _type_prefix_map = {'machine': 'ami', - 'kernel': 'aki', - 'ramdisk': 'ari'} + @staticmethod + def _image_type(image_type): + """Converts to a three letter image type. - def _image_ec2_id(self, image_id, image_type='machine'): - prefix = self._type_prefix_map[image_type] - template = prefix + '-%08x' + aki, kernel => aki + ari, ramdisk => ari + anything else => ami + + """ + if image_type == 'kernel': + return 'aki' + if image_type == 'ramdisk': + return 'ari' + if image_type not in ['aki', 'ari']: + return 'ami' + return image_type + + @staticmethod + def _image_ec2_id(image_id, image_type='ami'): + """Returns image ec2_id using id and three letter type""" + template = image_type + '-%08x' return ec2utils.id_to_ec2_id(int(image_id), template=template) def _get_image(self, context, ec2_id): @@ -881,7 +895,7 @@ class CloudController(object): def _format_image(self, image): """Convert from format defined by BaseImageService to S3 format.""" i = {} - image_type = image['properties'].get('type') + image_type = self._image_type(image.get('container_format')) ec2_id = self._image_ec2_id(image.get('id'), image_type) name = image.get('name') if name: @@ -890,16 +904,19 @@ class CloudController(object): i['imageId'] = ec2_id kernel_id = image['properties'].get('kernel_id') if kernel_id: - i['kernelId'] = self._image_ec2_id(kernel_id, 'kernel') + i['kernelId'] = self._image_ec2_id(kernel_id, 'aki') ramdisk_id = image['properties'].get('ramdisk_id') if ramdisk_id: - i['ramdiskId'] = self._image_ec2_id(ramdisk_id, 'ramdisk') + i['ramdiskId'] = self._image_ec2_id(ramdisk_id, 'ari') i['imageOwnerId'] = image['properties'].get('owner_id') i['imageLocation'] = image['properties'].get('image_location') i['imageState'] = image['properties'].get('image_state') i['displayName'] = image.get('name') i['description'] = image.get('description') - i['type'] = image_type + display_mapping = {'aki': 'kernel', + 'ari': 'ramdisk', + 'ami': 'machine'} + i['type'] = display_mapping.get(image_type) i['isPublic'] = str(image['properties'].get('is_public', '')) == 'True' i['architecture'] = image['properties'].get('architecture') return i @@ -932,8 +949,9 @@ class CloudController(object): image_location = kwargs['name'] metadata = {'properties': {'image_location': image_location}} image = self.image_service.create(context, metadata) + image_type = self._image_type(image.get('container_format')) image_id = self._image_ec2_id(image['id'], - image['properties']['type']) + image_type) msg = _("Registered image %(image_location)s with" " id %(image_id)s") % locals() LOG.audit(msg, context=context) -- cgit From 1703592992ebdd5bbf19952f79f05022a4cdc849 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Mar 2011 12:34:10 -0700 Subject: remove all references to image_type and change nova-manage upload to set container format more intelligently --- bin/nova-manage | 27 +++++++++++++------------ nova/api/openstack/servers.py | 2 +- nova/image/fake.py | 6 +++--- nova/image/s3.py | 1 - nova/tests/api/openstack/test_image_metadata.py | 2 -- nova/virt/libvirt_conn.py | 1 - 6 files changed, 18 insertions(+), 21 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 6789efba8..4e14b6cab 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -894,20 +894,18 @@ class ImageCommands(object): def __init__(self, *args, **kwargs): self.image_service = utils.import_object(FLAGS.image_service) - def _register(self, image_type, disk_format, container_format, + def _register(self, container_format, disk_format, path, owner, name=None, is_public='T', architecture='x86_64', kernel_id=None, ramdisk_id=None): meta = {'is_public': True, 'name': name, - 'disk_format': disk_format, 'container_format': container_format, + 'disk_format': disk_format, 'properties': {'image_state': 'available', 'owner_id': owner, - 'type': image_type, 'architecture': architecture, 'image_location': 'local', 'is_public': (is_public == 'T')}} - print image_type, meta if kernel_id: meta['properties']['kernel_id'] = int(kernel_id) if ramdisk_id: @@ -932,16 +930,17 @@ class ImageCommands(object): ramdisk_id = self.ramdisk_register(ramdisk, owner, None, is_public, architecture) self.image_register(image, owner, name, is_public, - architecture, kernel_id, ramdisk_id) + architecture, 'ami', 'ami', + kernel_id, ramdisk_id) def image_register(self, path, owner, name=None, is_public='T', - architecture='x86_64', kernel_id=None, ramdisk_id=None, - disk_format='ami', container_format='ami'): + architecture='x86_64', container_format='bare', + disk_format='raw', kernel_id=None, ramdisk_id=None): """Uploads an image into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] - [kernel_id=None] [ramdisk_id=None] - [disk_format='ami'] [container_format='ami']""" - return self._register('machine', disk_format, container_format, path, + [container_format='bare'] [disk_format='raw'] + [kernel_id=None] [ramdisk_id=None]""" + return self._register(container_format, disk_format, path, owner, name, is_public, architecture, kernel_id, ramdisk_id) @@ -950,7 +949,7 @@ class ImageCommands(object): """Uploads a kernel into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] """ - return self._register('kernel', 'aki', 'aki', path, owner, name, + return self._register('aki', 'aki', path, owner, name, is_public, architecture) def ramdisk_register(self, path, owner, name=None, is_public='T', @@ -958,7 +957,7 @@ class ImageCommands(object): """Uploads a ramdisk into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] """ - return self._register('ramdisk', 'ari', 'ari', path, owner, name, + return self._register('ari', 'ari', path, owner, name, is_public, architecture) def _lookup(self, old_image_id): @@ -975,6 +974,9 @@ class ImageCommands(object): 'ramdisk': 'ari'} container_format = mapping[old['type']] disk_format = container_format + if container_format == 'ami' and not old.get('kernelId'): + container_format = 'bare' + disk_format = 'raw' new = {'disk_format': disk_format, 'container_format': container_format, 'is_public': True, @@ -982,7 +984,6 @@ class ImageCommands(object): 'properties': {'image_state': old['imageState'], 'owner_id': old['imageOwnerId'], 'architecture': old['architecture'], - 'type': old['type'], 'image_location': old['imageLocation'], 'is_public': old['isPublic']}} if old.get('kernelId'): diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index f7696d918..d640e8684 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -562,7 +562,7 @@ class Controller(wsgi.Controller): _("Cannot build from image %(image_id)s, status not active") % locals()) - if image_meta['properties']['disk_format'] != 'ami': + if image_meta.get('container_format') != 'ami': return None, None try: diff --git a/nova/image/fake.py b/nova/image/fake.py index 08302d6eb..d1c62757f 100644 --- a/nova/image/fake.py +++ b/nova/image/fake.py @@ -44,10 +44,10 @@ class FakeImageService(service.BaseImageService): 'created_at': timestamp, 'updated_at': timestamp, 'status': 'active', - 'type': 'machine', + 'container_format': 'ami', + 'disk_format': 'raw', 'properties': {'kernel_id': FLAGS.null_kernel, - 'ramdisk_id': FLAGS.null_kernel, - 'disk_format': 'ami'} + 'ramdisk_id': FLAGS.null_kernel} } self.create(None, image) super(FakeImageService, self).__init__() diff --git a/nova/image/s3.py b/nova/image/s3.py index ddec5f3aa..203bedc49 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -177,7 +177,6 @@ class S3ImageService(service.BaseImageService): properties['ramdisk_id'] = ec2utils.ec2_id_to_id(ramdisk_id) properties['is_public'] = False - properties['type'] = image_type metadata.update({'disk_format': image_format, 'container_format': image_format, 'status': 'queued', diff --git a/nova/tests/api/openstack/test_image_metadata.py b/nova/tests/api/openstack/test_image_metadata.py index 9be753f84..7c3287006 100644 --- a/nova/tests/api/openstack/test_image_metadata.py +++ b/nova/tests/api/openstack/test_image_metadata.py @@ -45,7 +45,6 @@ class ImageMetaDataTest(unittest.TestCase): 'is_public': True, 'deleted_at': None, 'properties': { - 'type': 'ramdisk', 'key1': 'value1', 'key2': 'value2' }, @@ -62,7 +61,6 @@ class ImageMetaDataTest(unittest.TestCase): 'is_public': True, 'deleted_at': None, 'properties': { - 'type': 'ramdisk', 'key1': 'value1', 'key2': 'value2' }, diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index f34ea7225..adcb2ffa3 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -424,7 +424,6 @@ class LibvirtConnection(driver.ComputeDriver): 'container_format': base['container_format'], 'is_public': False, 'properties': {'architecture': base['architecture'], - 'type': base['type'], 'name': '%s.%s' % (base['name'], image_id), 'kernel_id': instance['kernel_id'], 'image_location': 'snapshot', -- cgit From b8173ba9fe80045665064208e66f46cca75fd3ba Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 30 Mar 2011 13:48:07 -0700 Subject: review cleanup --- bin/nova-manage | 3 ++- nova/api/ec2/cloud.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 4e14b6cab..fbf16f570 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -939,7 +939,8 @@ class ImageCommands(object): """Uploads an image into the image_service arguments: path owner [name] [is_public='T'] [architecture='x86_64'] [container_format='bare'] [disk_format='raw'] - [kernel_id=None] [ramdisk_id=None]""" + [kernel_id=None] [ramdisk_id=None] + """ return self._register(container_format, disk_format, path, owner, name, is_public, architecture, kernel_id, ramdisk_id) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 1e6c4d1db..3541e49ca 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -881,7 +881,7 @@ class CloudController(object): @staticmethod def _image_ec2_id(image_id, image_type='ami'): - """Returns image ec2_id using id and three letter type""" + """Returns image ec2_id using id and three letter type.""" template = image_type + '-%08x' return ec2utils.id_to_ec2_id(int(image_id), template=template) -- cgit From 1654afaabba498ab690c07ccc6de858783dc1742 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 31 Mar 2011 12:38:05 -0700 Subject: makes sure s3 filtering works even without metadata set properly --- nova/image/s3.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/image/s3.py b/nova/image/s3.py index 203bedc49..def49f682 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -80,9 +80,10 @@ class S3ImageService(service.BaseImageService): @classmethod def _is_visible(cls, context, image): + return (context.is_admin - or context.project_id == image['properties']['owner_id'] - or image['properties']['is_public'] == 'True') + or context.project_id == image['properties'].get('owner_id') + or str(image['properties'].get('is_public')) == 'True') @classmethod def _filter(cls, context, images): -- cgit From 56eb682a169c20a9a8275b3b9bfd423f0d562cbc Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:01:33 -0700 Subject: unite the filtering done by glance client and s3 --- bin/nova-manage | 14 ++++++-------- nova/api/ec2/cloud.py | 2 +- nova/image/glance.py | 28 ---------------------------- nova/image/local.py | 4 +++- nova/image/s3.py | 45 +++------------------------------------------ nova/image/service.py | 27 +++++++++++++++++++++++++++ 6 files changed, 40 insertions(+), 80 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index fbf16f570..bfff227c9 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -897,15 +897,14 @@ class ImageCommands(object): def _register(self, container_format, disk_format, path, owner, name=None, is_public='T', architecture='x86_64', kernel_id=None, ramdisk_id=None): - meta = {'is_public': True, + meta = {'is_public': (is_public == 'T'), 'name': name, 'container_format': container_format, 'disk_format': disk_format, 'properties': {'image_state': 'available', - 'owner_id': owner, + 'project_id': owner, 'architecture': architecture, - 'image_location': 'local', - 'is_public': (is_public == 'T')}} + 'image_location': 'local'}} if kernel_id: meta['properties']['kernel_id'] = int(kernel_id) if ramdisk_id: @@ -980,13 +979,12 @@ class ImageCommands(object): disk_format = 'raw' new = {'disk_format': disk_format, 'container_format': container_format, - 'is_public': True, + 'is_public': old['isPublic'], 'name': old['imageId'], 'properties': {'image_state': old['imageState'], - 'owner_id': old['imageOwnerId'], + 'project_id': old['imageOwnerId'], 'architecture': old['architecture'], - 'image_location': old['imageLocation'], - 'is_public': old['isPublic']}} + 'image_location': old['imageLocation']}} if old.get('kernelId'): new['properties']['kernel_id'] = self._lookup(old['kernelId']) if old.get('ramdiskId'): diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 02a30220a..b4705820e 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -927,7 +927,7 @@ class CloudController(object): 'ari': 'ramdisk', 'ami': 'machine'} i['imageType'] = display_mapping.get(image_type) - i['isPublic'] = str(image['properties'].get('is_public', '')) == 'True' + i['isPublic'] = image.get('is_public') == True i['architecture'] = image['properties'].get('architecture') return i diff --git a/nova/image/glance.py b/nova/image/glance.py index fdf468594..e583c504d 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -186,34 +186,6 @@ class GlanceImageService(service.BaseImageService): image_meta = _convert_timestamps_to_datetimes(image_meta) return image_meta - @staticmethod - def _is_image_available(context, image_meta): - """ - Images are always available if they are public or if the user is an - admin. - - Otherwise, we filter by project_id (if present) and then fall-back to - images owned by user. - """ - # FIXME(sirp): We should be filtering by user_id on the Glance side - # for security; however, we can't do that until we get authn/authz - # sorted out. Until then, filtering in Nova. - if image_meta['is_public'] or context.is_admin: - return True - - properties = image_meta['properties'] - - if context.project_id and ('project_id' in properties): - return str(properties['project_id']) == str(project_id) - - try: - user_id = properties['user_id'] - except KeyError: - return False - - return str(user_id) == str(context.user_id) - - # utility functions def _convert_timestamps_to_datetimes(image_meta): """ diff --git a/nova/image/local.py b/nova/image/local.py index 1fb6e1f13..92d3dabd4 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -84,7 +84,9 @@ class LocalImageService(service.BaseImageService): def show(self, context, image_id): try: with open(self._path_to(image_id)) as metadata_file: - return json.load(metadata_file) + image_meta = json.load(metadata_file) + if not self._is_image_available(context, image_meta): + raise exception.NotFound except (IOError, ValueError): raise exception.NotFound diff --git a/nova/image/s3.py b/nova/image/s3.py index def49f682..93af21022 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -58,54 +58,16 @@ class S3ImageService(service.BaseImageService): return image def delete(self, context, image_id): - # FIXME(vish): call to show is to check filter + # FIXME(vish): call to show is to check visibility self.show(context, image_id) self.service.delete(context, image_id) def update(self, context, image_id, metadata, data=None): - # FIXME(vish): call to show is to check filter + # FIXME(vish): call to show is to check visibility self.show(context, image_id) image = self.service.update(context, image_id, metadata, data) return image - def index(self, context): - images = self.service.index(context) - # FIXME(vish): index doesn't filter so we do it manually - return self._filter(context, images) - - def detail(self, context): - images = self.service.detail(context) - # FIXME(vish): detail doesn't filter so we do it manually - return self._filter(context, images) - - @classmethod - def _is_visible(cls, context, image): - - return (context.is_admin - or context.project_id == image['properties'].get('owner_id') - or str(image['properties'].get('is_public')) == 'True') - - @classmethod - def _filter(cls, context, images): - filtered = [] - for image in images: - if not cls._is_visible(context, image): - continue - filtered.append(image) - return filtered - - def show(self, context, image_id): - image = self.service.show(context, image_id) - if not self._is_visible(context, image): - raise exception.NotFound - return image - - def show_by_name(self, context, name): - image = self.service.show_by_name(context, name) - if not self._is_visible(context, image): - raise exception.NotFound - return image - @staticmethod def _conn(context): # TODO(vish): is there a better way to get creds to sign @@ -168,7 +130,7 @@ class S3ImageService(service.BaseImageService): arch = 'x86_64' properties = metadata['properties'] - properties['owner_id'] = context.project_id + properties['project_id'] = context.project_id properties['architecture'] = arch if kernel_id: @@ -177,7 +139,6 @@ class S3ImageService(service.BaseImageService): if ramdisk_id: properties['ramdisk_id'] = ec2utils.ec2_id_to_id(ramdisk_id) - properties['is_public'] = False metadata.update({'disk_format': image_format, 'container_format': image_format, 'status': 'queued', diff --git a/nova/image/service.py b/nova/image/service.py index b9897ecae..fddc72409 100644 --- a/nova/image/service.py +++ b/nova/image/service.py @@ -136,6 +136,33 @@ class BaseImageService(object): """ raise NotImplementedError + @staticmethod + def _is_image_available(context, image_meta): + """ + Images are always available if they are public or if the user is an + admin. + + Otherwise, we filter by project_id (if present) and then fall-back to + images owned by user. + """ + # FIXME(sirp): We should be filtering by user_id on the Glance side + # for security; however, we can't do that until we get authn/authz + # sorted out. Until then, filtering in Nova. + if image_meta['is_public'] or context.is_admin: + return True + + properties = image_meta['properties'] + + if context.project_id and ('project_id' in properties): + return str(properties['project_id']) == str(context.project_id) + + try: + user_id = properties['user_id'] + except KeyError: + return False + + return str(user_id) == str(context.user_id) + @classmethod def _translate_to_base(cls, metadata): """Return a metadata dictionary that is BaseImageService compliant. -- cgit From 8eff0a91bbc475d8559b39fb81e1a62beda841f3 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:35:17 -0700 Subject: update and fix tests --- nova/image/local.py | 1 + nova/image/s3.py | 13 +++++++++++++ nova/tests/api/openstack/test_servers.py | 16 +++++++--------- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/nova/image/local.py b/nova/image/local.py index 92d3dabd4..bad4c2d85 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -87,6 +87,7 @@ class LocalImageService(service.BaseImageService): image_meta = json.load(metadata_file) if not self._is_image_available(context, image_meta): raise exception.NotFound + return image_meta except (IOError, ValueError): raise exception.NotFound diff --git a/nova/image/s3.py b/nova/image/s3.py index 93af21022..a7301eefd 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -46,6 +46,7 @@ flags.DEFINE_string('image_decryption_dir', '/tmp', class S3ImageService(service.BaseImageService): + """Wraps an existing image service to support s3 based register""" def __init__(self, service=None, *args, **kwargs): if service == None: service = utils.import_object(FLAGS.image_service) @@ -68,6 +69,18 @@ class S3ImageService(service.BaseImageService): image = self.service.update(context, image_id, metadata, data) return image + def index(self, context): + return self.service.index(context) + + def detail(self, context): + return self.service.detail(context) + + def show(self, context, image_id): + return self.service.show(context, image_id) + + def show_by_name(self, context, name): + return self.service.show(context, name) + @staticmethod def _conn(context): # TODO(vish): is there a better way to get creds to sign diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 313676e72..1ec01bb4c 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1525,29 +1525,27 @@ class TestGetKernelRamdiskFromImage(test.TestCase): def test_not_ami(self): """Anything other than ami should return no kernel and no ramdisk""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'vhd'}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'vhd'} kernel_id, ramdisk_id = self._get_k_r(image_meta) self.assertEqual(kernel_id, None) self.assertEqual(ramdisk_id, None) def test_ami_no_kernel(self): """If an ami is missing a kernel it should raise NotFound""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'ami', 'ramdisk_id': 1}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'ami', + 'properties': {'ramdisk_id': 1}} self.assertRaises(exception.NotFound, self._get_k_r, image_meta) def test_ami_no_ramdisk(self): """If an ami is missing a ramdisk it should raise NotFound""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'ami', 'kernel_id': 1}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'ami', + 'properties': {'kernel_id': 1}} self.assertRaises(exception.NotFound, self._get_k_r, image_meta) def test_ami_kernel_ramdisk_present(self): """Return IDs if both kernel and ramdisk are present""" - image_meta = {'id': 1, 'status': 'active', - 'properties': {'disk_format': 'ami', 'kernel_id': 1, - 'ramdisk_id': 2}} + image_meta = {'id': 1, 'status': 'active', 'container_format': 'ami', + 'properties': {'kernel_id': 1, 'ramdisk_id': 2}} kernel_id, ramdisk_id = self._get_k_r(image_meta) self.assertEqual(kernel_id, 1) self.assertEqual(ramdisk_id, 2) -- cgit From 5f414311fb581ba612cf152f3ec9983d5f39c2e4 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:45:01 -0700 Subject: fallback to status if image_state is not set --- nova/api/ec2/cloud.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index b4705820e..4e5b8bfc6 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -920,7 +920,11 @@ class CloudController(object): get('image_location'), name) else: i['imageLocation'] = image['properties'].get('image_location') - i['imageState'] = image['properties'].get('image_state') + # NOTE(vish): fallback status if image_state isn't set + state = image.get('status') + if state == 'active': + state = 'available' + i['imageState'] = image['properties'].get('image_state', state) i['displayName'] = name i['description'] = image.get('description') display_mapping = {'aki': 'kernel', -- cgit From 75082c44cdc0319c56805b5558927760636eaf4b Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 5 Apr 2011 15:49:43 -0700 Subject: pep8 --- nova/image/glance.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nova/image/glance.py b/nova/image/glance.py index e583c504d..f848f40d8 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -186,6 +186,7 @@ class GlanceImageService(service.BaseImageService): image_meta = _convert_timestamps_to_datetimes(image_meta) return image_meta + # utility functions def _convert_timestamps_to_datetimes(image_meta): """ -- cgit From cd279bec6391ef40d278f377f0039b507b14904b Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 6 Apr 2011 12:58:57 -0700 Subject: check visibility on delete and update --- nova/image/glance.py | 4 ++++ nova/image/local.py | 6 +++++- nova/image/s3.py | 4 ---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/nova/image/glance.py b/nova/image/glance.py index f848f40d8..bf49ca96c 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -151,6 +151,8 @@ class GlanceImageService(service.BaseImageService): :raises NotFound if the image does not exist. """ + # NOTE(vish): show is to check if image is available + self.show(context, image_id) try: image_meta = self.client.update_image(image_id, image_meta, data) except glance_exception.NotFound: @@ -165,6 +167,8 @@ class GlanceImageService(service.BaseImageService): :raises NotFound if the image does not exist. """ + # NOTE(vish): show is to check if image is available + self.show(context, image_id) try: result = self.client.delete_image(image_id) except glance_exception.NotFound: diff --git a/nova/image/local.py b/nova/image/local.py index bad4c2d85..8bf78b4c9 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -126,6 +126,8 @@ class LocalImageService(service.BaseImageService): def update(self, context, image_id, metadata, data=None): """Replace the contents of the given image with the new data.""" + # NOTE(vish): show is to check if image is available + self.show(context, image_id) metadata['id'] = image_id try: if data: @@ -143,9 +145,11 @@ class LocalImageService(service.BaseImageService): def delete(self, context, image_id): """Delete the given image. - Raises OSError if the image does not exist. + Raises NotFound if the image does not exist. """ + # NOTE(vish): show is to check if image is available + self.show(context, image_id) try: shutil.rmtree(self._path_to(image_id, None)) except (IOError, ValueError): diff --git a/nova/image/s3.py b/nova/image/s3.py index a7301eefd..554760d53 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -59,13 +59,9 @@ class S3ImageService(service.BaseImageService): return image def delete(self, context, image_id): - # FIXME(vish): call to show is to check visibility - self.show(context, image_id) self.service.delete(context, image_id) def update(self, context, image_id, metadata, data=None): - # FIXME(vish): call to show is to check visibility - self.show(context, image_id) image = self.service.update(context, image_id, metadata, data) return image -- cgit From b0ad20c796bd25dad0538ab85b1a56f421e16039 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 7 Apr 2011 16:42:33 -0700 Subject: fix tests from moving access check into update and delete --- nova/image/local.py | 5 ++++- nova/tests/image/test_glance.py | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/nova/image/local.py b/nova/image/local.py index 8bf78b4c9..d4fd62156 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -122,12 +122,15 @@ class LocalImageService(service.BaseImageService): image_path = self._path_to(image_id, None) if not os.path.exists(image_path): os.mkdir(image_path) - return self.update(context, image_id, metadata, data) + return self._store(context, image_id, metadata, data) def update(self, context, image_id, metadata, data=None): """Replace the contents of the given image with the new data.""" # NOTE(vish): show is to check if image is available self.show(context, image_id) + return self._store(context, image_id, metadata, data) + + def _store(self, context, image_id, metadata, data=None): metadata['id'] = image_id try: if data: diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py index 9d0b14613..109905ded 100644 --- a/nova/tests/image/test_glance.py +++ b/nova/tests/image/test_glance.py @@ -209,17 +209,17 @@ class TestMutatorDateTimeTests(BaseGlanceTest): self.assertDateTimesEmpty(image_meta) def test_update_handles_datetimes(self): + self.client.images = {'image1': self._make_datetime_fixture()} self.client.update_response = self._make_datetime_fixture() - dummy_id = 'dummy_id' dummy_meta = {} - image_meta = self.service.update(self.context, 'dummy_id', dummy_meta) + image_meta = self.service.update(self.context, 'image1', dummy_meta) self.assertDateTimesFilled(image_meta) def test_update_handles_none_datetimes(self): + self.client.images = {'image1': self._make_datetime_fixture()} self.client.update_response = self._make_none_datetime_fixture() - dummy_id = 'dummy_id' dummy_meta = {} - image_meta = self.service.update(self.context, 'dummy_id', dummy_meta) + image_meta = self.service.update(self.context, 'image1', dummy_meta) self.assertDateTimesEmpty(image_meta) def _make_datetime_fixture(self): -- cgit