Merge "Refactor router delete processing"

neutron/agent/l3/dvr_edge_ha_router.py

@@ -88,8 +88,8 @@ class DvrEdgeHaRouter(DvrEdgeRouter, HaRouter):
         self._create_snat_namespace()
         super(DvrEdgeHaRouter, self).initialize(process_monitor)
 
-    def process(self, agent, delete=False):
+    def process(self, agent):
-        super(DvrEdgeHaRouter, self).process(agent, delete)
+        super(DvrEdgeHaRouter, self).process(agent)
         if self.ha_port:
             self.enable_keepalived()
 

neutron/agent/l3/dvr_local_router.py

@@ -414,11 +414,11 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
     def _handle_router_snat_rules(self, ex_gw_port, interface_name):
         pass
 
-    def process_external(self, agent, delete=False):
+    def process_external(self, agent):
         ex_gw_port = self.get_ex_gw_port()
         if ex_gw_port:
             self.create_dvr_fip_interfaces(ex_gw_port)
-        super(DvrLocalRouter, self).process_external(agent, delete)
+        super(DvrLocalRouter, self).process_external(agent)
 
     def create_dvr_fip_interfaces(self, ex_gw_port):
         floating_ips = self.get_floating_ips()
@@ -455,10 +455,10 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
                 # configured
                 self.agent.process_router_add(self)
 
-    def process(self, agent, delete=False):
+    def process(self, agent):
         ex_gw_port = self.get_ex_gw_port()
         if ex_gw_port:
             self.fip_ns = agent.get_fip_ns(ex_gw_port['network_id'])
             self.fip_ns.scan_fip_ports(self)
 
-        super(DvrLocalRouter, self).process(agent, delete)
+        super(DvrLocalRouter, self).process(agent)

neutron/agent/l3/dvr_router_base.py

@@ -26,8 +26,8 @@ class DvrRouterBase(router.RouterInfo):
         self.agent = agent
         self.host = host
 
-    def process(self, agent, delete=False):
+    def process(self, agent):
-        super(DvrRouterBase, self).process(agent, delete)
+        super(DvrRouterBase, self).process(agent)
         # NOTE:  Keep a copy of the interfaces around for when they are removed
         self.snat_ports = self.get_snat_interfaces()
 

neutron/agent/l3/ha_router.py

@@ -380,8 +380,8 @@ class HaRouter(router.RouterInfo):
         self.ha_network_removed()
         self.disable_keepalived()
 
-    def process(self, agent, delete=False):
+    def process(self, agent):
-        super(HaRouter, self).process(agent, delete)
+        super(HaRouter, self).process(agent)
 
         if self.ha_port:
             self.enable_keepalived()

neutron/agent/l3/router_info.py

@@ -255,7 +255,7 @@ class RouterInfo(object):
         self.router['gw_port'] = None
         self.router[l3_constants.INTERFACE_KEY] = []
         self.router[l3_constants.FLOATINGIP_KEY] = []
-        self.process(agent, delete=True)
+        self.process_delete(agent)
         self.disable_radvd()
         self.router_namespace.delete()
 
@@ -646,35 +646,50 @@ class RouterInfo(object):
                              self.iptables_manager,
                              interface_name)
 
-    def process_external(self, agent, delete=False):
+    def _process_external_on_delete(self, agent):
         fip_statuses = {}
-        if not delete:
+        try:
-            try:
-                with self.iptables_manager.defer_apply():
-                    ex_gw_port = self.get_ex_gw_port()
-                    self._process_external_gateway(ex_gw_port, agent.pd)
-                    if not ex_gw_port:
-                        return
-
-                    # Process SNAT/DNAT rules and addresses for floating IPs
-                    self.process_snat_dnat_for_fip()
-
-                # Once NAT rules for floating IPs are safely in place
-                # configure their addresses on the external gateway port
-                interface_name = self.get_external_device_interface_name(
-                    ex_gw_port)
-                fip_statuses = self.configure_fip_addresses(interface_name)
-
-            except (n_exc.FloatingIpSetupException,
-                    n_exc.IpTablesApplyException):
-                    # All floating IPs must be put in error state
-                    LOG.exception(_LE("Failed to process floating IPs."))
-                    fip_statuses = self.put_fips_in_error_state()
-            finally:
-                self.update_fip_statuses(agent, fip_statuses)
-        else:
             ex_gw_port = self.get_ex_gw_port()
             self._process_external_gateway(ex_gw_port, agent.pd)
