Skip to content

Commit 2765119

Browse files
authored
Merge pull request #388 from OpenMS/claude/singularity-bind-mountpoints
fix(singularity): pre-create /workspaces and /mounted-data so :rw bin…
2 parents b36fb90 + f8d0106 commit 2765119

4 files changed

Lines changed: 64 additions & 6 deletions

File tree

.github/workflows/build-and-test.yml

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,16 @@ jobs:
142142
sudo apptainer build /tmp/openms.sif docker-archive:///tmp/image.tar
143143
sudo chmod a+r /tmp/openms.sif
144144
145-
- name: Start apptainer instance (read-only root, host UID)
145+
- name: Prepare host bind dirs (mountpoint contract)
146+
run: |
147+
# Host paths we'll bind into the SIF. Asserting writability through
148+
# singularity's bind machinery requires that the destination paths
149+
# exist as real directories in the squashfs (otherwise singularity
150+
# silently degrades the bind to read-only via underlay).
151+
mkdir -p /tmp/host-workspaces /tmp/host-mounted-data
152+
echo "from-host-pretest" > /tmp/host-mounted-data/sentinel.txt
153+
154+
- name: Start apptainer instance (read-only root, host UID, with binds)
146155
run: |
147156
# Default apptainer semantics: read-only root, no --writable-tmpfs.
148157
# This matches how users on HPC clusters run the SIF.
@@ -151,7 +160,10 @@ jobs:
151160
# Docker ENTRYPOINT but leaves %startscript as the default no-op
152161
# `exec "$@"`. `instance start` would launch an empty instance and
153162
# streamlit would never bind 8501.
154-
apptainer instance run /tmp/openms.sif openms-test
163+
apptainer instance run \
164+
--bind /tmp/host-workspaces:/workspaces-streamlit-template:rw \
165+
--bind /tmp/host-mounted-data:/mounted-data:ro \
166+
/tmp/openms.sif openms-test
155167
apptainer instance list
156168
# Record where this run's logs will land so subsequent steps can tail
157169
# them deterministically (path depends on hostname/user).
@@ -195,6 +207,29 @@ jobs:
195207
run: |
196208
apptainer exec instance://openms-test redis-cli ping | grep -i pong
197209
210+
- name: Verify bind mount is writable (workspaces) and readable (data)
211+
run: |
212+
# The whole point of pre-creating /workspaces-streamlit-template
213+
# and /mounted-data in the image: singularity now has a real
214+
# attach point and `:rw` actually sticks. Without the mkdir,
215+
# `apptainer exec ... touch` here would fail with EROFS.
216+
apptainer exec instance://openms-test sh -c \
217+
'echo from-container > /workspaces-streamlit-template/probe.txt'
218+
test -f /tmp/host-workspaces/probe.txt
219+
grep -q from-container /tmp/host-workspaces/probe.txt
220+
# Read-only data mount should also be visible inside the container.
221+
apptainer exec instance://openms-test grep -q from-host-pretest /mounted-data/sentinel.txt
222+
# The mounted-drive browser uses os.path.ismount() to gate
223+
# rendering (existence is no longer enough now that the image
224+
# pre-creates the dir). Assert the kernel reports both paths as
225+
# real mount points so the detection function returns truthy.
226+
apptainer exec instance://openms-test python3 -c "
227+
import os, sys
228+
for p in ('/mounted-data', '/workspaces-streamlit-template'):
229+
assert os.path.ismount(p), f'{p} not reported as mount point'
230+
print(f'ismount({p}) = True')
231+
"
232+
198233
- name: Dump entrypoint logs on failure
199234
if: failure()
200235
run: |

Dockerfile

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ RUN apt-get update && apt-get install -y --no-install-recommends redis-server ng
133133
# never written under apptainer.
134134
RUN mkdir -p /var/lib/redis
135135

136+
# Pre-create bind-mount targets so apptainer/singularity has a real attach
137+
# point. Docker auto-creates missing `-v` targets, but singularity uses a
138+
# read-only underlay and silently ignores `:rw` when the target isn't a
139+
# real directory in the SIF — writes then fail with EROFS even though the
140+
# host bind path is writable. Pre-creating these directories costs one
141+
# inode each and changes nothing in docker mode (the user's volume mount
142+
# shadows them).
143+
RUN mkdir -p /workspaces-streamlit-template /mounted-data
144+
136145
# Create workdir and copy over all streamlit related files/folders.
137146

138147
# note: specifying folder with slash as suffix and repeating the folder name seems important to preserve directory structure

Dockerfile_simple

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ RUN mamba install pip
6262
RUN python -m pip install --upgrade pip
6363
RUN python -m pip install -r requirements.txt
6464

65+
# Pre-create bind-mount targets so apptainer/singularity has a real attach
66+
# point. Docker auto-creates missing `-v` targets, but singularity uses a
67+
# read-only underlay and silently ignores `:rw` when the target isn't a
68+
# real directory in the SIF — writes then fail with EROFS even though the
69+
# host bind path is writable.
70+
RUN mkdir -p /workspaces-streamlit-template /mounted-data
6571

6672
# create workdir and copy over all streamlit related files/folders
6773
WORKDIR /app

src/workflow/StreamlitUI.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,13 @@
2626
def _mounted_data_root() -> Union[Path, None]:
2727
"""Return the validated mount root from the ``local_data_dir`` setting.
2828
29-
The browser renders only when that path resolves to an existing
30-
directory inside the container, i.e. when a host volume is actually
31-
mounted there.
29+
The browser renders only when ``local_data_dir`` is an actual mount
30+
point inside the container — i.e. the operator passed ``-v`` /
31+
``--bind`` / ``volumeMount`` to attach host data. Existence alone is
32+
no longer sufficient because the image now pre-creates the path so
33+
apptainer/singularity binds have a real attach target; without
34+
``os.path.ismount`` the browser would render an empty tree for every
35+
user who didn't mount anything.
3236
"""
3337
settings = st.session_state.get("settings") or {}
3438
raw = (settings.get("local_data_dir") or "").strip()
@@ -38,7 +42,11 @@ def _mounted_data_root() -> Union[Path, None]:
3842
p = Path(raw).expanduser().resolve(strict=True)
3943
except (OSError, RuntimeError):
4044
return None
41-
return p if p.is_dir() else None
45+
if not p.is_dir():
46+
return None
47+
if not os.path.ismount(p):
48+
return None
49+
return p
4250

4351

4452
class StreamlitUI:

0 commit comments

Comments
 (0)