[DVR] Check if SNAT iptables manager is initialized

Check if SNAT iptables manager is initialized before processing the
IP NAT rules. If the router never had an external GW port, the DVR
GW in the SNAT namespace has not been created and the SNAT iptables
manager has not been initialized.

In this case, the IP NAT rules for centralized FIPs (to be applied
on the SNAT namespace) cannot be set.

Closes-Bug: #1945215
Change-Id: I426602514805d728f8cd78e42f2b0979b2101089

neutron/agent/l3/dvr_edge_router.py

@@ -398,7 +398,10 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
 
     def process_floating_ip_nat_rules(self):
         if self._is_this_snat_host():
-            self.process_floating_ip_nat_rules_for_centralized_floatingip()
+            if not self.snat_iptables_manager:
+                LOG.debug("DVR router: no snat rules to be handled")
+            else:
+                self.process_floating_ip_nat_rules_for_centralized_floatingip()
 
         # Cover mixed dvr_snat and compute node, aka a dvr_snat node has both
         # centralized and distributed floating IPs.

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

@@ -524,19 +524,22 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
                 msg = 'PID file %s is not present.' % pid_file
             self.fail(msg)
 
-    def _assert_snat_chains(self, router):
+    def _assert_snat_chains(self, router, enable_gw=True):
-        self.assertFalse(router.iptables_manager.is_chain_empty(
+        check = self.assertFalse if enable_gw else self.assertTrue
-            'nat', 'snat'))
+        check(router.iptables_manager.is_chain_empty('nat', 'snat'))
-        self.assertFalse(router.iptables_manager.is_chain_empty(
+        check(router.iptables_manager.is_chain_empty('nat', 'POSTROUTING'))
-            'nat', 'POSTROUTING'))
 
-    def _assert_floating_ip_chains(self, router, snat_bound_fip=False):
+    def _assert_floating_ip_chains(self, router, snat_bound_fip=False,
+                                   enable_gw=True):
         if snat_bound_fip:
-            self.assertFalse(router.snat_iptables_manager.is_chain_empty(
+            if enable_gw:
-                'nat', 'float-snat'))
+                self.assertFalse(router.snat_iptables_manager.is_chain_empty(
+                    'nat', 'float-snat'))
+            else:
+                self.assertIsNone(router.snat_iptables_manager)
 
-        self.assertFalse(router.iptables_manager.is_chain_empty(
+        check = self.assertFalse if enable_gw else self.assertTrue
-            'nat', 'float-snat'))
+        check(router.iptables_manager.is_chain_empty('nat', 'float-snat'))
 
     def _assert_iptables_rules_converged(self, router):
         # if your code is failing on this line, it means you are not generating
@@ -563,7 +566,7 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
             self.assertTrue(self.device_exists_with_ips_and_mac(
                 device, router.get_internal_device_name, router.ns_name))
 
-    def _assert_extra_routes(self, router, namespace=None):
+    def _assert_extra_routes(self, router, namespace=None, enable_gw=True):
         if namespace is None:
             namespace = router.ns_name
         routes = ip_lib.list_ip_routes(namespace, constants.IP_VERSION_4)
@@ -571,17 +574,24 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
                    'destination': route['cidr']} for route in routes]
 
         for extra_route in router.router['routes']:
-            self.assertIn(extra_route, routes)
+            check = self.assertIn if enable_gw else self.assertNotIn
+            check(extra_route, routes)
 
     def _assert_onlink_subnet_routes(
-            self, router, ip_versions, namespace=None):
+            self, router, ip_versions, namespace=None, enable_gw=True):
         ns_name = namespace or router.ns_name
         routes = []
         for ip_version in ip_versions:
             _routes = ip_lib.list_ip_routes(ns_name, ip_version)
             routes.extend(_routes)
         routes = set(route['cidr'] for route in routes)
-        extra_subnets = router.get_ex_gw_port()['extra_subnets']
+        ex_gw_port = router.get_ex_gw_port()
+        if not ex_gw_port:
+            if not enable_gw:
+                return
+            self.fail('GW port is enabled but not present in the router')
+
+        extra_subnets = ex_gw_port['extra_subnets']
         for extra_subnet in (route['cidr'] for route in extra_subnets):
             self.assertIn(extra_subnet, routes)
 

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

