Error log for missing certs with NB and SB DBs

When the ovn-provider starts up,
it attempts to connect to the NB and SB databases
by retrieving SSL and cert files.
To avoid errors, the code will now check if these
files exist before using them.
If the files are missing,
connections will be skipped and an error message
will be displayed in the logs.

Refactoring _check_and_set_ssl_files method to be
public and reusable. it will now check to see if a
string value is set and will now check path and
LOG an error message if not found.

Adding unit tests for ovsdb_monitor to bring up test coverage.
Updated ovsdb_tests to improve code.

Closes-Bug: #2072978
Change-Id: I2a21b94fee03767a5f703486bdab2908cda18746

Author: chrisbug

ovn_octavia_provider/helper.py

@@ -29,7 +29,7 @@
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import strutils
-from ovs.stream import Stream
+from ovn_octavia_provider.ovsdb import ovsdb_monitor
 from ovsdbapp.backend.ovs_idl import connection
 from ovsdbapp.backend.ovs_idl import idlutils
 from ovsdbapp.schema.ovn_northbound import commands as cmd
@@ -57,7 +57,7 @@ def __init__(self, notifier=True):
         self.helper_thread = threading.Thread(target=self.request_handler)
         self.helper_thread.daemon = True
         self._octavia_driver_lib = o_driver_lib.DriverLibrary()
-        self._check_and_set_ssl_files()
+        ovsdb_monitor.check_and_set_ssl_files('OVN_Northbound')
         self._init_lb_actions()
 
         i = impl_idl_ovn.OvnNbIdlForLb(notifier=notifier)
@@ -106,21 +106,6 @@ def _delete_disabled_from_status(status):
                 for i in v]
             for k, v in status.items()}
 
-    def _check_and_set_ssl_files(self):
-        # TODO(reedip): Make ovsdb_monitor's _check_and_set_ssl_files() public
-        # This is a copy of ovsdb_monitor._check_and_set_ssl_files
-        priv_key_file = ovn_conf.get_ovn_nb_private_key()
-        cert_file = ovn_conf.get_ovn_nb_certificate()
-        ca_cert_file = ovn_conf.get_ovn_nb_ca_cert()
-        if priv_key_file:
-            Stream.ssl_set_private_key_file(priv_key_file)
-
-        if cert_file:
-            Stream.ssl_set_certificate_file(cert_file)
-
-        if ca_cert_file:
-            Stream.ssl_set_ca_cert_file(ca_cert_file)
-
     def shutdown(self):
         self.requests.put({'type': ovn_const.REQ_TYPE_EXIT},
                           timeout=ovn_const.MAX_TIMEOUT_REQUEST)

ovn_octavia_provider/ovsdb/impl_idl_ovn.py

@@ -213,7 +213,7 @@ class OvnNbIdlForLb(ovsdb_monitor.OvnIdl):
 
     def __init__(self, event_lock_name=None, notifier=True):
         self.conn_string = config.get_ovn_nb_connection()
-        ovsdb_monitor._check_and_set_ssl_files(self.SCHEMA)
+        ovsdb_monitor.check_and_set_ssl_files(self.SCHEMA)
         helper = self._get_ovsdb_helper(self.conn_string)
         for table in OvnNbIdlForLb.TABLES:
             helper.register_table(table)
@@ -235,7 +235,7 @@ class OvnSbIdlForLb(ovsdb_monitor.OvnIdl):
 
     def __init__(self, event_lock_name=None):
         self.conn_string = config.get_ovn_sb_connection()
-        ovsdb_monitor._check_and_set_ssl_files(self.SCHEMA)
+        ovsdb_monitor.check_and_set_ssl_files(self.SCHEMA)
         helper = self._get_ovsdb_helper(self.conn_string)
         for table in OvnSbIdlForLb.TABLES:
             helper.register_table(table)

ovn_octavia_provider/ovsdb/ovsdb_monitor.py

@@ -13,6 +13,7 @@
 #    under the License.
 
 import abc
+import os
 
 from oslo_config import cfg
 from oslo_log import log
@@ -30,7 +31,7 @@
 class BaseOvnIdl(connection.OvsdbIdl):
     @classmethod
     def from_server(cls, connection_string, schema_name):
-        _check_and_set_ssl_files(schema_name)
+        check_and_set_ssl_files(schema_name)
         helper = idlutils.get_schema_helper(connection_string, schema_name)
         helper.register_all()
         return cls(connection_string, helper)
@@ -85,21 +86,33 @@ def __init__(self, driver):
         self.driver = driver
 
 
