diff --git a/config.yaml b/config.yaml index 724a9ddb..b9acbfec 100644 --- a/config.yaml +++ b/config.yaml @@ -277,4 +277,21 @@ options: description: | 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. \ No newline at end of file + 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. diff --git a/hooks/neutron_ovs_context.py b/hooks/neutron_ovs_context.py index b2e87b77..86fbd7ec 100644 --- a/hooks/neutron_ovs_context.py +++ b/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: diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index 4df3340b..7b97f46a 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/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: + packages_to_purge.extend(L3HA_PACKAGES) else: - purge_packages(DVR_PACKAGES) + 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 diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index db58eb47..92129d5d 100644 --- a/hooks/neutron_ovs_utils.py +++ b/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(): diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index fb496c95..16e59ec6 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/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') diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index 3f7bbc34..c3b5bf94 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/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( diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 1ffd9549..6ef9a7b0 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/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'}