openstack/ec2-api
Code Issues Proposed changes

Delayed delete DB items for instances, volumes

Browse Source 
Change-Id: I39c20eff1161c2660f73734f0d11290336546297
This commit is contained in:
Feodor Tersin 2015-02-04 14:59:23 +04:00
parent 47d0a7c35b
commit 5993031efc
9 changed files with 87 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
ec2api/api/address.py
View File

@ -115,7 +115,7 @@ class AddressDescriber(common.UniversalDescriber):
'instance-id': 'instanceId',
'network-interface-id': 'networkInterfaceId',
'network-interface-owner-id': 'networkInterfaceOwnerId',
'privateIpAddress': 'privateIpAddress',
'private-ip-address': 'privateIpAddress',
'public-ip': 'publicIp'}
def __init__(self, os_ports):
@ -127,6 +127,14 @@ class AddressDescriber(common.UniversalDescriber):
def get_os_items(self):
return address_engine.get_os_floating_ips(self.context)
def auto_update_db(self, item, os_item):
item = super(AddressDescriber, self).auto_update_db(item, os_item)
if (item and 'network_interface_id' in item and
(not os_item.get('port_id') or
os_item['fixed_ip_address'] != item['private_ip_address'])):
_disassociate_address_item(self.context, item)
return item
def get_name(self, os_item):
return os_item['floating_ip_address']

17
ec2api/api/instance.py
View File

@ -118,8 +118,8 @@ def terminate_instances(context, instance_id):
state_change = _format_state_change(instance, os_instance)
state_changes.append(state_change)
_remove_instances(context, instances, network_interfaces)
# NOTE(ft): don't delete items from DB until they disappear from OS.
# They will be auto deleted by a describe operation
return {'instancesSet': state_changes}
@ -272,7 +272,11 @@ class ReservationDescriber(common.NonOpenstackItemsDescriber):
formatted_instances = instance_describer.describe(
context, ids=ids, filter=instance_filters)
_remove_instances(context, instance_describer.obsolete_instances)
# NOTE(ft): remove obsolete instances' DB items only, because
# network interfaces and addresses are cleaned during appropriate
# describe operations called inside current operation
_remove_instances(context, instance_describer.obsolete_instances,
purge_linked_items=False)
self.reservations = instance_describer.reservations.values()
self.instances = instance_describer.reservation_instances
@ -562,16 +566,15 @@ def _format_state_change(instance, os_instance):
}
def _remove_instances(context, instances, network_interfaces=None):
def _remove_instances(context, instances, purge_linked_items=True):
if not instances:
return
if network_interfaces is None:
# TODO(ft): implement search db items by os_id in DB layer
network_interfaces = collections.defaultdict(list)
if purge_linked_items:
# TODO(ft): implement search db items by os_id in DB layer
for eni in db_api.get_items(context, 'eni'):
if 'instance_id' in eni:
network_interfaces[eni['instance_id']].append(eni)
addresses = db_api.get_items(context, 'eipalloc')
addresses = dict((a['network_interface_id'], a) for a in addresses
if 'network_interface_id' in a)

39
ec2api/api/network_interface.py
View File

