Allow dvr_snat l3 agent mode to be used with DVR

Currently it is a requirement to have a network node with an l3 agent
running in the dvr_snat mode even for DVR deployments that do not use
SNAT or have a very limited usage of SNAT.

It is not possible to disable snat completely:
https://bugs.launchpad.net/neutron/+bug/1761591

Neutron creates a network:router_centralized_snat port and if it is not
possible to find a dvr_snat agent to schedule it on there are various
side-effects which are not seen at first. For example, Designate stops
creating records for floating IPs and Neutron/Designate integration is,
therefore, not functional.

The Neutron DVR documentation says that dvr_snat should be used on
network nodes. However, there is nothing restricting a DVR deployment
from using dvr_snat l3 agents on every compute node and not having
dedicated network nodes.

This change modifies neutron-openvswitch to optionally enable dvr_snat
l3 agent mode (this includes supporting L3HA routers if enabled). As a
result, it is possible to have deployments without neutron-gateway thus
saving on the amount of required nodes. Care should be taken when a
large amount of L3HA routers is used and using DVR routers without L3HA
is a recommended.

Change-Id: Iad3a64967f91c81312911f6db856ce2271b0e068
Closes-Bug: #1808045

config.yaml

@@ -278,3 +278,20 @@ options:
       This option sets the maximum queue size for log entries.
       Can be used to avoid excessive memory consumption.
       WARNING: Should be NOT LESS than 25.
+  use-dvr-snat:
+    type: boolean
+    default: False
+    description: |
+      This option controls a mode of the l3 agent when DVR is used. There are
+      2 modes 'dvr' (default) and dvr_snat (gateway mode). Neutron server
+      (deployed by neutron-api charm) will schedule a network:router_centralized_snat port
+      and a (centralized) snat namespace to dvr_snat mode agents only. If this option
+      is enabled, all neutron-openvswitch nodes become candidates for being centralized
+      snat nodes. If l3ha is enabled on neutron-api, relevant packages are also installed
+      on every unit making them capable of hosting parts of an L3HA router. The min and max
+      numbers of L3 agents per router need to be taken into account in this case
+      (see max_l3_agents_per_router and min_l3_agents_per_router Neutron options).
+      Practically, this option can be used to allow DVR routers (L3HA or not) to
+      be scheduled without a requirement for a dedicated network node to host
+      centralized SNAT. This is especially important if only floating IPs are
+      used in the network design and SNAT traffic is minimal or non-existent.

hooks/neutron_ovs_context.py

@@ -280,7 +280,9 @@ class L3AgentContext(OSContextGenerator):
         neutron_api_settings = NeutronAPIContext()()
         ctxt = {}
         if neutron_api_settings['enable_dvr']:
-            ctxt['agent_mode'] = 'dvr'
+            use_dvr_snat = config('use-dvr-snat')
+            agent_mode = 'dvr_snat' if use_dvr_snat else 'dvr'
+            ctxt['agent_mode'] = agent_mode
             if not config('ext-port'):
                 ctxt['external_configuration_new'] = True
         else:

hooks/neutron_ovs_hooks.py

