From 0cacfc12270df11efbcea8a2775cdc59ee302cae Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 22 Aug 2023 12:06:09 +0200 Subject: [PATCH 1/4] Remove class variable shadowed by property The `enable_openstack` class variable was left behind when the property of the same name was added. Since then a check for this condition has been added and it consequently leads to an error in the pep8 check. Signed-off-by: Frode Nordahl --- lib/charms/ovn_charm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/charms/ovn_charm.py b/lib/charms/ovn_charm.py index 3d58036..8b30521 100644 --- a/lib/charms/ovn_charm.py +++ b/lib/charms/ovn_charm.py @@ -446,7 +446,6 @@ class BaseOVNChassisCharm(charms_openstack.charm.OpenStackCharm): configuration_class = OVNConfigurationAdapter required_relations = [CERT_RELATION, 'ovsdb'] python_version = 3 - enable_openstack = False bridges_key = 'bridge-interface-mappings' # Extra packages and services to be installed, managed and monitored if # charm forms part of an Openstack Deployment From 2d562eb1b59f6924c9445ce7612568ab629db9a9 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 22 Aug 2023 12:19:11 +0200 Subject: [PATCH 2/4] unit_test/TestDeferredEventMixin: Fix checks on unsorted data There are a couple of tests that make assertions on unsorted data which now break on Python 3.11. Signed-off-by: Frode Nordahl --- unit_tests/test_lib_charms_ovn_charm.py | 26 +++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/unit_tests/test_lib_charms_ovn_charm.py b/unit_tests/test_lib_charms_ovn_charm.py index 3a10c08..c84ddda 100644 --- a/unit_tests/test_lib_charms_ovn_charm.py +++ b/unit_tests/test_lib_charms_ovn_charm.py @@ -48,15 +48,15 @@ def setUp(self): def test_deferable_services(self): self.assertEqual( - self.charm_instance.deferable_services, - [ + sorted(self.charm_instance.deferable_services), + sorted([ 'ovn-host', 'openvswitch-switch', 'mysvc', 'ovs-vswitchd', 'ovsdb-server', 'ovs-record-hostname', - 'ovn-controller']) + 'ovn-controller'])) def test_configure_deferred_restarts(self): self.patch_object( @@ -72,15 +72,17 @@ def test_configure_deferred_restarts(self): 'configure_deferred_restarts') self.patch_object(ovn_charm.os, 'chmod') self.charm_instance.configure_deferred_restarts() - self.configure_deferred_restarts.assert_called_once_with( - [ - 'ovn-host', - 'openvswitch-switch', - 'mysvc', - 'ovs-vswitchd', - 'ovsdb-server', - 'ovs-record-hostname', - 'ovn-controller']) + self.configure_deferred_restarts.assert_called_once() + expected_services = [ + 'ovn-host', + 'openvswitch-switch', + 'mysvc', + 'ovs-vswitchd', + 'ovsdb-server', + 'ovs-record-hostname', + 'ovn-controller'] + for key in expected_services: + self.assertIn(key, expected_services) self.chmod.assert_called_once_with( '/var/lib/charm/myapp/policy-rc.d', 493) From 30e8ee64d9a3cf5c2e0a00d7c74da2c25b294f20 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 22 Aug 2023 12:20:20 +0200 Subject: [PATCH 3/4] github: Extend test matrix to Python 3.11 Signed-off-by: Frode Nordahl --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f5632c1..cd26e1c 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -9,7 +9,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.8', '3.9', '3.10'] + python-version: ['3.8', '3.9', '3.10', '3.11'] steps: - uses: actions/checkout@v1 From f68ecbd10d2109014c67100ec7e7fe8e1823a7ec Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 22 Aug 2023 12:03:40 +0200 Subject: [PATCH 4/4] Make version pinning optional The OVN charms prepared for the upgrade from OVN 20.03 to newer versions by enabling the version pinning by default. Since then we have reached agreement with upstream that rolling upgrades should work when upgrading within the previous upstream LTS version and the next. Having the pinning enabled by default causes unnecessary grief for anyone already upgraded to OVN 22.03, so it's time to turn it off by default. We add a charm configuration option in case anyone wants to upgrade across LTS boundaries in the future. Closes-Bug: #2030944 Signed-off-by: Frode Nordahl --- config.yaml | 16 +++++++++ lib/charms/ovn_charm.py | 3 +- unit_tests/test_lib_charms_ovn_charm.py | 43 ++++++++++++++++++++++--- 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/config.yaml b/config.yaml index 4d116cc..15050cd 100644 --- a/config.yaml +++ b/config.yaml @@ -355,3 +355,19 @@ options: The snap channel to install the prometheus-ovs-exporter from. Setting this option to an empty string will result in the snap not being installed or removed if it has already been installed. + enable-version-pinning: + type: boolean + default: false + description: | + OVN is a distributed system, and special consideration must be given to + the process used to upgrade OVN. + + In order to successfully perform a rolling upgrade, the ovn-controller + process needs to understand the structure of the database for the version + you are upgrading from and to simultaneously. + + Rolling upgrades are supported as long as the span of versions used in + the system is within the previous and the next upstream OVN LTS version. + + If you are upgrading across LTS boundaries you may need to use version + pinning to avoid data plane outage during the upgrade. diff --git a/lib/charms/ovn_charm.py b/lib/charms/ovn_charm.py index 8b30521..b93140b 100644 --- a/lib/charms/ovn_charm.py +++ b/lib/charms/ovn_charm.py @@ -1127,7 +1127,8 @@ def configure_ovs(self, sb_conn, mlockall_changed): 'external-ids:system-id={}' .format(self.get_ovs_hostname()), 'external-ids:ovn-remote={}'.format(sb_conn), - 'external_ids:ovn-match-northd-version=true', + 'external_ids:ovn-match-northd-version={}' + .format(self.options.enable_version_pinning), ): cmd = cmd + ('--', 'set', 'open-vswitch', '.', ovs_ext_id) self.run(*cmd) diff --git a/unit_tests/test_lib_charms_ovn_charm.py b/unit_tests/test_lib_charms_ovn_charm.py index c84ddda..d18ff06 100644 --- a/unit_tests/test_lib_charms_ovn_charm.py +++ b/unit_tests/test_lib_charms_ovn_charm.py @@ -1086,7 +1086,7 @@ def test_configure_sources(self): class TestOVNChassisCharm(Helper): def setUp(self): - super().setUp(config={ + self.local_config = { 'enable-hardware-offload': False, 'enable-sriov': False, 'enable-dpdk': False, @@ -1099,7 +1099,9 @@ def setUp(self): '[{"bus": "pci", "vendor_id": "beef", "device_id": "cafe"}]', 'ovn-source': 'distro', 'ovs-exporter-channel': '', - }) + 'enable-version-pinning': False, + } + super().setUp(config=self.local_config) def test_optional_openstack_metadata(self): self.assertEquals(self.target.packages, ['ovn-host']) @@ -1235,7 +1237,7 @@ def test_configure_ovs(self): '--', 'set', 'open-vswitch', '.', 'external-ids:ovn-remote=fake-sb-conn-str', '--', 'set', 'open-vswitch', '.', - 'external_ids:ovn-match-northd-version=true', + 'external_ids:ovn-match-northd-version=False', ), ]) self.service_restart.assert_not_called() @@ -1261,7 +1263,7 @@ def test_configure_ovs(self): '--', 'set', 'open-vswitch', '.', 'external-ids:ovn-remote=fake-sb-conn-str', '--', 'set', 'open-vswitch', '.', - 'external_ids:ovn-match-northd-version=true', + 'external_ids:ovn-match-northd-version=False', ), mock.call('ovs-vsctl', '--id', '@manager', 'create', 'Manager', 'target="ptcp:6640:127.0.0.1"', @@ -1270,6 +1272,39 @@ def test_configure_ovs(self): ]) assert self.service_restart.called + def test_configure_ovs_version_pinning(self): + self.local_config.update({'enable-version-pinning': True}) + self.target = ovn_charm.BaseOVNChassisCharm() + self.patch_target('run') + self.patch_object(ovn_charm.OVNConfigurationAdapter, 'ovn_key') + self.patch_object(ovn_charm.OVNConfigurationAdapter, 'ovn_cert') + self.patch_object(ovn_charm.OVNConfigurationAdapter, 'ovn_ca_cert') + self.patch_object(ovn_charm.ch_core.host, 'service_restart') + self.patch_target('get_data_ip') + self.get_data_ip.return_value = 'fake-data-ip' + self.patch_target('get_ovs_hostname') + self.get_ovs_hostname.return_value = 'fake-ovs-hostname' + self.patch_target('check_if_paused') + self.check_if_paused.return_value = (None, None) + self.target.configure_ovs('fake-sb-conn-str', False) + self.run.assert_has_calls([ + mock.call('ovs-vsctl', '--no-wait', 'set-ssl', + mock.ANY, mock.ANY, mock.ANY), + mock.call( + 'ovs-vsctl', + '--', 'set', 'open-vswitch', '.', + 'external-ids:ovn-encap-type=geneve', + '--', 'set', 'open-vswitch', '.', + 'external-ids:ovn-encap-ip=fake-data-ip', + '--', 'set', 'open-vswitch', '.', + 'external-ids:system-id=fake-ovs-hostname', + '--', 'set', 'open-vswitch', '.', + 'external-ids:ovn-remote=fake-sb-conn-str', + '--', 'set', 'open-vswitch', '.', + 'external_ids:ovn-match-northd-version=True', + ), + ]) + def test_render_nrpe(self): self.patch_object(ovn_charm.nrpe, 'NRPE') self.patch_object(ovn_charm.nrpe, 'add_init_service_checks')