From f945d84fc973220e4bdf0897c003a39640960f0b Mon Sep 17 00:00:00 2001 From: Miguel Lavalle Date: Wed, 17 Dec 2014 19:21:53 -0600 Subject: [PATCH] Refactor of floating ip processing in L3 Agent Breaks down the processing of floating ip processing in 3 methods. A key goal is to make the handling of ip tables application deferal more logically cohesive in the process_router method Partially-Implements: bp restructure-l3-agent Change-Id: I7594c16a3ee3b9c988c7db856f9f401e4742f4c2 --- neutron/agent/l3/agent.py | 61 +++++++++++++++++-------- neutron/agent/linux/iptables_manager.py | 15 ++++++ neutron/common/exceptions.py | 12 +++++ neutron/tests/unit/test_l3_agent.py | 37 +++++++++++++++ 4 files changed, 106 insertions(+), 19 deletions(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index b06752824..566cc1c61 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -494,25 +494,30 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, ri.perform_snat_action(self._handle_router_snat_rules, interface_name) - def _process_snat_dnat_for_fip(self, ri): - ex_gw_port = self._get_ex_gw_port(ri) - if not ex_gw_port: - return + def _put_fips_in_error_state(self, ri): fip_statuses = {} + for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []): + fip_statuses[fip['id']] = l3_constants.FLOATINGIP_STATUS_ERROR + return fip_statuses + + def _process_snat_dnat_for_fip(self, ri): try: - existing_floating_ips = ri.floating_ips self.process_router_floating_ip_nat_rules(ri) - ri.iptables_manager.defer_apply_off() - # Once NAT rules for floating IPs are safely in place - # configure their addresses on the external gateway port - fip_statuses = self.process_router_floating_ip_addresses( + except Exception: + # TODO(salv-orlando): Less broad catching + raise n_exc.FloatingIpSetupException('L3 agent failure to setup ' + 'NAT for floating IPs') + + def _configure_fip_addresses(self, ri, ex_gw_port): + try: + return self.process_router_floating_ip_addresses( ri, ex_gw_port) except Exception: # TODO(salv-orlando): Less broad catching - # All floating IPs must be put in error state - for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []): - fip_statuses[fip['id']] = l3_constants.FLOATINGIP_STATUS_ERROR + raise n_exc.FloatingIpSetupException('L3 agent failure to setup ' + 'floating IPs') + def _update_fip_statuses(self, ri, existing_floating_ips, fip_statuses): # Identify floating IPs which were disabled ri.floating_ips = set(fip_statuses.keys()) for fip_id in existing_floating_ips - ri.floating_ips: @@ -528,22 +533,40 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, else: ri.disable_keepalived() + def _process_external(self, ri): + try: + with ri.iptables_manager.defer_apply(): + self._process_external_gateway(ri) + ex_gw_port = self._get_ex_gw_port(ri) + # TODO(Carl) Return after setting existing_floating_ips and + # still call _update_fip_statuses? + if not ex_gw_port: + return + + # Process SNAT/DNAT rules and addresses for floating IPs + existing_floating_ips = ri.floating_ips + self._process_snat_dnat_for_fip(ri) + + # Once NAT rules for floating IPs are safely in place + # configure their addresses on the external gateway port + fip_statuses = self._configure_fip_addresses(ri, ex_gw_port) + + except (n_exc.FloatingIpSetupException, n_exc.IpTablesApplyException): + # All floating IPs must be put in error state + fip_statuses = self._put_fips_in_error_state(ri) + + self._update_fip_statuses(ri, existing_floating_ips, fip_statuses) + @common_utils.exception_logger() def process_router(self, ri): # TODO(mrsmith) - we shouldn't need to check here if 'distributed' not in ri.router: ri.router['distributed'] = False - - ri.iptables_manager.defer_apply_on() self._process_internal_ports(ri) - self._process_external_gateway(ri) - + self._process_external(ri) # Process static routes for router self.routes_updated(ri) - # Process SNAT/DNAT rules for floating IPs - self._process_snat_dnat_for_fip(ri) - # Enable or disable keepalived for ha routers self._process_ha_router(ri) diff --git a/neutron/agent/linux/iptables_manager.py b/neutron/agent/linux/iptables_manager.py index ce8dae365..5c04b5e64 100644 --- a/neutron/agent/linux/iptables_manager.py +++ b/neutron/agent/linux/iptables_manager.py @@ -18,6 +18,7 @@ """Implements iptables rules using linux utilities.""" +import contextlib import os import re import sys @@ -28,6 +29,7 @@ from oslo.utils import excutils from neutron.agent.common import config from neutron.agent.linux import iptables_comments as ic from neutron.agent.linux import utils as linux_utils +from neutron.common import exceptions as n_exc from neutron.common import utils from neutron.i18n import _LE, _LW from neutron.openstack.common import lockutils @@ -374,6 +376,19 @@ class IptablesManager(object): def is_chain_empty(self, table, chain, ip_version=4, wrap=True): return not self.get_chain(table, chain, ip_version, wrap) + @contextlib.contextmanager + def defer_apply(self): + """Defer apply context.""" + self.defer_apply_on() + try: + yield + finally: + try: + self.defer_apply_off() + except Exception: + raise n_exc.IpTablesApplyException('Failure applying ip ' + 'tables rules') + def defer_apply_on(self): self.iptables_apply_deferred = True diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index a629f26cd..a8d50c3e3 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -344,3 +344,15 @@ class RouterNotCompatibleWithAgent(NeutronException): class FailToDropPrivilegesExit(SystemExit): """Exit exception raised when a drop privileges action fails.""" code = 99 + + +class FloatingIpSetupException(NeutronException): + def __init__(self, message=None): + self.message = message + super(FloatingIpSetupException, self).__init__() + + +class IpTablesApplyException(NeutronException): + def __init__(self, message=None): + self.message = message + super(IpTablesApplyException, self).__init__() diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index f5ca6e63a..865072ca9 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -2139,3 +2139,40 @@ class TestBasicRouterOperations(base.BaseTestCase): asserter = self.assertIn if flag_set else self.assertNotIn asserter('AdvOtherConfigFlag on;', self.utils_replace_file.call_args[0][1]) + + def test__put_fips_in_error_state(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + ri = mock.Mock() + ri.router.get.return_value = [{'id': mock.sentinel.id1}, + {'id': mock.sentinel.id2}] + + statuses = agent._put_fips_in_error_state(ri) + + expected = [{mock.sentinel.id1: l3_constants.FLOATINGIP_STATUS_ERROR, + mock.sentinel.id2: l3_constants.FLOATINGIP_STATUS_ERROR}] + self.assertNotEqual(expected, statuses) + + def test__process_snat_dnat_for_fip(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + agent.process_router_floating_ip_nat_rules = mock.Mock( + side_effect=Exception) + + self.assertRaises(n_exc.FloatingIpSetupException, + agent._process_snat_dnat_for_fip, + mock.sentinel.ri) + + agent.process_router_floating_ip_nat_rules.assert_called_with( + mock.sentinel.ri) + + def test__configure_fip_addresses(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + agent.process_router_floating_ip_addresses = mock.Mock( + side_effect=Exception) + + self.assertRaises(n_exc.FloatingIpSetupException, + agent._configure_fip_addresses, + mock.sentinel.ri, + mock.sentinel.ex_gw_port) + + agent.process_router_floating_ip_addresses.assert_called_with( + mock.sentinel.ri, mock.sentinel.ex_gw_port) -- 2.45.2