diff options
author | Jenkins <jenkins@review.openstack.org> | 2013-01-25 12:48:41 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2013-01-25 12:48:41 +0000 |
commit | f6081d01878f0021a499f304c511b6e1e9c8f138 (patch) | |
tree | 085233d81ca3c022c658b302c32736ca79926b97 | |
parent | 1709c8e755da9d9a31332608bd7f8d971098262c (diff) | |
parent | 5a6681222999873f0df9816125fe9888498d91c2 (diff) | |
download | nova-f6081d01878f0021a499f304c511b6e1e9c8f138.tar.gz |
Merge "Eliminate race conditions in floating association" into stable/folsom
-rw-r--r-- | nova/db/api.py | 10 | ||||
-rw-r--r-- | nova/db/sqlalchemy/api.py | 3 | ||||
-rw-r--r-- | nova/network/manager.py | 82 | ||||
-rw-r--r-- | nova/tests/network/test_manager.py | 2 | ||||
-rw-r--r-- | nova/tests/test_db_api.py | 15 |
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'} |