Handle failures on update_dhcp_port

Ensure exceptions due to conflicting
state of network or subnet resources
are dealt with by the dhcp agent.

Closes-bug:  #1253344
Related-bug: #1243726

Change-Id: I4fd51442c034fabc91d5a3f065f4df98f5fad35b

diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py
index db744ba218b9787fd30e464f81dbd0810727a1f2..a815267e1658b37038e37cf193320bbcaf673b9e 100644 (file)
--- a/neutron/agent/dhcp_agent.py
+++ b/neutron/agent/dhcp_agent.py
@@ -448,12 +448,14 @@ class DhcpPluginApi(proxy.RpcProxy):
 
     def update_dhcp_port(self, port_id, port):
         """Make a remote process call to update the dhcp port."""
-        return dhcp.DictModel(self.call(self.context,
-                                        self.make_msg('update_dhcp_port',
-                                                      port_id=port_id,
-                                                      port=port,
-                                                      host=self.host),
-                                        topic=self.topic))
+        port = self.call(self.context,
+                         self.make_msg('update_dhcp_port',
+                                       port_id=port_id,
+                                       port=port,
+                                       host=self.host),
+                         topic=self.topic)
+        if port:
+            return dhcp.DictModel(port)
 
     def release_dhcp_port(self, network_id, device_id):
         """Make a remote process call to release the dhcp port."""

diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py
index de426ffc8db9450f1af54955195ad4b61a194ee7..423184a0dba3e748137758d2a7e0acecf1f09c2d 100644 (file)
--- a/neutron/agent/linux/dhcp.py
+++ b/neutron/agent/linux/dhcp.py
@@ -672,6 +672,8 @@ class DeviceManager(object):
                         [dict(subnet_id=s) for s in dhcp_enabled_subnet_ids])
                     dhcp_port = self.plugin.update_dhcp_port(
                         port.id, {'port': {'fixed_ips': port_fixed_ips}})
+                    if not dhcp_port:
+                        raise exceptions.Conflict()
                 else:
                     dhcp_port = port
                 # break since we found port that matches device_id

diff --git a/neutron/db/dhcp_rpc_base.py b/neutron/db/dhcp_rpc_base.py
index dced81a29e7c4fa17ae07a6c1240b07f99b4c0b2..056141270d2f79803db30430295d5465f1c541b0 100644 (file)
--- a/neutron/db/dhcp_rpc_base.py
+++ b/neutron/db/dhcp_rpc_base.py
@@ -51,6 +51,8 @@ class DhcpRpcCallbackMixin(object):
         try:
             if action == 'create_port':
                 return plugin.create_port(context, port)
+            elif action == 'update_port':
+                return plugin.update_port(context, port['id'], port['port'])
             else:
                 msg = _('Unrecognized action')
                 raise n_exc.Invalid(message=msg)
@@ -278,4 +280,6 @@ class DhcpRpcCallbackMixin(object):
                   {'port': port,
                    'host': host})
         plugin = manager.NeutronManager.get_plugin()
-        return plugin.update_port(context, port_id, port)
+        return self._port_action(plugin, context,
+                                 {'id': port_id, 'port': port},
+                                 'update_port')

diff --git a/neutron/tests/unit/test_db_rpc_base.py b/neutron/tests/unit/test_db_rpc_base.py
index 970cb4c5426fb331be3f490a162c259741a3ebba..ba225fcbf7f3ca049488ed55e80e23203a1db008 100644 (file)
--- a/neutron/tests/unit/test_db_rpc_base.py
+++ b/neutron/tests/unit/test_db_rpc_base.py
@@ -63,6 +63,22 @@ class TestDhcpRpcCallackMixin(base.BaseTestCase):
                                                       {'port': port},
                                                       action))
 
