From 6d30c69895fe00cbfec90ba3c551893e0ac72a70 Mon Sep 17 00:00:00 2001 From: Andrey Pavlov Date: Tue, 28 Mar 2017 07:22:49 +0300 Subject: [PATCH] use glanceclient version '2'. fix keystone error message. current devstack deploys glance version '2' only. use it. messages in keystone's exceptions was changed. fix unit tests that checks it. Change-Id: I7ed1f0ff518efa374a5e3b693c5785958c77340d --- devstack/create_config | 2 +- ec2api/api/ec2utils.py | 10 +- ec2api/api/image.py | 98 ++++++----- ec2api/api/instance.py | 2 +- ec2api/clients.py | 2 +- ec2api/tests/functional/api/test_images.py | 4 +- ec2api/tests/unit/base.py | 5 +- ec2api/tests/unit/fakes.py | 128 +++++++-------- ec2api/tests/unit/test_api_init.py | 2 +- ec2api/tests/unit/test_clients.py | 2 +- ec2api/tests/unit/test_image.py | 154 +++++++++--------- ec2api/tests/unit/test_instance.py | 35 ++-- ec2api/tests/unit/test_integrated_scenario.py | 14 +- 13 files changed, 224 insertions(+), 234 deletions(-) diff --git a/devstack/create_config b/devstack/create_config index a0d727f1..7f6344a0 100755 --- a/devstack/create_config +++ b/devstack/create_config @@ -126,7 +126,7 @@ auth="--os-project-name $project_name --os-username $user_name --os-password pas volume_status() { cinder $auth show $1|awk '/ status / {print $4}'; } instance_status() { nova $auth show $1|awk '/ status / {print $4}'; } -openstack_image_id=$(openstack $auth image list --long | grep "cirros" | grep " ami " | head -1 | awk '{print $2}') +openstack_image_id=$(openstack $auth image list --long | grep "cirros" | grep " bare " | head -1 | awk '{print $2}') if [[ -n "$openstack_image_id" ]]; then volume_id=$(cinder $auth create --image-id $openstack_image_id 1 | awk '/ id / {print $4}') [[ -n "$volume_id" ]] || { echo "can't create volume for EBS image creation"; exit 1; } diff --git a/ec2api/api/ec2utils.py b/ec2api/api/ec2utils.py index 08318553..cda33d63 100644 --- a/ec2api/api/ec2utils.py +++ b/ec2api/api/ec2utils.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy import datetime import json import re @@ -373,13 +372,14 @@ def get_os_image(context, ec2_image_id): def deserialize_os_image_properties(os_image): def prepare_property(property_name): - if property_name in properties: - properties[property_name] = json.loads(properties[property_name]) + if property_name in os_image_dict: + os_image_dict[property_name] = json.loads( + os_image_dict[property_name]) - properties = copy.copy(os_image.properties) + os_image_dict = dict(os_image) prepare_property('mappings') prepare_property('block_device_mapping') - return properties + return os_image_dict def create_virtual_bdm(device_name, virtual_name): diff --git a/ec2api/api/image.py b/ec2api/api/image.py index cbe276ea..ae553496 100644 --- a/ec2api/api/image.py +++ b/ec2api/api/image.py @@ -213,18 +213,17 @@ def register_image(context, name=None, image_location=None, raise exception.MissingParameter(msg) # TODO(ft): check parameters - properties = {} - metadata = {'properties': properties} + metadata = {} if name: # TODO(ft): check the name is unique (at least for EBS image case) metadata['name'] = name if image_location: - properties['image_location'] = image_location + metadata['image_location'] = image_location if 'name' not in metadata: # NOTE(ft): it's needed for backward compatibility metadata['name'] = image_location if root_device_name: - properties['root_device_name'] = root_device_name + metadata['root_device_name'] = root_device_name cinder = clients.cinder(context) if block_device_mapping: mappings = instance_api._parse_block_device_mapping( @@ -246,27 +245,28 @@ def register_image(context, name=None, image_location=None, bdm['volume_size'] = volume.size except cinder_exception.NotFound: pass - properties['bdm_v2'] = True - properties['block_device_mapping'] = json.dumps(mappings) + metadata['bdm_v2'] = 'True' + metadata['block_device_mapping'] = json.dumps(mappings) if architecture is not None: - properties['architecture'] = architecture + metadata['architecture'] = architecture if kernel_id: - properties['kernel_id'] = ec2utils.get_os_image(context, + metadata['kernel_id'] = ec2utils.get_os_image(context, kernel_id).id if ramdisk_id: - properties['ramdisk_id'] = ec2utils.get_os_image(context, + metadata['ramdisk_id'] = ec2utils.get_os_image(context, ramdisk_id).id with common.OnCrashCleaner() as cleaner: - if 'image_location' in properties: + glance = clients.glance(context) + if 'image_location' in metadata: os_image = _s3_create(context, metadata) else: - metadata.update({'size': 0, - 'is_public': False}) - # TODO(ft): set default values of image properties - glance = clients.glance(context) + metadata.update({'visibility': 'private', + 'container_format': 'bare', + 'disk_format': 'raw'}) os_image = glance.images.create(**metadata) - cleaner.addCleanup(os_image.delete) + glance.images.upload(os_image.id, '', image_size=0) + cleaner.addCleanup(glance.images.delete, os_image.id) kind = _get_os_image_kind(os_image) image = db_api.add_item(context, kind, {'os_id': os_image.id, 'is_public': False, @@ -366,9 +366,9 @@ class ImageDescriber(common.TaggableItemsDescriber): def get_os_items(self): os_images = list(clients.glance(self.context).images.list()) self.ec2_created_os_images = { - os_image.properties['ec2_id']: os_image + os_image.ec2_id: os_image for os_image in os_images - if (os_image.properties.get('ec2_id') and + if (hasattr(os_image, 'ec2_id') and self.context.project_id == os_image.owner)} return os_images @@ -376,12 +376,12 @@ class ImageDescriber(common.TaggableItemsDescriber): if not image: kind = _get_os_image_kind(os_image) if self.context.project_id == os_image.owner: - if os_image.properties.get('ec2_id') in self.pending_images: + if getattr(os_image, 'ec2_id', None) in self.pending_images: # NOTE(ft): the image is being creating, Glance had created # image, but creating thread doesn't yet update db item - image = self.pending_images[os_image.properties['ec2_id']] + image = self.pending_images[os_image.ec2_id] image['os_id'] = os_image.id - image['is_public'] = os_image.is_public + image['is_public'] = os_image.visibility == 'public' db_api.update_item(self.context, image) else: image = ec2utils.get_db_item_by_os_id( @@ -394,8 +394,8 @@ class ImageDescriber(common.TaggableItemsDescriber): image = {'id': image_id, 'os_id': os_image.id} elif (self.context.project_id == os_image.owner and - image.get('is_public') != os_image.is_public): - image['is_public'] = os_image.is_public + image.get('is_public') != os_image.visibility == 'public'): + image['is_public'] = os_image.visibility == 'public' if image['id'] in self.local_images_os_ids: db_api.update_item(self.context, image) else: @@ -430,7 +430,7 @@ class ImageDescriber(common.TaggableItemsDescriber): os_image = self.ec2_created_os_images.get(item['id']) if os_image: item['os_id'] = os_image.id - item['is_public'] = os_image.is_public + item['is_public'] = os_image.visibility == 'public' db_api.update_item(self.context, item) image = self.format(item, os_image) else: @@ -468,18 +468,18 @@ def describe_image_attribute(context, image_id, attribute): def _launch_permission_attribute(os_image, image, result): result['launchPermission'] = [] - if os_image.is_public: + if os_image.visibility == 'public': result['launchPermission'].append({'group': 'all'}) def _kernel_attribute(os_image, image, result): - kernel_id = os_image.properties.get('kernel_id') + kernel_id = getattr(os_image, 'kernel_id', None) if kernel_id: result['kernel'] = { 'value': ec2utils.os_id_to_ec2_id(context, 'aki', kernel_id) } def _ramdisk_attribute(os_image, image, result): - ramdisk_id = os_image.properties.get('ramdisk_id') + ramdisk_id = getattr(os_image, 'ramdisk_id', None) if ramdisk_id: result['ramdisk'] = { 'value': ec2utils.os_id_to_ec2_id(context, 'ari', ramdisk_id) @@ -589,7 +589,9 @@ def modify_image_attribute(context, image_id, attribute=None, reason=msg) _check_owner(context, os_image) - os_image.update(is_public=(operation_type == 'add')) + glance = clients.glance(context) + visibility = 'public' if operation_type == 'add' else 'private' + glance.images.update(os_image.id, visibility=visibility) return True if 'description' in attributes: @@ -610,8 +612,8 @@ def reset_image_attribute(context, image_id, attribute): os_image = ec2utils.get_os_image(context, image_id) _check_owner(context, os_image) - - os_image.update(is_public=False) + glance = clients.glance(context) + glance.images.update(os_image.id, visibility='private') return True @@ -627,8 +629,8 @@ def _format_image(context, image, os_image, images_dict, ids_dict, 'imageOwnerId': os_image.owner, 'imageType': IMAGE_TYPES[ ec2utils.get_ec2_id_kind(image['id'])], - 'isPublic': os_image.is_public, - 'architecture': os_image.properties.get('architecture'), + 'isPublic': os_image.visibility == 'public', + 'architecture': getattr(os_image, 'architecture', None), 'creationDate': os_image.created_at } if 'description' in image: @@ -638,23 +640,23 @@ def _format_image(context, image, os_image, images_dict, ids_dict, else: state = GLANCE_STATUS_TO_EC2.get(os_image.status, 'error') if state in ('available', 'pending'): - state = _s3_image_state_map.get(os_image.properties.get('image_state'), + state = _s3_image_state_map.get(getattr(os_image, 'image_state', None), state) ec2_image['imageState'] = state - kernel_id = os_image.properties.get('kernel_id') + kernel_id = getattr(os_image, 'kernel_id', None) if kernel_id: ec2_image['kernelId'] = ec2utils.os_id_to_ec2_id( context, 'aki', kernel_id, items_by_os_id=images_dict, ids_by_os_id=ids_dict) - ramdisk_id = os_image.properties.get('ramdisk_id') + ramdisk_id = getattr(os_image, 'ramdisk_id', None) if ramdisk_id: ec2_image['ramdiskId'] = ec2utils.os_id_to_ec2_id( context, 'ari', ramdisk_id, items_by_os_id=images_dict, ids_by_os_id=ids_dict) name = os_image.name - img_loc = os_image.properties.get('image_location') + img_loc = getattr(os_image, 'image_location', None) if img_loc: ec2_image['imageLocation'] = img_loc else: @@ -783,7 +785,7 @@ def _get_os_image_kind(os_image): def _auto_create_image_extension(context, image, os_image): - image['is_public'] = os_image.is_public + image['is_public'] = os_image.visibility == 'public' ec2utils.register_auto_create_db_item_extension( @@ -810,7 +812,7 @@ _s3_image_state_map = {'downloading': 'pending', def _s3_create(context, metadata): """Gets a manifest from s3 and makes an image.""" - image_location = metadata['properties']['image_location'].lstrip('/') + image_location = metadata['image_location'].lstrip('/') bucket_name = image_location.split('/')[0] manifest_path = image_location[len(bucket_name) + 1:] bucket = _s3_conn(context).get_bucket(bucket_name) @@ -819,12 +821,9 @@ def _s3_create(context, metadata): (image_metadata, image_parts, encrypted_key, encrypted_iv) = _s3_parse_manifest(context, manifest) - properties = metadata['properties'] - properties.update(image_metadata['properties']) - properties['image_state'] = 'pending' metadata.update(image_metadata) - metadata.update({'properties': properties, - 'is_public': False}) + metadata.update({'image_state': 'pending', + 'visibility': 'private'}) # TODO(bcwaldon): right now, this removes user-defined ids # We need to re-enable this. @@ -834,7 +833,7 @@ def _s3_create(context, metadata): image = glance.images.create(**metadata) def _update_image_state(image_state): - image.update(properties={'image_state': image_state}) + glance.images.update(image.id, image_state=image_state) def delayed_create(): """This handles the fetching and decrypting of the part files.""" @@ -887,7 +886,7 @@ def _s3_create(context, metadata): _update_image_state('uploading') try: with open(unz_filename) as image_file: - image.update(data=image_file) + glance.images.upload(image.id, image_file) except Exception: LOG.exception(_LE('Failed to upload %(image_location)s ' 'to %(image_path)s'), log_vars) @@ -916,7 +915,7 @@ def _s3_parse_manifest(context, manifest): except Exception: arch = 'x86_64' - properties = {'architecture': arch} + metadata = {'architecture': arch} mappings = [] try: @@ -930,7 +929,7 @@ def _s3_parse_manifest(context, manifest): mappings = [] if mappings: - properties['mappings'] = mappings + metadata['mappings'] = mappings def set_dependent_image_id(image_key): try: @@ -942,7 +941,7 @@ def _s3_parse_manifest(context, manifest): if image_id == 'true': return True os_image = ec2utils.get_os_image(context, image_id) - properties[image_key] = os_image.id + metadata[image_key] = os_image.id image_format = 'ami' if set_dependent_image_id('kernel_id'): @@ -950,9 +949,8 @@ def _s3_parse_manifest(context, manifest): if set_dependent_image_id('ramdisk_id'): image_format = 'ari' - metadata = {'disk_format': image_format, - 'container_format': image_format, - 'properties': properties} + metadata.update({'disk_format': image_format, + 'container_format': image_format}) image_parts = [ fn_element.text for fn_element in manifest.find('image').getiterator('filename')] diff --git a/ec2api/api/instance.py b/ec2api/api/instance.py index 49697cdc..dc3e9350 100644 --- a/ec2api/api/instance.py +++ b/ec2api/api/instance.py @@ -1518,7 +1518,7 @@ def _cloud_get_image_state(image): state = image.status if state == 'active': state = 'available' - return image.properties.get('image_state', state) + return getattr(image, 'image_state', state) def _cloud_format_kernel_id(context, os_instance, image_ids=None): diff --git a/ec2api/clients.py b/ec2api/clients.py index b21b25a4..b59e6f10 100644 --- a/ec2api/clients.py +++ b/ec2api/clients.py @@ -117,7 +117,7 @@ def neutron(context): def glance(context): - return glanceclient.Client('1', service_type='image', + return glanceclient.Client(version='2', service_type='image', session=context.session) diff --git a/ec2api/tests/functional/api/test_images.py b/ec2api/tests/functional/api/test_images.py index dafb2c46..0ab566c5 100644 --- a/ec2api/tests/functional/api/test_images.py +++ b/ec2api/tests/functional/api/test_images.py @@ -218,11 +218,11 @@ class ImageTest(base.EC2TestCase): data = self.client.describe_image_attribute( ImageId=image_id, Attribute='kernel') - self.assertIn('KernelId', data) + self.assertNotIn('KernelId', data) data = self.client.describe_image_attribute( ImageId=image_id, Attribute='ramdisk') - self.assertIn('RamdiskId', data) + self.assertNotIn('RamdiskId', data) # description data = self.client.describe_image_attribute( diff --git a/ec2api/tests/unit/base.py b/ec2api/tests/unit/base.py index e8f7cae8..113c6efa 100644 --- a/ec2api/tests/unit/base.py +++ b/ec2api/tests/unit/base.py @@ -111,8 +111,9 @@ class MockOSMixin(object): def mock_glance(self): glance_patcher = mock.patch('glanceclient.client.Client') - glance = mock.create_autospec(glanceclient.Client(endpoint='v1')) - glance_patcher.start().return_value = glance + with mock.patch('glanceclient.v2.schemas.Controller'): + glance = mock.create_autospec(glanceclient.Client(endpoint='v2')) + glance_patcher.start().return_value = glance self.addCleanup(glance_patcher.stop) return glance diff --git a/ec2api/tests/unit/fakes.py b/ec2api/tests/unit/fakes.py index dec7b4b8..ac87b024 100644 --- a/ec2api/tests/unit/fakes.py +++ b/ec2api/tests/unit/fakes.py @@ -1597,31 +1597,33 @@ class OSImage(object): def __init__(self, image_dict, from_get=False): - def set_attr(attr): - if not from_get or image_dict.get(attr) is not None: - setattr(self, attr, image_dict.get(attr)) - - self.id = image_dict['id'] - set_attr('owner') - set_attr('created_at') - set_attr('is_public') - set_attr('status') - set_attr('container_format') - set_attr('name') - self.properties = copy.deepcopy(image_dict.get('properties', {})) + if from_get: + attrs = [k for k in image_dict.keys() + if image_dict[k] is not None] + else: + attrs = list(image_dict) + attrs.extend( + ['owner', 'created_at', 'visibility', 'status', + 'container_format', 'name']) + self._image_dict = {'id': image_dict['id']} + self._image_dict.update({k: image_dict.get(k) + for k in attrs}) for complex_attr in ('mappings', 'block_device_mapping'): - if complex_attr in self.properties: - self.properties[complex_attr] = ( - json.dumps(self.properties[complex_attr])) + if complex_attr in self._image_dict: + self._image_dict[complex_attr] = json.dumps( + self._image_dict[complex_attr]) + for k in self._image_dict: + setattr(self, k, self._image_dict[k]) def __eq__(self, other): return type(self) == type(other) and self.__dict__ == other.__dict__ - def update(self, **kwargs): - pass + def __iter__(self): + for key in self._image_dict.items(): + yield key - def delete(self): - pass + def __getitem__(self, key): + return self._image_dict.get(key) TIME_CREATE_IMAGE = timeutils.isotime(None, True) @@ -1709,63 +1711,59 @@ OS_IMAGE_1 = { 'id': ID_OS_IMAGE_1, 'owner': ID_OS_PROJECT, 'created_at': TIME_CREATE_IMAGE, - 'is_public': False, + 'visibility': 'private', 'status': 'active', 'container_format': 'ami', 'name': 'fake_name', - 'properties': { - 'kernel_id': ID_OS_IMAGE_AKI_1, - 'ramdisk_id': ID_OS_IMAGE_ARI_1, - 'type': 'machine', - 'image_state': 'available', - 'image_location': LOCATION_IMAGE_1, - 'mappings': [ - {'device': '/dev/sda1', 'virtual': 'root'}, - {'device': 'sdb0', 'virtual': 'ephemeral0'}, - {'device': 'sdb1', 'virtual': 'ephemeral1'}, - {'device': 'sdb2', 'virtual': 'ephemeral2'}, - {'device': 'sdb3', 'virtual': 'ephemeral3'}, - {'device': 'sdb4', 'virtual': 'ephemeral4'}, - {'device': 'sdc0', 'virtual': 'swap'}, - {'device': 'sdc1', 'virtual': 'swap'}, - {'device': 'sdc2', 'virtual': 'swap'}, - {'device': 'sdc3', 'virtual': 'swap'}, - {'device': 'sdc4', 'virtual': 'swap'}], - 'block_device_mapping': [ - {'device_name': '/dev/sdb1', - 'snapshot_id': ID_OS_SNAPSHOT_1, - 'volume_size': 22}, - {'device_name': '/dev/sdb2', - 'volume_id': ID_OS_VOLUME_1}, - {'device_name': '/dev/sdb3', 'virtual_name': 'ephemeral5'}, - {'device_name': '/dev/sdb4', 'no_device': True}, - {'device_name': '/dev/sdc1', - 'snapshot_id': ID_OS_SNAPSHOT_2}, - {'device_name': '/dev/sdc2', - 'volume_id': ID_OS_VOLUME_2}, - {'device_name': '/dev/sdc3', 'virtual_name': 'ephemeral6'}, - {'device_name': '/dev/sdc4', 'no_device': True}], - } + 'kernel_id': ID_OS_IMAGE_AKI_1, + 'ramdisk_id': ID_OS_IMAGE_ARI_1, + 'type': 'machine', + 'image_state': 'available', + 'image_location': LOCATION_IMAGE_1, + 'mappings': [ + {'device': '/dev/sda1', 'virtual': 'root'}, + {'device': 'sdb0', 'virtual': 'ephemeral0'}, + {'device': 'sdb1', 'virtual': 'ephemeral1'}, + {'device': 'sdb2', 'virtual': 'ephemeral2'}, + {'device': 'sdb3', 'virtual': 'ephemeral3'}, + {'device': 'sdb4', 'virtual': 'ephemeral4'}, + {'device': 'sdc0', 'virtual': 'swap'}, + {'device': 'sdc1', 'virtual': 'swap'}, + {'device': 'sdc2', 'virtual': 'swap'}, + {'device': 'sdc3', 'virtual': 'swap'}, + {'device': 'sdc4', 'virtual': 'swap'}], + 'block_device_mapping': [ + {'device_name': '/dev/sdb1', + 'snapshot_id': ID_OS_SNAPSHOT_1, + 'volume_size': 22}, + {'device_name': '/dev/sdb2', + 'volume_id': ID_OS_VOLUME_1}, + {'device_name': '/dev/sdb3', 'virtual_name': 'ephemeral5'}, + {'device_name': '/dev/sdb4', 'no_device': True}, + {'device_name': '/dev/sdc1', + 'snapshot_id': ID_OS_SNAPSHOT_2}, + {'device_name': '/dev/sdc2', + 'volume_id': ID_OS_VOLUME_2}, + {'device_name': '/dev/sdc3', 'virtual_name': 'ephemeral6'}, + {'device_name': '/dev/sdc4', 'no_device': True}] } OS_IMAGE_2 = { 'id': ID_OS_IMAGE_2, 'owner': ID_OS_PROJECT, 'created_at': TIME_CREATE_IMAGE, - 'is_public': True, + 'visibility': 'public', 'status': 'active', 'container_format': None, 'name': None, - 'properties': { - 'type': 'machine', - 'root_device_name': '/dev/sdb1', - 'architecture': 'x86_64', - 'mappings': [{'device': '/dev/sda1', - 'virtual': 'root'}], - 'block_device_mapping': [ - {'device_name': '/dev/sdb1', - 'snapshot_id': ID_OS_SNAPSHOT_1, - 'delete_on_termination': True}], - } + 'type': 'machine', + 'root_device_name': '/dev/sdb1', + 'architecture': 'x86_64', + 'mappings': [{'device': '/dev/sda1', + 'virtual': 'root'}], + 'block_device_mapping': [ + {'device_name': '/dev/sdb1', + 'snapshot_id': ID_OS_SNAPSHOT_1, + 'delete_on_termination': True}], } OS_IMAGE_AKI_1 = { 'id': ID_OS_IMAGE_AKI_1, diff --git a/ec2api/tests/unit/test_api_init.py b/ec2api/tests/unit/test_api_init.py index f8e13915..c9b44143 100644 --- a/ec2api/tests/unit/test_api_init.py +++ b/ec2api/tests/unit/test_api_init.py @@ -99,6 +99,6 @@ class ApiInitTestCase(base.BaseTestCase): do_check(neutron_exception.BadRequest(message='fake_msg'), 400, 'BadRequest', 'fake_msg') do_check(keystone_exception.BadRequest(message='fake_msg'), - 400, 'BadRequest', 'fake_msg') + 400, 'BadRequest', 'fake_msg (HTTP 400)') do_check(boto_exception.S3ResponseError(400, '', 'fake_msg'), 400, 'S3ResponseError', 'fake_msg') diff --git a/ec2api/tests/unit/test_clients.py b/ec2api/tests/unit/test_clients.py index 1ba92dff..a8754f04 100644 --- a/ec2api/tests/unit/test_clients.py +++ b/ec2api/tests/unit/test_clients.py @@ -107,7 +107,7 @@ class ClientsTestCase(base.BaseTestCase): context = mock.NonCallableMock(session=mock.sentinel.session) res = clients.glance(context) self.assertEqual(glance.return_value, res) - glance.assert_called_with('1', service_type='image', + glance.assert_called_with(version='2', service_type='image', session=mock.sentinel.session) @mock.patch('cinderclient.client.Client') diff --git a/ec2api/tests/unit/test_image.py b/ec2api/tests/unit/test_image.py index 7bd2b6d6..7dce03c5 100644 --- a/ec2api/tests/unit/test_image.py +++ b/ec2api/tests/unit/test_image.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy import json import os import tempfile @@ -179,7 +178,7 @@ class ImageTestCase(base.ApiTestCase): s3_create.assert_called_once_with( mock.ANY, {'name': fakes.LOCATION_IMAGE_1, - 'properties': {'image_location': fakes.LOCATION_IMAGE_1}}) + 'image_location': fakes.LOCATION_IMAGE_1}) s3_create.reset_mock() resp = self.execute( @@ -192,12 +191,14 @@ class ImageTestCase(base.ApiTestCase): s3_create.assert_called_once_with( mock.ANY, {'name': 'an image name', - 'properties': {'image_location': fakes.LOCATION_IMAGE_1}}) + 'image_location': fakes.LOCATION_IMAGE_1}) @mock.patch('ec2api.api.ec2utils.get_os_image') def test_register_image_by_bdm(self, get_os_image): self.glance.images.create.return_value = ( fakes.OSImage(fakes.OS_IMAGE_2)) + self.glance.images.upload.return_value = ( + fakes.OSImage(fakes.OS_IMAGE_2)) self.cinder.volume_snapshots.get.side_effect = ( tools.get_by_1st_arg_getter( {fakes.ID_OS_SNAPSHOT_1: ( @@ -235,21 +236,19 @@ class ImageTestCase(base.ApiTestCase): 'description': None}) self.assertEqual(1, self.glance.images.create.call_count) self.assertEqual((), self.glance.images.create.call_args[0]) - self.assertIn('properties', self.glance.images.create.call_args[1]) self.assertIsInstance( - self.glance.images.create.call_args[1]['properties'], - dict) - bdm = self.glance.images.create.call_args[1]['properties'].pop( + self.glance.images.create.call_args[1], dict) + bdm = self.glance.images.create.call_args[1].pop( 'block_device_mapping', 'null') self.assertEqual( - {'is_public': False, - 'size': 0, + {'visibility': 'private', 'name': 'fake_name', - 'properties': { - 'root_device_name': fakes.ROOT_DEVICE_NAME_IMAGE_2, - 'kernel_id': fakes.ID_OS_IMAGE_AKI_1, - 'ramdisk_id': fakes.ID_OS_IMAGE_ARI_1, - 'bdm_v2': True}}, + 'kernel_id': fakes.ID_OS_IMAGE_AKI_1, + 'ramdisk_id': fakes.ID_OS_IMAGE_ARI_1, + 'root_device_name': fakes.ROOT_DEVICE_NAME_IMAGE_2, + 'container_format': 'bare', + 'disk_format': 'raw', + 'bdm_v2': 'True'}, self.glance.images.create.call_args[1]) self.assertEqual([{'boot_index': 0, 'delete_on_termination': True, @@ -438,8 +437,7 @@ class ImageTestCase(base.ApiTestCase): {'ImageId': image_id, 'Attribute': 'kernel'}) - @mock.patch.object(fakes.OSImage, 'update', autospec=True) - def test_modify_image_attributes(self, osimage_update): + def test_modify_image_attributes(self): self._setup_model() resp = self.execute('ModifyImageAttribute', @@ -448,10 +446,8 @@ class ImageTestCase(base.ApiTestCase): 'operationType': 'add', 'userGroup.1': 'all'}) self.assertThat(resp, matchers.DictMatches({'return': True})) - osimage_update.assert_called_once_with( - mock.ANY, is_public=True) - self.assertEqual(fakes.ID_OS_IMAGE_1, - osimage_update.call_args[0][0].id) + self.glance.images.update.assert_called_once_with( + fakes.ID_OS_IMAGE_1, visibility='public') def test_modify_image_attributes_invalid_parameters(self): image_id = fakes.random_ec2_id('ami') @@ -487,10 +483,16 @@ class ImagePrivateTestCase(base.BaseTestCase): image_ids = {fakes.ID_OS_IMAGE_1: fakes.ID_EC2_IMAGE_1, fakes.ID_OS_IMAGE_AKI_1: fakes.ID_EC2_IMAGE_AKI_1, fakes.ID_OS_IMAGE_ARI_1: fakes.ID_EC2_IMAGE_ARI_1} - os_image = copy.deepcopy(fakes.OS_IMAGE_1) + os_image = {'id': fakes.ID_OS_IMAGE_1, + 'owner': fakes.ID_OS_PROJECT, + 'created_at': fakes.TIME_CREATE_IMAGE, + 'visibility': 'private', + 'status': 'active', + 'container_format': 'ami', + 'name': 'fake_name'} # check name and location attributes for an unnamed image - os_image['properties'] = {'image_location': 'location'} + os_image['image_location'] = 'location' os_image['name'] = None image = image_api._format_image( @@ -501,7 +503,7 @@ class ImagePrivateTestCase(base.BaseTestCase): self.assertEqual('location', image['name']) # check name and location attributes for complete image - os_image['properties'] = {} + os_image['image_location'] = None os_image['name'] = 'fake_name' image = image_api._format_image( @@ -512,14 +514,13 @@ class ImagePrivateTestCase(base.BaseTestCase): self.assertEqual('fake_name', image['name']) # check ebs image type for bdm_v2 mapping type - os_image['properties'] = { - 'bdm_v2': True, - 'root_device_name': '/dev/vda', - 'block_device_mapping': [ - {'boot_index': 0, - 'snapshot_id': fakes.ID_OS_SNAPSHOT_2, - 'source_type': 'snapshot', - 'destination_type': 'volume'}]} + os_image['bdm_v2'] = True + os_image['root_device_name'] = '/dev/vda' + os_image['block_device_mapping'] = [ + {'boot_index': 0, + 'snapshot_id': fakes.ID_OS_SNAPSHOT_2, + 'source_type': 'snapshot', + 'destination_type': 'volume'}] image = image_api._format_image( 'fake_context', fakes.DB_IMAGE_1, fakes.OSImage(os_image), @@ -529,7 +530,9 @@ class ImagePrivateTestCase(base.BaseTestCase): self.assertEqual('ebs', image['rootDeviceType']) # check instance-store image attributes with no any device mappings - os_image['properties'] = {'root_device_name': '/dev/vda'} + os_image['bdm_v2'] = False + os_image['root_device_name'] = '/dev/vda' + os_image['block_device_mapping'] = [] image = image_api._format_image( 'fake_context', fakes.DB_IMAGE_1, fakes.OSImage(os_image), None, None) @@ -559,7 +562,7 @@ class ImagePrivateTestCase(base.BaseTestCase): os_image.status = 'queued' def check_state_translation(state, expected): - os_image.properties['image_state'] = state + os_image.image_state = state image = image_api._format_image( 'fake_context', fakes.DB_IMAGE_1, os_image, None, None) self.assertEqual(expected, image['imageState'], @@ -741,8 +744,8 @@ class ImagePrivateTestCase(base.BaseTestCase): os_image = {'id': os_image_id, 'owner': fakes.ID_OS_PROJECT, 'status': 'active', - 'is_public': False, - 'properties': {'ec2_id': image_id}} + 'visibility': 'private', + 'ec2_id': image_id} glance.images.list.return_value = [fakes.OSImage(os_image)] expected['imagesSet'] = [{ 'architecture': None, @@ -788,14 +791,14 @@ class S3TestCase(base.BaseTestCase): expected_metadata = { 'disk_format': 'ami', 'container_format': 'ami', - 'properties': {'architecture': 'x86_64', - 'kernel_id': fakes.ID_OS_IMAGE_AKI_1, - 'ramdisk_id': fakes.ID_OS_IMAGE_ARI_1, - 'mappings': [ - {"device": "sda1", "virtual": "ami"}, - {"device": "/dev/sda1", "virtual": "root"}, - {"device": "sda2", "virtual": "ephemeral0"}, - {"device": "sda3", "virtual": "swap"}]}} + 'architecture': 'x86_64', + 'kernel_id': fakes.ID_OS_IMAGE_AKI_1, + 'ramdisk_id': fakes.ID_OS_IMAGE_ARI_1, + 'mappings': [ + {"device": "sda1", "virtual": "ami"}, + {"device": "/dev/sda1", "virtual": "root"}, + {"device": "sda2", "virtual": "ephemeral0"}, + {"device": "sda3", "virtual": "swap"}]} self.assertThat(metadata, matchers.DictMatches(expected_metadata, orderless_lists=True)) @@ -810,8 +813,7 @@ class S3TestCase(base.BaseTestCase): mock.ANY, 'ari', item_ids=(fakes.ID_EC2_IMAGE_ARI_1,), item_os_ids=None) - @mock.patch.object(fakes.OSImage, 'update', autospec=True) - def test_s3_create_image_locations(self, osimage_update): + def test_s3_create_image_locations(self): self.configure(image_decryption_dir=None) glance = self.mock_glance() _handle, tempf = tempfile.mkstemp() @@ -829,24 +831,23 @@ class S3TestCase(base.BaseTestCase): get_contents_as_string.return_value) = FILE_MANIFEST_XML s3_download_file.return_value = tempf s3_untarzip_image.return_value = tempf + os_image_id = fakes.random_os_id() (glance.images.create.return_value) = ( - fakes.OSImage({'id': fakes.random_os_id(), + fakes.OSImage({'id': os_image_id, 'status': 'queued'})) data = [ - ({'properties': { - 'image_location': 'testbucket_1/test.img.manifest.xml'}}, + ({'image_location': 'testbucket_1/test.img.manifest.xml'}, 'testbucket_1', 'test.img.manifest.xml'), - ({'properties': { - 'image_location': '/testbucket_2/test.img.manifest.xml'}}, + ({'image_location': '/testbucket_2/test.img.manifest.xml'}, 'testbucket_2', 'test.img.manifest.xml')] for mdata, bucket, manifest in data: image = image_api._s3_create(fake_context, mdata) eventlet.sleep() - osimage_update.assert_called_with( - image, properties={'image_state': 'available'}) - osimage_update.assert_any_call( - image, data=mock.ANY) + self.glance.images.update.assert_called_with( + os_image_id, image_state='available') + self.glance.images.upload.assert_any_call( + os_image_id, mock.ANY) s3_conn.return_value.get_bucket.assert_called_with(bucket) (s3_conn.return_value.get_bucket.return_value. get_key.assert_called_with(manifest)) @@ -865,17 +866,16 @@ class S3TestCase(base.BaseTestCase): @mock.patch('ec2api.api.image.eventlet.spawn_n') def test_s3_create_bdm(self, spawn_n): glance = self.mock_glance() - metadata = {'properties': { - 'image_location': 'fake_bucket/fake_manifest', - 'root_device_name': '/dev/sda1', - 'block_device_mapping': [ - {'device_name': '/dev/sda1', - 'snapshot_id': fakes.ID_OS_SNAPSHOT_1, - 'delete_on_termination': True}, - {'device_name': '/dev/sda2', - 'virtual_name': 'ephemeral0'}, - {'device_name': '/dev/sdb0', - 'no_device': True}]}} + metadata = {'image_location': 'fake_bucket/fake_manifest', + 'root_device_name': '/dev/sda1', + 'block_device_mapping': [ + {'device_name': '/dev/sda1', + 'snapshot_id': fakes.ID_OS_SNAPSHOT_1, + 'delete_on_termination': True}, + {'device_name': '/dev/sda2', + 'virtual_name': 'ephemeral0'}, + {'device_name': '/dev/sdb0', + 'no_device': True}]} fake_context = base.create_context() with mock.patch('ec2api.api.image._s3_conn') as s3_conn: @@ -887,19 +887,17 @@ class S3TestCase(base.BaseTestCase): image_api._s3_create(fake_context, metadata) glance.images.create.assert_called_once_with( - disk_format='ami', container_format='ami', is_public=False, - properties={'architecture': 'x86_64', - 'image_state': 'pending', - 'root_device_name': '/dev/sda1', - 'block_device_mapping': [ - {'device_name': '/dev/sda1', - 'snapshot_id': fakes.ID_OS_SNAPSHOT_1, - 'delete_on_termination': True}, - {'device_name': '/dev/sda2', - 'virtual_name': 'ephemeral0'}, - {'device_name': '/dev/sdb0', - 'no_device': True}], - 'image_location': 'fake_bucket/fake_manifest'}) + disk_format='ami', container_format='ami', + visibility='private', architecture='x86_64', + image_state='pending', root_device_name='/dev/sda1', + block_device_mapping=[{'device_name': '/dev/sda1', + 'snapshot_id': fakes.ID_OS_SNAPSHOT_1, + 'delete_on_termination': True}, + {'device_name': '/dev/sda2', + 'virtual_name': 'ephemeral0'}, + {'device_name': '/dev/sdb0', + 'no_device': True}], + image_location='fake_bucket/fake_manifest') def test_s3_malicious_tarballs(self): self.assertRaises( diff --git a/ec2api/tests/unit/test_instance.py b/ec2api/tests/unit/test_instance.py index 9a36352e..66721faa 100644 --- a/ec2api/tests/unit/test_instance.py +++ b/ec2api/tests/unit/test_instance.py @@ -1487,8 +1487,7 @@ class InstancePrivateTestCase(base.BaseTestCase): # NOTE(ft): check cases of not available image os_image = fakes.OSImage({ 'id': fakes.random_os_id(), - 'status': None, - 'properties': {}}) + 'status': None}) get_os_image.return_value = os_image self.assertRaises( @@ -1497,7 +1496,7 @@ class InstancePrivateTestCase(base.BaseTestCase): fake_context, fakes.random_ec2_id('ami'), None, None) os_image.status = 'active' - os_image.properties['image_state'] = 'decrypting' + os_image.image_state = 'decrypting' self.assertRaises( exception.InvalidAMIIDUnavailable, @@ -1622,12 +1621,12 @@ class InstancePrivateTestCase(base.BaseTestCase): fake_image_template = { 'id': fakes.random_os_id(), - 'properties': {'root_device_name': '/dev/vda', - 'bdm_v2': True, - 'block_device_mapping': []}} + 'root_device_name': '/dev/vda', + 'bdm_v2': True, + 'block_device_mapping': []} # check merging with image bdms - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'boot_index': 0, 'device_name': '/dev/vda', 'source_type': 'snapshot', @@ -1676,7 +1675,7 @@ class InstancePrivateTestCase(base.BaseTestCase): self.assertEqual(expected, result) # check result order for adjusting some bdm of all - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': '/dev/vdc', 'source_type': 'blank', 'volume_size': 10}, @@ -1722,7 +1721,7 @@ class InstancePrivateTestCase(base.BaseTestCase): self.assertEqual(expected, result) # check conflict of short and full device names - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': '/dev/vdc', 'source_type': 'blank', 'volume_size': 10}, @@ -1737,7 +1736,7 @@ class InstancePrivateTestCase(base.BaseTestCase): fakes.OSImage(fake_image_template)) # opposit combination of the same case - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': 'vdc', 'source_type': 'blank', 'volume_size': 10}, @@ -1752,7 +1751,7 @@ class InstancePrivateTestCase(base.BaseTestCase): fakes.OSImage(fake_image_template)) # check fault on root device snapshot changing - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'boot_index': 0, 'source_type': 'snapshot', 'snapshot_id': fakes.ID_EC2_SNAPSHOT_1}, @@ -1767,11 +1766,11 @@ class InstancePrivateTestCase(base.BaseTestCase): fakes.OSImage(fake_image_template)) # same case for legacy bdm - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': '/dev/vda', 'snapshot_id': fakes.ID_EC2_SNAPSHOT_1}, ] - fake_image_template['properties']['bdm_v2'] = False + fake_image_template['bdm_v2'] = False bdms = [ {'device_name': '/dev/vda', 'ebs': {'snapshot_id': fakes.ID_EC2_SNAPSHOT_2}}, @@ -1782,11 +1781,11 @@ class InstancePrivateTestCase(base.BaseTestCase): fakes.OSImage(fake_image_template)) # same case for legacy bdm with short names - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': 'vda', 'snapshot_id': fakes.ID_EC2_SNAPSHOT_1}, ] - fake_image_template['properties']['bdm_v2'] = False + fake_image_template['bdm_v2'] = False bdms = [ {'device_name': 'vda', 'ebs': {'snapshot_id': fakes.ID_EC2_SNAPSHOT_2}}, @@ -1796,10 +1795,10 @@ class InstancePrivateTestCase(base.BaseTestCase): fake_context, bdms, fakes.OSImage(fake_image_template)) - fake_image_template['properties']['bdm_v2'] = True + fake_image_template['bdm_v2'] = True # check fault on reduce volume size - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': 'vdc', 'source_type': 'blank', 'volume_size': 15}, @@ -1814,7 +1813,7 @@ class InstancePrivateTestCase(base.BaseTestCase): fakes.OSImage(fake_image_template)) # check fault on set snapshot id if bdm doesn't have one - fake_image_template['properties']['block_device_mapping'] = [ + fake_image_template['block_device_mapping'] = [ {'device_name': 'vdc', 'source_type': 'blank', 'volume_size': 10}, diff --git a/ec2api/tests/unit/test_integrated_scenario.py b/ec2api/tests/unit/test_integrated_scenario.py index e8b967d4..aea31950 100644 --- a/ec2api/tests/unit/test_integrated_scenario.py +++ b/ec2api/tests/unit/test_integrated_scenario.py @@ -76,9 +76,7 @@ class DBItemsAutoCreationTestCase(base.DbTestCase): 'owner': image_project_id, 'is_public': True, 'container_format': 'ami', - 'properties': { - 'kernel_id': os_aki_image_id, - }, + 'kernel_id': os_aki_image_id, } os_aki_image = { 'id': os_aki_image_id, @@ -173,16 +171,14 @@ class DBItemsAutoCreationTestCase(base.DbTestCase): 'owner': image_project_id, 'is_public': True, 'container_format': 'ami', - 'properties': { - 'bdm_v2': True, - 'block_device_mapping': [{'device_name': '/dev/vds', + 'bdm_v2': True, + 'block_device_mapping': [{'device_name': '/dev/vds', 'source_type': 'snapshot', 'destination_type': 'volume', 'snapshot_id': os_snapshot_id}], - }, } if os_bdm_image_id: - os_image['properties']['block_device_mapping'].append({ + os_image['block_device_mapping'].append({ 'device_name': '/dev/vdi', 'source_type': 'image', 'destination_type': 'volume', @@ -263,7 +259,7 @@ class DBItemsAutoCreationTestCase(base.DbTestCase): os_image = { 'id': os_image_id, 'owner': image_project_id, - 'is_public': True, + 'visibility': 'public', } self.nova_admin.servers.list.return_value = [ fakes.OSInstance_full(os_instance)]