@@ -24,6 +24,8 @@ from charmhelpers.contrib.openstack.utils import (
     series_upgrade_prepare,
     series_upgrade_complete,
     is_unit_paused_set,
+    CompareOpenStackReleases,
+    os_release,
 )
 
 from charmhelpers.core.hookenv import (
@@ -38,6 +40,7 @@ from charmhelpers.core.hookenv import (
 from neutron_ovs_utils import (
     DHCP_PACKAGES,
     DVR_PACKAGES,
+    L3HA_PACKAGES,
     METADATA_PACKAGES,
     OVS_DEFAULT,
     configure_ovs,
@@ -46,9 +49,11 @@ from neutron_ovs_utils import (
     register_configs,
     restart_map,
     use_dvr,
+    use_l3ha,
     enable_nova_metadata,
     enable_local_dhcp,
     install_packages,
+    install_l3ha_packages,
     purge_packages,
     assess_status,
     install_tmpfilesd,
@@ -126,10 +131,23 @@ def config_changed():
 @hooks.hook('neutron-plugin-api-relation-changed')
 @restart_on_change(restart_map())
 def neutron_plugin_api_changed():
+    packages_to_purge = []
     if use_dvr():
         install_packages()
+        # per 17.08 release notes L3HA + DVR is a Newton+ feature
+        _os_release = os_release('neutron-common', base='icehouse')
+        if (use_l3ha() and
+           CompareOpenStackReleases(_os_release) >= 'newton'):
+            install_l3ha_packages()
         else:
-        purge_packages(DVR_PACKAGES)
+            packages_to_purge.extend(L3HA_PACKAGES)
+    else:
+        packages_to_purge = deepcopy(DVR_PACKAGES)
+        packages_to_purge.extend(L3HA_PACKAGES)
+
+    if packages_to_purge:
+        purge_packages(packages_to_purge)
+
     configure_ovs()
     CONFIGS.write_all()
     # If dvr setting has changed, need to pass that on

hooks/neutron_ovs_utils.py

@@ -109,6 +109,8 @@ NEUTRON_METADATA_AGENT_CONF = "/etc/neutron/metadata_agent.ini"
 DVR_PACKAGES = ['neutron-l3-agent']
 DHCP_PACKAGES = ['neutron-dhcp-agent']
 METADATA_PACKAGES = ['neutron-metadata-agent']
+# conntrack is a dependency of neutron-l3-agent and hence is not added
+L3HA_PACKAGES = ['keepalived']
 
 PY3_PACKAGES = [
     'python3-neutron',
@@ -254,6 +256,11 @@ def install_packages():
         enable_ovs_dpdk()
 
 
+def install_l3ha_packages():
+    apt_update()
+    apt_install(L3HA_PACKAGES, fatal=True)
+
+
 def purge_packages(pkg_list):
     purge_pkgs = []
     required_packages = determine_packages()
@@ -276,6 +283,11 @@ def determine_packages():
     if use_dvr():
         pkgs.extend(DVR_PACKAGES)
         py3_pkgs.append('python3-neutron-fwaas')
+        _os_release = os_release('neutron-common', base='icehouse')
+        # per 17.08 release notes L3HA + DVR is a Newton+ feature
+        if (use_l3ha() and
+           CompareOpenStackReleases(_os_release) >= 'newton'):
+            pkgs.extend(L3HA_PACKAGES)
     if enable_local_dhcp():
         pkgs.extend(DHCP_PACKAGES)
         pkgs.extend(METADATA_PACKAGES)
@@ -679,7 +691,11 @@ def get_shared_secret():
 
 
 def use_dvr():
-    return context.NeutronAPIContext()()['enable_dvr']
+    return context.NeutronAPIContext()().get('enable_dvr', False)
+
+
+def use_l3ha():
+    return context.NeutronAPIContext()().get('enable_l3ha', False)
 
 
 def determine_datapath_type():

unit_tests/test_neutron_ovs_context.py

@@ -439,6 +439,26 @@ class L3AgentContextTest(CharmTestCase):
                                          'external_configuration_new': True}
         )
 
+    @patch.object(charmhelpers.contrib.openstack.context, 'relation_get')
+    @patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
+    @patch.object(charmhelpers.contrib.openstack.context, 'related_units')
+    def test_dvr_enabled_dvr_snat_enabled(self, _runits, _rids, _rget):
+        self.test_config.set('use-dvr-snat', True)
+        _runits.return_value = ['unit1']
+        _rids.return_value = ['rid2']
+        rdata = {
+            'neutron-security-groups': 'True',
+            'enable-dvr': 'True',
+            'l2-population': 'True',
+            'overlay-network-type': 'vxlan',
+            'network-device-mtu': 1500,
+        }
+        _rget.side_effect = lambda *args, **kwargs: rdata
+        self.assertEqual(
+            context.L3AgentContext()(), {'agent_mode': 'dvr_snat',
+                                         'external_configuration_new': True}
+        )
+
     @patch.object(charmhelpers.contrib.openstack.context, 'relation_get')
     @patch.object(charmhelpers.contrib.openstack.context, 'relation_ids')
     @patch.object(charmhelpers.contrib.openstack.context, 'related_units')

unit_tests/test_neutron_ovs_hooks.py

@@ -41,7 +41,9 @@ TO_PATCH = [
     'configure_ovs',
     'configure_sriov',
     'use_dvr',
+    'use_l3ha',
     'install_packages',
+    'install_l3ha_packages',
     'purge_packages',
     'enable_nova_metadata',
     'enable_local_dhcp',
@@ -122,8 +124,12 @@ class NeutronOVSHooksTests(CharmTestCase):
             relation_id='neutron-plugin:42',
             request_restart=True)
 
+    @patch.object(hooks, 'os_release')
     @patch.object(hooks, 'neutron_plugin_joined')
-    def test_neutron_plugin_api(self, _plugin_joined):
+    def test_neutron_plugin_api(self, _plugin_joined, _os_release):
+        _os_release.return_value = 'newton'
+        self.use_dvr.return_value = True
+        self.use_l3ha.return_value = False
         self.relation_ids.return_value = ['rid']
         self._call_hook('neutron-plugin-api-relation-changed')
         self.configure_ovs.assert_called_with()
@@ -134,12 +140,14 @@ class NeutronOVSHooksTests(CharmTestCase):
     @patch.object(hooks, 'neutron_plugin_joined')
     def test_neutron_plugin_api_nodvr(self, _plugin_joined):
         self.use_dvr.return_value = False
+        self.use_l3ha.return_value = False
         self.relation_ids.return_value = ['rid']
         self._call_hook('neutron-plugin-api-relation-changed')
         self.configure_ovs.assert_called_with()
         self.assertTrue(self.CONFIGS.write_all.called)
         _plugin_joined.assert_called_with(relation_id='rid')
-        self.purge_packages.assert_called_with(['neutron-l3-agent'])
+        self.purge_packages.assert_called_with(['neutron-l3-agent',
+                                                'keepalived'])
 
     def test_neutron_plugin_joined_dvr_dhcp(self):
         self.enable_nova_metadata.return_value = True
@@ -189,6 +197,33 @@ class NeutronOVSHooksTests(CharmTestCase):
                                                 'neutron-metadata-agent'])
         self.assertFalse(self.install_packages.called)
 
+    @patch.object(hooks, 'os_release')
+    @patch.object(hooks, 'neutron_plugin_joined')
+    def test_neutron_plugin_api_dvr_no_l3ha(self, _plugin_joined, _os_release):
+        _os_release.return_value = 'newton'
+        self.use_dvr.return_value = True
+        self.use_l3ha.return_value = False
+        self.relation_ids.return_value = ['rid']
+        self._call_hook('neutron-plugin-api-relation-changed')
+        self.configure_ovs.assert_called_with()
+        self.assertTrue(self.CONFIGS.write_all.called)
+        _plugin_joined.assert_called_with(relation_id='rid')
+        self.purge_packages.assert_called_with(['keepalived'])
+
+    @patch.object(hooks, 'os_release')
+    @patch.object(hooks, 'neutron_plugin_joined')
+    def test_neutron_plugin_api_dvr_l3ha(self, _plugin_joined, _os_release):
+        _os_release.return_value = 'newton'
+        self.use_dvr.return_value = True
+        self.use_l3ha.return_value = True
+        self.relation_ids.return_value = ['rid']
+        self._call_hook('neutron-plugin-api-relation-changed')
+        self.configure_ovs.assert_called_with()
+        self.assertTrue(self.CONFIGS.write_all.called)
+        _plugin_joined.assert_called_with(relation_id='rid')
+        self.install_packages.assert_called_with()
+        self.install_l3ha_packages.assert_called_with()
+
     def test_amqp_joined(self):
         self._call_hook('amqp-relation-joined')
         self.relation_set.assert_called_with(

unit_tests/test_neutron_ovs_utils.py

@@ -132,13 +132,15 @@ class TestNeutronOVSUtils(CharmTestCase):
             call(self.filter_installed_packages(), fatal=True),
         ])
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
     def test_determine_packages(self, _head_pkgs, _os_rel,
-                                _use_dvr):
+                                _use_dvr, _use_l3ha):
         self.test_config.set('enable-local-dhcp-and-metadata', False)
         _use_dvr.return_value = False
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'icehouse'
         self.os_release.return_value = 'icehouse'
         _head_pkgs.return_value = head_pkg
