From 8a3607bc1e4013c5a37bd21318f7b90d3514995a 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 b7f8e63..790eba4 100644
--- a/lib/charms/ovn_charm.py
+++ b/lib/charms/ovn_charm.py
@@ -1099,20 +1099,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 eb161b5..1a720be 100644
--- a/unit_tests/test_reactive_ovn_chassis_charm_handlers.py
+++ b/unit_tests/test_reactive_ovn_chassis_charm_handlers.py
@@ -175,6 +175,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'