fix(web): answer capacity exhaustion with 409, not 500

simplyblock_core/controllers/backup_controller.py

@@ -17,7 +17,7 @@
     lvol_dek_path, pool_kek_name,
 )
 from simplyblock_core.utils.secrets import unwrap_secret
-from simplyblock_core.exceptions import PreconditionError
+from simplyblock_core.exceptions import InsufficientCapacityError, PreconditionError
 from simplyblock_core.rpc_client import RPCException
 
 logger = logging.getLogger()
@@ -487,6 +487,8 @@ def restore_backup(backup_id: str, lvol_name: str, pool_id_or_name: str,
         fabric="tcp",
     )
     if error or not lvol_id:
+        if lvol_controller.is_capacity_error(error):
+            raise InsufficientCapacityError(str(error))
         raise RuntimeError(f"Failed to create restore volume: {error}")
 
     # Mark volume as restoring

simplyblock_core/controllers/lvol_controller.py

@@ -30,6 +30,28 @@
 logger = utils.get_logger(__name__)
 
 
+#: The cluster-wide placement refusal: no node can hold another object.
+ERR_NO_NODE_WITH_CAPACITY = "No nodes found with enough resources to create the LVol"
+
+#: The per-lvstore refusal, raised when a pinned node is already at its object cap.
+#: A prefix rather than a whole message: the rest names the node and the counts.
+ERR_OBJECT_LIMIT_PREFIX = "Object limit reached on lvstore"
+
+
+def is_capacity_error(error) -> bool:
+    """True when ``error`` is one of the placement refusals above.
+
+    Callers converting ``(id, error)`` returns into exceptions use this to tell a
+    cluster that has no room from one that genuinely failed. Both refusals are
+    matched through the constants they are built from, so a reworded message
+    cannot silently stop being recognized.
+    """
+    if not error:
+        return False
+    error = str(error)
+    return ERR_NO_NODE_WITH_CAPACITY in error or ERR_OBJECT_LIMIT_PREFIX in error
+
+
 def _create_crypto_lvol(rpc_client, lvol, cluster):
     name = lvol.crypto_bdev
     base_name = f"{lvol.lvs_name}/{lvol.lvol_bdev}"
@@ -404,7 +426,7 @@ def check_lvstore_object_limit(host_node, all_lvols, all_snaps, new_objects=1):
                      if s.lvol and s.lvol.node_id == node_id and not s.deleted)
     total = lvol_count + snap_count
     if total + new_objects > limit:
