From 34458c702c3c292840d401bd8d2e5365c3ffd96c Mon Sep 17 00:00:00 2001 From: menzelj Date: Fri, 18 Sep 2026 16:29:37 +0200 Subject: [PATCH] Mount NFS destinations with nolock so they need no nfs-common (0.66.2) An NFSv3 mount with locking makes the kernel register its lock manager with rpcbind on the Docker host itself. On a host without nfs-common the mount fails with a bare "connection refused" that reads like the NAS is unreachable, even though the NAS and its ports are fine. Only StackPilot ever writes the backup files on an NFS destination, one writer per file, so nothing needs locking. mount_options() now adds nolock unless the options already choose (lock, nolock, local_lock=) or ask for NFSv4, which locks in-protocol and needs no rpcbind. Existing destinations get their volume recreated once with the new options. The refused-mount hint now points at rpcbind/nfs-common on the Docker host first, and "protocol not supported" explains the missing kernel module (modprobe nfsv4). The form labels the option. Co-Authored-By: Claude Opus 5 --- README.md | 4 ++ .../services/backup_destination_service.py | 39 +++++++++++-- backend/tests/test_helper_containers.py | 57 ++++++++++++++++++- backend/version.py | 2 +- frontend/package.json | 2 +- frontend/src/pages/Settings.tsx | 2 +- 6 files changed, 96 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 355b8e9..db11c02 100644 --- a/README.md +++ b/README.md @@ -636,6 +636,10 @@ it is what your saved destination credentials are encrypted with. NFS needs no privileges in the StackPilot container: the Docker daemon mounts the export as a named volume (`stackpilot-nfs-dest-`, recreated when the config changes) and file I/O runs through throwaway helper containers. + StackPilot adds `nolock` to NFSv2/3 mount options unless you set `lock` or + `local_lock=` yourself: with locking the kernel needs rpcbind on the Docker + host (nfs-common), and without it the mount fails with "connection refused". + Only StackPilot writes the backup files, so nothing needs locking (0.66.2). - **Push & restore**: the stack Backup dialog can push straight to a destination instead of downloading; the Restore dialog can browse a destination's backups and restore (volumes included) directly from it. Backups can also be diff --git a/backend/services/backup_destination_service.py b/backend/services/backup_destination_service.py index bcf7569..b181ada 100644 --- a/backend/services/backup_destination_service.py +++ b/backend/services/backup_destination_service.py @@ -291,6 +291,27 @@ def _nfs_subdir(cfg: dict) -> str: return "/".join(parts) +def mount_options(cfg: dict) -> str: + """The destination's mount options, with ``nolock`` added for NFSv2/3. + + An NFSv3 mount with locking makes the kernel register its lock manager + with rpcbind on the *Docker host*, which is only there when nfs-common (or + the distribution's equivalent) is installed; without it the mount fails + with a bare "connection refused". Only StackPilot ever writes these backup + files, one writer per file, so there is nothing to lock. An explicit + lock/nolock/local_lock option is left as given, and NFSv4 has locking + built into the protocol and needs no rpcbind at all. + """ + raw = (cfg.get("options") or "rw").strip().strip(",") + parts = [p.strip() for p in raw.split(",") if p.strip()] + chosen = any(p in ("lock", "nolock") or p.startswith("local_lock=") for p in parts) + v4 = any(p.split("=", 1)[0] in ("nfsvers", "vers") and p.split("=", 1)[-1].startswith("4") + for p in parts) + if not chosen and not v4: + parts.append("nolock") + return ",".join(parts) + + def _nfs_volume(dest: BackupDestination, cfg: dict) -> str: """Ensure the named volume describing this NFS mount exists; recreate it when the destination's server/path/options changed (opts are immutable).""" @@ -302,8 +323,7 @@ def _nfs_volume(dest: BackupDestination, cfg: dict) -> str: raise DestinationError("NFS server is required") if not path.startswith("/"): raise DestinationError("NFS export path must be absolute (start with /)") - options = (cfg.get("options") or "rw").strip().strip(",") - driver_opts = {"type": "nfs", "o": f"addr={server},{options}", "device": f":{path}"} + driver_opts = {"type": "nfs", "o": f"addr={server},{mount_options(cfg)}", "device": f":{path}"} name = f"{_NFS_VOLUME_PREFIX}{dest.id}" client = get_client() @@ -379,11 +399,18 @@ def _mount_error(exc: Exception) -> DestinationError: """A mount failure, with what to do about the common one.""" text = str(exc) message = f"NFS mount failed: {text}" - if "connection refused" in text.lower(): + lowered = text.lower() + if "connection refused" in lowered: message += ( - " — the NFS server refused the Docker host's connection. Check that NFS is " - "enabled on the server and reachable from the Docker host. NFSv3 also needs " - "rpcbind (port 111) and mountd; mount options 'rw,nfsvers=4' need only port 2049." + " — a connection was refused. With locking enabled ('lock' in the mount " + "options) NFSv3 needs rpcbind running on the Docker host itself: install " + "nfs-common there, or drop 'lock'. Otherwise check that NFS is enabled on the " + "server and that ports 111, 2049 and mountd are reachable from the Docker host." + ) + elif "protocol not supported" in lowered: + message += ( + " — the Docker host's kernel cannot speak this NFS version: load the module " + "(e.g. 'modprobe nfsv4') or remove nfsvers from the mount options." ) return DestinationError(message) diff --git a/backend/tests/test_helper_containers.py b/backend/tests/test_helper_containers.py index 4b84bc9..ea3e0da 100644 --- a/backend/tests/test_helper_containers.py +++ b/backend/tests/test_helper_containers.py @@ -204,5 +204,60 @@ def test_refused_mount_says_what_to_do(monkeypatch): with pytest.raises(svc.DestinationError) as info: svc._nfs_run("stackpilot-nfs-dest-1", ["mkdir", "-p", "/nfs"]) - assert "nfsvers=4" in str(info.value) and "port 2049" in str(info.value) + assert "rpcbind" in str(info.value) and "2049" in str(info.value) assert client.all == [] # and nothing is left behind + + +# --------------------------------------------------------------------------- # +# nolock: NFS destinations that work without nfs-common on the Docker host +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "options, expected", + [ + (None, "rw,nolock"), + ("", "rw,nolock"), + ("rw", "rw,nolock"), + ("rw,soft,", "rw,soft,nolock"), + ("rw,nfsvers=3", "rw,nfsvers=3,nolock"), + ("rw,lock", "rw,lock"), # asked for locking: respected + ("rw,nolock", "rw,nolock"), + ("rw,local_lock=all", "rw,local_lock=all"), + ("rw,nfsvers=4", "rw,nfsvers=4"), # v4 locks in-protocol, no rpcbind + ("rw,vers=4.1", "rw,vers=4.1"), + ], +) +def test_nolock_is_added_for_nfs_v3_only(options, expected): + from services.backup_destination_service import mount_options + + assert mount_options({"options": options}) == expected + + +def test_existing_destination_is_remounted_with_nolock(monkeypatch): + """A destination saved with plain "rw" gets its volume recreated once.""" + from services import backup_destination_service as svc + + client = FakeClient() + monkeypatch.setattr("docker_client.get_client", lambda: client) + dest, cfg = _nfs_dest("rw") + name = f"{svc._NFS_VOLUME_PREFIX}{dest.id}" + # As 0.66.1 and earlier created it. + client.volumes.create(name, "local", { + "type": "nfs", "o": "addr=10.10.1.80,rw", "device": ":/mnt/user/Share", + }, {"stackpilot.nfs-destination": "1"}) + + svc._nfs_volume(dest, cfg) + + assert client.volumes.items[name].attrs["Options"]["o"] == "addr=10.10.1.80,rw,nolock" + + +def test_refused_mount_points_at_rpcbind_on_the_docker_host(monkeypatch): + from services import backup_destination_service as svc + + client = FakeClient(start_error="failed to mount local volume: connection refused") + monkeypatch.setattr("docker_client.get_long_client", lambda: client) + monkeypatch.setattr("services.stack_assets_service.ensure_helper_image", lambda c: None) + with pytest.raises(svc.DestinationError) as info: + svc._nfs_run("stackpilot-nfs-dest-1", ["true"]) + assert "nfs-common" in str(info.value) diff --git a/backend/version.py b/backend/version.py index 094951e..0968c78 100644 --- a/backend/version.py +++ b/backend/version.py @@ -1,3 +1,3 @@ """Single source of truth for the StackPilot release version.""" -APP_VERSION = "0.66.1" +APP_VERSION = "0.66.2" diff --git a/frontend/package.json b/frontend/package.json index c20156e..851bc23 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "stackpilot-frontend", "private": true, - "version": "0.66.1", + "version": "0.66.2", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/pages/Settings.tsx b/frontend/src/pages/Settings.tsx index 8d1077c..812da11 100644 --- a/frontend/src/pages/Settings.tsx +++ b/frontend/src/pages/Settings.tsx @@ -526,7 +526,7 @@ const FIELDS: Record