From 4f8fdef22e8415232f48189192008cf14a88f7f0 Mon Sep 17 00:00:00 2001 From: Feodor Tersin Date: Sun, 21 Dec 2014 15:05:19 +0400 Subject: [PATCH] Use image DB items for instances Change-Id: I8d85f9fba9c5d2c034a0a1305ce8636b2511bb2b --- ec2api/api/instance.py | 75 +++++++++++++++++++++-------------- ec2api/db/api.py | 2 +- ec2api/db/sqlalchemy/api.py | 2 +- ec2api/tests/fakes.py | 44 +++++++++++++++++++- ec2api/tests/test_instance.py | 29 ++++++++------ 5 files changed, 107 insertions(+), 45 deletions(-) diff --git a/ec2api/api/instance.py b/ec2api/api/instance.py index 06548d83..b8ec7f44 100644 --- a/ec2api/api/instance.py +++ b/ec2api/api/instance.py @@ -19,6 +19,7 @@ import itertools import random import re +from glanceclient import exc as glance_exception from novaclient import exceptions as nova_exception from oslo.config import cfg @@ -240,7 +241,8 @@ def run_instances(context, image_id, min_count, max_count, ec2_network_interfaces = _format_network_interfaces( context, instance_network_interfaces) return _format_reservation(context, ec2_reservation_id, instances_info, - ec2_network_interfaces) + ec2_network_interfaces, + image_ids={os_image.id: image_id}) def terminate_instances(context, instance_id): @@ -306,6 +308,10 @@ def describe_instances(context, instance_id=None, filter=None, ec2_network_interfaces = _get_ec2_network_interfaces(context, instance_id) volumes = dict((v['os_id'], v) for v in db_api.get_items(context, 'vol')) + image_ids = dict((i['os_id'], i['id']) + for i in itertools.chain( + db_api.get_items(context, 'ami'), + db_api.get_public_items(context, 'ami'))) reservation_filters = [] instance_filters = [] for f in filter or []: @@ -317,7 +323,8 @@ def describe_instances(context, instance_id=None, filter=None, for reservation_id, instances_info in reservations.iteritems(): formatted_reservation = _format_reservation( context, reservation_id, instances_info, - ec2_network_interfaces, instance_filters, volumes=volumes) + ec2_network_interfaces, instance_filters, volumes=volumes, + image_ids=image_ids) if (formatted_reservation['instancesSet'] and not utils.filtered_out(formatted_reservation, reservation_filters, @@ -461,16 +468,18 @@ def _get_idempotent_run(context, client_token): return ec2_network_interfaces = _get_ec2_network_interfaces(context, instance_ids) return _format_reservation(context, instance['reservation_id'], - instances_info, ec2_network_interfaces) + instances_info, ec2_network_interfaces, + image_ids={}) def _format_reservation(context, reservation_id, instances_info, - ec2_network_interfaces, filters=None, volumes=None): + ec2_network_interfaces, filters=None, volumes=None, + image_ids=None): formatted_instances = [] for (instance, os_instance, novadb_instance) in instances_info: ec2_instance = _format_instance( context, instance, os_instance, novadb_instance, - ec2_network_interfaces.get(instance['id']), volumes) + ec2_network_interfaces.get(instance['id']), volumes, image_ids) if not utils.filtered_out(ec2_instance, filters, INSTANCE_FILTER_MAP): formatted_instances.append(ec2_instance) formatted_reservation = {'reservationId': reservation_id, @@ -483,15 +492,16 @@ def _format_reservation(context, reservation_id, instances_info, def _format_instance(context, instance, os_instance, novadb_instance, - ec2_network_interfaces, volumes): + ec2_network_interfaces, volumes, image_ids): ec2_instance = {} ec2_instance['instanceId'] = instance['id'] - image_uuid = os_instance.image['id'] if os_instance.image else '' - ec2_instance['imageId'] = ec2utils.glance_id_to_ec2_id(context, image_uuid) - kernel_id = _cloud_format_kernel_id(context, novadb_instance) + os_image_id = os_instance.image['id'] if os_instance.image else None + ec2_instance['imageId'] = ec2utils.os_id_to_ec2_id( + context, 'ami', os_image_id, ids_by_os_id=image_ids) + kernel_id = _cloud_format_kernel_id(context, novadb_instance, image_ids) if kernel_id: ec2_instance['kernelId'] = kernel_id - ramdisk_id = _cloud_format_ramdisk_id(context, novadb_instance) + ramdisk_id = _cloud_format_ramdisk_id(context, novadb_instance, image_ids) if ramdisk_id: ec2_instance['ramdiskId'] = ramdisk_id ec2_instance['instanceState'] = _cloud_state_description( @@ -673,29 +683,32 @@ def _check_min_max_count(min_count, max_count): def _parse_image_parameters(context, image_id, kernel_id, ramdisk_id): - - def get_os_image_id(ec2_image_id): - try: - return ec2utils.ec2_id_to_glance_id(context, ec2_image_id) - except exception.NovaDbImageNotFound: - raise exception.NovaDbImageNotFound(image_id=ec2_image_id) - glance = clients.glance(context) - if kernel_id: - os_kernel_id = get_os_image_id(kernel_id) - glance.images.get(os_kernel_id) - if ramdisk_id: - os_ramdisk_id = get_os_image_id(ramdisk_id) - glance.images.get(os_ramdisk_id) - os_image_id = get_os_image_id(image_id) - os_image = glance.images.get(os_image_id) + + # TODO(ft): we can't get all images from DB per one request due different + # kinds. It's need to refactor DB API and ec2utils functions to work with + # kind smarter + def get_os_image(kind, ec2_image_id): + try: + ids = db_api.get_item_ids(context, kind, (ec2_image_id,)) + _id, os_image_id = ids[0] + os_image = glance.images.get(os_image_id) + except (IndexError, glance_exception.HTTPNotFound): + raise exception.InvalidAMIIDNotFound(id=ec2_image_id) + return os_image + + os_kernel_id = (get_os_image('aki', kernel_id)['os_id'] + if kernel_id else None) + os_ramdisk_id = (get_os_image('ari', ramdisk_id)['os_id'] + if ramdisk_id else None) + os_image = get_os_image('ami', image_id) if _cloud_get_image_state(os_image) != 'available': # TODO(ft): Change the message with the real AWS message msg = _('Image must be available') raise exception.ImageNotActive(message=msg) - return os_image, kernel_id, ramdisk_id + return os_image, os_kernel_id, os_ramdisk_id def _parse_block_device_mapping(context, block_device_mapping, os_image): @@ -1064,18 +1077,20 @@ def _cloud_get_image_state(image): return image.properties.get('image_state', state) -def _cloud_format_kernel_id(context, instance_ref): +def _cloud_format_kernel_id(context, instance_ref, image_ids=None): kernel_uuid = instance_ref['kernel_id'] if kernel_uuid is None or kernel_uuid == '': return - return ec2utils.glance_id_to_ec2_id(context, kernel_uuid, 'aki') + return ec2utils.os_id_to_ec2_id(context, 'aki', kernel_uuid, + ids_by_os_id=image_ids) -def _cloud_format_ramdisk_id(context, instance_ref): +def _cloud_format_ramdisk_id(context, instance_ref, image_ids=None): ramdisk_uuid = instance_ref['ramdisk_id'] if ramdisk_uuid is None or ramdisk_uuid == '': return - return ec2utils.glance_id_to_ec2_id(context, ramdisk_uuid, 'ari') + return ec2utils.os_id_to_ec2_id(context, 'ari', ramdisk_uuid, + ids_by_os_id=image_ids) def _cloud_format_instance_type(context, os_instance): diff --git a/ec2api/db/api.py b/ec2api/db/api.py index f03f1c6b..f65d35ad 100644 --- a/ec2api/db/api.py +++ b/ec2api/db/api.py @@ -114,7 +114,7 @@ def get_items_by_ids(context, kind, item_ids): return IMPL.get_items_by_ids(context, kind, item_ids) -def get_public_items(context, kind, item_ids): +def get_public_items(context, kind, item_ids=None): return IMPL.get_public_items(context, kind, item_ids) diff --git a/ec2api/db/sqlalchemy/api.py b/ec2api/db/sqlalchemy/api.py index 32ee3d07..0301e35c 100644 --- a/ec2api/db/sqlalchemy/api.py +++ b/ec2api/db/sqlalchemy/api.py @@ -220,7 +220,7 @@ def get_items_by_ids(context, kind, item_ids): @require_context -def get_public_items(context, kind, item_ids): +def get_public_items(context, kind, item_ids=None): query = (model_query(context, models.Item). filter(models.Item.id.like('%s-%%' % kind)). filter(models.Item.data.like('%\'is_public\': True%'))) diff --git a/ec2api/tests/fakes.py b/ec2api/tests/fakes.py index f8b94318..1ae85648 100644 --- a/ec2api/tests/fakes.py +++ b/ec2api/tests/fakes.py @@ -181,6 +181,13 @@ ID_EC2_ROUTE_TABLE_ASSOCIATION_2 = ID_EC2_SUBNET_2.replace('subnet', CIDR_EXTERNAL_NETWORK = '192.168.50.0/24' +# image constants +ID_EC2_IMAGE_1 = random_ec2_id('ami') +ID_EC2_IMAGE_2 = random_ec2_id('ami') +ID_OS_IMAGE_1 = random_os_id() +ID_OS_IMAGE_2 = random_os_id() + + # Object constants section # Constant name notation: # [] @@ -481,7 +488,7 @@ EC2_INSTANCE_1 = { 'placement': {'availabilityZone': None}, 'dnsName': IP_ADDRESS_2, 'instanceState': {'code': 0, 'name': 'pending'}, - 'imageId': None, + 'imageId': ID_EC2_IMAGE_1, 'productCodesSet': [], 'privateDnsName': ID_EC2_INSTANCE_1, 'keyName': None, @@ -546,6 +553,7 @@ class OSInstance(object): OS_INSTANCE_1 = OSInstance( ID_OS_INSTANCE_1, {'id': 'fakeFlavorId'}, + image={'id': ID_OS_IMAGE_1}, addresses={ ID_EC2_SUBNET_2: [{'addr': IP_NETWORK_INTERFACE_2, 'version': 4, @@ -924,6 +932,40 @@ EC2_ROUTE_TABLE_2 = { } +# image objects +class OSImage(object): + + def __init__(self, image_dict): + self.id = image_dict['id'] + self.is_public = image_dict['is_public'] + self.status = image_dict['status'] + self.properties = image_dict['properties'] + +DB_IMAGE_1 = { + 'id': ID_EC2_IMAGE_1, + 'os_id': ID_OS_IMAGE_1, + 'public': False, +} +DB_IMAGE_2 = { + 'id': ID_EC2_IMAGE_2, + 'os_id': ID_OS_IMAGE_2, + 'public': True, +} + +OS_IMAGE_1 = { + 'id': ID_OS_IMAGE_1, + 'is_public': False, + 'status': 'active', + 'properties': {}, +} +OS_IMAGE_2 = { + 'id': ID_OS_IMAGE_2, + 'public': True, + 'status': 'active', + 'properties': {}, +} + + # keypair objects class NovaKeyPair(object): diff --git a/ec2api/tests/test_instance.py b/ec2api/tests/test_instance.py index b6918ac7..b172c43a 100644 --- a/ec2api/tests/test_instance.py +++ b/ec2api/tests/test_instance.py @@ -75,6 +75,8 @@ class InstanceTestCase(base.ApiTestCase): {fakes.ID_EC2_SUBNET_1: fakes.DB_SUBNET_1, fakes.ID_EC2_NETWORK_INTERFACE_1: copy.deepcopy(fakes.DB_NETWORK_INTERFACE_1)})) + self.db_api.get_item_ids.return_value = [ + (fakes.ID_EC2_IMAGE_1, fakes.ID_OS_IMAGE_1)] self.neutron.list_ports.return_value = ( {'ports': [fakes.OS_PORT_1, fakes.OS_PORT_2]}) self.create_network_interface.return_value = ( @@ -83,10 +85,9 @@ class InstanceTestCase(base.ApiTestCase): self.db_api.add_item.return_value = fakes.DB_INSTANCE_1 self.utils_generate_uid.return_value = fakes.ID_EC2_RESERVATION_1 - self.glance.images.get.return_value = self.fake_image_class( - 'fake_image_id', 'active', {}) - self.ec2_id_to_glance_id.return_value = 'fake_image_id' - self.glance_id_to_ec2_id.return_value = None + self.glance.images.get.return_value = fakes.OSImage(fakes.OS_IMAGE_1) +# self.ec2_id_to_glance_id.return_value = 'fake_image_id' +# self.glance_id_to_ec2_id.return_value = None fake_flavor = self.fake_flavor_class('fake_flavor') self.nova_flavors.list.return_value = [fake_flavor] self.nova_servers.create.return_value = ( @@ -104,7 +105,7 @@ class InstanceTestCase(base.ApiTestCase): _get_vpc_default_security_group_id.return_value = None def do_check(params, new_port=True, delete_on_termination=None): - params.update({'ImageId': 'ami-00000001', + params.update({'ImageId': fakes.ID_EC2_IMAGE_1, 'InstanceType': 'fake_flavor', 'MinCount': '1', 'MaxCount': '1'}) resp = self.execute('RunInstances', params) @@ -140,7 +141,7 @@ class InstanceTestCase(base.ApiTestCase): self.create_network_interface.assert_called_once_with( mock.ANY, fakes.EC2_SUBNET_1['subnetId']) self.nova_servers.create.assert_called_once_with( - 'EC2 server', 'fake_image_id', fake_flavor, + 'EC2 server', fakes.ID_OS_IMAGE_1, fake_flavor, min_count=1, max_count=1, kernel_id=None, ramdisk_id=None, availability_zone=None, @@ -148,6 +149,8 @@ class InstanceTestCase(base.ApiTestCase): security_groups=None, nics=[{'port-id': fakes.ID_OS_PORT_1}], key_name=None, userdata=None) + self.db_api.get_item_ids.assert_called_once_with( + mock.ANY, 'ami', (fakes.ID_EC2_IMAGE_1,)) self.db_api.update_item.assert_called_once_with( mock.ANY, db_attached_eni) self.isotime.assert_called_once_with(None, True) @@ -276,6 +279,8 @@ class InstanceTestCase(base.ApiTestCase): {fakes.ID_EC2_SUBNET_1: fakes.DB_SUBNET_1, fakes.ID_EC2_NETWORK_INTERFACE_1: copy.deepcopy(fakes.DB_NETWORK_INTERFACE_1)})) + self.db_api.get_item_ids.return_value = [ + (fakes.ID_EC2_IMAGE_1, fakes.ID_OS_IMAGE_1)] self.neutron.list_ports.return_value = ( {'ports': [fakes.OS_PORT_1, fakes.OS_PORT_2]}) self.create_network_interface.return_value = ( @@ -284,13 +289,12 @@ class InstanceTestCase(base.ApiTestCase): self.db_api.add_item.return_value = fakes.DB_INSTANCE_1 self.utils_generate_uid.return_value = fakes.ID_EC2_RESERVATION_1 - self.glance.images.get.return_value = self.fake_image_class( - 'fake_image_id', 'active', {}) - self.ec2_id_to_glance_id.return_value = 'fake_image_id' + self.glance.images.get.return_value = fakes.OSImage(fakes.OS_IMAGE_1) fake_flavor = self.fake_flavor_class('fake_flavor') self.nova_flavors.list.return_value = [fake_flavor] self.nova_servers.create.return_value = ( fakes.OSInstance(fakes.ID_OS_INSTANCE_1, {'id': 'fakeFlavorId'}, + image={'id': fakes.ID_OS_IMAGE_1}, addresses={ fakes.ID_EC2_SUBNET_1: [ {'addr': fakes.IP_NETWORK_INTERFACE_1, @@ -299,7 +303,7 @@ class InstanceTestCase(base.ApiTestCase): self.db_api.update_item.side_effect = Exception() def do_check(params, new_port=True, delete_on_termination=None): - params.update({'ImageId': 'ami-00000001', + params.update({'ImageId': fakes.ID_EC2_IMAGE_1, 'InstanceType': 'fake_flavor', 'MinCount': '1', 'MaxCount': '1'}) self.execute('RunInstances', params) @@ -512,7 +516,9 @@ class InstanceTestCase(base.ApiTestCase): [fakes.DB_ADDRESS_1, fakes.DB_ADDRESS_2] if kind == 'eipalloc' else [fakes.DB_INSTANCE_1, fakes.DB_INSTANCE_2] - if kind == 'i' else []) + if kind == 'i' else + [fakes.DB_IMAGE_1, fakes.DB_IMAGE_2] + if kind == 'ami' else []) self.neutron.list_floatingips.return_value = ( {'floatingips': [fakes.OS_FLOATING_IP_1, fakes.OS_FLOATING_IP_2]}) @@ -526,7 +532,6 @@ class InstanceTestCase(base.ApiTestCase): instance_get_by_uuid(context, None, item_id)) fake_flavor = self.fake_flavor_class('fake_flavor') self.nova_flavors.get.return_value = fake_flavor - self.glance_id_to_ec2_id.return_value = None format_security_groups_ids_names.return_value = {} self.novadb.block_device_mapping_get_all_by_instance.return_value = []