From 4f09e06b6b0573ba1946ccf92a15df3202d44424 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Fri, 8 Nov 2019 20:08:47 +0100 Subject: [PATCH] Remove dependency on openvswitch-switch daemons While the ovn packages have a dependency on the `openvswitch-switch` and `openvswitch-common` packages the OVN central components have no need for having the daemons running. Also fixes an issue with the ovn-central systemd service file which lead to unwanted reverse dependency between `ovn-northd` and the database services. I.e. we do not want a restart of `ovn-northd` to lead to a reastart of the `ovn-nb-ovsdb` service. Remove loading of `openvswitch` kernel module from LXD profile, add in iptables instead as we will need this to protect the SB DB listener created for accepting connections from `ovn-northd` Change-Id: I134ec560c5522f0c657c203a4cd828e0e5c6f3cb --- src/lib/charm/openstack/ovn_central.py | 34 ++++++------------ src/lxd-profile.yaml | 6 +--- src/reactive/ovn_central_handlers.py | 1 - src/templates/ovn-central.service | 4 +-- src/templates/ovn-nb-ovsdb.service | 2 +- src/templates/ovn-sb-ovsdb.service | 2 +- .../test_lib_charm_openstack_ovn_central.py | 36 +++---------------- 7 files changed, 18 insertions(+), 67 deletions(-) diff --git a/src/lib/charm/openstack/ovn_central.py b/src/lib/charm/openstack/ovn_central.py index 84974d6..614e637 100644 --- a/src/lib/charm/openstack/ovn_central.py +++ b/src/lib/charm/openstack/ovn_central.py @@ -94,12 +94,19 @@ class OVNCentralCharm(charms_openstack.charm.OpenStackCharm): We also configure source before installing as OpenvSwitch and OVN packages are distributed as part of the UCA. """ + # NOTE(fnordahl) The OVN central components are currently packaged with + # a dependency on openvswitch-switch, but it does not need the switch + # or stock ovsdb running. service_masks = [ - '/etc/systemd/system/ovn-central.service', + 'openvswitch-switch.service', + 'ovs-vswitchd.service', + 'ovsdb-server.service', + 'ovn-central.service', ] for service_file in service_masks: - if not os.path.islink(service_file): - os.symlink('/dev/null', service_file) + abs_path_svc = os.path.join('/etc/systemd/system', service_file) + if not os.path.islink(abs_path_svc): + os.symlink('/dev/null', abs_path_svc) self.configure_source() super().install() @@ -196,11 +203,6 @@ class OVNCentralCharm(charms_openstack.charm.OpenStackCharm): tls_object['cert'], tls_object['key'], cn='host') - self.run('ovs-vsctl', - 'set-ssl', - ovn_key(self.adapters_instance), - ovn_cert(self.adapters_instance), - ovn_ca_cert(self.adapters_instance)) if (reactive.is_flag_set('leadership.is_leader') and not reactive.is_flag_set('leadership.set.ready')): # This is one-time set up at cluster creation and can only be @@ -238,19 +240,3 @@ class OVNCentralCharm(charms_openstack.charm.OpenStackCharm): 'add', 'SB_Global', '.', 'connections', '@connection') self.restart_all() break - - def configure_ovn_remote(self, ovsdb_interface): - """Configure the OVN remote setting in the local OVSDB. - - The value is used by command line tools run on this unit. - - :param ovsdb_interface: OVSDB interface instance - :type ovsdb_interface: reactive.Endpoint derived class - :raises: subprocess.CalledProcessError - """ - self.run('ovs-vsctl', - 'set', - 'open', - '.', - 'external-ids:ovn-remote={}' - .format(','.join(ovsdb_interface.db_sb_connection_strs))) diff --git a/src/lxd-profile.yaml b/src/lxd-profile.yaml index 39bb3a5..e5a65ad 100644 --- a/src/lxd-profile.yaml +++ b/src/lxd-profile.yaml @@ -1,6 +1,2 @@ -comment: | - NOTE(fnordahl): This is not required by the services the charm ultimately - will run, but is currently required by how it is packaged. We should work - towards not having this dependency. config: - linux.kernel_modules: openvswitch + linux.kernel_modules: ip_tables,ip6_tables diff --git a/src/reactive/ovn_central_handlers.py b/src/reactive/ovn_central_handlers.py index 316d985..49df1dc 100644 --- a/src/reactive/ovn_central_handlers.py +++ b/src/reactive/ovn_central_handlers.py @@ -146,6 +146,5 @@ def render(): ovsdb_peer.cluster_remote_addrs, ovsdb_peer.db_sb_cluster_port)) if ovn_charm.enable_services(): - ovn_charm.configure_ovn_remote(ovsdb_peer) reactive.set_flag('config.rendered') ovn_charm.assess_status() diff --git a/src/templates/ovn-central.service b/src/templates/ovn-central.service index 072da7f..622d841 100644 --- a/src/templates/ovn-central.service +++ b/src/templates/ovn-central.service @@ -6,10 +6,8 @@ [Unit] Description=Open Virtual Network central components After=network.target -After=openvswitch-switch.service Requires=network.target -Requires=openvswitch-switch.service -Requires=ovn-northd.service +Wants=ovn-northd.service # Facilitate spread placement of the DBs if someone should choose to do that Wants=ovn-nb-ovsdb.service Wants=ovn-sb-ovsdb.service diff --git a/src/templates/ovn-nb-ovsdb.service b/src/templates/ovn-nb-ovsdb.service index 8dc2e27..455faeb 100644 --- a/src/templates/ovn-nb-ovsdb.service +++ b/src/templates/ovn-nb-ovsdb.service @@ -5,7 +5,7 @@ ############################################################################### [Unit] Description=Open vSwitch database server for OVN Northbound database -After=network.target openvswitch-switch.service +After=network.target PartOf=ovn-central.service DefaultDependencies=no diff --git a/src/templates/ovn-sb-ovsdb.service b/src/templates/ovn-sb-ovsdb.service index 0487cdb..09da88b 100644 --- a/src/templates/ovn-sb-ovsdb.service +++ b/src/templates/ovn-sb-ovsdb.service @@ -5,7 +5,7 @@ ############################################################################### [Unit] Description=Open vSwitch database server for OVN Southbound database -After=network.target openvswitch-switch.service +After=network.target PartOf=ovn-central.service DefaultDependencies=no diff --git a/unit_tests/test_lib_charm_openstack_ovn_central.py b/unit_tests/test_lib_charm_openstack_ovn_central.py index 386382d..e2dde99 100644 --- a/unit_tests/test_lib_charm_openstack_ovn_central.py +++ b/unit_tests/test_lib_charm_openstack_ovn_central.py @@ -65,12 +65,14 @@ class TestOVNCentralCharm(Helper): self.patch_target('configure_source') self.target.install() calls = [] - for service in self.target.services: + for service in ('openvswitch-switch', 'ovs-vswitchd', 'ovsdb-server', + self.target.services[0],): calls.append( mock.call('/etc/systemd/system/{}.service'.format(service))) self.islink.assert_has_calls(calls) calls = [] - for service in self.target.services: + for service in ('openvswitch-switch', 'ovs-vswitchd', 'ovsdb-server', + self.target.services[0],): calls.append( mock.call('/dev/null', '/etc/systemd/system/{}.service'.format(service))) @@ -161,11 +163,6 @@ class TestOVNCentralCharm(Helper): 'fakekey', cn='host') self.target.run.assert_has_calls([ - mock.call('ovs-vsctl', - 'set-ssl', - '/etc/openvswitch/key_host', - '/etc/openvswitch/cert_host', - '/etc/openvswitch/ovn-central.crt'), mock.call('ovn-nbctl', 'set-connection', 'pssl:6641'), @@ -178,28 +175,3 @@ class TestOVNCentralCharm(Helper): 'add', 'SB_Global', '.', 'connections', '@connection'), ]) - self.is_flag_set.side_effect = [False, True] - self.target.run.reset_mock() - self.target.configure_tls() - self.target.run.assert_has_calls([ - mock.call('ovs-vsctl', - 'set-ssl', - '/etc/openvswitch/key_host', - '/etc/openvswitch/cert_host', - '/etc/openvswitch/ovn-central.crt'), - ]) - - def test_configure_ovn_remote(self): - self.patch_target('run') - ovsdb_interface = mock.MagicMock() - ovsdb_interface.db_sb_connection_strs = \ - mock.PropertyMock().return_value = [ - 'ssl:a.b.c.d:6642', - 'ssl:a.b.c.d:6642', - 'ssl:a.b.c.d:6642', - ] - self.target.configure_ovn_remote(ovsdb_interface) - self.run.assert_called_once_with( - 'ovs-vsctl', 'set', 'open', '.', - 'external-ids:ovn-remote=' - 'ssl:a.b.c.d:6642,ssl:a.b.c.d:6642,ssl:a.b.c.d:6642')