+            if not ex_gw_port:
+                return
+
+            interface_name = self.get_external_device_interface_name(
+                ex_gw_port)
+            fip_statuses = self.configure_fip_addresses(interface_name)
+
+        except (n_exc.FloatingIpSetupException):
+                # All floating IPs must be put in error state
+                LOG.exception(_LE("Failed to process floating IPs."))
+                fip_statuses = self.put_fips_in_error_state()
+        finally:
+            self.update_fip_statuses(agent, fip_statuses)
+
+    def process_external(self, agent):
+        fip_statuses = {}
+        try:
+            with self.iptables_manager.defer_apply():
+                ex_gw_port = self.get_ex_gw_port()
+                self._process_external_gateway(ex_gw_port, agent.pd)
+                if not ex_gw_port:
+                    return
+
+                # Process SNAT/DNAT rules and addresses for floating IPs
+                self.process_snat_dnat_for_fip()
+
+            # Once NAT rules for floating IPs are safely in place
+            # configure their addresses on the external gateway port
+            interface_name = self.get_external_device_interface_name(
+                ex_gw_port)
+            fip_statuses = self.configure_fip_addresses(interface_name)
+
+        except (n_exc.FloatingIpSetupException,
+                n_exc.IpTablesApplyException):
+                # All floating IPs must be put in error state
+                LOG.exception(_LE("Failed to process floating IPs."))
+                fip_statuses = self.put_fips_in_error_state()
+        finally:
+            self.update_fip_statuses(agent, fip_statuses)
 
     def update_fip_statuses(self, agent, fip_statuses):
         # Identify floating IPs which were disabled
@@ -693,19 +708,34 @@ class RouterInfo(object):
             agent.context, self.router_id, fip_statuses)
 
     @common_utils.exception_logger()
-    def process(self, agent, delete=False):
+    def process_delete(self, agent):
+        """Process the delete of this router
+
+        This method is the point where the agent requests that this router
+        be deleted. This is a separate code path from process in that it
+        avoids any changes to the qrouter namespace that will be removed
+        at the end of the operation.
+
+        :param agent: Passes the agent in order to send RPC messages.
+        """
+        LOG.debug("process router delete")
+        self._process_internal_ports(agent.pd)
+        agent.pd.sync_router(self.router['id'])
+        self._process_external_on_delete(agent)
+
+    @common_utils.exception_logger()
+    def process(self, agent):
         """Process updates to this router
 
         This method is the point where the agent requests that updates be
         applied to this router.
 
         :param agent: Passes the agent in order to send RPC messages.
-        :param delete: Indicates whether this update is from a delete operation
         """
         LOG.debug("process router updates")
         self._process_internal_ports(agent.pd)
         agent.pd.sync_router(self.router['id'])
-        self.process_external(agent, delete)
+        self.process_external(agent)
         # Process static routes for router
         self.routes_updated(self.routes, self.router['routes'])
         self.routes = self.router['routes']

neutron/tests/functional/agent/l3/test_dvr_router.py

@@ -172,6 +172,13 @@ class TestDvrRouter(framework.L3AgentTestFramework):
             self._assert_onlink_subnet_routes(
                 router, ip_versions, snat_ns_name)
             self._assert_extra_routes(router, namespace=snat_ns_name)
+
+        # During normal operation, a router-gateway-clear followed by
+        # a router delete results in two notifications to the agent.  This
+        # code flow simulates the exceptional case where the notification of
+        # the clearing of the gateway hast been missed, so we are checking
+        # that the L3 agent is robust enough to handle that case and delete
+        # the router correctly.
         self._delete_router(self.agent, router.router_id)
         self._assert_fip_namespace_deleted(ext_gateway_port)
         self._assert_router_does_not_exist(router)