Skip to content

Commit bce2e27

Browse files
authored
Merge pull request #389 from OpenMS/claude/singularity-bind-mountpoints
fix(apptainer): use unix socket for Redis so host:6379 can't shadow us
2 parents 9128698 + a40e0c6 commit bce2e27

2 files changed

Lines changed: 63 additions & 12 deletions

File tree

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,23 @@ jobs:
205205
- name: Verify Redis is reachable inside container (full variant)
206206
if: matrix.variant == 'full'
207207
run: |
208-
apptainer exec instance://openms-test redis-cli ping | grep -i pong
208+
# In apptainer mode the entrypoint uses a unix socket (TCP 6379 on
209+
# localhost is the host's, since net namespace is shared). The
210+
# entrypoint writes the resolved URL to /tmp/openms-redis-url for
211+
# out-of-band discovery, since `apptainer exec` spawns a fresh
212+
# shell that doesn't inherit the daemon's exported env.
213+
URL=$(apptainer exec instance://openms-test cat /tmp/openms-redis-url 2>/dev/null || true)
214+
case "$URL" in
215+
unix://*)
216+
SOCK="${URL#unix://}"
217+
echo "Redis URL is unix socket: $SOCK"
218+
apptainer exec instance://openms-test redis-cli -s "$SOCK" ping | grep -i pong
219+
;;
220+
*)
221+
echo "Redis URL is TCP (or unset): ${URL:-default}"
222+
apptainer exec instance://openms-test redis-cli ping | grep -i pong
223+
;;
224+
esac
209225
210226
- name: Verify bind mount is writable (workspaces) and readable (data)
211227
run: |

docker/entrypoint.sh

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,30 @@ if [ "$READONLY_ROOT" -eq 1 ]; then
4444
mkdir -p "$RUNTIME_DIR"
4545
REDIS_DATA_DIR="$RUNTIME_DIR/redis"
4646
REDIS_PID_FILE="$RUNTIME_DIR/redis.pid"
47+
# Apptainer/singularity share the host's network namespace by default. If
48+
# the host has anything listening on 6379 (a system redis-server, a docker
49+
# container, a previous singularity instance that didn't clean up), our
50+
# `redis-server --daemonize` silently fails with EADDRINUSE and the local
51+
# redis-cli ping happily connects to the host's redis instead — which
52+
# leaves stale `worker-1` records lying around and ultimately runs the
53+
# workflow's mkdir outside our mount namespace (no bind → EROFS). A unix
54+
# socket sidesteps the network stack entirely; the path is unambiguously
55+
# ours.
56+
REDIS_SOCKET="$RUNTIME_DIR/redis.sock"
57+
REDIS_URL="unix://${REDIS_SOCKET}"
58+
export REDIS_URL
4759
NGINX_CONF_DIR="$RUNTIME_DIR/nginx"
4860
NGINX_PID_FILE="$RUNTIME_DIR/nginx.pid"
4961
mkdir -p "$REDIS_DATA_DIR" "$NGINX_CONF_DIR"
62+
# Marker for out-of-band discovery (e.g. `apptainer exec ... redis-cli`
63+
# from CI). The entrypoint's exported env doesn't propagate to fresh
64+
# exec invocations, so write the resolved URL to a stable path.
65+
echo "$REDIS_URL" > /tmp/openms-redis-url 2>/dev/null || true
5066
else
5167
RUNTIME_DIR="/var/run"
5268
REDIS_DATA_DIR="/var/lib/redis"
5369
REDIS_PID_FILE="/var/run/redis.pid"
70+
REDIS_SOCKET=""
5471
NGINX_CONF_DIR="/etc/nginx"
5572
NGINX_PID_FILE="/run/nginx.pid"
5673
fi
@@ -74,25 +91,43 @@ fi
7491
# The simple image does not install redis-server. Skip the whole queue section
7592
# when the binary is missing, so this entrypoint can be shared by both images.
7693
if command -v redis-server >/dev/null 2>&1; then
77-
echo "Starting Redis server (data=$REDIS_DATA_DIR)..."
78-
redis-server --daemonize yes \
79-
--dir "$REDIS_DATA_DIR" \
80-
--pidfile "$REDIS_PID_FILE" \
81-
--appendonly no
82-
83-
# Bounded wait so a broken redis-server (e.g. port already bound by the
84-
# host under apptainer's shared net namespace) fails the container fast
85-
# instead of hanging forever and never serving /_stcore/health.
94+
if [ -n "$REDIS_SOCKET" ]; then
95+
echo "Starting Redis server (data=$REDIS_DATA_DIR, socket=$REDIS_SOCKET)..."
96+
# --port 0 disables the TCP listener entirely — we only accept the
97+
# unix socket. This is the whole point of switching to a socket in
98+
# apptainer mode: the host's network namespace (shared by default)
99+
# cannot conflict with us, and there is no fall-through to a stray
100+
# host redis-server.
101+
redis-server --daemonize yes \
102+
--dir "$REDIS_DATA_DIR" \
103+
--pidfile "$REDIS_PID_FILE" \
104+
--unixsocket "$REDIS_SOCKET" \
105+
--unixsocketperm 700 \
106+
--port 0 \
107+
--appendonly no
108+
REDIS_CLI_ARGS=(-s "$REDIS_SOCKET")
109+
else
110+
echo "Starting Redis server (data=$REDIS_DATA_DIR)..."
111+
redis-server --daemonize yes \
112+
--dir "$REDIS_DATA_DIR" \
113+
--pidfile "$REDIS_PID_FILE" \
114+
--appendonly no
115+
REDIS_CLI_ARGS=()
116+
fi
117+
118+
# Bounded wait so a broken redis-server (e.g. socket can't be created or
119+
# an unexpected fork failure) fails the container fast instead of hanging
120+
# forever and never serving /_stcore/health.
86121
REDIS_STARTUP_RETRIES="${REDIS_STARTUP_RETRIES:-30}"
87122
for i in $(seq 1 "$REDIS_STARTUP_RETRIES"); do
88-
if redis-cli ping >/dev/null 2>&1; then
123+
if redis-cli "${REDIS_CLI_ARGS[@]}" ping >/dev/null 2>&1; then
89124
echo "Redis is ready"
90125
break
91126
fi
92127
echo "Waiting for Redis... attempt $i/$REDIS_STARTUP_RETRIES"
93128
sleep 1
94129
done
95-
if ! redis-cli ping >/dev/null 2>&1; then
130+
if ! redis-cli "${REDIS_CLI_ARGS[@]}" ping >/dev/null 2>&1; then
96131
echo "ERROR: Redis failed to become ready within ${REDIS_STARTUP_RETRIES}s" >&2
97132
exit 1
98133
fi

0 commit comments

Comments
 (0)