From 635c5b611ae765bca3a888b1fcabe6bf0027c948 Mon Sep 17 00:00:00 2001 From: jichenjc Date: Tue, 25 Feb 2014 22:09:55 +0800 Subject: [PATCH] Add Quota roll back for deallocate fix ip in nova-network in nova-network, deallocate_fixed_ip function reserve quota first then do deallocate operations if any operation failed, the quota reserve operation need to be rollback. Change-Id: If57d2b8a24b2f6ed3d90a1357e89782194df013c Closes-Bug: #1284930 --- nova/network/manager.py | 116 ++++++++++++++++------------- nova/tests/network/test_manager.py | 16 ++++ 2 files changed, 80 insertions(+), 52 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 35e8628a1b..6b11204c3c 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -39,7 +39,7 @@ from oslo import messaging from nova import conductor from nova import context from nova import exception -from nova.i18n import _, _LE +from nova.i18n import _, _LE, _LW from nova import ipv6 from nova import manager from nova.network import api as network_api @@ -988,65 +988,77 @@ class NetworkManager(manager.Manager): LOG.exception(_("Failed to update usages deallocating " "fixed IP")) - self._do_trigger_security_group_members_refresh_for_instance( - instance_uuid) + try: + self._do_trigger_security_group_members_refresh_for_instance( + instance_uuid) - if self._validate_instance_zone_for_dns_domain(context, instance): - for n in self.instance_dns_manager.get_entries_by_address(address, - self.instance_dns_domain): - self.instance_dns_manager.delete_entry(n, - self.instance_dns_domain) + if self._validate_instance_zone_for_dns_domain(context, instance): + for n in self.instance_dns_manager.get_entries_by_address( + address, self.instance_dns_domain): + self.instance_dns_manager.delete_entry(n, + self.instance_dns_domain) - fixed_ip_ref.allocated = False - fixed_ip_ref.save() + fixed_ip_ref.allocated = False + fixed_ip_ref.save() - if teardown: - network = fixed_ip_ref.network + if teardown: + network = fixed_ip_ref.network - if CONF.force_dhcp_release: - dev = self.driver.get_dev(network) - # NOTE(vish): The below errors should never happen, but there - # may be a race condition that is causing them per - # https://code.launchpad.net/bugs/968457, so we log - # an error to help track down the possible race. - msg = _("Unable to release %s because vif doesn't exist.") - if not vif_id: - LOG.error(msg % address) - return + if CONF.force_dhcp_release: + dev = self.driver.get_dev(network) + # NOTE(vish): The below errors should never happen, but + # there may be a race condition that is causing + # them per + # https://code.launchpad.net/bugs/968457, + # so we log an error to help track down + # the possible race. + msg = _("Unable to release %s because vif doesn't exist.") + if not vif_id: + LOG.error(msg % address) + return - vif = objects.VirtualInterface.get_by_id(context, vif_id) + vif = objects.VirtualInterface.get_by_id(context, vif_id) - if not vif: - LOG.error(msg % address) - return + if not vif: + LOG.error(msg % address) + return - # NOTE(cfb): Call teardown before release_dhcp to ensure - # that the IP can't be re-leased after a release - # packet is sent. - self._teardown_network_on_host(context, network) - # NOTE(vish): This forces a packet so that the release_fixed_ip - # callback will get called by nova-dhcpbridge. + # NOTE(cfb): Call teardown before release_dhcp to ensure + # that the IP can't be re-leased after a release + # packet is sent. + self._teardown_network_on_host(context, network) + # NOTE(vish): This forces a packet so that the + # release_fixed_ip callback will + # get called by nova-dhcpbridge. + try: + self.driver.release_dhcp(dev, address, vif.address) + except exception.NetworkDhcpReleaseFailed: + LOG.error(_LE("Error releasing DHCP for IP %(address)s" + " with MAC %(mac_address)s"), + {'address': address, + 'mac_address': vif.address}, + instance=instance) + + # NOTE(yufang521247): This is probably a failed dhcp fixed + # ip. DHCPRELEASE packet sent to dnsmasq would not trigger + # dhcp-bridge to run. Thus it is better to disassociate + # such fixed ip here. + fixed_ip_ref = objects.FixedIP.get_by_address( + context, address) + if (instance_uuid == fixed_ip_ref.instance_uuid and + not fixed_ip_ref.leased): + fixed_ip_ref.disassociate() + else: + # We can't try to free the IP address so just call teardown + self._teardown_network_on_host(context, network) + except Exception: + with excutils.save_and_reraise_exception(): try: - self.driver.release_dhcp(dev, address, vif.address) - except exception.NetworkDhcpReleaseFailed: - LOG.error(_LE("Error releasing DHCP for IP %(address)s " - "with MAC %(mac_address)s"), - {'address': address, - 'mac_address': vif.address}, - instance=instance) - - # NOTE(yufang521247): This is probably a failed dhcp fixed ip. - # DHCPRELEASE packet sent to dnsmasq would not trigger - # dhcp-bridge to run. Thus it is better to disassociate such - # fixed ip here. - fixed_ip_ref = objects.FixedIP.get_by_address( - context, address) - if (instance_uuid == fixed_ip_ref.instance_uuid and - not fixed_ip_ref.leased): - fixed_ip_ref.disassociate() - else: - # We can't try to free the IP address so just call teardown - self._teardown_network_on_host(context, network) + quotas.rollback(context) + except Exception: + LOG.warn(_LW("Failed to rollback quota for " + "deallocate fixed ip: %s"), address, + instance=instance) # Commit the reservations quotas.commit(context) diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py index 0086fe9f43..cd71f68c65 100644 --- a/nova/tests/network/test_manager.py +++ b/nova/tests/network/test_manager.py @@ -2501,6 +2501,22 @@ class CommonNetworkTestCase(test.TestCase): # Determine networks to NAT based on lookup self._test_init_host_dynamic_fixed_range(self.network) + @mock.patch('nova.objects.quotas.Quotas.rollback') + @mock.patch('nova.objects.fixed_ip.FixedIP.get_by_address') + @mock.patch('nova.network.manager.NetworkManager.' + '_do_trigger_security_group_members_refresh_for_instance') + def test_fixed_ip_cleanup_rollback(self, fake_trig, + fixed_get, rollback): + manager = network_manager.NetworkManager() + + fake_trig.side_effect = test.TestingException + + self.assertRaises(test.TestingException, + manager.deallocate_fixed_ip, + self.context, 'fake', 'fake', + instance=fake_inst(uuid='ignoreduuid')) + rollback.assert_called_once_with(self.context) + class TestRPCFixedManager(network_manager.RPCAllocateFixedIP, network_manager.NetworkManager):