controller: don't strand the API stopped on a failed primary unmount

Review feedback: skip the service stop when the primary mount is already
unmounted (a no-op request must not cause an API outage), and restart
wrolpi-api (best effort) when umount fails since the drive is still
mounted and usable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-07-23 19:17:09 -06:00
parent 43f6ed2b8e
commit f635671266
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
2 changed files with 48 additions and 12 deletions

View file

@ -54,7 +54,7 @@ from controller.lib.smart import (
get_all_smart_status,
is_smart_available,
)
from controller.lib.systemd import stop_service
from controller.lib.systemd import start_service, stop_service
from controller.lib.wrol_mode import require_normal_mode
logger = logging.getLogger(__name__)
@ -274,18 +274,34 @@ def _unmount_primary(normalized: str) -> dict:
reconciler never manages the primary mount, so umount directly. The
/etc/fstab entry is left in place so a reboot restores the mount.
"""
if normalized not in _executor.current_mount_points():
# Nothing to unmount — do not take the API service down for a no-op.
return {
"success": True,
"mount_point": normalized,
"removed_from_fstab": False,
"stopped_services": [],
}
stopped = stop_service("wrolpi-api")
if not stopped.get("success"):
logger.warning("Could not stop wrolpi-api before unmounting %s: %s",
normalized, stopped.get("error"))
if normalized in _executor.current_mount_points():
result = _executor.unmount(normalized)
if not result.ok:
raise HTTPException(
status_code=500,
detail=f"{normalized} is still mounted: {result.error}",
)
result = _executor.unmount(normalized)
if not result.ok:
# The drive is still mounted, so don't leave the API stopped for
# nothing; best effort — the 500 below is the primary signal.
if stopped.get("success"):
restarted = start_service("wrolpi-api")
if not restarted.get("success"):
logger.warning(
"Could not restart wrolpi-api after failed unmount of %s: %s",
normalized, restarted.get("error"))
raise HTTPException(
status_code=500,
detail=f"{normalized} is still mounted: {result.error}",
)
return {
"success": True,

View file

@ -292,8 +292,10 @@ class TestUnmountPrimaryEndpoint:
with mock.patch("controller.api.disks.get_media_directory",
return_value=Path("/media/wrolpi")), \
mock.patch("controller.api.disks.stop_service",
return_value={"success": True}) as stop:
yield {**disks_env, "stop_service": stop}
return_value={"success": True}) as stop, \
mock.patch("controller.api.disks.start_service",
return_value={"success": True}) as start:
yield {**disks_env, "stop_service": stop, "start_service": start}
@pytest.mark.parametrize("mount_point", [
"/media/wrolpi",
@ -314,13 +316,16 @@ class TestUnmountPrimaryEndpoint:
assert body["removed_from_fstab"] is False
assert body["stopped_services"] == ["wrolpi-api"]
primary_env["stop_service"].assert_called_once_with("wrolpi-api")
# A successful unmount must not restart the API behind the user's back.
primary_env["start_service"].assert_not_called()
assert primary_env["fake"].current_mount_points() == set()
def test_unmount_primary_returns_500_when_still_mounted(
self, test_client, primary_env,
):
# umount fails (e.g. busy) — surface the failure, but the service
# stop should still have been attempted first.
# umount fails (e.g. busy) — surface the failure. The drive is
# still mounted, so the API service must be started again rather
# than left stopped for nothing.
primary_env["fake"]._live.add("/media/wrolpi")
primary_env["fake"].fail_for_unmount = frozenset({"/media/wrolpi"})
@ -331,6 +336,21 @@ class TestUnmountPrimaryEndpoint:
assert response.status_code == 500
assert "still mounted" in response.json()["detail"]
primary_env["stop_service"].assert_called_once_with("wrolpi-api")
primary_env["start_service"].assert_called_once_with("wrolpi-api")
def test_unmount_primary_already_unmounted_does_not_stop_api(
self, test_client, primary_env,
):
# Nothing to unmount — succeed without touching the API service.
response = test_client.post(
"/api/disks/unmount",
json={"mount_point": "/media/wrolpi"},
)
assert response.status_code == 200
body = response.json()
assert body["success"] is True
assert body["stopped_services"] == []
primary_env["stop_service"].assert_not_called()
def test_unmount_primary_stop_failure_still_unmounts(
self, test_client, primary_env,