@@ -149,13 +151,15 @@ class TestNeutronOVSUtils(CharmTestCase):
         ]
         self.assertEqual(pkg_list, expect)
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
     def test_determine_packages_mitaka(self, _head_pkgs, _os_rel,
-                                       _use_dvr):
+                                       _use_dvr, _use_l3ha):
         self.test_config.set('enable-local-dhcp-and-metadata', False)
         _use_dvr.return_value = False
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'mitaka'
         self.os_release.return_value = 'mitaka'
         _head_pkgs.return_value = head_pkg
@@ -166,13 +170,15 @@ class TestNeutronOVSUtils(CharmTestCase):
         ]
         self.assertEqual(pkg_list, expect)
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
     def test_determine_packages_metadata(self, _head_pkgs, _os_rel,
-                                         _use_dvr):
+                                         _use_dvr, _use_l3ha):
         self.test_config.set('enable-local-dhcp-and-metadata', True)
         _use_dvr.return_value = False
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'icehouse'
         self.os_release.return_value = 'icehouse'
         _head_pkgs.return_value = head_pkg
@@ -185,11 +191,14 @@ class TestNeutronOVSUtils(CharmTestCase):
         ]
         self.assertEqual(pkg_list, expect)
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
-    def test_determine_packages_dvr(self, _head_pkgs, _os_rel, _use_dvr):
+    def test_determine_packages_dvr(self, _head_pkgs, _os_rel, _use_dvr,
+                                    _use_l3ha):
         _use_dvr.return_value = True
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'icehouse'
         self.os_release.return_value = 'icehouse'
         _head_pkgs.return_value = head_pkg
