summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJenkins <jenkins@review.openstack.org>2013-01-25 12:48:41 +0000
committerGerrit Code Review <review@openstack.org>2013-01-25 12:48:41 +0000
commitf6081d01878f0021a499f304c511b6e1e9c8f138 (patch)
tree085233d81ca3c022c658b302c32736ca79926b97
parent1709c8e755da9d9a31332608bd7f8d971098262c (diff)
parent5a6681222999873f0df9816125fe9888498d91c2 (diff)
downloadnova-f6081d01878f0021a499f304c511b6e1e9c8f138.tar.gz
Merge "Eliminate race conditions in floating association" into stable/folsom
-rw-r--r--nova/db/api.py10
-rw-r--r--nova/db/sqlalchemy/api.py3
-rw-r--r--nova/network/manager.py82
-rw-r--r--nova/tests/network/test_manager.py2
-rw-r--r--nova/tests/test_db_api.py15
5 files changed, 79 insertions, 33 deletions
diff --git a/nova/db/api.py b/nova/db/api.py
index cb651793e9..bb69558378 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -303,7 +303,8 @@ def floating_ip_destroy(context, address):
def floating_ip_disassociate(context, address):
"""Disassociate a floating ip from a fixed ip by address.
- :returns: the address of the existing fixed ip.
+ :returns: the address of the previous fixed ip or None
+ if the ip was not associated to an ip.
"""
return IMPL.floating_ip_disassociate(context, address)
@@ -311,7 +312,12 @@ def floating_ip_disassociate(context, address):
def floating_ip_fixed_ip_associate(context, floating_address,
fixed_address, host):
- """Associate a floating ip to a fixed_ip by address."""
+ """Associate a floating ip to a fixed_ip by address.
+
+ :returns: the address of the new fixed ip (fixed_address) or None
+ if the ip was already associated to the fixed ip.
+ """
+
return IMPL.floating_ip_fixed_ip_associate(context,
floating_address,
fixed_address,
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index f363dc764f..4bdab492b4 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -812,9 +812,12 @@ def floating_ip_fixed_ip_associate(context, floating_address,
fixed_ip_ref = fixed_ip_get_by_address(context,
fixed_address,
session=session)
+ if floating_ip_ref.fixed_ip_id == fixed_ip_ref["id"]:
+ return None
floating_ip_ref.fixed_ip_id = fixed_ip_ref["id"]
floating_ip_ref.host = host
floating_ip_ref.save(session=session)
+ return fixed_address
@require_context
diff --git a/nova/network/manager.py b/nova/network/manager.py
index 2306e0ca68..00a6e58c43 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -555,27 +555,34 @@ class FloatingIP(object):
def _associate_floating_ip(self, context, floating_address, fixed_address,
interface):
"""Performs db and driver calls to associate floating ip & fixed ip"""
- # associate floating ip
- self.db.floating_ip_fixed_ip_associate(context,
- floating_address,
- fixed_address,
- self.host)
- try:
- # gogo driver time
- self.l3driver.add_floating_ip(floating_address, fixed_address,
- interface)
- except exception.ProcessExecutionError as e:
- fixed_address = self.db.floating_ip_disassociate(context,
- floating_address)
- if "Cannot find device" in str(e):
- LOG.error(_('Interface %(interface)s not found'), locals())
- raise exception.NoFloatingIpInterface(interface=interface)
- payload = dict(project_id=context.project_id,
- floating_ip=floating_address)
- notifier.notify(context,
- notifier.publisher_id("network"),
- 'network.floating_ip.associate',
- notifier.INFO, payload=payload)
+
+ @utils.synchronized(unicode(floating_address))
+ def do_associate():
+ # associate floating ip
+ res = self.db.floating_ip_fixed_ip_associate(context,
+ floating_address,
+ fixed_address,
+ self.host)
+ if not res:
+ # NOTE(vish): ip was already associated
+ return
+ try:
+ # gogo driver time
+ self.l3driver.add_floating_ip(floating_address, fixed_address,
+ interface)
+ except exception.ProcessExecutionError as e:
+ self.db.floating_ip_disassociate(context, floating_address)
+ if "Cannot find device" in str(e):
+ LOG.error(_('Interface %(interface)s not found'), locals())
+ raise exception.NoFloatingIpInterface(interface=interface)
+
+ payload = dict(project_id=context.project_id,
+ floating_ip=floating_address)
+ notifier.notify(context,
+ notifier.publisher_id("network"),
+ 'network.floating_ip.associate',
+ notifier.INFO, payload=payload)
+ do_associate()
@wrap_check_policy
def disassociate_floating_ip(self, context, address,
@@ -635,16 +642,31 @@ class FloatingIP(object):
def _disassociate_floating_ip(self, context, address, interface):
"""Performs db and driver calls to disassociate floating ip"""
# disassociate floating ip
- fixed_address = self.db.floating_ip_disassociate(context, address)
- if interface:
- # go go driver time
- self.l3driver.remove_floating_ip(address, fixed_address, interface)
- payload = dict(project_id=context.project_id, floating_ip=address)
- notifier.notify(context,
- notifier.publisher_id("network"),
- 'network.floating_ip.disassociate',
- notifier.INFO, payload=payload)
+ @utils.synchronized(unicode(address))
+ def do_disassociate():
+ # NOTE(vish): Note that we are disassociating in the db before we
+ # actually remove the ip address on the host. We are
+ # safe from races on this host due to the decorator,
+ # but another host might grab the ip right away. We
+ # don't worry about this case because the miniscule
+ # window where the ip is on both hosts shouldn't cause
+ # any problems.
+ fixed_address = self.db.floating_ip_disassociate(context, address)
+
+ if not fixed_address:
+ # NOTE(vish): ip was already disassociated
+ return
+ if interface:
+ # go go driver time
+ self.l3driver.remove_floating_ip(address, fixed_address,
+ interface)
+ payload = dict(project_id=context.project_id, floating_ip=address)
+ notifier.notify(context,
+ notifier.publisher_id("network"),
+ 'network.floating_ip.disassociate',
+ notifier.INFO, payload=payload)
+ do_disassociate()
@wrap_check_policy
def get_floating_ip(self, context, id):
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index 75a7cb5958..e983ad6a5d 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -639,7 +639,7 @@ class VlanNetworkTestCase(test.TestCase):
is_admin=False)
def fake1(*args, **kwargs):
- pass
+ return '10.0.0.1'
# floating ip that's already associated
def fake2(*args, **kwargs):
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 641c7c8cd9..82063db4cc 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -207,6 +207,21 @@ class DbApiTestCase(test.TestCase):
self.assertEqual(0, len(results))
db.instance_update(ctxt, instance['uuid'], {"task_state": None})
+ def test_multi_associate_disassociate(self):
+ ctxt = context.get_admin_context()
+ values = {'address': 'floating'}
+ floating = db.floating_ip_create(ctxt, values)
+ values = {'address': 'fixed'}
+ fixed = db.fixed_ip_create(ctxt, values)
+ res = db.floating_ip_fixed_ip_associate(ctxt, floating, fixed, 'foo')
+ self.assertEqual(res, fixed)
+ res = db.floating_ip_fixed_ip_associate(ctxt, floating, fixed, 'foo')
+ self.assertEqual(res, None)
+ res = db.floating_ip_disassociate(ctxt, floating)
+ self.assertEqual(res, fixed)
+ res = db.floating_ip_disassociate(ctxt, floating)
+ self.assertEqual(res, None)
+
def test_network_create_safe(self):
ctxt = context.get_admin_context()
values = {'host': 'localhost', 'project_id': 'project1'}