Merge pull request #69702 from avanthakkar/fix-smb-sqlite-error-handling

mgr/smb: gracefully handle transient sqlitedb errors

Reviewed-by: John Mulligan <jmulligan@redhat.com>
Reviewed-by: Rabinarayan Panigrahi <rapanigr@redhat.com>

src/pybind/mgr/smb/cli.py

@@ -7,7 +7,7 @@ import functools
 import object_format
 from mgr_module import CLICommandBase
 
-from . import resourcelib
+from . import resourcelib, sqlite_store
 from .proto import Self
 
 
@@ -81,6 +81,11 @@ class NoMatchingValue(object_format.ErrorResponseBase):
         return -errno.ENOENT, "", str(self)
 
 
+class SMBDBUnavailable(object_format.ErrorResponseBase):
+    def format_response(self) -> Tuple[int, str, str]:
+        return -errno.EAGAIN, "", str(self)
+
+
 @contextlib.contextmanager
 def error_wrapper() -> Iterator[None]:
     """Context-decorator that converts between certain common exception types."""
@@ -89,3 +94,6 @@ def error_wrapper() -> Iterator[None]:
     except resourcelib.ResourceTypeError as err:
         msg = f'failed to parse input: {err}'
         raise InvalidInputValue(msg) from err
+    except sqlite_store.StoreUnavailable as err:
+        msg = f'smb database temporarily unavailable: {err}'
+        raise SMBDBUnavailable(msg) from err

src/pybind/mgr/smb/module.py

@@ -207,6 +207,17 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule):
             return results.ResultGroup(
                 [results.InvalidResourceResult(err.resource_data, str(err))]
             )
+        except sqlite_store.StoreUnavailable as err:
+            # Reached only via remote() calls (e.g. dashboard), which bypass
+            # cli.py's error_wrapper. Return a result instead of raising so a
+            # transient db outage doesn't surface as an unhandled exception.
+            return results.ResultGroup(
+                [
+                    results.InvalidResourceResult(
+                        {}, f'smb database temporarily unavailable: {err}'
+                    )
+                ]
+            )
 
     @SMBCLICommand('cluster ls', perm='r')
     def cluster_ls(self) -> List[str]:
@@ -783,20 +794,31 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule):
         """Show resources fetched from the local config store based on resource
         type or resource type and id(s).
         """
-        if not resource_names:
-            resources = self._handler.all_resources()
-        else:
-            try:
-                resources = self._handler.matching_resources(resource_names)
-            except handler.InvalidResourceMatch as err:
-                raise cli.InvalidInputValue(str(err)) from err
-        if password_filter is not PasswordFilter.NONE:
-            op = (PasswordFilter.NONE, password_filter)
-            log.debug('Password filtering for smb show: %r', op)
-            resources = [r.convert(op) for r in resources]
-        if len(resources) == 1 and results is ShowResults.COLLAPSED:
-            return resources[0].to_simplified()
-        return {"resources": [r.to_simplified() for r in resources]}
+        try:
+            if not resource_names:
+                resources = self._handler.all_resources()
+            else:
+                try:
+                    resources = self._handler.matching_resources(
+                        resource_names
+                    )
+                except handler.InvalidResourceMatch as err:
+                    raise cli.InvalidInputValue(str(err)) from err
+            if password_filter is not PasswordFilter.NONE:
+                op = (PasswordFilter.NONE, password_filter)
+                log.debug('Password filtering for smb show: %r', op)
+                resources = [r.convert(op) for r in resources]
+            if len(resources) == 1 and results is ShowResults.COLLAPSED:
+                return resources[0].to_simplified()
+            return {"resources": [r.to_simplified() for r in resources]}
+        except sqlite_store.StoreUnavailable as err:
+            # Reached only via remote() calls (e.g. dashboard), which bypass
+            # cli.py's error_wrapper. Return a value instead of raising so a
+            # transient db outage doesn't surface as an unhandled exception.
+            return {
+                "error": f"smb database temporarily unavailable: {err}",
+                "resources": [],
+            }
 
     def submit_smb_spec(self, spec: SMBSpec) -> None:
         """Submit a new or updated smb spec object to ceph orchestration."""

src/pybind/mgr/smb/sqlite_store.py

@@ -21,6 +21,7 @@ import contextlib
 import copy
 import json
 import logging
+import sqlite3
 import threading
 
 from .config_store import ObjectCachingEntry
@@ -36,6 +37,11 @@ from .proto import (
 log = logging.getLogger(__name__)
 
 
+class StoreUnavailable(RuntimeError):
+    """Raised when the underlying store backend cannot be reached (e.g. a
+    transient RADOS/lock-loss condition during cluster instability)."""
+
+
 class DirectDBAcessor(Protocol):
     """A simple protocol describing the minimal per-mgr-module (mon) store interface
     provided by the fairly giganto MgrModule class.
@@ -290,26 +296,29 @@ class SqliteStore:
 
     @contextlib.contextmanager
     def _db(self) -> Iterator[Cursor]:
-        if self._cursor is not None:
-            log.debug('fetching cached cursor')
-            yield self._cursor
-            return
-        if hasattr(self._backend, 'exclusive_db_cursor'):
-            log.debug('fetching exclusive db cursor')
-            with self._backend.exclusive_db_cursor() as cursor:
+        try:
+            if self._cursor is not None:
+                log.debug('fetching cached cursor')
+                yield self._cursor
+                return
+            if hasattr(self._backend, 'exclusive_db_cursor'):
+                log.debug('fetching exclusive db cursor')
+                with self._backend.exclusive_db_cursor() as cursor:
+                    try:
+                        self._cursor = cursor
+                        yield cursor
+                    finally:
+                        self._cursor = None
+                return
+            log.debug('fetching default db cursor')
+            with self._backend.db:
                 try:
-                    self._cursor = cursor
-                    yield cursor
+                    self._cursor = self._backend.db.cursor()
+                    yield self._cursor
                 finally:
                     self._cursor = None
-            return
-        log.debug('fetching default db cursor')
-        with self._backend.db:
-            try:
-                self._cursor = self._backend.db.cursor()
-                yield self._cursor
-            finally:
-                self._cursor = None
+        except sqlite3.DatabaseError as e:
+            raise StoreUnavailable(str(e)) from e
 
     def __getitem__(self, key: EntryKey) -> SqliteStoreEntry:
         """Return an entry object given a namespaced entry key. This entry does