-        return (f"Object limit reached on lvstore of node {node_id}: {total} "
+        return (f"{ERR_OBJECT_LIMIT_PREFIX} of node {node_id}: {total} "
                 f"objects (lvols/clones: {lvol_count}, snapshots: "
                 f"{snap_count}); the hard limit is {limit} per lvstore")
     return None
@@ -628,7 +650,7 @@ def add_lvol_ha(name, size, host_id_or_name, ha_type, pool_id_or_name, use_comp=
     if not host_node:
         nodes = _get_next_3_nodes(cl.get_id(), lvol.size, all_lvols, namespaced=bool(namespaced))
         if not nodes:
-            return False, "No nodes found with enough resources to create the LVol"
+            return False, ERR_NO_NODE_WITH_CAPACITY
         host_node = nodes[0]
 
     limit_error = check_lvstore_object_limit(host_node, all_lvols, all_snaps)
@@ -4098,7 +4120,7 @@ def replicate_lvol_on_source_cluster(lvol_id, cluster_id=None, pool_uuid=None):
         # get new source node from the new cluster
         nodes = _get_next_3_nodes(new_source_cluster.get_id(), lvol.size)
         if not nodes:
-            return False, "No nodes found with enough resources to create the LVol"
+            return False, ERR_NO_NODE_WITH_CAPACITY
         source_node = nodes[0]
 
     if not source_node:

simplyblock_core/exceptions.py

@@ -4,3 +4,13 @@ class PreconditionError(Exception):
 
 class MigrationConflictError(Exception):
     """Raised when a conflicting active migration already exists."""
+
+
+class InsufficientCapacityError(Exception):
+    """Raised when the cluster has nowhere to place a new logical volume.
+
+    A refusal, not a fault: the request is well-formed and the control plane is
+    healthy, but no node can hold another object until capacity is freed or a
+    node is added. Retrying an identical request cannot change the answer, so
+    this surfaces as a 409 rather than a 5xx.
+    """

simplyblock_web/api/v2/cluster/storage_pool/volume/__init__.py

@@ -7,6 +7,7 @@
 from simplyblock_core.db_controller import DBController
 from simplyblock_core import utils as core_utils
 from simplyblock_core.controllers import backup_controller, lvol_controller, snapshot_controller
+from simplyblock_core.exceptions import InsufficientCapacityError
 from simplyblock_core.models.lvol_model import LVol
 
 from ...._dependencies import Cluster, StoragePool, Volume
@@ -121,6 +122,8 @@ def add(
         raise AssertionError('unreachable')
 
     if volume_id_or_false == False:  # noqa
+        if lvol_controller.is_capacity_error(error):
+            raise InsufficientCapacityError(str(error))
         raise ValueError(error)
 
     return util.creation_response(

simplyblock_web/app.py

@@ -18,7 +18,7 @@
 from simplyblock_web.settings import Settings as WebSettings
 from simplyblock_core import constants, utils as core_utils
 from simplyblock_core.settings import Settings
-from simplyblock_core.exceptions import PreconditionError
+from simplyblock_core.exceptions import InsufficientCapacityError, PreconditionError
 
 logger = core_utils.get_logger(__name__)
 logger.setLevel(constants.LOG_WEB_LEVEL)
@@ -78,22 +78,46 @@ async def dispatch(self, request: Request, call_next):
 app: FastAPI = FastAPI()
 Instrumentator().instrument(app).expose(app, endpoint="/_meta/metrics")
 
-@app.exception_handler(PreconditionError)
-async def precondition_handler(request: Request, exc: PreconditionError):
-    logger.exception("Preciondition checks failed", exc_info=exc)
-    return JSONResponse(status_code=400, content={
-        "error": "Preconditions are not met",
-        "detail": str(exc),
-    })
-
-
-@app.exception_handler(RuntimeError)
-async def runtime_error_handler(request: Request, exc: RuntimeError):
-    logger.exception("Unexcpected error while processing request", exc_info=exc)
-    return JSONResponse(status_code=500, content={
-        "status": "An error occured while processing the request",
-        "detail": str(exc),
-    })
+
+def register_exception_handlers(app: FastAPI) -> None:
+    """Map the core exception types onto their HTTP responses.
+
+    Kept as a function so tests can build an app that answers the way the real
+    one does. An endpoint's status code is part of its contract, and a test app
+    without these handlers cannot observe it.
+    """
+
+    @app.exception_handler(PreconditionError)
+    async def precondition_handler(request: Request, exc: PreconditionError):
+        logger.exception("Precondition checks failed", exc_info=exc)
+        return JSONResponse(status_code=400, content={
+            "error": "Preconditions are not met",
+            "detail": str(exc),
+        })
+
+    @app.exception_handler(InsufficientCapacityError)
+    async def insufficient_capacity_handler(request: Request, exc: InsufficientCapacityError):
+        # Deliberately 409 and not 507: 507 is the more precise name for the
+        # condition, but it is a 5xx, and generic retry policies retry those. A
+        # refusal that no retry can change has to land in the 4xx class to be
+        # read correctly by a client that has not special-cased it.
+        logger.warning("Cluster has no capacity for the requested volume: %s", exc)
+        return JSONResponse(status_code=409, content={
+            "error": "Insufficient capacity",
+            "code": "insufficient_capacity",
+            "detail": str(exc),
+        })
+
+    @app.exception_handler(RuntimeError)
+    async def runtime_error_handler(request: Request, exc: RuntimeError):
+        logger.exception("Unexpected error while processing request", exc_info=exc)
+        return JSONResponse(status_code=500, content={
+            "status": "An error occured while processing the request",
+            "detail": str(exc),
+        })
+
+
+register_exception_handlers(app)
 
 
 _web_settings = WebSettings()

tests/unit/test_backup_restore_node_selection.py

@@ -11,7 +11,8 @@
 
 import pytest
 
-from simplyblock_core.exceptions import PreconditionError
+from simplyblock_core.controllers import lvol_controller
+from simplyblock_core.exceptions import InsufficientCapacityError, PreconditionError
 from simplyblock_core.models.backup import Backup
 from simplyblock_core.models.storage_node import StorageNode
 
@@ -194,6 +195,22 @@ def test_volume_creation_failure_is_a_runtime_error(self, db, add_lvol_ha):
         with pytest.raises(RuntimeError, match="Failed to create restore volume"):
             _restore()
 
+    @pytest.mark.parametrize("refusal", [
+        lvol_controller.ERR_NO_NODE_WITH_CAPACITY,
+        f"{lvol_controller.ERR_OBJECT_LIMIT_PREFIX} of node abc: 35 objects "
+        f"(lvols/clones: 35, snapshots: 0); the hard limit is 35 per lvstore",
+    ])
+    def test_capacity_exhaustion_is_not_a_server_error(self, db, add_lvol_ha, refusal):
+        """A cluster with nowhere to put the volume has not failed, it has declined.
+
+        Reported as a RuntimeError, it reached clients as a 500 and every generic
+        retry policy treated a permanent refusal as a transient fault.
+        """
+        add_lvol_ha.return_value = (None, refusal)
+
+        with pytest.raises(InsufficientCapacityError):
+            _restore()
+
     def test_task_creation_failure_is_a_runtime_error(self, db, add_lvol_ha, tasks):
         tasks.add_backup_restore_task.return_value = False
 

tests/unit/web/api/v2/conftest.py

@@ -12,6 +12,8 @@
   parameters.
 - Authentication is bypassed via ``app.dependency_overrides``; it has its own
   unit tests in ``test_auth.py``.
+- The app registers the same exception handlers as the real one, so a test sees
+  the status code a client would actually get rather than a raised exception.
 - ``Thread`` is replaced with an inline runner so fire-and-forget endpoints
   (cluster start/shutdown, node restart, …) can be asserted synchronously.
 """
@@ -23,6 +25,7 @@
 from fastapi.testclient import TestClient
 
 from simplyblock_core.db_controller import DBController
+from simplyblock_web.app import register_exception_handlers
 
 import simplyblock_web.api.v2 as v2
 import simplyblock_web.api.v2._auth as auth_module
@@ -105,6 +108,7 @@ def db(monkeypatch):
 @pytest.fixture(scope='session')
 def app():
     app = FastAPI()
+    register_exception_handlers(app)
     app.include_router(v2.api, prefix='/api/v2')
     app.dependency_overrides[auth_module.verify_api_token] = lambda: None
     app.dependency_overrides[auth_module.verify_metrics_token] = lambda: None

tests/unit/web/api/v2/test_backup_endpoints.py

@@ -1,6 +1,8 @@
 # coding=utf-8
 """Unit tests for /api/v2/clusters/{id}/backups endpoints (backup_controller mocked)."""
 
+from simplyblock_core.exceptions import InsufficientCapacityError
+
 from tests.unit.web.api.v2 import _factories as factories
 from tests.unit.web.api.v2._factories import (
     BACKUP_ID,
@@ -83,6 +85,20 @@ def test_restores_backup(self, client, db, cluster, backup_controller):
         backup_controller.restore_backup.assert_called_once_with(
             BACKUP_ID, 'restored-volume', 'pool-1', target_node_id=None)
 
+    def test_capacity_exhaustion_is_a_conflict_not_a_server_error(
+            self, client, db, cluster, backup_controller):
+        backup_controller.restore_backup.side_effect = InsufficientCapacityError(
+            'No nodes found with enough resources to create the LVol')
+
+        response = client.post(f'{BASE}/restore', json={
+            'backup_id': BACKUP_ID,
+            'lvol_name': 'restored-volume',
+            'pool': 'pool-1',
+        })
+
+        assert response.status_code == 409
+        assert response.json()['code'] == 'insufficient_capacity'
+
 
 class TestBackupPolicies:
 

tests/unit/web/api/v2/test_volume_endpoints.py

@@ -31,6 +31,17 @@ def test_returns_volumes_of_pool(self, client, db, volume):
 
 class TestCreateVolume:
 
+    def test_capacity_exhaustion_is_a_conflict_not_a_server_error(
+            self, client, db, pool, lvol_controller):
+        db.get_lvol_by_name.side_effect = KeyError('LVol not found')
+        lvol_controller.add_lvol_ha.return_value = (
+            False, 'No nodes found with enough resources to create the LVol')
+
+        response = client.post(f'{BASE}/', json={'name': 'volume-1', 'size': '10G'})
+
+        assert response.status_code == 409
+        assert response.json()['code'] == 'insufficient_capacity'
+
     def test_calls_add_lvol_ha(self, client, db, pool, lvol_controller):
         db.get_lvol_by_name.side_effect = KeyError('LVol not found')
         lvol_controller.add_lvol_ha.return_value = (VOLUME_ID, None)