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