+    def _test__port_action_good_action(self, action, port, expected_call):
+        self.callbacks._port_action(self.plugin, mock.Mock(),
+                                    port, action)
+        self.plugin.assert_has_calls(expected_call)
+
+    def test_port_action_create_port(self):
+        self._test__port_action_good_action(
+            'create_port', mock.Mock(),
+            mock.call.create_port(mock.ANY, mock.ANY))
+
+    def test_port_action_update_port(self):
+        fake_port = {'id': 'foo_port_id', 'port': mock.Mock()}
+        self._test__port_action_good_action(
+            'update_port', fake_port,
+            mock.call.update_port(mock.ANY, 'foo_port_id', mock.ANY))
+
     def test__port_action_bad_action(self):
         self.assertRaises(
             n_exc.Invalid,
@@ -149,6 +165,14 @@ class TestDhcpRpcCallackMixin(base.BaseTestCase):
         self.plugin.assert_has_calls(expected)
         return retval
 
+    def test_update_dhcp_port(self):
+        self.callbacks.update_dhcp_port(mock.Mock(),
+                                        host='foo_host',
+                                        port_id='foo_port_id',
+                                        port=mock.Mock())
+        self.plugin.assert_has_calls(
+            mock.call.update_port(mock.ANY, 'foo_port_id', mock.ANY))
+
     def test_get_dhcp_port_existing(self):
         port_retval = dict(id='port_id', fixed_ips=[dict(subnet_id='a')])
         expectations = [

diff --git a/neutron/tests/unit/test_dhcp_agent.py b/neutron/tests/unit/test_dhcp_agent.py
index ae4d5a3b7092e250dc24cd3abfab3f5e329de254..7ba71d5bf1105b2ca8882b175b51ad2faff0fd0b 100644 (file)
--- a/neutron/tests/unit/test_dhcp_agent.py
+++ b/neutron/tests/unit/test_dhcp_agent.py
@@ -899,7 +899,13 @@ class TestDhcpPluginApiProxy(base.BaseTestCase):
                  'fixed_ips': [{'subnet_id': fake_fixed_ip1.subnet_id}],
                  'device_id': mock.ANY}})
         self.assertIsNone(self.proxy.create_dhcp_port(port_body))
-        self.proxy.create_dhcp_port(port_body)
+
+    def test_update_dhcp_port_none(self):
+        self.call.return_value = None
+        port_body = {'port': {'fixed_ips':
+                              [{'subnet_id': fake_fixed_ip1.subnet_id}]}}
+        self.assertIsNone(self.proxy.update_dhcp_port(fake_port1.id,
+                                                      port_body))
 
     def test_update_dhcp_port(self):
         port_body = {'port': {'fixed_ips':
@@ -1166,6 +1172,14 @@ class TestDeviceManager(base.BaseTestCase):
     def test_setup_device_exists_reuse(self):
         self._test_setup_helper(True, True)
 
+    def test_create_dhcp_port_raise_conflict(self):
+        plugin = mock.Mock()
+        dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, plugin)
+        plugin.create_dhcp_port.return_value = None
+        self.assertRaises(exceptions.Conflict,
+                          dh.setup_dhcp_port,
+                          fake_network)
+
     def test_create_dhcp_port_create_new(self):
         plugin = mock.Mock()
         dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, plugin)
@@ -1197,6 +1211,17 @@ class TestDeviceManager(base.BaseTestCase):
             mock.call.update_dhcp_port(fake_network_copy.ports[0].id,
                                        port_body)])
 
+    def test_update_dhcp_port_raises_conflict(self):
+        plugin = mock.Mock()
+        dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, plugin)
+        fake_network_copy = copy.deepcopy(fake_network)
+        fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network)
+        fake_network_copy.subnets[1].enable_dhcp = True
+        plugin.update_dhcp_port.return_value = None
+        self.assertRaises(exceptions.Conflict,
+                          dh.setup_dhcp_port,
+                          fake_network_copy)
+
     def test_create_dhcp_port_no_update_or_create(self):
         plugin = mock.Mock()
         dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, plugin)