@@ -215,6 +215,15 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         self._dvr_router_lifecycle(enable_ha=True, enable_snat=True,
                                    snat_bound_fip=True)
 
+    def test_dvr_lifecycle_no_ha_with_snat_with_fips_with_cent_fips_no_gw(
+            self):
+        self._dvr_router_lifecycle(enable_ha=False, enable_snat=True,
+                                   snat_bound_fip=True, enable_gw=False)
+
+    def test_dvr_lifecycle_ha_with_snat_with_fips_with_cent_fips_no_gw(self):
+        self._dvr_router_lifecycle(enable_ha=True, enable_snat=True,
+                                   snat_bound_fip=True, enable_gw=False)
+
     def _check_routes(self, expected_routes, actual_routes):
         actual_routes = [{key: route[key] for key in expected_routes[0].keys()}
                          for route in actual_routes]
@@ -541,7 +550,8 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
                               custom_mtu=2000,
                               ip_version=lib_constants.IP_VERSION_4,
                               dual_stack=False,
-                              snat_bound_fip=False):
+                              snat_bound_fip=False,
+                              enable_gw=True):
         '''Test dvr router lifecycle
 
         :param enable_ha: sets the ha value for the router.
@@ -557,12 +567,13 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         # We get the router info particular to a dvr router
         router_info = self.generate_dvr_router_info(
             enable_ha, enable_snat, extra_routes=True,
-            snat_bound_fip=snat_bound_fip)
+            snat_bound_fip=snat_bound_fip, enable_gw=enable_gw)
         for key in ('_interfaces', '_snat_router_interfaces',
                     '_floatingip_agent_interfaces'):
-            for port in router_info[key]:
+            for port in router_info.get(key, []):
                 port['mtu'] = custom_mtu
-        router_info['gw_port']['mtu'] = custom_mtu
+        if router_info['gw_port']:
+            router_info['gw_port']['mtu'] = custom_mtu
         if enable_ha:
             router_info['_ha_interface']['mtu'] = custom_mtu
 
@@ -580,7 +591,10 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         # manage the router (create it, create namespaces,
         # attach interfaces, etc...)
         router = self.manage_router(self.agent, router_info)
-        if enable_ha:
+        if enable_ha and not enable_gw:
+            port = router.get_ex_gw_port()
+            self.assertEqual({}, port)
+        elif enable_ha and enable_gw:
             port = router.get_ex_gw_port()
             interface_name = router.get_external_device_name(port['id'])
             self._assert_no_ip_addresses_on_interface(router.ha_namespace,
@@ -607,13 +621,15 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         utils.wait_until_true(
             lambda: self._metadata_proxy_exists(self.agent.conf, router))
         self._assert_internal_devices(router)
-        self._assert_dvr_external_device(router)
+        self._assert_dvr_external_device(router, enable_gw)
-        self._assert_dvr_gateway(router)
+        self._assert_dvr_gateway(router, enable_gw)
-        self._assert_dvr_floating_ips(router, snat_bound_fip=snat_bound_fip)
+        self._assert_dvr_floating_ips(router, snat_bound_fip=snat_bound_fip,
-        self._assert_snat_chains(router)
+                                      enable_gw=enable_gw)
-        self._assert_floating_ip_chains(router, snat_bound_fip=snat_bound_fip)
+        self._assert_snat_chains(router, enable_gw=enable_gw)
+        self._assert_floating_ip_chains(router, snat_bound_fip=snat_bound_fip,
+                                        enable_gw=enable_gw)
         self._assert_metadata_chains(router)
-        self._assert_rfp_fpr_mtu(router, custom_mtu)
+        self._assert_rfp_fpr_mtu(router, custom_mtu, enable_gw=enable_gw)
         if enable_snat:
             if (ip_version == lib_constants.IP_VERSION_6 or dual_stack):
                 ip_versions = [lib_constants.IP_VERSION_4,
@@ -623,8 +639,9 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
             snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
                 router.router_id)
             self._assert_onlink_subnet_routes(
-                router, ip_versions, snat_ns_name)
+                router, ip_versions, snat_ns_name, enable_gw=enable_gw)
-            self._assert_extra_routes(router, namespace=snat_ns_name)
+            self._assert_extra_routes(router, namespace=snat_ns_name,
+                                      enable_gw=enable_gw)
 
         # During normal operation, a router-gateway-clear followed by
         # a router delete results in two notifications to the agent.  This
@@ -633,7 +650,8 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         # 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_fip_namespace_deleted(ext_gateway_port,
+                                           enable_gw=enable_gw)
         self._assert_router_does_not_exist(router)
         self._assert_snat_namespace_does_not_exist(router)
 
@@ -666,8 +684,13 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
             }
             return fip_agent_gw_port_info
 
-    def _assert_dvr_external_device(self, router):
+    def _assert_dvr_external_device(self, router, enable_gw):
         external_port = router.get_ex_gw_port()
+        if not external_port:
+            if not enable_gw:
+                return
+            self.fail('GW port is enabled but not present in the router')
+
         snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
             router.router_id)
 
@@ -694,12 +717,12 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         else:
             self.fail("Agent not configured for dvr or dvr_snat")
 
-    def _assert_dvr_gateway(self, router):
+    def _assert_dvr_gateway(self, router, enable_gw):
         gateway_expected_in_snat_namespace = (
             self.agent.conf.agent_mode == 'dvr_snat'
         )
         if gateway_expected_in_snat_namespace:
-            self._assert_dvr_snat_gateway(router)
+            self._assert_dvr_snat_gateway(router, enable_gw=enable_gw)
             self._assert_removal_of_already_deleted_gateway_device(router)
 
         snat_namespace_should_not_exist = (
@@ -708,10 +731,15 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         if snat_namespace_should_not_exist:
             self._assert_snat_namespace_does_not_exist(router)
 
-    def _assert_dvr_snat_gateway(self, router):
+    def _assert_dvr_snat_gateway(self, router, enable_gw=True):
         namespace = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
             router.router_id)
         external_port = router.get_ex_gw_port()
+        if not external_port:
+            if not enable_gw:
+                return
+            self.fail('GW port is enabled but not present in the router')
+
         external_device_name = router.get_external_device_name(
             external_port['id'])
         external_device = ip_lib.IPDevice(external_device_name,
@@ -736,7 +764,8 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
             router.router_id)
         self.assertFalse(self._namespace_exists(namespace))
 
-    def _assert_dvr_floating_ips(self, router, snat_bound_fip=False):
+    def _assert_dvr_floating_ips(self, router, snat_bound_fip=False,
+                                 enable_gw=True):
         # in the fip namespace:
         # Check that the fg-<port-id> (floatingip_agent_gateway)
         # is created with the ip address of the external gateway port
@@ -744,6 +773,9 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         self.assertTrue(floating_ips)
         # We need to fetch the floatingip agent gateway port info
         # from the router_info
+        if not enable_gw:
+            return
+
         floating_agent_gw_port = (
             router.router[lib_constants.FLOATINGIP_AGENT_INTF_KEY])
         self.assertTrue(floating_agent_gw_port)
@@ -1037,7 +1069,11 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
                 dst=str(not_expected_neighbor))
             self.assertEqual([], neighbor)
 
-    def _assert_rfp_fpr_mtu(self, router, expected_mtu=1500):
+    def _assert_rfp_fpr_mtu(self, router, expected_mtu=1500, enable_gw=True):
+        if not enable_gw:
+            self.assertIsNone(router.fip_ns)
+            return
+
         dev_mtu = self.get_device_mtu(
             router.router_id, router.fip_ns.get_rtr_ext_device_name,
             router.ns_name)
@@ -2040,7 +2076,12 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
         self.assertIsNone(fg_device.route.get_gateway())
 
     def _assert_fip_namespace_deleted(self, ext_gateway_port,
-                                      assert_ovs_interface=True):
+                                      assert_ovs_interface=True,
+                                      enable_gw=True):
+        if not enable_gw:
+            self.assertEqual({}, ext_gateway_port)
+            return
+
         ext_net_id = ext_gateway_port['network_id']
         fip_ns = self.agent.get_fip_ns(ext_net_id)
         fip_ns.unsubscribe = mock.Mock()