@@ -201,11 +210,14 @@ class TestNeutronOVSUtils(CharmTestCase):
         ]
         self.assertEqual(pkg_list, expect)
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
-    def test_determine_packages_dvr_rocky(self, _head_pkgs, _os_rel, _use_dvr):
+    def test_determine_packages_dvr_rocky(self, _head_pkgs, _os_rel, _use_dvr,
+                                          _use_l3ha):
         _use_dvr.return_value = True
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'rocky'
         self.os_release.return_value = 'rocky'
         _head_pkgs.return_value = head_pkg
@@ -219,14 +231,77 @@ class TestNeutronOVSUtils(CharmTestCase):
         ]
         self.assertEqual(pkg_list, expect)
 
+    @patch.object(nutils, 'use_l3ha')
+    @patch.object(nutils, 'use_dvr')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
+    def test_determine_packages_newton_dvr_l3ha(self, _head_pkgs, _os_rel,
+                                                _use_dvr, _use_l3ha):
+        self.test_config.set('enable-local-dhcp-and-metadata', False)
+        _use_dvr.return_value = True
+        _use_l3ha.return_value = True
+        _os_rel.return_value = 'newton'
+        self.os_release.return_value = 'newton'
+        _head_pkgs.return_value = head_pkg
+        pkg_list = nutils.determine_packages()
+        expect = [
+            head_pkg,
+            'neutron-l3-agent',
+            'keepalived',
+            'neutron-openvswitch-agent',
+        ]
+        self.assertEqual(pkg_list, expect)
+
+    @patch.object(nutils, 'use_l3ha')
+    @patch.object(nutils, 'use_dvr')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
+    def test_determine_packages_newton_dvr_no_l3ha(self, _head_pkgs, _os_rel,
+                                                   _use_dvr, _use_l3ha):
+        self.test_config.set('enable-local-dhcp-and-metadata', False)
+        _use_dvr.return_value = True
+        _use_l3ha.return_value = False
+        _os_rel.return_value = 'newton'
+        self.os_release.return_value = 'newton'
+        _head_pkgs.return_value = head_pkg
+        pkg_list = nutils.determine_packages()
+        expect = [
+            head_pkg,
+            'neutron-l3-agent',
+            'neutron-openvswitch-agent',
+        ]
+        self.assertEqual(pkg_list, expect)
+
+    @patch.object(nutils, 'use_l3ha')
+    @patch.object(nutils, 'use_dvr')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
+    @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
+    def test_determine_packages_mitaka_dvr_l3ha(self, _head_pkgs, _os_rel,
+                                                _use_dvr, _use_l3ha):
+        self.test_config.set('enable-local-dhcp-and-metadata', False)
+        _use_dvr.return_value = True
+        _use_l3ha.return_value = True
+        _os_rel.return_value = 'mitaka'
+        self.os_release.return_value = 'mitaka'
+        _head_pkgs.return_value = head_pkg
+        pkg_list = nutils.determine_packages()
+        expect = [
+            head_pkg,
+            'neutron-l3-agent',
+            'neutron-openvswitch-agent',
+        ]
+        self.assertEqual(pkg_list, expect)
+
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
     def test_determine_pkgs_sriov(self, _head_pkgs, _os_rel,
-                                  _use_dvr):
+                                  _use_dvr, _use_l3ha):
         self.test_config.set('enable-local-dhcp-and-metadata', False)
         self.test_config.set('enable-sriov', True)
         _use_dvr.return_value = False
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'kilo'
         self.os_release.return_value = 'kilo'
         _head_pkgs.return_value = head_pkg
