Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Makes sure instance deletion ok with deleted data
Commit 5ad1dea added changed the network deallocation code to
work with deleted instances. This was done by setting the context
to read deleted records. Unfortunately this was done a little too
broadly, leading to a new bug where a deleted floating_ip will
cause an instance to not be able to be deleted.

This fixes the issue by limiting the use of read_deleted context
to only the places it is trying to access the instance record. It
adds a test  to verify that the code works with a duplicate
deleted floating_ip and updates the existing test for a deleted
instance to exercise the entire code path.

Fixes bug 1038266

Change-Id: I1aef94369e5bcf951e78e89b1eded5305cf36b53
  • Loading branch information
vishvananda committed Aug 17, 2012
1 parent a10be15 commit 1f98e28
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 21 deletions.
6 changes: 4 additions & 2 deletions nova/db/sqlalchemy/api.py
Expand Up @@ -1170,7 +1170,8 @@ def fixed_ip_get(context, id, session=None):
# FIXME(sirp): shouldn't we just use project_only here to restrict the
# results?
if is_user_context(context) and result['instance_uuid'] is not None:
instance = instance_get_by_uuid(context, result['instance_uuid'],
instance = instance_get_by_uuid(context.elevated(read_deleted='yes'),
result['instance_uuid'],
session)
authorize_project_context(context, instance.project_id)

Expand Down Expand Up @@ -1199,7 +1200,8 @@ def fixed_ip_get_by_address(context, address, session=None):
# NOTE(sirp): shouldn't we just use project_only here to restrict the
# results?
if is_user_context(context) and result['instance_uuid'] is not None:
instance = instance_get_by_uuid(context, result['instance_uuid'],
instance = instance_get_by_uuid(context.elevated(read_deleted='yes'),
result['instance_uuid'],
session)
authorize_project_context(context, instance.project_id)

Expand Down
18 changes: 8 additions & 10 deletions nova/network/manager.py
Expand Up @@ -353,14 +353,11 @@ def deallocate_for_instance(self, context, **kwargs):
# NOTE(francois.charlier): in some cases the instance might be
# deleted before the IPs are released, so we need to get deleted
# instances too
read_deleted_context = context.elevated(read_deleted='yes')
instance = self.db.instance_get(read_deleted_context, instance_id)

LOG.debug(_("floating IP deallocation for instance |%s|"),
instance=instance, context=read_deleted_context)
instance = self.db.instance_get(
context.elevated(read_deleted='yes'), instance_id)

try:
fixed_ips = self.db.fixed_ip_get_by_instance(read_deleted_context,
fixed_ips = self.db.fixed_ip_get_by_instance(context,
instance['uuid'])
except exception.FixedIpNotFoundForInstance:
fixed_ips = []
Expand All @@ -374,14 +371,14 @@ def deallocate_for_instance(self, context, **kwargs):
for floating_ip in floating_ips:
address = floating_ip['address']
try:
self.disassociate_floating_ip(read_deleted_context,
self.disassociate_floating_ip(context,
address,
affect_auto_assigned=True)
except exception.FloatingIpNotAssociated:
LOG.exception(_("Floating IP is not associated. Ignore."))
# deallocate if auto_assigned
if floating_ip['auto_assigned']:
self.deallocate_floating_ip(read_deleted_context, address,
self.deallocate_floating_ip(context, address,
affect_auto_assigned=True)

# call the next inherited class's deallocate_for_instance()
Expand Down Expand Up @@ -1276,8 +1273,9 @@ def deallocate_fixed_ip(self, context, address, **kwargs):
"""Returns a fixed ip to the pool."""
fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
vif_id = fixed_ip_ref['virtual_interface_id']
instance = self.db.instance_get_by_uuid(context,
fixed_ip_ref['instance_uuid'])
instance = self.db.instance_get_by_uuid(
context.elevated(read_deleted='yes'),
fixed_ip_ref['instance_uuid'])

self._do_trigger_security_group_members_refresh_for_instance(
instance['uuid'])
Expand Down
55 changes: 46 additions & 9 deletions nova/tests/network/test_manager.py
Expand Up @@ -1042,12 +1042,14 @@ def network_get(_context, network_id):
fixed = db.fixed_ip_get_by_address(elevated, fix_addr)
network = db.network_get(elevated, fixed['network_id'])

db.instance_destroy(self.context.elevated(), instance['uuid'])
self.assertRaises(exception.InstanceNotFound,
self.network.deallocate_fixed_ip,
context1,
fix_addr,
'fake')
def fake_refresh(instance_uuid):
raise test.TestingException()
self.stubs.Set(self.network,
'_do_trigger_security_group_members_refresh_for_instance',
fake_refresh)
self.assertRaises(test.TestingException,
self.network.deallocate_fixed_ip,
context1, fix_addr, 'fake')
fixed = db.fixed_ip_get_by_address(elevated, fix_addr)
self.assertTrue(fixed['allocated'])

Expand Down Expand Up @@ -1541,10 +1543,45 @@ def test_double_deallocation(self):
instance_id=instance_ref['id'])

def test_deallocation_deleted_instance(self):
instance_ref = db.api.instance_create(self.context,
{"project_id": self.project_id, "deleted": True})
self.stubs.Set(self.network, '_teardown_network_on_host',
lambda *args, **kwargs: None)
instance = db.api.instance_create(self.context, {
'project_id': self.project_id, 'deleted': True})
network = db.api.network_create_safe(self.context.elevated(), {
'project_id': self.project_id})
addr = db.fixed_ip_create(self.context, {'allocated': True,
'instance_uuid': instance['uuid'], 'address': '10.1.1.1',
'network_id': network['id']})
fixed = db.fixed_ip_get_by_address(
self.context.elevated(read_deleted='yes'), addr)
db.api.floating_ip_create(self.context, {
'address': '10.10.10.10', 'instance_uuid': instance['uuid'],
'fixed_ip_id': fixed['id'],
'project_id': self.project_id})
self.network.deallocate_for_instance(self.context,
instance_id=instance_ref['id'])
instance_id=instance['id'])

def test_deallocation_duplicate_floating_ip(self):
self.stubs.Set(self.network, '_teardown_network_on_host',
lambda *args, **kwargs: None)
instance = db.api.instance_create(self.context, {
'project_id': self.project_id})
network = db.api.network_create_safe(self.context.elevated(), {
'project_id': self.project_id})
addr = db.fixed_ip_create(self.context, {'allocated': True,
'instance_uuid': instance['uuid'], 'address': '10.1.1.1',
'network_id': network['id']})
fixed = db.fixed_ip_get_by_address(
self.context.elevated(read_deleted='yes'), addr)
db.api.floating_ip_create(self.context, {
'address': '10.10.10.10',
'deleted': True})
db.api.floating_ip_create(self.context, {
'address': '10.10.10.10', 'instance_uuid': instance['uuid'],
'fixed_ip_id': fixed['id'],
'project_id': self.project_id})
self.network.deallocate_for_instance(self.context,
instance_id=instance['id'])

def test_floating_dns_create_conflict(self):
zone = "example.org"
Expand Down

0 comments on commit 1f98e28

Please sign in to comment.