Fix recording shutdown for node docker (#3106)

Author: VietND96

Makefile

@@ -980,7 +980,7 @@ test_parallel: hub chrome firefox edge chromium video
 		cd ./tests || true ; \
 		echo TAG=$(TAG_VERSION) > .env ; \
 		echo VIDEO_TAG=$(FFMPEG_TAG_VERSION)-$(BUILD_DATE) >> .env ; \
-		echo TEST_DELAY_AFTER_TEST=$(or $(TEST_DELAY_AFTER_TEST), 2) >> .env ; \
+		echo TEST_DELAY_AFTER_TEST=$(or $(TEST_DELAY_AFTER_TEST), 0) >> .env ; \
 		echo TEST_DRAIN_AFTER_SESSION_COUNT=$(or $(TEST_DRAIN_AFTER_SESSION_COUNT), 2) >> .env ; \
 		echo TEST_PARALLEL_HARDENING=$(or $(TEST_PARALLEL_HARDENING), "true") >> .env ; \
 		echo TEST_PARALLEL_COUNT=$(or $(TEST_PARALLEL_COUNT), 5) >> .env ; \
@@ -1007,10 +1007,10 @@ test_parallel: hub chrome firefox edge chromium video
 	make test_video_integrity
 
 test_video_standalone: standalone_chrome standalone_chromium standalone_firefox standalone_edge
-	DOCKER_COMPOSE_FILE=docker-compose-v3-test-standalone.yml TEST_DELAY_AFTER_TEST=2 HUB_CHECKS_INTERVAL=45 make test_video
+	DOCKER_COMPOSE_FILE=docker-compose-v3-test-standalone.yml TEST_DELAY_AFTER_TEST=0 HUB_CHECKS_INTERVAL=45 make test_video
 
 test_video_dynamic_name:
-	VIDEO_FILE_NAME=auto TEST_DELAY_AFTER_TEST=2 HUB_CHECKS_INTERVAL=45 TEST_ADD_CAPS_RECORD_VIDEO=false \
+	VIDEO_FILE_NAME=auto TEST_DELAY_AFTER_TEST=0 HUB_CHECKS_INTERVAL=45 TEST_ADD_CAPS_RECORD_VIDEO=false \
 	make test_video
 
 # This should run on its own CI job. There is no need to combine it with the other tests.
@@ -1039,7 +1039,7 @@ test_video: video hub chrome firefox edge chromium
 		echo UID=$$(id -u) >> .env ; \
 		echo BINDING_VERSION=$(BINDING_VERSION) >> .env ; \
 		echo BASE_VERSION=$(BASE_VERSION) >> .env ; \
-		echo TEST_DELAY_AFTER_TEST=$(or $(TEST_DELAY_AFTER_TEST), 2) >> .env ; \
+		echo TEST_DELAY_AFTER_TEST=$(or $(TEST_DELAY_AFTER_TEST), 0) >> .env ; \
 		echo HUB_CHECKS_INTERVAL=$(or $(HUB_CHECKS_INTERVAL), 45) >> .env ; \
 		echo SELENIUM_ENABLE_MANAGED_DOWNLOADS=$(or $(SELENIUM_ENABLE_MANAGED_DOWNLOADS), "true") >> .env ; \
 		echo TEST_FIREFOX_INSTALL_LANG_PACKAGE=$${TEST_FIREFOX_INSTALL_LANG_PACKAGE} >> .env ; \
@@ -1161,7 +1161,7 @@ test_node_docker: hub standalone_docker standalone_chrome standalone_firefox sta
 		echo LOG_LEVEL=$(or $(LOG_LEVEL), "INFO") >> .env ; \
 		echo REQUEST_TIMEOUT=$(or $(REQUEST_TIMEOUT), 300) >> .env ; \
 		echo SELENIUM_ENABLE_MANAGED_DOWNLOADS=$(or $(SELENIUM_ENABLE_MANAGED_DOWNLOADS), "false") >> .env ; \
-		echo TEST_DELAY_AFTER_TEST=$(or $(TEST_DELAY_AFTER_TEST), 2) >> .env ; \
+		echo TEST_DELAY_AFTER_TEST=$(or $(TEST_DELAY_AFTER_TEST), 0) >> .env ; \
 		echo RECORD_STANDALONE=$(or $(RECORD_STANDALONE), "true") >> .env ; \
 		echo GRID_URL=$(or $(GRID_URL), "") >> .env ; \
 		echo HUB_CHECKS_INTERVAL=$(or $(HUB_CHECKS_INTERVAL), 20) >> .env ; \
@@ -1170,6 +1170,7 @@ test_node_docker: hub standalone_docker standalone_chrome standalone_firefox sta
 		echo UID=$$(id -u) >> .env ; \
 		echo BINDING_VERSION=$(BINDING_VERSION) >> .env ; \
 		echo BASE_VERSION=$(BASE_VERSION) >> .env ; \
+		echo VIDEO_EVENT_DRIVEN=$(or $(VIDEO_EVENT_DRIVEN), "true") >> .env ; \
 		if [ "$$(uname)" != "Darwin" ]; then \
 			echo HOST_IP=$$(hostname -I | awk '{print $$1}') >> .env ; \
 		else \
@@ -1238,7 +1239,7 @@ test_video_integrity:
 	fi; \
 	for file in $$list_files; do \
 		echo "Checking video file: $$file"; \
-		docker run -u $$(id -u) -v $$(pwd):$$(pwd) -w $$(pwd) --entrypoint="" $(NAME)/video:$(FFMPEG_TAG_VERSION)-$(BUILD_DATE) ffmpeg -v error -i "$$file" -f null - ; \
+		docker run --rm -u $$(id -u) -v $$(pwd):$$(pwd) -w $$(pwd) --entrypoint="" $(NAME)/video:$(FFMPEG_TAG_VERSION)-$(BUILD_DATE) ffmpeg -v error -i "$$file" -f null - ; \
 		if [ $$? -ne 0 ]; then \
 			echo "Video file $$file is corrupted"; \
 			number_corrupted_files=$$((number_corrupted_files+1)); \
@@ -1275,7 +1276,7 @@ chart_test_autoscaling_deployment:
 	PLATFORMS=$(PLATFORMS) TEST_EXISTING_KEDA=true RELEASE_NAME=selenium CHART_ENABLE_TRACING=true TEST_PATCHED_KEDA=$(TEST_PATCHED_KEDA) AUTOSCALING_COOLDOWN_PERIOD=30 \
 	TRACING_EXPORTER_ENDPOINT=$(TRACING_EXPORTER_ENDPOINT) TEST_CUSTOM_SPECIFIC_NAME=true \
 	SECURE_CONNECTION_SERVER=true SECURE_USE_EXTERNAL_CERT=true SERVICE_TYPE_NODEPORT=true SELENIUM_GRID_PROTOCOL=https SELENIUM_GRID_HOST=$$(hostname -I | cut -d' ' -f1) SELENIUM_GRID_PORT=31444 \
-	SELENIUM_GRID_AUTOSCALING_MIN_REPLICA=1 SET_MAX_REPLICAS=3 TEST_DELAY_AFTER_TEST=2 TEST_NODE_DRAIN_AFTER_SESSION_COUNT=3 SELENIUM_GRID_MONITORING=false \
+	SELENIUM_GRID_AUTOSCALING_MIN_REPLICA=1 SET_MAX_REPLICAS=3 TEST_DELAY_AFTER_TEST=0 TEST_NODE_DRAIN_AFTER_SESSION_COUNT=3 SELENIUM_GRID_MONITORING=false \
 	VERSION=$(TAG_VERSION) VIDEO_TAG=$(FFMPEG_TAG_VERSION)-$(BUILD_DATE) KEDA_BASED_NAME=$(KEDA_BASED_NAME) KEDA_BASED_TAG=$(KEDA_BASED_TAG) NAMESPACE=$(NAMESPACE) BINDING_VERSION=$(BINDING_VERSION) BASE_VERSION=$(BASE_VERSION) \
 	TEMPLATE_OUTPUT_FILENAME="k8s_prefixSelenium_enableTracing_secureServer_externalCerts_nodePort_autoScaling_scaledObject_existingKEDA_subPath.yaml" \
 	./tests/charts/make/chart_test.sh DeploymentAutoscaling

StandaloneDocker/Dockerfile

@@ -21,4 +21,6 @@ ENV SE_SESSION_REQUEST_TIMEOUT="300" \
 # Boolean value, maps "--relax-checks"
     SE_RELAX_CHECKS="true" \
     SE_OTEL_SERVICE_NAME="selenium-standalone-docker" \
-    SE_NODE_ENABLE_MANAGED_DOWNLOADS="true"
+    SE_NODE_ENABLE_MANAGED_DOWNLOADS="true" \
+    SE_BIND_BUS="true" \
+    SE_EVENT_BUS_IMPLEMENTATION=""

StandaloneDocker/start-selenium-grid-docker.sh

@@ -108,6 +108,19 @@ if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
   append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
 fi
 
+if [ ! -z "${SE_EVENT_BUS_IMPLEMENTATION}" ]; then
+  append_se_opts "--events-implementation" "${SE_EVENT_BUS_IMPLEMENTATION}"
+fi
+
+if [ "${SE_BIND_BUS}" = "true" ]; then
+  append_se_opts "--bind-bus" "${SE_BIND_BUS}"
+  append_se_opts "--publish-events" "tcp://*:${SE_EVENT_BUS_PUBLISH_PORT}"
+  append_se_opts "--subscribe-events" "tcp://*:${SE_EVENT_BUS_SUBSCRIBE_PORT}"
+  if [ -z "${SE_EVENT_BUS_IMPLEMENTATION}" ]; then
+    append_se_opts "--events-implementation" "org.openqa.selenium.events.zeromq.ZeroMqEventBus"
+  fi
+fi
+
 if [ "${SE_ENABLE_TLS}" = "true" ]; then
   # Configure truststore for the server
   if [ ! -z "$SE_JAVA_SSL_TRUST_STORE" ]; then

Video/recorder.conf

@@ -6,7 +6,7 @@ autostart=%(ENV_SE_RECORD_VIDEO)s
 startsecs=0
 autorestart=%(ENV_SE_RECORD_VIDEO)s
 stopsignal=TERM
-stopwaitsecs=60
+stopwaitsecs=30
 
 ;Logs (all activity redirected to stdout so it can be seen through "docker logs"
 redirect_stderr=true
@@ -20,7 +20,8 @@ killasgroup=true
 autostart=%(ENV_SE_RECORD_VIDEO)s
 startsecs=0
 autorestart=%(ENV_SE_RECORD_VIDEO)s
-stopsignal=KILL
+stopsignal=TERM
+stopwaitsecs=5
 
 ;Logs (all activity redirected to stdout so it can be seen through "docker logs"
 redirect_stderr=true

Video/video.sh

@@ -212,9 +212,15 @@ function graceful_exit() {
   wait_util_uploader_shutdown
 }
 
+_graceful_exit_done=false
 function graceful_exit_force() {
+  if [[ "$_graceful_exit_done" = "true" ]]; then
+    return
+  fi
+  _graceful_exit_done=true
   graceful_exit
-  kill -SIGTERM "$(cat ${SE_SUPERVISORD_PID_FILE})" 2>/dev/null
+  # Supervisord signaling is delegated to the Python controller (video_recorder.py)
+  # which handles it uniformly for both shell and event-driven modes.
   echo "$(date -u +"${ts_format}") [${process_name}] - Ready to shutdown the recorder"
   exit 0
 }
@@ -234,7 +240,7 @@ if [[ "${VIDEO_UPLOAD_ENABLED}" != "true" ]] && [[ "${VIDEO_FILE_NAME}" != "auto
     -probesize 32M -analyzeduration 0 -y -f x11grab -video_size ${VIDEO_SIZE} -r ${FRAME_RATE} \
     -i ${DISPLAY} ${SE_AUDIO_SOURCE} -codec:v ${CODEC} ${PRESET:-"-preset veryfast"} \
     -tune zerolatency -crf ${SE_VIDEO_CRF:-28} -maxrate ${SE_VIDEO_MAXRATE:-1000k} -bufsize ${SE_VIDEO_BUFSIZE:-2000k} \
-    -pix_fmt yuv420p -movflags +faststart "$video_file" &
+    -pix_fmt yuv420p -movflags frag_keyframe+empty_moov+default_base_moof "$video_file" &
   FFMPEG_PID=$!
   if ps -p $FFMPEG_PID >/dev/null; then
     wait $FFMPEG_PID
@@ -270,7 +276,7 @@ else
           -probesize 32M -analyzeduration 0 -y -f x11grab -video_size ${VIDEO_SIZE} -r ${FRAME_RATE} \
           -i ${DISPLAY} ${SE_AUDIO_SOURCE} -codec:v ${CODEC} ${PRESET:-"-preset veryfast"} \
           -tune zerolatency -crf ${SE_VIDEO_CRF:-28} -maxrate ${SE_VIDEO_MAXRATE:-1000k} -bufsize ${SE_VIDEO_BUFSIZE:-2000k} \
-          -pix_fmt yuv420p -movflags +faststart "$video_file" &
+          -pix_fmt yuv420p -movflags frag_keyframe+empty_moov+default_base_moof "$video_file" &
         FFMPEG_PID=$!
         if ps -p $FFMPEG_PID >/dev/null; then
           recording_started="true"

Video/video_ready.py

@@ -28,7 +28,10 @@ def do_GET(self):
 
 def graceful_shutdown(signum, frame):
     print("Trapped SIGTERM/SIGINT/x so shutting down video-ready...")
-    httpd.shutdown()
+    # httpd.shutdown() must be called from a different thread than serve_forever()
+    # or it deadlocks.  video-ready has no state to drain, so close the socket
+    # and exit directly — supervisord will see the clean exit immediately.
+    httpd.server_close()
     sys.exit(0)
 
 

Video/video_recorder.py

@@ -6,6 +6,12 @@
 
 When event-driven mode is enabled, this launches a single unified service
 that handles both recording and uploading with shared state management.
+
+After the video service exits for any reason (normal drain, session end, or
+supervisord-initiated shutdown), this controller signals supervisord so the
+container shuts down.  Centralising this here means both shell and event-driven
+modes have identical container-lifecycle behaviour without video.sh needing to
+know about supervisord.
 """
 
 import os
@@ -14,12 +20,47 @@
 import sys
 
 
+def _signal_supervisord() -> None:
+    """Signal supervisord to initiate a container-wide shutdown.
+
+    Safe to call even when supervisord is already shutting down — it will
+    simply ignore a repeated SIGTERM if it is already in SHUTDOWN state.
+    """
+    pid_file = os.environ.get("SE_SUPERVISORD_PID_FILE", "")
+    if not pid_file:
+        return
+    try:
+        with open(pid_file) as f:
+            pid = int(f.read().strip())
+        os.kill(pid, signal.SIGTERM)
+        print("[video.recorder] - Signaled supervisord to shut down")
+    except (OSError, ValueError, FileNotFoundError):
+        pass
+
+
 def main():
     event_driven = os.environ.get("SE_VIDEO_EVENT_DRIVEN", "false").lower() == "true"
 
     if event_driven:
         print("Starting unified event-driven video service...")
         print("This service handles both recording and uploading with shared state.")
+
+        # Capture whether shutdown was externally initiated (SIGTERM/SIGINT)
+        # before asyncio.run() replaces the signal handlers via add_signal_handler.
+        _external_shutdown = [False]
+
+        def _mark_external_shutdown(signum, frame):
+            _external_shutdown[0] = True
+            # This handler is only reachable before asyncio.run() installs its
+            # own handlers via loop.add_signal_handler().  Setting the flag and
+            # returning would swallow the signal — nothing would act on it and
+            # the process would hang inside asyncio.run() indefinitely.
+            # Exit immediately so supervisord sees a clean stop.
+            sys.exit(0)
+
+        signal.signal(signal.SIGTERM, _mark_external_shutdown)
+        signal.signal(signal.SIGINT, _mark_external_shutdown)
+
         try:
             import asyncio
 
@@ -31,24 +72,46 @@ def main():
             print("Ensure pyzmq is installed: pip install pyzmq")
             print("Falling back to shell-based recording...")
             _run_shell_recorder()
+            return
+
+        # Only trigger container shutdown for self-initiated exits (drain).
+        if not _external_shutdown[0]:
+            _signal_supervisord()
     else:
         print("Starting shell-based video recording...")
         _run_shell_recorder()
 
 
 def _run_shell_recorder():
     proc = subprocess.Popen(["/opt/bin/video.sh"])
+    _external_shutdown = False  # True when supervisord (or user) told us to stop
 
     def forward_signal(signum, frame):
-        try:
-            proc.send_signal(signum)
-        except ProcessLookupError:
-            pass  # Process already exited before signal was forwarded
+        nonlocal _external_shutdown
+        # Forward the signal to video.sh at most once.  supervisord uses
+        # killasgroup=true so video.sh already received the signal directly;
+        # re-forwarding on every re-entrant call amplifies the SIGTERM
+        # ping-pong and can keep the process alive for 60 s.
+        if not _external_shutdown:
+            _external_shutdown = True
+            try:
+                proc.send_signal(signum)
+            except ProcessLookupError:
+                pass  # Process already exited before signal was forwarded
         proc.wait()
 
     signal.signal(signal.SIGTERM, forward_signal)
     signal.signal(signal.SIGINT, forward_signal)
     rc = proc.wait()
+
+    # Signal supervisord only for self-initiated exits (drain, node gone).
+    # If the shutdown came FROM supervisord (_external_shutdown=True) it is
+    # already in SHUTDOWN state — signalling it again is a no-op at best and
+    # confusing at worst.  If the recorder crashed (rc != 0) we must not bring
+    # down the Selenium process alongside it.
+    if not _external_shutdown and rc == 0:
+        _signal_supervisord()
+
     if rc != 0:
         sys.exit(rc)