From 86eb58317fc1d28dccc76e201ddff2c9f5fd1235 Mon Sep 17 00:00:00 2001 From: Pedro Guimaraes Date: Mon, 29 Jun 2020 18:40:33 +0200 Subject: [PATCH] Add disable-neutron-lbaas option Since Rocky, Octavia is a valid alternative as LBaaS. If enabled, we should not configure Neutron LBaaS(v2) agent at the same time. The fact that we configure both means neutron-lbaas-agent will generate messages on rabbitmq which never gets consumed and creating alarms on NRPE without any actual issues. This change introduces an option to disable neutron LBaaS solution. Once activated, it masks lbaas agent service. Change-Id: I10c4cc2983245efb5bef3d7cbc8e3b6963448a7d Closes-Bug: #1825906 --- config.yaml | 6 ++++ hooks/neutron_hooks.py | 10 ++++++ hooks/neutron_utils.py | 53 +++++++++++++++++++++++++++----- unit_tests/test_neutron_hooks.py | 31 +++++++++++++++++++ unit_tests/test_neutron_utils.py | 51 ++++++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 8 deletions(-) diff --git a/config.yaml b/config.yaml index 09287484..1ffd9df5 100644 --- a/config.yaml +++ b/config.yaml @@ -349,3 +349,9 @@ options: order to support kernels with limited namespace support. i.e. Trusty. Changing the value after neutron DHCP agents are created will break access. The charm will go into a blocked state if this is attempted. + disable-neutron-lbaas: + type: boolean + default: False + description: | + Manually disable lbaas services. Set this option to True if Octavia + is used with neutron. This option is ignored for Train+ OpenStack. diff --git a/hooks/neutron_hooks.py b/hooks/neutron_hooks.py index 993dbbe5..63b17177 100755 --- a/hooks/neutron_hooks.py +++ b/hooks/neutron_hooks.py @@ -70,7 +70,9 @@ from neutron_utils import ( pause_unit_helper, resume_unit_helper, remove_legacy_nova_metadata, + remove_legacy_neutron_lbaas, disable_nova_metadata, + disable_neutron_lbaas, remove_old_packages, deprecated_services, ) @@ -126,6 +128,12 @@ def install(): # n-gateway and n-cloud-controller services. install_systemd_override() + # LP #1825906: prefer to install the lbaas package and then mask it + # instead of checking if we need to install that package on each + # config-changed hook + if disable_neutron_lbaas(): + remove_legacy_neutron_lbaas() + @hooks.hook('config-changed') @restart_on_change(restart_map) @@ -172,6 +180,8 @@ def config_changed(): # Disable nova metadata if possible, if disable_nova_metadata(): remove_legacy_nova_metadata() + if disable_neutron_lbaas(): + remove_legacy_neutron_lbaas() @hooks.hook('upgrade-charm') diff --git a/hooks/neutron_utils.py b/hooks/neutron_utils.py index 1a5fe971..6ccd4ac0 100644 --- a/hooks/neutron_utils.py +++ b/hooks/neutron_utils.py @@ -684,11 +684,18 @@ def resolve_config_files(plugin, release): drop_config.extend([NEUTRON_LBAAS_AA_PROFILE_PATH]) # Drop lbaasv2 at train - if cmp_os_release >= 'train': - drop_config.extend([ - NEUTRON_LBAASV2_AA_PROFILE_PATH, - NEUTRON_LBAAS_AGENT_CONF, - ]) + # or drop if disable-lbaas option is true + if disable_neutron_lbaas(): + if cmp_os_release >= 'newton': + drop_config.extend([ + NEUTRON_LBAASV2_AA_PROFILE_PATH, + NEUTRON_LBAAS_AGENT_CONF, + ]) + else: + drop_config.extend([ + NEUTRON_LBAAS_AA_PROFILE_PATH, + NEUTRON_LBAAS_AGENT_CONF, + ]) if disable_nova_metadata(cmp_os_release): drop_config.extend(get_nova_config_files().keys()) @@ -770,6 +777,11 @@ def restart_map(release=None): svcs.add('neutron-lbaasv2-agent') if cmp_release >= 'train' and 'neutron-lbaasv2-agent' in svcs: svcs.remove('neutron-lbaasv2-agent') + if disable_neutron_lbaas(): + if cmp_release < 'newton' and 'neutron-lbaas-agent' in svcs: + svcs.remove('neutron-lbaas-agent') + elif cmp_release >= 'newton' and 'neutron-lbaasv2-agent' in svcs: + svcs.remove('neutron-lbaasv2-agent') if svcs: _map[f] = list(svcs) return _map @@ -930,8 +942,21 @@ def remove_legacy_nova_metadata(): remove_file(f) +def remove_legacy_neutron_lbaas(): + """Remove neutron lbaas files.""" + cmp_os_source = CompareOpenStackReleases(os_release('neutron-common')) + service_name = 'neutron-lbaas-agent' + if cmp_os_source >= 'train': + return + if cmp_os_source >= 'newton': + service_name = 'neutron-lbaasv2-agent' + service_stop(service_name) + service('disable', service_name) + service('mask', service_name) + + def disable_nova_metadata(cmp_os_source=None): - """Check whether nova mnetadata service should be disabled.""" + """Check whether nova metadata service should be disabled.""" if not cmp_os_source: cmp_os_source = CompareOpenStackReleases(os_release('neutron-common')) if cmp_os_source >= 'rocky': @@ -951,6 +976,15 @@ def disable_nova_metadata(cmp_os_source=None): return disable +def disable_neutron_lbaas(cmp_os_source=None): + """Check whether neutron lbaas service should be disabled.""" + if not cmp_os_source: + cmp_os_source = CompareOpenStackReleases(os_release('neutron-common')) + if cmp_os_source >= 'train': + return True + return config('disable-neutron-lbaas') or False + + def cache_env_data(): env = NetworkServiceContext()() if not env: @@ -1127,7 +1161,10 @@ def deprecated_services(): services = [] if disable_nova_metadata(): services.append('nova-api-metadata') - if cmp_release >= 'train': - services.append('neutron-lbaasv2-agent') + if disable_neutron_lbaas(): + if cmp_release >= 'newton': + services.append('neutron-lbaasv2-agent') + else: + services.append('neutron-lbaas-agent') return services diff --git a/unit_tests/test_neutron_hooks.py b/unit_tests/test_neutron_hooks.py index bac69aa6..e82caf25 100644 --- a/unit_tests/test_neutron_hooks.py +++ b/unit_tests/test_neutron_hooks.py @@ -53,7 +53,9 @@ TO_PATCH = [ 'install_systemd_override', 'configure_apparmor', 'disable_nova_metadata', + 'disable_neutron_lbaas', 'remove_legacy_nova_metadata', + 'remove_legacy_neutron_lbaas', 'services', 'remove_old_packages', 'is_container', @@ -109,12 +111,14 @@ class TestQuantumHooks(CharmTestCase): @patch('sys.exit') def test_install_hook_invalid_plugin(self, _exit): self.valid_plugin.return_value = False + self.disable_neutron_lbaas.return_value = False self._call_hook('install') self.assertTrue(self.log.called) _exit.assert_called_with(1) def test_config_changed(self): self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = False def mock_relids(rel): return ['relid'] @@ -136,8 +140,26 @@ class TestQuantumHooks(CharmTestCase): '{foo : bar}', '/etc/sysctl.d/50-quantum-gateway.conf') + def test_config_change_disable_lbaas(self): + self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = True + + def mock_relids(rel): + return ['relid'] + self.test_config.set( + 'sysctl', + '{foo : bar}' + ) + self.openstack_upgrade_available.return_value = True + self.valid_plugin.return_value = True + self.relation_ids.side_effect = mock_relids + self._call_hook('config-changed') + self.assertTrue(self.disable_neutron_lbaas.called) + self.assertTrue(self.remove_legacy_neutron_lbaas.called) + def test_config_changed_in_container(self): self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = False def mock_relids(rel): return ['relid'] @@ -160,6 +182,7 @@ class TestQuantumHooks(CharmTestCase): def test_config_changed_upgrade(self): self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = False self.openstack_upgrade_available.return_value = True self.valid_plugin.return_value = True self._call_hook('config-changed') @@ -169,6 +192,7 @@ class TestQuantumHooks(CharmTestCase): def test_config_changed_n1kv(self): self.openstack_upgrade_available.return_value = False self.valid_plugin.return_value = True + self.disable_neutron_lbaas.return_value = False self.filter_installed_packages.side_effect = lambda p: p self.test_config.set('plugin', 'n1kv') self._call_hook('config-changed') @@ -179,6 +203,7 @@ class TestQuantumHooks(CharmTestCase): @patch('sys.exit') def test_config_changed_invalid_plugin(self, _exit): + self.disable_neutron_lbaas.return_value = False self.disable_nova_metadata.return_value = False self.valid_plugin.return_value = False self._call_hook('config-changed') @@ -248,6 +273,7 @@ class TestQuantumHooks(CharmTestCase): def test_nm_changed(self): self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = False def _relation_get(key): data = { @@ -271,6 +297,7 @@ class TestQuantumHooks(CharmTestCase): mock_get_packages): '''Ensure first set of restart_trigger restarts nova-api-metadata''' self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = False # as restart_map is embedded into the decorator, we have to mock out # the bits in the restart_map to be able to make it pass. mock_os_release.return_value = 'mitaka' @@ -309,6 +336,7 @@ class TestQuantumHooks(CharmTestCase): mock_get_packages): '''Ensure change of restart_trigger restarts nova-api-metadata''' self.disable_nova_metadata.return_value = False + self.disable_neutron_lbaas.return_value = False # as restart_map is embedded into the decorator, we have to mock out # the bits in the restart_map to be able to make it pass. mock_os_release.return_value = 'mitaka' @@ -348,6 +376,7 @@ class TestQuantumHooks(CharmTestCase): '''Ensure no change in restart_trigger skips restarts''' self.patch_object(hooks, 'disable_nova_metadata', return_value=False) + self.disable_neutron_lbaas.return_value = False # as restart_map is embedded into the decorator, we have to mock out # the bits in the restart_map to be able to make it pass. mock_os_release.return_value = 'mitaka' @@ -376,6 +405,7 @@ class TestQuantumHooks(CharmTestCase): def test_nm_changed_disable_meta(self): self.disable_nova_metadata.return_value = True + self.disable_neutron_lbaas.return_value = False def _relation_get(key): data = { @@ -420,6 +450,7 @@ class TestQuantumHooks(CharmTestCase): def test_quantum_network_service_relation_changed(self): self.patch_object(hooks, 'disable_nova_metadata', return_value=False) + self.disable_neutron_lbaas.return_value = False self.test_config.set('ha-legacy-mode', True) self._call_hook('quantum-network-service-relation-changed') self.assertTrue(self.cache_env_data.called) diff --git a/unit_tests/test_neutron_utils.py b/unit_tests/test_neutron_utils.py index c70fcf0a..548971e6 100644 --- a/unit_tests/test_neutron_utils.py +++ b/unit_tests/test_neutron_utils.py @@ -44,6 +44,7 @@ TO_PATCH = [ 'NeutronAPIContext', 'enable_ipfix', 'disable_ipfix', + 'disable_neutron_lbaas', ] @@ -152,6 +153,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs' + self.disable_neutron_lbaas.return_value = False self.os_release.return_value = 'newton' packages = neutron_utils.get_packages() self.assertTrue('neutron-metering-agent' in packages) @@ -241,6 +243,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs-odl' + self.disable_neutron_lbaas.return_value = False self.os_release.return_value = 'icehouse' packages = neutron_utils.get_packages() self.assertTrue('neutron-metering-agent' in packages) @@ -253,6 +256,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs-odl' + self.disable_neutron_lbaas.return_value = False self.os_release.return_value = 'newton' packages = neutron_utils.get_packages() self.assertTrue('neutron-metering-agent' in packages) @@ -513,6 +517,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs' + self.disable_neutron_lbaas.return_value = False self.get_os_codename_install_source.return_value = 'havana' mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'icehouse' @@ -560,6 +565,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs' + self.disable_neutron_lbaas.return_value = False mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'mitaka' ex_map = { @@ -605,6 +611,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs' + self.disable_neutron_lbaas.return_value = False mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'newton' ex_map = { @@ -645,6 +652,47 @@ class TestNeutronUtils(CharmTestCase): } self.assertEqual(neutron_utils.restart_map(), ex_map) + @patch.object(neutron_utils, 'get_packages') + def test_restart_map_ovs_stein_disable_lbaas(self, mock_get_packages): + self.patch_object(neutron_utils, 'disable_nova_metadata', + return_value=False) + self.config.return_value = 'ovs' + self.disable_neutron_lbaas.return_value = True + mock_get_packages.return_value = ['neutron-vpn-agent'] + self.os_release.return_value = 'stein' + ex_map = { + neutron_utils.NEUTRON_CONF: ['neutron-vpn-agent', + 'neutron-dhcp-agent', + 'neutron-metering-agent', + 'neutron-metadata-agent', + 'neutron-openvswitch-agent'], + neutron_utils.NEUTRON_DNSMASQ_CONF: ['neutron-dhcp-agent'], + neutron_utils.NEUTRON_OVS_AGENT_CONF: + ['neutron-openvswitch-agent'], + neutron_utils.NEUTRON_METADATA_AGENT_CONF: + ['neutron-metadata-agent'], + neutron_utils.NEUTRON_VPNAAS_AGENT_CONF: ['neutron-vpn-agent'], + neutron_utils.NEUTRON_L3_AGENT_CONF: ['neutron-vpn-agent'], + neutron_utils.NEUTRON_DHCP_AGENT_CONF: ['neutron-dhcp-agent'], + neutron_utils.NEUTRON_FWAAS_CONF: ['neutron-vpn-agent'], + neutron_utils.NEUTRON_METERING_AGENT_CONF: + ['neutron-metering-agent'], + neutron_utils.NOVA_CONF: ['nova-api-metadata'], + neutron_utils.EXT_PORT_CONF: ['ext-port'], + neutron_utils.PHY_NIC_MTU_CONF: ['os-charm-phy-nic-mtu'], + neutron_utils.NEUTRON_DHCP_AA_PROFILE_PATH: ['neutron-dhcp-agent'], + neutron_utils.NEUTRON_OVS_AA_PROFILE_PATH: + ['neutron-openvswitch-agent'], + neutron_utils.NEUTRON_L3_AA_PROFILE_PATH: ['neutron-vpn-agent'], + neutron_utils.NEUTRON_METADATA_AA_PROFILE_PATH: + ['neutron-metadata-agent'], + neutron_utils.NEUTRON_METERING_AA_PROFILE_PATH: + ['neutron-metering-agent'], + neutron_utils.NOVA_API_METADATA_AA_PROFILE_PATH: + ['nova-api-metadata'], + } + self.assertEqual(neutron_utils.restart_map(), ex_map) + @patch.object(neutron_utils, 'get_packages') def test_restart_map_ovs_train(self, mock_get_packages): self.patch_object(neutron_utils, 'disable_nova_metadata', @@ -702,6 +750,7 @@ class TestNeutronUtils(CharmTestCase): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) self.config.return_value = 'ovs-odl' + self.disable_neutron_lbaas.return_value = False mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'icehouse' ex_map = { @@ -741,6 +790,7 @@ class TestNeutronUtils(CharmTestCase): def test_restart_map_ovs_odl_newton(self, mock_get_packages): self.patch_object(neutron_utils, 'disable_nova_metadata', return_value=False) + self.disable_neutron_lbaas.return_value = False self.config.return_value = 'ovs-odl' mock_get_packages.return_value = ['neutron-vpn-agent'] self.os_release.return_value = 'newton' @@ -897,6 +947,7 @@ class TestNeutronUtils(CharmTestCase): self._set_distrib_codename('trusty') self.os_release.return_value = 'mitaka' self.is_relation_made = False + self.disable_neutron_lbaas.return_value = False actual_map = neutron_utils.resolve_config_files(neutron_utils.OVS, 'mitaka') actual_configs = actual_map[neutron_utils.OVS].keys()