diff --git a/app/src/components/admin/ControllerPage.js b/app/src/components/admin/ControllerPage.js index 6d51f929..17714785 100644 --- a/app/src/components/admin/ControllerPage.js +++ b/app/src/components/admin/ControllerPage.js @@ -688,9 +688,10 @@ function SmartDetailsModal({drive, open, onClose}) { } -// The primary mount cannot be unmounted from the UI, but its persistence -// (an /etc/fstab entry) can be toggled. Any mount outside /media belongs to -// the system (/, /boot/efi, /boot/firmware, …) and is entirely untouchable. +// The primary mount can be unmounted (to swap in a backup drive), but doing +// so stops the WROLPi API service — the unmount modal warns about this. Any +// mount outside /media belongs to the system (/, /boot/efi, /boot/firmware, …) +// and is entirely untouchable. const PRIMARY_MOUNT = '/media/wrolpi'; // Helper to format disk size @@ -942,12 +943,15 @@ function DiskSection() { if (!unmountTarget) return; try { - await unmountDisk(unmountTarget); + const result = await unmountDisk(unmountTarget); + const stoppedServices = (result && result.stopped_services) || []; toast({ type: 'success', title: 'Disk Unmounted', - description: `Unmounted ${unmountTarget}`, - time: 3000, + description: stoppedServices.length > 0 ? + `Unmounted ${unmountTarget}; stopped ${stoppedServices.join(', ')}` : + `Unmounted ${unmountTarget}`, + time: 5000, }); fetchDiskInfo(); } catch (e) { @@ -1077,7 +1081,7 @@ function DiskSection() { )} - {isMounted && (isSystemMount || isPrimary) ? ( + {isMounted && isSystemMount ? ( System ) : isMounted ? ( `; @@ -1809,7 +1809,13 @@ } async function unmountDisk(mountPoint) { - if (!confirm(`Unmount ${mountPoint}?`)) return; + const message = mountPoint === '/media/wrolpi' ? + `Unmount ${mountPoint}?\n\nWARNING: This is the primary WROLPi drive. ` + + `The WROLPi API service will be STOPPED before unmounting, and your media ` + + `will be unavailable until a drive is mounted at /media/wrolpi and the API ` + + `service is started again.` : + `Unmount ${mountPoint}?`; + if (!confirm(message)) return; try { await apiCall('api/disks/unmount', 'POST', { diff --git a/controller/test/test_disks_api.py b/controller/test/test_disks_api.py index ab63c78b..7c1813bd 100644 --- a/controller/test/test_disks_api.py +++ b/controller/test/test_disks_api.py @@ -255,7 +255,6 @@ class TestUnmountEndpoint: @pytest.mark.parametrize("mount_point", [ "/media", # not strictly under /media/ "/media_evil", # prefix-collides with /media - "/media/wrolpi/../wrolpi", # normalizes to the reserved primary ]) def test_unmount_rejects_paths_outside_media( self, test_client, disks_env, mount_point, @@ -268,18 +267,89 @@ class TestUnmountEndpoint: assert disks_env["fake"].unmount_calls == [] def test_unmount_reserved_mount_point_rejected(self, test_client, disks_env): - # The primary WROLPi drive must never be unmountable from the API. - disks_env["fake"]._live.add("/media/wrolpi") + # The onboarding temp mount belongs to the Controller itself. + disks_env["fake"]._live.add("/media/wrolpi_temp_onboarding") + + response = test_client.post( + "/api/disks/unmount", + json={"mount_point": "/media/wrolpi_temp_onboarding"}, + ) + assert response.status_code == 400 + assert "reserved" in response.json()["detail"].lower() + # No umount was attempted, directly or via the reconciler. + assert disks_env["fake"].unmount_calls == [] + assert disks_env["fake"].current_mount_points() == { + "/media/wrolpi_temp_onboarding"} + + +class TestUnmountPrimaryEndpoint: + """Unmounting the primary drive stops the API service, then umounts + directly (the reconciler never manages the primary mount). The + /etc/fstab entry is left alone so a reboot restores the mount.""" + + @pytest.fixture + def primary_env(self, disks_env): + 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} + + @pytest.mark.parametrize("mount_point", [ + "/media/wrolpi", + "/media/wrolpi/../wrolpi", # normalizes to the primary + ]) + def test_unmount_primary_stops_api_and_unmounts( + self, test_client, primary_env, mount_point, + ): + primary_env["fake"]._live.add("/media/wrolpi") + + response = test_client.post( + "/api/disks/unmount", + json={"mount_point": mount_point}, + ) + assert response.status_code == 200 + body = response.json() + assert body["success"] is True + assert body["removed_from_fstab"] is False + assert body["stopped_services"] == ["wrolpi-api"] + primary_env["stop_service"].assert_called_once_with("wrolpi-api") + 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. + primary_env["fake"]._live.add("/media/wrolpi") + primary_env["fake"].fail_for_unmount = frozenset({"/media/wrolpi"}) response = test_client.post( "/api/disks/unmount", json={"mount_point": "/media/wrolpi"}, ) - assert response.status_code == 400 - assert "primary" in response.json()["detail"].lower() - # No umount was attempted, directly or via the reconciler. - assert disks_env["fake"].unmount_calls == [] - assert disks_env["fake"].current_mount_points() == {"/media/wrolpi"} + assert response.status_code == 500 + assert "still mounted" in response.json()["detail"] + primary_env["stop_service"].assert_called_once_with("wrolpi-api") + + def test_unmount_primary_stop_failure_still_unmounts( + self, test_client, primary_env, + ): + # A failed service stop (e.g. unit not installed) must not block + # the unmount; the umount result decides success. + primary_env["fake"]._live.add("/media/wrolpi") + primary_env["stop_service"].return_value = { + "success": False, "error": "unit not found"} + + 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"] == [] + assert primary_env["fake"].current_mount_points() == set() # --- /api/disks/fstab read endpoint ---------------------------------------