diff options
-rw-r--r-- | nova/db/api.py | 10 | ||||
-rw-r--r-- | nova/db/sqlalchemy/api.py | 3 | ||||
-rw-r--r-- | nova/network/manager.py | 91 | ||||
-rw-r--r-- | nova/tests/network/test_manager.py | 2 | ||||
-rw-r--r-- | nova/tests/test_db_api.py | 15 |
5 files changed, 83 insertions, 38 deletions
diff --git a/nova/db/api.py b/nova/db/api.py index 4acff8a99..3e350fc75 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -290,7 +290,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) @@ -298,7 +299,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 8d087f28d..a1cca73be 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -761,9 +761,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 0e8530d14..3b1c2abb1 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -582,29 +582,35 @@ class FloatingIP(object): def _associate_floating_ip(self, context, floating_address, fixed_address, interface, instance_uuid): """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, - instance_id=instance_uuid, - floating_ip=floating_address) - notifier.notify(context, - notifier.publisher_id("network"), - 'network.floating_ip.associate', - notifier.INFO, payload=payload) + + @lockutils.synchronized(unicode(floating_address), 'nova-') + 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, + instance_id=instance_uuid, + floating_ip=floating_address) + notifier.notify(context, + notifier.publisher_id("network"), + 'network.floating_ip.associate', + notifier.INFO, payload=payload) + do_associate() @rpc_common.client_exceptions(exception.FloatingIpNotFoundForAddress) @wrap_check_policy @@ -664,18 +670,33 @@ class FloatingIP(object): instance_uuid): """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, - instance_id=instance_uuid, - floating_ip=address) - notifier.notify(context, - notifier.publisher_id("network"), - 'network.floating_ip.disassociate', - notifier.INFO, payload=payload) + + @lockutils.synchronized(unicode(address), 'nova-') + 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, + instance_id=instance_uuid, + floating_ip=address) + notifier.notify(context, + notifier.publisher_id("network"), + 'network.floating_ip.disassociate', + notifier.INFO, payload=payload) + do_disassociate() @rpc_common.client_exceptions(exception.FloatingIpNotFound) @wrap_check_policy diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py index e80ea3936..eb3dcf14a 100644 --- a/nova/tests/network/test_manager.py +++ b/nova/tests/network/test_manager.py @@ -669,7 +669,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 ea6e9aea5..af329daf6 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -246,6 +246,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'} |