-def _check_and_set_ssl_files(schema_name):
-    if schema_name == 'OVN_Northbound':
-        priv_key_file = ovn_config.get_ovn_nb_private_key()
-        cert_file = ovn_config.get_ovn_nb_certificate()
-        ca_cert_file = ovn_config.get_ovn_nb_ca_cert()
-
-        Stream.ssl_set_private_key_file(priv_key_file)
-        Stream.ssl_set_certificate_file(cert_file)
-        Stream.ssl_set_ca_cert_file(ca_cert_file)
-
-    if schema_name == 'OVN_Southbound':
-        priv_key_file = ovn_config.get_ovn_sb_private_key()
-        cert_file = ovn_config.get_ovn_sb_certificate()
-        ca_cert_file = ovn_config.get_ovn_sb_ca_cert()
-
-        Stream.ssl_set_private_key_file(priv_key_file)
-        Stream.ssl_set_certificate_file(cert_file)
-        Stream.ssl_set_ca_cert_file(ca_cert_file)
+def check_and_set_ssl_files(schema_name):
+    if schema_name in ['OVN_Northbound', 'OVN_Southbound']:
+        if schema_name == 'OVN_Northbound':
+            priv_key_file = ovn_config.get_ovn_nb_private_key()
+            cert_file = ovn_config.get_ovn_nb_certificate()
+            ca_cert_file = ovn_config.get_ovn_nb_ca_cert()
+        else:
+            priv_key_file = ovn_config.get_ovn_sb_private_key()
+            cert_file = ovn_config.get_ovn_sb_certificate()
+            ca_cert_file = ovn_config.get_ovn_sb_ca_cert()
+
+        if priv_key_file:
+            if not os.path.exists(priv_key_file):
+                LOG.error(f"Cannot find private key file for {schema_name}")
+            else:
+                Stream.ssl_set_private_key_file(priv_key_file)
+
+        if cert_file:
+            if not os.path.exists(cert_file):
+                LOG.error(f"Cannot find private cert file for {schema_name}")
+            else:
+                Stream.ssl_set_certificate_file(cert_file)
+
+        if ca_cert_file:
+            if not os.path.exists(ca_cert_file):
+                LOG.error(f"Cannot find ca cert file for {schema_name}")
+            else:
+                Stream.ssl_set_ca_cert_file(ca_cert_file)
+    else:
+        LOG.error(f"{schema_name} not supported")

ovn_octavia_provider/tests/unit/ovsdb/test_ovsdb_monitor.py

@@ -0,0 +1,88 @@
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+from unittest import mock
+
+from oslo_config import cfg
+import testscenarios
+
+from ovn_octavia_provider.ovsdb import ovsdb_monitor
+from ovn_octavia_provider.tests.unit import base as base_test
+
+
+OPTS = ('ovn_nb_private_key', 'ovn_nb_certificate',
+        'ovn_nb_ca_cert', 'ovn_sb_private_key',
+        'ovn_sb_certificate', 'ovn_sb_ca_cert')
+
+
+class TestOvsdbMonitor(testscenarios.WithScenarios,
+                       base_test.TestOvnOctaviaBase):
+
+    scenarios = [
+        ('OVN_Northbound', {'schema': 'OVN_Northbound',
+                            'private_key': 'ovn_nb_private_key',
+                            'certificate': 'ovn_nb_certificate',
+                            'ca_cert': 'ovn_nb_ca_cert'}),
+        ('OVN_Southbound', {'schema': 'OVN_Southbound',
+                            'private_key': 'ovn_sb_private_key',
+                            'certificate': 'ovn_sb_certificate',
+                            'ca_cert': 'ovn_sb_ca_cert'})
+    ]
+
+    def setUp(self):
+        super().setUp()
+        self._register_opts()
+        self.mock_os_path = mock.patch('os.path.exists').start()
+        self.mock_stream = mock.patch.object(ovsdb_monitor, 'Stream').start()
+
+    @staticmethod
+    def _register_opts():
+        for opt in OPTS:
+            try:
+                cfg.CONF.register_opt(cfg.StrOpt(opt), group='ovn')
+            except cfg.DuplicateOptError:
+                pass
+
+    def test_set_ssl(self):
+        cfg.CONF.set_override(self.private_key, 'key', group='ovn')
+        cfg.CONF.set_override(self.certificate, 'cert', group='ovn')
+        cfg.CONF.set_override(self.ca_cert, 'ca-cert', group='ovn')
+        self.mock_os_path.return_value = True
+        ovsdb_monitor.check_and_set_ssl_files(self.schema)
+        self.mock_stream.ssl_set_private_key_file.assert_called_with('key')
+        self.mock_stream.ssl_set_certificate_file.assert_called_with('cert')
+        self.mock_stream.ssl_set_ca_cert_file.assert_called_with('ca-cert')
+
+    def test_no_key_and_certs(self):
+        cfg.CONF.set_override(self.private_key, '', group='ovn')
+        cfg.CONF.set_override(self.certificate, '', group='ovn')
+        cfg.CONF.set_override(self.ca_cert, '', group='ovn')
+        self.mock_os_path.return_value = False
+        ovsdb_monitor.check_and_set_ssl_files(self.schema)
+        self.mock_stream.ssl_set_private_key_file.assert_not_called()
+        self.mock_stream.ssl_set_certificate_file.assert_not_called()
+        self.mock_stream.ssl_set_ca_cert_file.assert_not_called()
+
+    def test_no_nonexisting_files(self):
+        cfg.CONF.set_override(self.private_key, 'key', group='ovn')
+        cfg.CONF.set_override(self.certificate, 'cert', group='ovn')
+        cfg.CONF.set_override(self.ca_cert, 'ca-cert', group='ovn')
+        self.mock_os_path.return_value = False
+
+        with self.assertLogs():
+            ovsdb_monitor.check_and_set_ssl_files(self.schema)
+
+        self.mock_stream.ssl_set_private_key_file.assert_not_called()
+        self.mock_stream.ssl_set_certificate_file.assert_not_called()
+        self.mock_stream.ssl_set_ca_cert_file.assert_not_called()