Merge "Move the assignment of existing_floating_ips before try block"

neutron/agent/l3/agent.py

@@ -385,6 +385,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
                 ri.disable_keepalived()
 
     def _process_external(self, ri):
+        existing_floating_ips = ri.floating_ips
         try:
             with ri.iptables_manager.defer_apply():
                 self._process_external_gateway(ri)
@@ -395,7 +396,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
                     return
 
                 # Process SNAT/DNAT rules and addresses for floating IPs
-                existing_floating_ips = ri.floating_ips
                 if ri.router['distributed']:
                     self.create_dvr_fip_interfaces(ri, ex_gw_port)
                 ri.process_snat_dnat_for_fip()

neutron/agent/linux/iptables_manager.py

@@ -384,8 +384,9 @@ class IptablesManager(object):
             try:
                 self.defer_apply_off()
             except Exception:
-                raise n_exc.IpTablesApplyException('Failure applying ip '
+                msg = _LE('Failure applying iptables rules')
-                                                   'tables rules')
+                LOG.exception(msg)
+                raise n_exc.IpTablesApplyException(msg)
 
     def defer_apply_on(self):
         self.iptables_apply_deferred = True

neutron/tests/unit/test_iptables_manager.py

@@ -18,10 +18,12 @@ import sys
 
 import mock
 from oslo_config import cfg
+import testtools
 
 from neutron.agent.common import config as a_cfg
 from neutron.agent.linux import iptables_comments as ic
 from neutron.agent.linux import iptables_manager
+from neutron.common import exceptions as n_exc
 from neutron.tests import base
 from neutron.tests import tools
 
@@ -247,6 +249,12 @@ class IptablesManagerStateFulTestCase(base.BaseTestCase):
         self.assertEqual(iptables_manager.get_chain_name(name, wrap=True),
                          name[:11])
 
+    def test_defer_apply_with_exception(self):
+        self.iptables._apply = mock.Mock(side_effect=Exception)
+        with testtools.ExpectedException(n_exc.IpTablesApplyException):
+            with self.iptables.defer_apply():
+                pass
+
     def _extend_with_ip6tables_filter(self, expected_calls, filter_dump):
         expected_calls.insert(2, (
             mock.call(['ip6tables-save', '-c'],

neutron/tests/unit/test_l3_agent.py

@@ -1286,6 +1286,30 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
                 mock.ANY, ri.router_id,
                 {fip_id: l3_constants.FLOATINGIP_STATUS_ERROR})
 
+    def test_process_external_iptables_exception(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        with mock.patch.object(
+            agent.plugin_rpc,
+            'update_floatingip_statuses') as mock_update_fip_status:
+            fip_id = _uuid()
+            router = prepare_router_data(num_internal_ports=1)
+            router[l3_constants.FLOATINGIP_KEY] = [
+                {'id': fip_id,
+                 'floating_ip_address': '8.8.8.8',
+                 'fixed_ip_address': '7.7.7.7',
+                 'port_id': router[l3_constants.INTERFACE_KEY][0]['id']}]
+
+            ri = l3router.RouterInfo(router['id'], router, **self.ri_kwargs)
+            ri.iptables_manager._apply = mock.Mock(side_effect=Exception)
+            agent._process_external(ri)
+            # Assess the call for putting the floating IP into Error
+            # was performed
+            mock_update_fip_status.assert_called_once_with(
+                mock.ANY, ri.router_id,
+                {fip_id: l3_constants.FLOATINGIP_STATUS_ERROR})
+
+            self.assertEqual(ri.iptables_manager._apply.call_count, 1)
+
     def test_handle_router_snat_rules_distributed_without_snat_manager(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         ri = dvr_router.DvrRouter(