Network: enable instance deletion when dhcp release fails

In the event that the 'dhcp_release' fails we should continue
with the deletion of the instance.

This will no longer the leave the instance in ERROR state.

Change-Id: Ie8259180a9df12865940907c728a8d9da3c9fda0
Closes-bug: #1289397

4 files changed, 63 insertions, 3 deletions

diff --git a/nova/exception.py b/nova/exception.py
index 0aa178db24..2bfd75e261 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -586,6 +586,10 @@ class NetworkDuplicated(Invalid):
     msg_fmt = _("Network %(network_id)s is duplicated.")
 
 
+class NetworkDhcpReleaseFailed(NovaException):
+    msg_fmt = _("Failed to release IP %(address)s with MAC %(mac_address)s")
+
+
 class NetworkInUse(NovaException):
     msg_fmt = _("Network %(network_id)s is still in use.")
 
diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index c929280a00..2cfd9f4501 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -1017,7 +1017,12 @@ def get_dhcp_opts(context, network_ref):
 
 
 def release_dhcp(dev, address, mac_address):
-    utils.execute('dhcp_release', dev, address, mac_address, run_as_root=True)
+    try:
+        utils.execute('dhcp_release', dev, address, mac_address,
+                      run_as_root=True)
+    except processutils.ProcessExecutionError:
+        raise exception.NetworkDhcpReleaseFailed(address=address,
+                                                 mac_address=mac_address)
 
 
 def update_dhcp(context, dev, network_ref):
diff --git a/nova/network/manager.py b/nova/network/manager.py
index 3b373c8ad4..bb128e56ca 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 _
+from nova.i18n import _, _LE
 from nova import ipv6
 from nova import manager
 from nova.network import api as network_api
@@ -1020,7 +1020,14 @@ class NetworkManager(manager.Manager):
                 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.
-                self.driver.release_dhcp(dev, address, vif.address)
+                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
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index a75a8528eb..0b1ef67bf1 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -1668,6 +1668,50 @@ class VlanNetworkTestCase(test.TestCase):
         fixed_update.assert_called_once_with(context1, fix_addr.address,
                                              {'allocated': False})
 
+    @mock.patch('nova.db.fixed_ip_get_by_address')
+    @mock.patch('nova.db.network_get')
+    @mock.patch('nova.db.fixed_ip_update')
+    def test_deallocate_fixed_with_dhcp_exception(self, fixed_update, net_get,
+                                                  fixed_get):
+        net_get.return_value = dict(test_network.fake_network,
+                                    **networks[1])
+
+        def vif_get(_context, _vif_id):
+            return vifs[0]
+
+        with contextlib.nested(
+            mock.patch.object(db, 'virtual_interface_get', vif_get),
+            mock.patch.object(
+                utils, 'execute',
+                side_effect=processutils.ProcessExecutionError()),
+        ) as (_vif_get, _execute):
+            context1 = context.RequestContext('user', 'project1')
+
+            instance = db.instance_create(context1,
+                    {'project_id': 'project1'})
+
+            elevated = context1.elevated()
+            fix_addr = db.fixed_ip_associate_pool(elevated, 1,
+                                                  instance['uuid'])
+            fixed_get.return_value = dict(test_fixed_ip.fake_fixed_ip,
+                                          address=fix_addr.address,
+                                          instance_uuid=instance.uuid,
+                                          allocated=True,
+                                          virtual_interface_id=3,
+                                          network=dict(
+                                              test_network.fake_network,
+                                              **networks[1]))
+            self.flags(force_dhcp_release=True)
+            self.network.deallocate_fixed_ip(context1, fix_addr.address,
+                                             'fake')
+            fixed_update.assert_called_once_with(context1, fix_addr.address,
+                                                 {'allocated': False})
+            _execute.assert_called_once_with('dhcp_release',
+                                             networks[1]['bridge'],
+                                             fix_addr.address,
+                                             'DE:AD:BE:EF:00:00',
+                                             run_as_root=True)
+
     def test_deallocate_fixed_deleted(self):
         # Verify doesn't deallocate deleted fixed_ip from deleted network.