From d8837e0985aeecd1282340d947fef3f356661e0f Mon Sep 17 00:00:00 2001
From: Edward Hope-Morley <edward.hope-morley@canonical.com>
Date: Mon, 7 Oct 2024 21:27:08 +0100
Subject: [PATCH] Bail out configuring ovs if no OVNSDB available

When the ovsdb relation is departing the hook will fail since it will
try to configure ovs while only partial information is available.

Closes-Bug: #1944983
---
 lib/charms/ovn_charm.py                       | 36 +++++++++++--------
 unit_tests/__init__.py                        | 10 +++++-
 ...est_reactive_ovn_chassis_charm_handlers.py | 20 +++++++++++
 3 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/lib/charms/ovn_charm.py b/lib/charms/ovn_charm.py
index b93140b..46ba7b0 100644
--- a/lib/charms/ovn_charm.py
+++ b/lib/charms/ovn_charm.py
@@ -1118,20 +1118,28 @@ def configure_ovs(self, sb_conn, mlockall_changed):
                  self.options.ovn_cert,
                  self.options.ovn_ca_cert)
 
-        # The local ``ovn-controller`` process will retrieve information about
-        # how to connect to OVN from the local Open vSwitch database.
-        cmd = ('ovs-vsctl',)
-        for ovs_ext_id in ('external-ids:ovn-encap-type=geneve',
-                           'external-ids:ovn-encap-ip={}'
-                           .format(self.get_data_ip()),
-                           'external-ids:system-id={}'
-                           .format(self.get_ovs_hostname()),
-                           'external-ids:ovn-remote={}'.format(sb_conn),
-                           'external_ids:ovn-match-northd-version={}'
-                           .format(self.options.enable_version_pinning),
-                           ):
-            cmd = cmd + ('--', 'set', 'open-vswitch', '.', ovs_ext_id)
-        self.run(*cmd)
+        if sb_conn:
+            # The local ``ovn-controller`` process will retrieve information
+            # about how to connect to OVN from the local Open vSwitch
+            # database.
+            cmd = ('ovs-vsctl',)
+            for ovs_ext_id in ('external-ids:ovn-encap-type=geneve',
+                               'external-ids:ovn-encap-ip={}'
+                               .format(self.get_data_ip()),
+                               'external-ids:system-id={}'
+                               .format(self.get_ovs_hostname()),
+                               'external-ids:ovn-remote={}'.format(sb_conn),
+                               'external_ids:ovn-match-northd-version={}'
+                               .format(self.options.enable_version_pinning),
+                               ):
+                cmd = cmd + ('--', 'set', 'open-vswitch', '.', ovs_ext_id)
+            self.run(*cmd)
+        else:
+            ch_core.hookenv.log('could not configure ovs due to unavailable '
+                                'sbdb connection info - ovn-central relation '
+                                'no longer available?',
+                                level=ch_core.hookenv.WARNING)
+
         if self.enable_openstack:
             # OpenStack Nova expects the local OVSDB server to listen to
             # TCP port 6640 on localhost.  We use this for the OVN metadata
diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py
index f2a3c67..b38c200 100644
--- a/unit_tests/__init__.py
+++ b/unit_tests/__init__.py
@@ -14,13 +14,21 @@
 
 import sys
 
+import mock
+
 sys.path.append('lib')
 
 # Mock out charmhelpers so that we can test without it.
 import charms_openstack.test_mocks  # noqa
+
+# charms.openstack (commit b90327) re-introduced a dependency on charmhelpers
+# so we need to mock that out explicitly here since we do not install
+# charmhelpers as a test dependency.
+sys.modules['charmhelpers.contrib.openstack.utils'] = mock.MagicMock()
+sys.modules['charmhelpers.contrib.openstack.utils'].\
+    CompareOpenStackReleases = mock.MagicMock()
 charms_openstack.test_mocks.mock_charmhelpers()
 
-import mock
 import charms
 
 
diff --git a/unit_tests/test_reactive_ovn_chassis_charm_handlers.py b/unit_tests/test_reactive_ovn_chassis_charm_handlers.py
index 5caf7f5..2544fe1 100644
--- a/unit_tests/test_reactive_ovn_chassis_charm_handlers.py
+++ b/unit_tests/test_reactive_ovn_chassis_charm_handlers.py
@@ -207,6 +207,26 @@ def test_configure_ovs(self):
         self.charm.configure_iptables_rules.assert_called_once_with()
         self.charm.assess_status.assert_called_once_with()
 
+    def test_configure_ovs_no_sb_conn(self):
+        self.patch_object(handlers.reactive, 'endpoint_from_flag')
+        self.patch_object(handlers.charm, 'optional_interfaces')
+        self.patch_object(handlers.reactive, 'set_flag')
+        self.patch_object(handlers.reactive, 'is_flag_set', return_value=True)
+        ovsdb = mock.MagicMock()
+        ovsdb.db_sb_connection_strs = []
+        self.endpoint_from_flag.return_value = ovsdb
+        handlers.configure_ovs()
+        self.charm.configure_ovs.assert_called_once_with(
+            ','.join(ovsdb.db_sb_connection_strs), True)
+        self.charm.render_with_interfaces.assert_called_once_with(
+            self.optional_interfaces((ovsdb,),
+                                     'nova-compute.connected',
+                                     'amqp.connected'))
+        self.set_flag.assert_called_once_with('config.rendered')
+        self.charm.configure_bridges.assert_called_once_with()
+        self.charm.configure_iptables_rules.assert_called_once_with()
+        self.charm.assess_status.assert_called_once_with()
+
     def test_configure_nrpe(self):
         self.patch_object(handlers.reactive, 'endpoint_from_flag')
         self.endpoint_from_flag.return_value = 'nrpe-external-master'