Skip to content

Commit 5e2fa13

Browse files
committed
Fix recording shutdown for node docker
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
1 parent aa36c2b commit 5e2fa13

2 files changed

Lines changed: 68 additions & 5 deletions

File tree

Video/video.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,15 @@ function graceful_exit() {
212212
wait_util_uploader_shutdown
213213
}
214214

215+
_graceful_exit_done=false
215216
function graceful_exit_force() {
217+
if [[ "$_graceful_exit_done" = "true" ]]; then
218+
return
219+
fi
220+
_graceful_exit_done=true
216221
graceful_exit
217-
kill -SIGTERM "$(cat ${SE_SUPERVISORD_PID_FILE})" 2>/dev/null
222+
# Supervisord signaling is delegated to the Python controller (video_recorder.py)
223+
# which handles it uniformly for both shell and event-driven modes.
218224
echo "$(date -u +"${ts_format}") [${process_name}] - Ready to shutdown the recorder"
219225
exit 0
220226
}

Video/video_recorder.py

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66
77
When event-driven mode is enabled, this launches a single unified service
88
that handles both recording and uploading with shared state management.
9+
10+
After the video service exits for any reason (normal drain, session end, or
11+
supervisord-initiated shutdown), this controller signals supervisord so the
12+
container shuts down. Centralising this here means both shell and event-driven
13+
modes have identical container-lifecycle behaviour without video.sh needing to
14+
know about supervisord.
915
"""
1016

1117
import os
@@ -14,12 +20,41 @@
1420
import sys
1521

1622

23+
def _signal_supervisord() -> None:
24+
"""Signal supervisord to initiate a container-wide shutdown.
25+
26+
Safe to call even when supervisord is already shutting down — it will
27+
simply ignore a repeated SIGTERM if it is already in SHUTDOWN state.
28+
"""
29+
pid_file = os.environ.get("SE_SUPERVISORD_PID_FILE", "")
30+
if not pid_file:
31+
return
32+
try:
33+
with open(pid_file) as f:
34+
pid = int(f.read().strip())
35+
os.kill(pid, signal.SIGTERM)
36+
print("[video.recorder] - Signaled supervisord to shut down")
37+
except (OSError, ValueError, FileNotFoundError):
38+
pass
39+
40+
1741
def main():
1842
event_driven = os.environ.get("SE_VIDEO_EVENT_DRIVEN", "false").lower() == "true"
1943

2044
if event_driven:
2145
print("Starting unified event-driven video service...")
2246
print("This service handles both recording and uploading with shared state.")
47+
48+
# Capture whether shutdown was externally initiated (SIGTERM/SIGINT)
49+
# before asyncio.run() replaces the signal handlers via add_signal_handler.
50+
_external_shutdown = [False]
51+
52+
def _mark_external_shutdown(signum, frame):
53+
_external_shutdown[0] = True
54+
55+
signal.signal(signal.SIGTERM, _mark_external_shutdown)
56+
signal.signal(signal.SIGINT, _mark_external_shutdown)
57+
2358
try:
2459
import asyncio
2560

@@ -31,24 +66,46 @@ def main():
3166
print("Ensure pyzmq is installed: pip install pyzmq")
3267
print("Falling back to shell-based recording...")
3368
_run_shell_recorder()
69+
return
70+
71+
# Only trigger container shutdown for self-initiated exits (drain).
72+
if not _external_shutdown[0]:
73+
_signal_supervisord()
3474
else:
3575
print("Starting shell-based video recording...")
3676
_run_shell_recorder()
3777

3878

3979
def _run_shell_recorder():
4080
proc = subprocess.Popen(["/opt/bin/video.sh"])
81+
_external_shutdown = False # True when supervisord (or user) told us to stop
4182

4283
def forward_signal(signum, frame):
43-
try:
44-
proc.send_signal(signum)
45-
except ProcessLookupError:
46-
pass # Process already exited before signal was forwarded
84+
nonlocal _external_shutdown
85+
# Forward the signal to video.sh at most once. supervisord uses
86+
# killasgroup=true so video.sh already received the signal directly;
87+
# re-forwarding on every re-entrant call amplifies the SIGTERM
88+
# ping-pong and can keep the process alive for 60 s.
89+
if not _external_shutdown:
90+
_external_shutdown = True
91+
try:
92+
proc.send_signal(signum)
93+
except ProcessLookupError:
94+
pass # Process already exited before signal was forwarded
4795
proc.wait()
4896

4997
signal.signal(signal.SIGTERM, forward_signal)
5098
signal.signal(signal.SIGINT, forward_signal)
5199
rc = proc.wait()
100+
101+
# Signal supervisord only for self-initiated exits (drain, node gone).
102+
# If the shutdown came FROM supervisord (_external_shutdown=True) it is
103+
# already in SHUTDOWN state — signalling it again is a no-op at best and
104+
# confusing at worst. If the recorder crashed (rc != 0) we must not bring
105+
# down the Selenium process alongside it.
106+
if not _external_shutdown and rc == 0:
107+
_signal_supervisord()
108+
52109
if rc != 0:
53110
sys.exit(rc)
54111

0 commit comments

Comments
 (0)