diff options
| -rw-r--r-- | nova/api/openstack/images.py | 152 | ||||
| -rw-r--r-- | nova/compute/api.py | 31 | ||||
| -rw-r--r-- | nova/image/glance.py | 160 | ||||
| -rw-r--r-- | nova/image/local.py | 9 | ||||
| -rw-r--r-- | nova/image/service.py | 61 | ||||
| -rw-r--r-- | nova/test.py | 57 | ||||
| -rw-r--r-- | nova/tests/api/openstack/fakes.py | 25 | ||||
| -rw-r--r-- | nova/tests/api/openstack/test_images.py | 263 | ||||
| -rw-r--r-- | nova/tests/image/test_glance.py | 265 | ||||
| -rw-r--r-- | nova/utils.py | 42 |
10 files changed, 746 insertions, 319 deletions
diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 99c14275a..79852ecc6 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -15,10 +15,14 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime + from webob import exc from nova import compute +from nova import exception from nova import flags +from nova import log from nova import utils from nova import wsgi import nova.api.openstack @@ -27,6 +31,8 @@ from nova.api.openstack import faults import nova.image.service +LOG = log.getLogger('nova.api.openstack.images') + FLAGS = flags.FLAGS @@ -84,8 +90,6 @@ def _translate_status(item): # S3ImageService pass - return item - def _filter_keys(item, keys): """ @@ -104,6 +108,100 @@ def _convert_image_id_to_hash(image): image['id'] = image_id +def _translate_s3_like_images(image_metadata): + """Work-around for leaky S3ImageService abstraction""" + api_metadata = image_metadata.copy() + _convert_image_id_to_hash(api_metadata) + api_metadata = _translate_keys(api_metadata) + _translate_status(api_metadata) + return api_metadata + + +def _translate_from_image_service_to_api(image_metadata): + """Translate from ImageService to OpenStack API style attribute names + + This involves 4 steps: + + 1. Filter out attributes that the OpenStack API doesn't need + + 2. Translate from base image attributes from names used by + BaseImageService to names used by OpenStack API + + 3. Add in any image properties + + 4. Format values according to API spec (for example dates must + look like "2010-08-10T12:00:00Z") + """ + service_metadata = image_metadata.copy() + properties = service_metadata.pop('properties', {}) + + # 1. Filter out unecessary attributes + api_keys = ['id', 'name', 'updated_at', 'created_at', 'status'] + api_metadata = utils.subset_dict(service_metadata, api_keys) + + # 2. Translate base image attributes + api_map = {'updated_at': 'updated', 'created_at': 'created'} + api_metadata = utils.map_dict_keys(api_metadata, api_map) + + # 3. Add in any image properties + # 3a. serverId is used for backups and snapshots + try: + api_metadata['serverId'] = int(properties['instance_id']) + except KeyError: + pass # skip if it's not present + except ValueError: + pass # skip if it's not an integer + + # 3b. Progress special case + # TODO(sirp): ImageService doesn't have a notion of progress yet, so for + # now just fake it + if service_metadata['status'] == 'saving': + api_metadata['progress'] = 0 + + # 4. Format values + # 4a. Format Image Status (API requires uppercase) + api_metadata['status'] = _format_status_for_api(api_metadata['status']) + + # 4b. Format timestamps + for attr in ('created', 'updated'): + if attr in api_metadata: + api_metadata[attr] = _format_datetime_for_api( + api_metadata[attr]) + + return api_metadata + + +def _format_status_for_api(status): + """Return status in a format compliant with OpenStack API""" + mapping = {'queued': 'QUEUED', + 'preparing': 'PREPARING', + 'saving': 'SAVING', + 'active': 'ACTIVE', + 'killed': 'FAILED'} + return mapping[status] + + +def _format_datetime_for_api(datetime_): + """Stringify datetime objects in a format compliant with OpenStack API""" + API_DATETIME_FMT = '%Y-%m-%dT%H:%M:%SZ' + return datetime_.strftime(API_DATETIME_FMT) + + +def _safe_translate(image_metadata): + """Translate attributes for OpenStack API, temporary workaround for + S3ImageService attribute leakage. + """ + # FIXME(sirp): The S3ImageService appears to be leaking implementation + # details, including its internal attribute names, and internal + # `status` values. Working around it for now. + s3_like_image = ('imageId' in image_metadata) + if s3_like_image: + translate = _translate_s3_like_images + else: + translate = _translate_from_image_service_to_api + return translate(image_metadata) + + class Controller(wsgi.Controller): _serialization_metadata = { @@ -117,34 +215,32 @@ class Controller(wsgi.Controller): def index(self, req): """Return all public images in brief""" - items = self._service.index(req.environ['nova.context']) - items = common.limited(items, req) - items = [_filter_keys(item, ('id', 'name')) for item in items] - return dict(images=items) + context = req.environ['nova.context'] + image_metas = self._service.index(context) + image_metas = common.limited(image_metas, req) + return dict(images=image_metas) def detail(self, req): """Return all public images in detail""" - try: - items = self._service.detail(req.environ['nova.context']) - except NotImplementedError: - items = self._service.index(req.environ['nova.context']) - for image in items: - _convert_image_id_to_hash(image) - - items = common.limited(items, req) - items = [_translate_keys(item) for item in items] - items = [_translate_status(item) for item in items] - return dict(images=items) + context = req.environ['nova.context'] + image_metas = self._service.detail(context) + image_metas = common.limited(image_metas, req) + api_image_metas = [_safe_translate(image_meta) + for image_meta in image_metas] + return dict(images=api_image_metas) def show(self, req, id): """Return data about the given image id""" - image_id = common.get_image_id_from_image_hash(self._service, - req.environ['nova.context'], id) + context = req.environ['nova.context'] + try: + image_id = common.get_image_id_from_image_hash( + self._service, context, id) + except exception.NotFound: + raise faults.Fault(exc.HTTPNotFound()) - image = self._service.show(req.environ['nova.context'], image_id) - _convert_image_id_to_hash(image) - self._format_image_dates(image) - return dict(image=image) + image_meta = self._service.show(context, image_id) + api_image_meta = _safe_translate(image_meta) + return dict(image=api_image_meta) def delete(self, req, id): # Only public images are supported for now. @@ -155,18 +251,12 @@ class Controller(wsgi.Controller): env = self._deserialize(req.body, req.get_content_type()) instance_id = env["image"]["serverId"] name = env["image"]["name"] - image_meta = compute.API().snapshot( context, instance_id, name) - - return dict(image=image_meta) + api_image_meta = _safe_translate(image_meta) + return dict(image=api_image_meta) def update(self, req, id): # Users may not modify public images, and that's all that # we support for now. raise faults.Fault(exc.HTTPNotFound()) - - def _format_image_dates(self, image): - for attr in ['created_at', 'updated_at', 'deleted_at']: - if image.get(attr) is not None: - image[attr] = image[attr].strftime('%Y-%m-%dT%H:%M:%SZ') diff --git a/nova/compute/api.py b/nova/compute/api.py index f4aab97de..af02e1a59 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -397,20 +397,26 @@ class API(base.Base): fixed_ip=None): """Get all instances, possibly filtered by one of the given parameters. If there is no filter and the context is - an admin, it will retreive all instances in the system.""" + an admin, it will retreive all instances in the system. + """ if reservation_id is not None: - return self.db.instance_get_all_by_reservation(context, - reservation_id) + return self.db.instance_get_all_by_reservation( + context, reservation_id) + if fixed_ip is not None: return self.db.fixed_ip_get_instance(context, fixed_ip) + if project_id or not context.is_admin: if not context.project: - return self.db.instance_get_all_by_user(context, - context.user_id) + return self.db.instance_get_all_by_user( + context, context.user_id) + if project_id is None: project_id = context.project_id - return self.db.instance_get_all_by_project(context, - project_id) + + return self.db.instance_get_all_by_project( + context, project_id) + return self.db.instance_get_all(context) def _cast_compute_message(self, method, context, instance_id, host=None, @@ -460,12 +466,15 @@ class API(base.Base): :retval: A dict containing image metadata """ - data = {'name': name, 'is_public': False} - image_meta = self.image_service.create(context, data) - params = {'image_id': image_meta['id']} + properties = {'instance_id': str(instance_id), + 'user_id': str(context.user_id)} + sent_meta = {'name': name, 'is_public': False, + 'properties': properties} + recv_meta = self.image_service.create(context, sent_meta) + params = {'image_id': recv_meta['id']} self._cast_compute_message('snapshot_instance', context, instance_id, params=params) - return image_meta + return recv_meta def reboot(self, context, instance_id): """Reboot the given instance.""" diff --git a/nova/image/glance.py b/nova/image/glance.py index 9984a3ba1..be9805b69 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -39,7 +39,17 @@ GlanceClient = utils.import_class('glance.client.Client') class GlanceImageService(service.BaseImageService): """Provides storage and retrieval of disk image objects within Glance.""" + GLANCE_ONLY_ATTRS = ["size", "location", "disk_format", + "container_format"] + + # NOTE(sirp): Overriding to use _translate_to_service provided by + # BaseImageService + SERVICE_IMAGE_ATTRS = service.BaseImageService.BASE_IMAGE_ATTRS +\ + GLANCE_ONLY_ATTRS + def __init__(self, client=None): + # FIXME(sirp): can we avoid dependency-injection here by using + # stubbing out a fake? if client is None: self.client = GlanceClient(FLAGS.glance_host, FLAGS.glance_port) else: @@ -49,39 +59,43 @@ class GlanceImageService(service.BaseImageService): """ Calls out to Glance for a list of images available """ - return self.client.get_images() + # NOTE(sirp): We need to use `get_images_detailed` and not + # `get_images` here because we need `is_public` and `properties` + # included so we can filter by user + filtered = [] + image_metas = self.client.get_images_detailed() + for image_meta in image_metas: + if self._is_image_available(context, image_meta): + meta_subset = utils.subset_dict(image_meta, ('id', 'name')) + filtered.append(meta_subset) + return filtered def detail(self, context): """ Calls out to Glance for a list of detailed image information """ - return [self._convert_timestamps_to_datetimes(image) - for image in self.client.get_images_detailed()] + filtered = [] + image_metas = self.client.get_images_detailed() + for image_meta in image_metas: + if self._is_image_available(context, image_meta): + base_image_meta = self._translate_to_base(image_meta) + filtered.append(base_image_meta) + return filtered def show(self, context, image_id): """ Returns a dict containing image data for the given opaque image id. """ try: - image = self.client.get_image_meta(image_id) + image_meta = self.client.get_image_meta(image_id) except glance_exception.NotFound: raise exception.NotFound - return self._convert_timestamps_to_datetimes(image) - def _convert_timestamps_to_datetimes(self, image): - """ - Returns image with known timestamp fields converted to datetime objects - """ - for attr in ['created_at', 'updated_at', 'deleted_at']: - if image.get(attr): - image[attr] = self._parse_glance_iso8601_timestamp(image[attr]) - return image + if not self._is_image_available(context, image_meta): + raise exception.NotFound - def _parse_glance_iso8601_timestamp(self, timestamp): - """ - Parse a subset of iso8601 timestamps into datetime objects - """ - return datetime.datetime.strptime(timestamp, "%Y-%m-%dT%H:%M:%S.%f") + base_image_meta = self._translate_to_base(image_meta) + return base_image_meta def show_by_name(self, context, name): """ @@ -89,56 +103,67 @@ class GlanceImageService(service.BaseImageService): """ # TODO(vish): replace this with more efficient call when glance # supports it. - images = self.detail(context) - image = None - for cantidate in images: - if name == cantidate.get('name'): - image = cantidate - break - if image is None: - raise exception.NotFound - return image + image_metas = self.detail(context) + for image_meta in image_metas: + if name == image_meta.get('name'): + return image_meta + raise exception.NotFound def get(self, context, image_id, data): """ Calls out to Glance for metadata and data and writes data. """ try: - metadata, image_chunks = self.client.get_image(image_id) + image_meta, image_chunks = self.client.get_image(image_id) except glance_exception.NotFound: raise exception.NotFound + for chunk in image_chunks: data.write(chunk) - return self._convert_timestamps_to_datetimes(metadata) - def create(self, context, metadata, data=None): + base_image_meta = self._translate_to_base(image_meta) + return base_image_meta + + def create(self, context, image_meta, data=None): """ Store the image data and return the new image id. :raises AlreadyExists if the image already exist. - """ - return self._convert_timestamps_to_datetimes( - self.client.add_image(metadata, data)) + # Translate Base -> Service + LOG.debug(_("Creating image in Glance. Metadata passed in %s"), + image_meta) + sent_service_image_meta = self._translate_to_service(image_meta) + LOG.debug(_("Metadata after formatting for Glance %s"), + sent_service_image_meta) + + recv_service_image_meta = self.client.add_image( + sent_service_image_meta, data) + + # Translate Service -> Base + base_image_meta = self._translate_to_base(recv_service_image_meta) + LOG.debug(_("Metadata returned from Glance formatted for Base %s"), + base_image_meta) + return base_image_meta - def update(self, context, image_id, metadata, data=None): + def update(self, context, image_id, image_meta, data=None): """Replace the contents of the given image with the new data. :raises NotFound if the image does not exist. - """ try: - metadata = self.client.update_image(image_id, metadata, data) + image_meta = self.client.update_image(image_id, image_meta, data) except glance_exception.NotFound: raise exception.NotFound - return self._convert_timestamps_to_datetimes(metadata) + + base_image_meta = self._translate_to_base(image_meta) + return base_image_meta def delete(self, context, image_id): """ Delete the given image. :raises NotFound if the image does not exist. - """ try: result = self.client.delete_image(image_id) @@ -151,3 +176,62 @@ class GlanceImageService(service.BaseImageService): Clears out all images """ pass + + @classmethod + def _translate_to_base(cls, image_meta): + """Overriding the base translation to handle conversion to datetime + objects + """ + image_meta = service.BaseImageService._translate_to_base(image_meta) + 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): + """ + Returns image with known timestamp fields converted to datetime objects + """ + for attr in ['created_at', 'updated_at', 'deleted_at']: + if image_meta.get(attr) is not None: + image_meta[attr] = _parse_glance_iso8601_timestamp( + image_meta[attr]) + return image_meta + + +def _parse_glance_iso8601_timestamp(timestamp): + """ + Parse a subset of iso8601 timestamps into datetime objects + """ + GLANCE_FMT = "%Y-%m-%dT%H:%M:%S" + ISO_FMT = "%Y-%m-%dT%H:%M:%S.%f" + # FIXME(sirp): Glance is not returning in ISO format, we should fix Glance + # to do so, and then switch to parsing it here + return datetime.datetime.strptime(timestamp, GLANCE_FMT) diff --git a/nova/image/local.py b/nova/image/local.py index 609d6c42a..1fb6e1f13 100644 --- a/nova/image/local.py +++ b/nova/image/local.py @@ -24,6 +24,7 @@ from nova import exception from nova import flags from nova import log as logging from nova.image import service +from nova import utils FLAGS = flags.FLAGS @@ -63,8 +64,12 @@ class LocalImageService(service.BaseImageService): return images def index(self, context): - return [dict(image_id=i['id'], name=i.get('name')) - for i in self.detail(context)] + filtered = [] + image_metas = self.detail(context) + for image_meta in image_metas: + meta = utils.subset_dict(image_meta, ('id', 'name')) + filtered.append(meta) + return filtered def detail(self, context): images = [] diff --git a/nova/image/service.py b/nova/image/service.py index e907381c9..b9897ecae 100644 --- a/nova/image/service.py +++ b/nova/image/service.py @@ -16,9 +16,33 @@ # under the License. +from nova import utils + + class BaseImageService(object): + """Base class for providing image search and retrieval services + + ImageService exposes two concepts of metadata: + + 1. First-class attributes: This is metadata that is common to all + ImageService subclasses and is shared across all hypervisors. These + attributes are defined by IMAGE_ATTRS. + + 2. Properties: This is metdata that is specific to an ImageService, + and Image, or a particular hypervisor. Any attribute not present in + BASE_IMAGE_ATTRS should be considered an image property. + + This means that ImageServices will return BASE_IMAGE_ATTRS as keys in the + metadata dict, all other attributes will be returned as keys in the nested + 'properties' dict. + """ + BASE_IMAGE_ATTRS = ['id', 'name', 'created_at', 'updated_at', + 'deleted_at', 'deleted', 'status', 'is_public'] - """Base class for providing image search and retrieval services""" + # NOTE(sirp): ImageService subclasses may override this to aid translation + # between BaseImageService attributes and additional metadata stored by + # the ImageService subclass + SERVICE_IMAGE_ATTRS = [] def index(self, context): """ @@ -111,3 +135,38 @@ class BaseImageService(object): """ raise NotImplementedError + + @classmethod + def _translate_to_base(cls, metadata): + """Return a metadata dictionary that is BaseImageService compliant. + + This is used by subclasses to expose only a metadata dictionary that + is the same across ImageService implementations. + """ + return cls._propertify_metadata(metadata, cls.BASE_IMAGE_ATTRS) + + @classmethod + def _translate_to_service(cls, metadata): + """Return a metadata dictionary that is usable by the ImageService + subclass. + + As an example, Glance has additional attributes (like 'location'); the + BaseImageService considers these properties, but we need to translate + these back to first-class attrs for sending to Glance. This method + handles this by allowing you to specify the attributes an ImageService + considers first-class. + """ + if not cls.SERVICE_IMAGE_ATTRS: + raise NotImplementedError(_("Cannot use this without specifying " + "SERVICE_IMAGE_ATTRS for subclass")) + return cls._propertify_metadata(metadata, cls.SERVICE_IMAGE_ATTRS) + + @staticmethod + def _propertify_metadata(metadata, keys): + """Return a dict with any unrecognized keys placed in the nested + 'properties' dict. + """ + flattened = utils.flatten_dict(metadata) + attributes, properties = utils.partition_dict(flattened, keys) + attributes['properties'] = properties + return attributes diff --git a/nova/test.py b/nova/test.py index d8a47464f..e0fef6101 100644 --- a/nova/test.py +++ b/nova/test.py @@ -150,3 +150,60 @@ class TestCase(unittest.TestCase): _wrapped.func_name = self.originalAttach.func_name rpc.Consumer.attach_to_eventlet = _wrapped + + # Useful assertions + def assertDictMatch(self, d1, d2): + """Assert two dicts are equivalent. + + This is a 'deep' match in the sense that it handles nested + dictionaries appropriately. + + NOTE: + + If you don't care (or don't know) a given value, you can specify + the string DONTCARE as the value. This will cause that dict-item + to be skipped. + """ + def raise_assertion(msg): + d1str = str(d1) + d2str = str(d2) + base_msg = ("Dictionaries do not match. %(msg)s d1: %(d1str)s " + "d2: %(d2str)s" % locals()) + raise AssertionError(base_msg) + + d1keys = set(d1.keys()) + d2keys = set(d2.keys()) + if d1keys != d2keys: + d1only = d1keys - d2keys + d2only = d2keys - d1keys + raise_assertion("Keys in d1 and not d2: %(d1only)s. " + "Keys in d2 and not d1: %(d2only)s" % locals()) + + for key in d1keys: + d1value = d1[key] + d2value = d2[key] + if hasattr(d1value, 'keys') and hasattr(d2value, 'keys'): + self.assertDictMatch(d1value, d2value) + elif 'DONTCARE' in (d1value, d2value): + continue + elif d1value != d2value: + raise_assertion("d1['%(key)s']=%(d1value)s != " + "d2['%(key)s']=%(d2value)s" % locals()) + + def assertDictListMatch(self, L1, L2): + """Assert a list of dicts are equivalent""" + def raise_assertion(msg): + L1str = str(L1) + L2str = str(L2) + base_msg = ("List of dictionaries do not match: %(msg)s " + "L1: %(L1str)s L2: %(L2str)s" % locals()) + raise AssertionError(base_msg) + + L1count = len(L1) + L2count = len(L2) + if L1count != L2count: + raise_assertion("Length mismatch: len(L1)=%(L1count)d != " + "len(L2)=%(L2count)d" % locals()) + + for d1, d2 in zip(L1, L2): + self.assertDictMatch(d1, d2) diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 56143114d..3cc68a536 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -143,6 +143,21 @@ def stub_out_compute_api_snapshot(stubs): stubs.Set(nova.compute.API, 'snapshot', snapshot) +def stub_out_glance_add_image(stubs, sent_to_glance): + """ + We return the metadata sent to glance by modifying the sent_to_glance dict + in place. + """ + orig_add_image = glance_client.Client.add_image + + def fake_add_image(context, metadata, data=None): + sent_to_glance['metadata'] = metadata + sent_to_glance['data'] = data + return orig_add_image(metadata, data) + + stubs.Set(glance_client.Client, 'add_image', fake_add_image) + + def stub_out_glance(stubs, initial_fixtures=None): class FakeGlanceClient: @@ -165,8 +180,9 @@ def stub_out_glance(stubs, initial_fixtures=None): def fake_add_image(self, image_meta, data=None): image_meta = copy.deepcopy(image_meta) - id = ''.join(random.choice(string.letters) for _ in range(20)) - image_meta['id'] = id + image_id = ''.join(random.choice(string.letters) + for _ in range(20)) + image_meta['id'] = image_id self.fixtures.append(image_meta) return image_meta @@ -185,9 +201,6 @@ def stub_out_glance(stubs, initial_fixtures=None): self.fixtures.remove(f) - ##def fake_delete_all(self): - ## self.fixtures = [] - def _find_image(self, image_id): for f in self.fixtures: if f['id'] == image_id: @@ -204,10 +217,10 @@ def stub_out_glance(stubs, initial_fixtures=None): stubs.Set(GlanceClient, 'add_image', fake.fake_add_image) stubs.Set(GlanceClient, 'update_image', fake.fake_update_image) stubs.Set(GlanceClient, 'delete_image', fake.fake_delete_image) - #stubs.Set(GlanceClient, 'delete_all', fake.fake_delete_all) class FakeToken(object): + # FIXME(sirp): let's not use id here id = 0 def __init__(self, **kwargs): diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index feb32ed9f..1cdccadd6 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -29,6 +29,7 @@ import tempfile import stubout import webob +from glance import client as glance_client from nova import context from nova import exception from nova import flags @@ -43,78 +44,44 @@ FLAGS = flags.FLAGS class BaseImageServiceTests(object): - """Tasks to test for all image services""" def test_create(self): - - fixture = {'name': 'test image', - 'updated': None, - 'created': None, - 'status': None, - 'instance_id': None, - 'progress': None} - + fixture = self._make_fixture('test image') num_images = len(self.service.index(self.context)) - id = self.service.create(self.context, fixture)['id'] + image_id = self.service.create(self.context, fixture)['id'] - self.assertNotEquals(None, id) + self.assertNotEquals(None, image_id) self.assertEquals(num_images + 1, len(self.service.index(self.context))) def test_create_and_show_non_existing_image(self): - - fixture = {'name': 'test image', - 'updated': None, - 'created': None, - 'status': None, - 'instance_id': None, - 'progress': None} - + fixture = self._make_fixture('test image') num_images = len(self.service.index(self.context)) - id = self.service.create(self.context, fixture)['id'] - - self.assertNotEquals(None, id) + image_id = self.service.create(self.context, fixture)['id'] + self.assertNotEquals(None, image_id) self.assertRaises(exception.NotFound, self.service.show, self.context, 'bad image id') def test_update(self): - - fixture = {'name': 'test image', - 'updated': None, - 'created': None, - 'status': None, - 'instance_id': None, - 'progress': None} - - id = self.service.create(self.context, fixture)['id'] - + fixture = self._make_fixture('test image') + image_id = self.service.create(self.context, fixture)['id'] fixture['status'] = 'in progress' - self.service.update(self.context, id, fixture) - new_image_data = self.service.show(self.context, id) + self.service.update(self.context, image_id, fixture) + + new_image_data = self.service.show(self.context, image_id) self.assertEquals('in progress', new_image_data['status']) def test_delete(self): - - fixtures = [ - {'name': 'test image 1', - 'updated': None, - 'created': None, - 'status': None, - 'instance_id': None, - 'progress': None}, - {'name': 'test image 2', - 'updated': None, - 'created': None, - 'status': None, - 'instance_id': None, - 'progress': None}] + fixture1 = self._make_fixture('test image 1') + fixture2 = self._make_fixture('test image 2') + fixtures = [fixture1, fixture2] num_images = len(self.service.index(self.context)) self.assertEquals(0, num_images, str(self.service.index(self.context))) @@ -132,6 +99,22 @@ class BaseImageServiceTests(object): num_images = len(self.service.index(self.context)) self.assertEquals(1, num_images) + def test_index(self): + fixture = self._make_fixture('test image') + image_id = self.service.create(self.context, fixture)['id'] + image_metas = self.service.index(self.context) + expected = [{'id': 'DONTCARE', 'name': 'test image'}] + self.assertDictListMatch(image_metas, expected) + + @staticmethod + def _make_fixture(name): + fixture = {'name': 'test image', + 'updated': None, + 'created': None, + 'status': None, + 'is_public': True} + return fixture + class LocalImageServiceTest(test.TestCase, BaseImageServiceTests): @@ -167,8 +150,17 @@ class LocalImageServiceTest(test.TestCase, class GlanceImageServiceTest(test.TestCase, BaseImageServiceTests): - """Tests the local image service""" + """Tests the Glance image service, in particular that metadata translation + works properly. + + At a high level, the translations involved are: + 1. Glance -> ImageService - This is needed so we can support + multple ImageServices (Glance, Local, etc) + + 2. ImageService -> API - This is needed so we can support multple + APIs (OpenStack, EC2) + """ def setUp(self): super(GlanceImageServiceTest, self).setUp() self.stubs = stubout.StubOutForTesting() @@ -176,41 +168,53 @@ class GlanceImageServiceTest(test.TestCase, fakes.stub_out_compute_api_snapshot(self.stubs) service_class = 'nova.image.glance.GlanceImageService' self.service = utils.import_object(service_class) - self.context = context.RequestContext(None, None) + self.context = context.RequestContext(1, None) self.service.delete_all() + self.sent_to_glance = {} + fakes.stub_out_glance_add_image(self.stubs, self.sent_to_glance) def tearDown(self): self.stubs.UnsetAll() super(GlanceImageServiceTest, self).tearDown() + def test_create_with_instance_id(self): + """Ensure instance_id is persisted as an image-property""" + fixture = {'name': 'test image', + 'is_public': False, + 'properties': {'instance_id': '42', 'user_id': '1'}} -class ImageControllerWithGlanceServiceTest(test.TestCase): + image_id = self.service.create(self.context, fixture)['id'] + expected = fixture + self.assertDictMatch(self.sent_to_glance['metadata'], expected) + + image_meta = self.service.show(self.context, image_id) + expected = {'id': image_id, + 'name': 'test image', + 'is_public': False, + 'properties': {'instance_id': '42', 'user_id': '1'}} + self.assertDictMatch(image_meta, expected) + + image_metas = self.service.detail(self.context) + self.assertDictMatch(image_metas[0], expected) + + def test_create_without_instance_id(self): + """ + Ensure we can create an image without having to specify an + instance_id. Public images are an example of an image not tied to an + instance. + """ + fixture = {'name': 'test image'} + image_id = self.service.create(self.context, fixture)['id'] + + expected = {'name': 'test image', 'properties': {}} + self.assertDictMatch(self.sent_to_glance['metadata'], expected) + +class ImageControllerWithGlanceServiceTest(test.TestCase): """Test of the OpenStack API /images application controller""" - # Registered images at start of each test. - now = datetime.datetime.utcnow() - IMAGE_FIXTURES = [ - {'id': '23g2ogk23k4hhkk4k42l', - 'imageId': '23g2ogk23k4hhkk4k42l', - 'name': 'public image #1', - 'created_at': now.isoformat(), - 'updated_at': now.isoformat(), - 'deleted_at': None, - 'deleted': False, - 'is_public': True, - 'status': 'available', - 'image_type': 'kernel'}, - {'id': 'slkduhfas73kkaskgdas', - 'imageId': 'slkduhfas73kkaskgdas', - 'name': 'public image #2', - 'created_at': now.isoformat(), - 'updated_at': now.isoformat(), - 'deleted_at': None, - 'deleted': False, - 'is_public': True, - 'status': 'available', - 'image_type': 'ramdisk'}] + NOW_GLANCE_FORMAT = "2010-10-11T10:30:22" + NOW_API_FORMAT = "2010-10-11T10:30:22Z" def setUp(self): super(ImageControllerWithGlanceServiceTest, self).setUp() @@ -223,7 +227,8 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): fakes.stub_out_rate_limiting(self.stubs) fakes.stub_out_auth(self.stubs) fakes.stub_out_key_pair_funcs(self.stubs) - fakes.stub_out_glance(self.stubs, initial_fixtures=self.IMAGE_FIXTURES) + fixtures = self._make_image_fixtures() + fakes.stub_out_glance(self.stubs, initial_fixtures=fixtures) def tearDown(self): self.stubs.UnsetAll() @@ -233,34 +238,94 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): def test_get_image_index(self): req = webob.Request.blank('/v1.0/images') res = req.get_response(fakes.wsgi_app()) - res_dict = json.loads(res.body) + image_metas = json.loads(res.body)['images'] - fixture_index = [dict(id=f['id'], name=f['name']) for f - in self.IMAGE_FIXTURES] + expected = [{'id': 123, 'name': 'public image'}, + {'id': 124, 'name': 'queued backup'}, + {'id': 125, 'name': 'saving backup'}, + {'id': 126, 'name': 'active backup'}, + {'id': 127, 'name': 'killed backup'}] - for image in res_dict['images']: - self.assertEquals(1, fixture_index.count(image), - "image %s not in fixture index!" % str(image)) + self.assertDictListMatch(image_metas, expected) def test_get_image_details(self): req = webob.Request.blank('/v1.0/images/detail') res = req.get_response(fakes.wsgi_app()) - res_dict = json.loads(res.body) - - for image in self.IMAGE_FIXTURES: - expected = { - 'id': abs(hash(image['imageId'])), - 'name': image['name'], - 'status': 'active', - } - self.assertTrue(expected in res_dict['images']) - - def test_show_image(self): - expected = self.IMAGE_FIXTURES[0] - id = abs(hash(expected['id'])) - expected_time = self.now.strftime('%Y-%m-%dT%H:%M:%SZ') - req = webob.Request.blank('/v1.0/images/%s' % id) + image_metas = json.loads(res.body)['images'] + + now = self.NOW_API_FORMAT + expected = [ + {'id': 123, 'name': 'public image', 'updated': now, + 'created': now, 'status': 'ACTIVE'}, + {'id': 124, 'name': 'queued backup', 'serverId': 42, + 'updated': now, 'created': now, + 'status': 'QUEUED'}, + {'id': 125, 'name': 'saving backup', 'serverId': 42, + 'updated': now, 'created': now, + 'status': 'SAVING', 'progress': 0}, + {'id': 126, 'name': 'active backup', 'serverId': 42, + 'updated': now, 'created': now, + 'status': 'ACTIVE'}, + {'id': 127, 'name': 'killed backup', 'serverId': 42, + 'updated': now, 'created': now, + 'status': 'FAILED'} + ] + + self.assertDictListMatch(image_metas, expected) + + def test_get_image_found(self): + req = webob.Request.blank('/v1.0/images/123') + res = req.get_response(fakes.wsgi_app()) + image_meta = json.loads(res.body)['image'] + expected = {'id': 123, 'name': 'public image', + 'updated': self.NOW_API_FORMAT, + 'created': self.NOW_API_FORMAT, 'status': 'ACTIVE'} + self.assertDictMatch(image_meta, expected) + + def test_get_image_non_existent(self): + req = webob.Request.blank('/v1.0/images/4242') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 404) + + def test_get_image_not_owned(self): + """We should return a 404 if we request an image that doesn't belong + to us + """ + req = webob.Request.blank('/v1.0/images/128') res = req.get_response(fakes.wsgi_app()) - actual = json.loads(res.body)['image'] - self.assertEqual(expected_time, actual['created_at']) - self.assertEqual(expected_time, actual['updated_at']) + self.assertEqual(res.status_int, 404) + + @classmethod + def _make_image_fixtures(cls): + image_id = 123 + base_attrs = {'created_at': cls.NOW_GLANCE_FORMAT, + 'updated_at': cls.NOW_GLANCE_FORMAT, + 'deleted_at': None, + 'deleted': False} + + fixtures = [] + + def add_fixture(**kwargs): + kwargs.update(base_attrs) + fixtures.append(kwargs) + + # Public image + add_fixture(id=image_id, name='public image', is_public=True, + status='active', properties={}) + image_id += 1 + + # Backup for User 1 + backup_properties = {'instance_id': '42', 'user_id': '1'} + for status in ('queued', 'saving', 'active', 'killed'): + add_fixture(id=image_id, name='%s backup' % status, + is_public=False, status=status, + properties=backup_properties) + image_id += 1 + + # Backup for User 2 + other_backup_properties = {'instance_id': '43', 'user_id': '2'} + add_fixture(id=image_id, name='someone elses backup', is_public=False, + status='active', properties=other_backup_properties) + image_id += 1 + + return fixtures diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py index f1f8504f3..d03aa9cc8 100644 --- a/nova/tests/image/test_glance.py +++ b/nova/tests/image/test_glance.py @@ -19,6 +19,8 @@ import datetime import unittest +from nova import context +from nova import test from nova.image import glance @@ -29,14 +31,14 @@ class StubGlanceClient(object): self.add_response = add_response self.update_response = update_response - def get_image_meta(self, id): - return self.images[id] + def get_image_meta(self, image_id): + return self.images[image_id] def get_images_detailed(self): return self.images.itervalues() - def get_image(self, id): - return self.images[id], [] + def get_image(self, image_id): + return self.images[image_id], [] def add_image(self, metadata, data): return self.add_response @@ -46,143 +48,144 @@ class StubGlanceClient(object): class NullWriter(object): + """Used to test ImageService.get which takes a writer object""" def write(self, *arg, **kwargs): pass -class TestGlanceImageServiceDatetimes(unittest.TestCase): +class BaseGlanceTest(unittest.TestCase): + NOW_GLANCE_FORMAT = "2010-10-11T10:30:22" + NOW_DATETIME = datetime.datetime(2010, 10, 11, 10, 30, 22) def setUp(self): + # FIXME(sirp): we can probably use stubs library here rather than + # dependency injection self.client = StubGlanceClient(None) self.service = glance.GlanceImageService(self.client) + self.context = context.RequestContext(None, None) + def assertDateTimesFilled(self, image_meta): + self.assertEqual(image_meta['created_at'], self.NOW_DATETIME) + self.assertEqual(image_meta['updated_at'], self.NOW_DATETIME) + self.assertEqual(image_meta['deleted_at'], self.NOW_DATETIME) + + def assertDateTimesEmpty(self, image_meta): + self.assertEqual(image_meta['updated_at'], None) + self.assertEqual(image_meta['deleted_at'], None) + + +class TestGlanceImageServiceProperties(BaseGlanceTest): def test_show_passes_through_to_client(self): - self.client.images = {'xyz': {'foo': 'bar'}} - self.assertEqual(self.service.show({}, 'xyz'), {'foo': 'bar'}) + """Ensure attributes which aren't BASE_IMAGE_ATTRS are stored in the + properties dict + """ + fixtures = {'image1': {'name': 'image1', 'is_public': True, + 'foo': 'bar', + 'properties': {'prop1': 'propvalue1'}}} + self.client.images = fixtures + image_meta = self.service.show(self.context, 'image1') + + expected = {'name': 'image1', 'is_public': True, + 'properties': {'prop1': 'propvalue1', 'foo': 'bar'}} + self.assertEqual(image_meta, expected) def test_detail_passes_through_to_client(self): - self.client.images = {1: {'foo': 'bar'}} - self.assertEqual(list(self.service.detail({})), [{'foo': 'bar'}]) - - def test_show_makes_create_datetimes(self): - create_time = datetime.datetime.utcnow() - self.client.images = {'xyz': { - 'id': "id", - 'name': "my awesome image", - 'created_at': create_time.isoformat(), - }} - actual = self.service.show({}, 'xyz') - self.assertEqual(actual['created_at'], create_time) - - def test_show_makes_update_datetimes(self): - update_time = datetime.datetime.utcnow() - self.client.images = {'abc': { - 'id': "id", - 'name': "my okay image", - 'updated_at': update_time.isoformat(), - }} - actual = self.service.show({}, 'abc') - self.assertEqual(actual['updated_at'], update_time) - - def test_show_makes_delete_datetimes(self): - delete_time = datetime.datetime.utcnow() - self.client.images = {'123': { - 'id': "123", - 'name': "my lame image", - 'deleted_at': delete_time.isoformat(), - }} - actual = self.service.show({}, '123') - self.assertEqual(actual['deleted_at'], delete_time) - - def test_show_handles_deleted_at_none(self): - self.client.images = {'747': { - 'id': "747", - 'name': "not deleted", - 'deleted_at': None, - }} - actual = self.service.show({}, '747') - self.assertEqual(actual['deleted_at'], None) - - def test_detail_handles_timestamps(self): - now = datetime.datetime.utcnow() - image1 = { - 'id': 1, - 'name': 'image 1', - 'created_at': now.isoformat(), - 'updated_at': now.isoformat(), - 'deleted_at': None, - } - image2 = { - 'id': 2, - 'name': 'image 2', - 'deleted_at': now.isoformat(), - } - self.client.images = {1: image1, 2: image2} - i1, i2 = self.service.detail({}) - self.assertEqual(i1['created_at'], now) - self.assertEqual(i1['updated_at'], now) - self.assertEqual(i1['deleted_at'], None) - self.assertEqual(i2['deleted_at'], now) - - def test_get_handles_timestamps(self): - now = datetime.datetime.utcnow() - self.client.images = {'abcd': { - 'id': 'abcd', - 'name': 'nifty image', - 'created_at': now.isoformat(), - 'updated_at': now.isoformat(), - 'deleted_at': now.isoformat(), - }} - actual = self.service.get({}, 'abcd', NullWriter()) - for attr in ('created_at', 'updated_at', 'deleted_at'): - self.assertEqual(actual[attr], now) - - def test_get_handles_deleted_at_none(self): - self.client.images = {'abcd': {'deleted_at': None}} - actual = self.service.get({}, 'abcd', NullWriter()) - self.assertEqual(actual['deleted_at'], None) - - def test_create_handles_timestamps(self): - now = datetime.datetime.utcnow() - self.client.add_response = { - 'id': 'abcd', - 'name': 'blah', - 'created_at': now.isoformat(), - 'updated_at': now.isoformat(), - 'deleted_at': now.isoformat(), - } - actual = self.service.create({}, {}) - for attr in ('created_at', 'updated_at', 'deleted_at'): - self.assertEqual(actual[attr], now) - - def test_create_handles_deleted_at_none(self): - self.client.add_response = { - 'id': 'abcd', - 'name': 'blah', - 'deleted_at': None, - } - actual = self.service.create({}, {}) - self.assertEqual(actual['deleted_at'], None) - - def test_update_handles_timestamps(self): - now = datetime.datetime.utcnow() - self.client.update_response = { - 'id': 'abcd', - 'name': 'blah', - 'created_at': now.isoformat(), - 'updated_at': now.isoformat(), - 'deleted_at': now.isoformat(), - } - actual = self.service.update({}, 'dummy_id', {}) - for attr in ('created_at', 'updated_at', 'deleted_at'): - self.assertEqual(actual[attr], now) - - def test_create_handles_deleted_at_none(self): - self.client.update_response = { - 'id': 'abcd', - 'name': 'blah', - 'deleted_at': None, - } - actual = self.service.update({}, 'dummy_id', {}) - self.assertEqual(actual['deleted_at'], None) + fixtures = {'image1': {'name': 'image1', 'is_public': True, + 'foo': 'bar', + 'properties': {'prop1': 'propvalue1'}}} + self.client.images = fixtures + image_meta = self.service.detail(self.context) + expected = [{'name': 'image1', 'is_public': True, + 'properties': {'prop1': 'propvalue1', 'foo': 'bar'}}] + self.assertEqual(image_meta, expected) + + +class TestGetterDateTimeNoneTests(BaseGlanceTest): + + def test_show_handles_none_datetimes(self): + self.client.images = self._make_none_datetime_fixtures() + image_meta = self.service.show(self.context, 'image1') + self.assertDateTimesEmpty(image_meta) + + def test_detail_handles_none_datetimes(self): + self.client.images = self._make_none_datetime_fixtures() + image_meta = self.service.detail(self.context)[0] + self.assertDateTimesEmpty(image_meta) + + def test_get_handles_none_datetimes(self): + self.client.images = self._make_none_datetime_fixtures() + writer = NullWriter() + image_meta = self.service.get(self.context, 'image1', writer) + self.assertDateTimesEmpty(image_meta) + + def test_show_makes_datetimes(self): + self.client.images = self._make_datetime_fixtures() + image_meta = self.service.show(self.context, 'image1') + self.assertDateTimesFilled(image_meta) + + def test_detail_makes_datetimes(self): + self.client.images = self._make_datetime_fixtures() + image_meta = self.service.detail(self.context)[0] + self.assertDateTimesFilled(image_meta) + + def test_get_makes_datetimes(self): + self.client.images = self._make_datetime_fixtures() + writer = NullWriter() + image_meta = self.service.get(self.context, 'image1', writer) + self.assertDateTimesFilled(image_meta) + + def _make_datetime_fixtures(self): + fixtures = {'image1': {'name': 'image1', 'is_public': True, + 'created_at': self.NOW_GLANCE_FORMAT, + 'updated_at': self.NOW_GLANCE_FORMAT, + 'deleted_at': self.NOW_GLANCE_FORMAT}} + return fixtures + + def _make_none_datetime_fixtures(self): + fixtures = {'image1': {'name': 'image1', 'is_public': True, + 'updated_at': None, + 'deleted_at': None}} + return fixtures + + +class TestMutatorDateTimeTests(BaseGlanceTest): + """Tests create(), update()""" + + def test_create_handles_datetimes(self): + self.client.add_response = self._make_datetime_fixture() + image_meta = self.service.create(self.context, {}) + self.assertDateTimesFilled(image_meta) + + def test_create_handles_none_datetimes(self): + self.client.add_response = self._make_none_datetime_fixture() + dummy_meta = {} + image_meta = self.service.create(self.context, dummy_meta) + self.assertDateTimesEmpty(image_meta) + + def test_update_handles_datetimes(self): + 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) + self.assertDateTimesFilled(image_meta) + + def test_update_handles_none_datetimes(self): + 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) + self.assertDateTimesEmpty(image_meta) + + def _make_datetime_fixture(self): + fixture = {'id': 'image1', 'name': 'image1', 'is_public': True, + 'created_at': self.NOW_GLANCE_FORMAT, + 'updated_at': self.NOW_GLANCE_FORMAT, + 'deleted_at': self.NOW_GLANCE_FORMAT} + return fixture + + def _make_none_datetime_fixture(self): + fixture = {'id': 'image1', 'name': 'image1', 'is_public': True, + 'updated_at': None, + 'deleted_at': None} + return fixture diff --git a/nova/utils.py b/nova/utils.py index fa01f2c94..3f6f9fc8a 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -665,6 +665,48 @@ def get_from_path(items, path): return get_from_path(results, remainder) +def flatten_dict(dict_, flattened=None): + """Recursively flatten a nested dictionary""" + flattened = flattened or {} + for key, value in dict_.iteritems(): + if hasattr(value, 'iteritems'): + flatten_dict(value, flattened) + else: + flattened[key] = value + return flattened + + +def partition_dict(dict_, keys): + """Return two dicts, one containing only `keys` the other containing + everything but `keys` + """ + intersection = {} + difference = {} + for key, value in dict_.iteritems(): + if key in keys: + intersection[key] = value + else: + difference[key] = value + return intersection, difference + + +def map_dict_keys(dict_, key_map): + """Return a dictionary in which the dictionaries keys are mapped to + new keys. + """ + mapped = {} + for key, value in dict_.iteritems(): + mapped_key = key_map[key] if key in key_map else key + mapped[mapped_key] = value + return mapped + + +def subset_dict(dict_, keys): + """Return a dict that only contains a subset of keys""" + subset = partition_dict(dict_, keys)[0] + return subset + + def check_isinstance(obj, cls): """Checks that obj is of type cls, and lets PyLint infer types""" if isinstance(obj, cls): |