@@ -238,14 +313,16 @@ class TestNeutronOVSUtils(CharmTestCase):
         ]
         self.assertEqual(pkg_list, expect)
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dvr')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'os_release')
     @patch.object(charmhelpers.contrib.openstack.neutron, 'headers_package')
     def test_determine_pkgs_sriov_mitaka(self, _head_pkgs, _os_rel,
-                                         _use_dvr):
+                                         _use_dvr, _use_l3ha):
         self.test_config.set('enable-local-dhcp-and-metadata', False)
         self.test_config.set('enable-sriov', True)
         _use_dvr.return_value = False
+        _use_l3ha.return_value = False
         _os_rel.return_value = 'mitaka'
         self.os_release.return_value = 'mitaka'
         _head_pkgs.return_value = head_pkg
@@ -407,10 +484,12 @@ class TestNeutronOVSUtils(CharmTestCase):
         _map = nutils.resource_map()
         self.assertFalse(nutils.EXT_PORT_CONF in _map.keys())
 
+    @patch.object(nutils, 'use_l3ha')
     @patch.object(nutils, 'use_dpdk')
     @patch.object(nutils, 'use_dvr')
-    def test_restart_map(self, mock_use_dvr, mock_use_dpdk):
+    def test_restart_map(self, mock_use_dvr, mock_use_dpdk, mock_use_l3ha):
         mock_use_dvr.return_value = False
+        mock_use_l3ha.return_value = False
         mock_use_dpdk.return_value = False
         self.os_release.return_value = "mitaka"
         self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}