@ -20,6 +20,7 @@ from neutronclient.common import exceptions as neutron_exception
from novaclient import exceptions as nova_exception
from oslo.config import cfg
from ec2api.api import address as address_api
from ec2api.api import clients
from ec2api.api import common
from ec2api.api import dhcp_options
@ -163,9 +164,7 @@ def delete_network_interface(context, network_interface_id):
for address in db_api.get_items(context, 'eipalloc'):
if address.get('network_interface_id') == network_interface['id']:
address.pop('network_interface_id')
address.pop('private_ip_address')
db_api.update_item(context, address)
address_api._disassociate_address_item(context, address)
neutron = clients.neutron(context)
with common.OnCrashCleaner() as cleaner:
@ -207,20 +206,19 @@ class NetworkInterfaceDescriber(common.TaggableItemsDescriber):
return None
return _format_network_interface(
self.context, network_interface, os_port,
self.addresses[network_interface['id']], self.security_groups)
self.ec2_addresses[network_interface['id']],
self.security_groups)
def get_os_items(self):
neutron = clients.neutron(self.context)
os_floating_ips = neutron.list_floatingips()['floatingips']
os_floating_ip_ids = set(ip['id'] for ip in os_floating_ips)
addresses = collections.defaultdict(list)
for address in db_api.get_items(self.context, 'eipalloc'):
if ('network_interface_id' in address and
address['os_id'] in os_floating_ip_ids):
addresses[address['network_interface_id']].append(address)
self.addresses = addresses
addresses = address_api.describe_addresses(self.context)
self.ec2_addresses = collections.defaultdict(list)
for address in addresses['addressesSet']:
if 'networkInterfaceId' in address:
self.ec2_addresses[
address['networkInterfaceId']].append(address)
self.security_groups = (
security_group_api._format_security_groups_ids_names(self.context))
neutron = clients.neutron(self.context)
return neutron.list_ports()['ports']
def get_name(self, os_item):
@ -398,7 +396,7 @@ def detach_network_interface(context, attachment_id, force=None):
def _format_network_interface(context, network_interface, os_port,
associated_addresses=[], security_groups={}):
associated_ec2_addresses=[], security_groups={}):
ec2_network_interface = {}
ec2_network_interface['networkInterfaceId'] = network_interface['id']
ec2_network_interface['subnetId'] = network_interface['subnet_id']
@ -439,16 +437,17 @@ def _format_network_interface(context, network_interface, os_port,
ip['ip_address'])
item = {'privateIpAddress': ip['ip_address'],
'primary': primary}
address = next((addr for addr in associated_addresses
if addr['private_ip_address'] == ip['ip_address']),
ec2_address = next(
(addr for addr in associated_ec2_addresses
if addr['privateIpAddress'] == ip['ip_address']),
None)
if address:
if ec2_address:
item['association'] = {
'associationId': ec2utils.change_ec2_id_kind(address['id'],
'eipassoc'),
'associationId': ec2utils.change_ec2_id_kind(
ec2_address['allocationId'], 'eipassoc'),
'ipOwnerId': context.project_id,
'publicDnsName': None,
'publicIp': address['public_ip'],
'publicIp': ec2_address['publicIp'],
}
if primary:
ipsSet.insert(0, item)

3
ec2api/api/volume.py
View File

@ -101,7 +101,8 @@ def delete_volume(context, volume_id):
raise exception.UnsupportedOperation()
except cinder_exception.NotFound:
pass
db_api.delete_item(context, volume['id'])
# NOTE(andrey-mp) Don't delete item from DB until it disappears from Cloud
# It will be deleted by describer in the future
return True

2
ec2api/tests/test_address.py
View File

@ -635,7 +635,7 @@ class AddressTestCase(base.ApiTestCase):
('instance-id', fakes.ID_EC2_INSTANCE_1),
('network-interface-id', fakes.ID_EC2_NETWORK_INTERFACE_2),
('network-interface-owner-id', fakes.ID_OS_PROJECT),
('privateIpAddress', fakes.IP_NETWORK_INTERFACE_2),
('private-ip-address', fakes.IP_NETWORK_INTERFACE_2),
('public-ip', fakes.IP_ADDRESS_2)])
def test_describe_addresses_ec2_classic(self):

13
ec2api/tests/test_instance.py
View File

@ -701,9 +701,7 @@ class InstanceTestCase(base.ApiTestCase):
self.nova_servers.get.assert_any_call(fakes.ID_OS_INSTANCE_2)
self.assertEqual(
0, self.address_api.dissassociate_address_item.call_count)
self.assertEqual(2, self.db_api.delete_item.call_count)
for inst_id in (fakes.ID_EC2_INSTANCE_1, fakes.ID_EC2_INSTANCE_2):
self.db_api.delete_item.assert_any_call(mock.ANY, inst_id)
self.assertFalse(self.db_api.delete_item.called)
self.assertEqual(2, os_instance_delete.call_count)
self.assertEqual(2, os_instance_get.call_count)
for call_num, inst_id in enumerate([fakes.OS_INSTANCE_1,
@ -752,12 +750,7 @@ class InstanceTestCase(base.ApiTestCase):
detach_network_interface.assert_any_call(
mock.ANY,
('eni-attach-%s' % ec2_eni['id'].split('-')[-1]))
self.assertEqual(len(deleted_enis) + 2,
self.db_api.delete_item.call_count)
for eni in deleted_enis:
self.db_api.delete_item.assert_any_call(mock.ANY, eni['id'])
for inst_id in (fakes.ID_EC2_INSTANCE_1, fakes.ID_EC2_INSTANCE_2):
self.db_api.delete_item.assert_any_call(mock.ANY, inst_id)
self.assertFalse(self.db_api.delete_item.called)
detach_network_interface.reset_mock()
self.db_api.delete_item.reset_mock()
@ -1097,7 +1090,7 @@ class InstanceTestCase(base.ApiTestCase):
'reservationSet': [fakes.EC2_RESERVATION_2]},
orderless_lists=True))
remove_instances.assert_called_once_with(
mock.ANY, [fakes.DB_INSTANCE_1])
mock.ANY, [fakes.DB_INSTANCE_1], purge_linked_items=False)
@mock.patch('ec2api.api.instance._format_instance')
def test_describe_instances_sorting(self, format_instance):

10
ec2api/tests/test_network_interface.py
View File

@ -348,11 +348,11 @@ class NetworkInterfaceTestCase(base.ApiTestCase):
def test_describe_network_interfaces(self):
self.db_api.get_items.side_effect = (
lambda _, kind: [fakes.DB_NETWORK_INTERFACE_1,
fakes.DB_NETWORK_INTERFACE_2]
if kind == 'eni' else
[fakes.DB_ADDRESS_1, fakes.DB_ADDRESS_2]
if kind == 'eipalloc' else [])
fakes.get_db_api_get_items({
'eni': [fakes.DB_NETWORK_INTERFACE_1,
fakes.DB_NETWORK_INTERFACE_2],
'eipalloc': [fakes.DB_ADDRESS_1, fakes.DB_ADDRESS_2],
'i': [fakes.DB_INSTANCE_1, fakes.DB_INSTANCE_2]}))
self.neutron.list_ports.return_value = (
{'ports': [fakes.OS_PORT_1, fakes.OS_PORT_2]})
self.neutron.list_floatingips.return_value = (

10
ec2api/tests/test_subnet.py
View File

@ -223,11 +223,11 @@ class SubnetTestCase(base.ApiTestCase):
{fakes.ID_EC2_VPC_1: fakes.DB_VPC_1,
fakes.ID_EC2_SUBNET_1: fakes.DB_SUBNET_1}))
self.db_api.get_items.side_effect = (
lambda _, kind: [fakes.DB_NETWORK_INTERFACE_1,
fakes.DB_NETWORK_INTERFACE_2]
if kind == 'eni' else
[fakes.DB_ADDRESS_1, fakes.DB_ADDRESS_2]
if kind == 'eipalloc' else [])
fakes.get_db_api_get_items({
'eni': [fakes.DB_NETWORK_INTERFACE_1,
fakes.DB_NETWORK_INTERFACE_2],
'eipalloc': [fakes.DB_ADDRESS_1, fakes.DB_ADDRESS_2],
'i': [fakes.DB_INSTANCE_1, fakes.DB_INSTANCE_2]}))
self.neutron.list_ports.return_value = (
{'ports': [fakes.OS_PORT_1, fakes.OS_PORT_2]})
self.neutron.list_floatingips.return_value = (

29
ec2api/tests/test_volume.py
View File

@ -72,6 +72,24 @@ class VolumeTestCase(base.ApiTestCase):
'DescribeVolumes', 'volumeSet',
fakes.ID_EC2_VOLUME_1, 'volumeId')
def test_describe_volumes_auto_remove(self):
self.cinder.volumes.list.return_value = []
self.db_api.get_items.side_effect = (
fakes.get_db_api_get_items({
'vol': [fakes.DB_VOLUME_1, fakes.DB_VOLUME_2],
'i': [],
'snap': []}))
resp = self.execute('DescribeVolumes', {})
self.assertEqual(200, resp['http_status_code'])
resp.pop('http_status_code')
self.assertThat(resp, matchers.DictMatches(
{'volumeSet': []}))
self.db_api.delete_item.assert_any_call(
mock.ANY, fakes.ID_EC2_VOLUME_1)
self.db_api.delete_item.assert_any_call(
mock.ANY, fakes.ID_EC2_VOLUME_2)
def test_describe_volumes_invalid_parameters(self):
self.cinder.volumes.list.return_value = [
fakes.CinderVolume(fakes.OS_VOLUME_1),
@ -135,6 +153,17 @@ class VolumeTestCase(base.ApiTestCase):
None, snapshot_id=fakes.ID_OS_SNAPSHOT_1, volume_type=None,
availability_zone=fakes.NAME_AVAILABILITY_ZONE)
def test_delete_volume(self):
self.db_api.get_item_by_id.return_value = fakes.DB_VOLUME_1
resp = self.execute('DeleteVolume',
{'VolumeId': fakes.ID_EC2_VOLUME_1})
self.assertEqual(200, resp['http_status_code'])
resp.pop('http_status_code')
self.assertEqual({'return': True}, resp)
self.cinder.volumes.delete.assert_called_once_with(
fakes.ID_OS_VOLUME_1)
self.assertFalse(self.db_api.delete_item.called)
def test_format_volume_maps_status(self):
fake_volume = fakes.CinderVolume(fakes.OS_VOLUME_1)
self.cinder.volumes.list.return_value = [fake_volume]