feat: video record and upload for standalone and dynamic grid

[VietND96]

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

• Update for video container (record & upload) work with standalone roles and dynamic grid (node-docker, standalone-docker)
• Add test to cover upload function by starting a local FTP server and passing env variables to config RCLONE upload.
• Add docker compose files for reference.

Motivation and Context

Reference

Configure video recording and uploading for Hub and Nodes: docker-compose-v3-video-upload.yml

Configure video recording and uploading for Standalone roles: docker-compose-v3-video-upload-standalone.yml

Configure video recording and uploading for Dynamic Grid (node-docker): docker-compose-v3-video-upload-dynamic-grid.yml

Configure video recording and uploading for Dynamic Grid standalone (standalone-docker): tests/docker-compose-v3-test-standalone-docker.yaml

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Enhanced video upload script with batch processing and improved logging.
• Added support for standalone video recording and improved session ID handling.
• Updated documentation to reflect new video recording and uploading configurations.
• Introduced new Docker Compose configurations for dynamic grid and standalone setups.
• Added new test strategies and targets for standalone video recording.

---

Changes walkthrough 📝

	Relevant files
Enhancement	9 files upload.sh Enhance video upload script with batch processing and logging Video/upload.sh Added process name for logging. Implemented batch processing for RCLONE uploads. Enhanced logging with timestamps. Refactored functions for better clarity. +51/-37  video.sh Support standalone video recording and improve logging      Video/video.sh Added support for standalone video recording. Improved logging with timestamps. Refactored session ID handling. +23/-14  video_graphQLQuery.sh Adjust GraphQL endpoint configuration for video queries    Video/video_graphQLQuery.sh Adjusted GraphQL endpoint configuration. Improved handling of environment variables. +5/-5      video_gridUrl.sh Add script to determine Selenium grid URL                                Video/video_gridUrl.sh Introduced script to determine grid URL. Added support for basic authentication. +30/-0    chart_test.sh Enhance Kubernetes resource description and uploader config tests/charts/make/chart_test.sh Enhanced resource description for Kubernetes. Added support for external uploader configuration. +8/-3      video_ready.py Modify video readiness check logic                                              Video/video_ready.py Modified video readiness check logic. Improved environment variable handling. +4/-1      Makefile Add test targets and improve resource preparation                Makefile Added new test targets for standalone video recording. Improved resource preparation steps. +45/-65  Dockerfile Add video grid URL script to Docker image                                Video/Dockerfile Added new script to Docker image. +1/-1      supervisord.conf Update supervisor configuration for video services              Video/supervisord.conf Updated supervisor configuration for video services. +5/-1
Tests	3 files bootstrap.sh Add delay after test execution in bootstrap script              tests/bootstrap.sh Added delay after test execution. +2/-0      config.yml Add test strategy for standalone video recording                  .circleci/config.yml Added new test strategy for standalone video recording. +6/-0      docker-test.yml Include standalone video recording test strategy                  .github/workflows/docker-test.yml Included standalone video recording test strategy. +4/-0
Documentation	1 files README.md Update documentation for video recording and uploading      README.md Updated documentation for video recording and uploading. Added configuration references for various setups. +16/-2
Configuration changes	2 files docker-compose-v3-video-upload-dynamic-grid.yml Add Docker Compose config for dynamic grid                              docker-compose-v3-video-upload-dynamic-grid.yml Added new Docker Compose configuration for dynamic grid. +50/-0    docker-compose-v3-video-upload-standalone.yml Add Docker Compose config for standalone video upload        docker-compose-v3-video-upload-standalone.yml Added Docker Compose configuration for standalone video upload. +120/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Potential Bug The new code for handling different session ID queries based on standalone or grid mode may not cover all possible scenarios, potentially leading to issues in session identification. Performance Concern The new batch processing for rclone uploads might introduce delays or bottlenecks if not properly tuned, especially with the fixed batch size of 10. Logic Change The modification to the video_ready check logic might affect the reliability of the readiness probe, potentially leading to false positives in certain scenarios.

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Use environment variables for sensitive information instead of hardcoding Consider using environment variables for sensitive information like passwords instead of hardcoding them in the docker-compose file. This improves security and makes the configuration more flexible. docker-compose-v3-video-upload.yml [65-66] # Password encrypted using command: rclone obscure <your_password> -- RCLONE_CONFIG_MYFTP_PASS=KkK8RsUIba-MMTBUSnuYIdAKvcnFyLl2pdhQig +- RCLONE_CONFIG_MYFTP_PASS=${RCLONE_FTP_PASSWORD} Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion addresses a significant security concern by recommending the use of environment variables for sensitive information like passwords, which enhances security and flexibility.	9
	Use secrets management for sensitive information instead of environment variables Consider using secrets management for sensitive information like passwords instead of environment variables. This provides an additional layer of security. tests/docker-compose-v3-test-standalone-docker.yaml [26] -- SE_RCLONE_CONFIG_MYFTP_PASS=KkK8RsUIba-MMTBUSnuYIdAKvcnFyLl2pdhQig +- SE_RCLONE_CONFIG_MYFTP_PASS_FILE=/run/secrets/ftp_password Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion enhances security by recommending the use of secrets management for handling sensitive information, providing an additional layer of protection compared to environment variables.	9
Enhancement	Replace multiple if-else conditions with a case statement for better code structure Consider using a case statement instead of multiple if-else conditions for better readability and maintainability when setting JQ_SESSION_ID_QUERY, SE_NODE_PORT, and NODE_STATUS_ENDPOINT based on SE_VIDEO_RECORD_STANDALONE. Video/video.sh [20-28] -if [ "${SE_VIDEO_RECORD_STANDALONE}" = "true" ]; then - JQ_SESSION_ID_QUERY=".value.nodes[]?.slots[]?.session?.sessionId" - SE_NODE_PORT=${SE_NODE_PORT:-"4444"} - NODE_STATUS_ENDPOINT="$(/opt/bin/video_gridUrl.sh)/status" -else - JQ_SESSION_ID_QUERY=".[]?.node?.slots | .[0]?.session?.sessionId" - SE_NODE_PORT=${SE_NODE_PORT:-"5555"} - NODE_STATUS_ENDPOINT="${SE_SERVER_PROTOCOL}://${DISPLAY_CONTAINER_NAME}:${SE_NODE_PORT}/status" -fi +case "${SE_VIDEO_RECORD_STANDALONE}" in + true) + JQ_SESSION_ID_QUERY=".value.nodes[]?.slots[]?.session?.sessionId" + SE_NODE_PORT=${SE_NODE_PORT:-"4444"} + NODE_STATUS_ENDPOINT="$(/opt/bin/video_gridUrl.sh)/status" + ;; + *) + JQ_SESSION_ID_QUERY=".[]?.node?.slots | .[0]?.session?.sessionId" + SE_NODE_PORT=${SE_NODE_PORT:-"5555"} + NODE_STATUS_ENDPOINT="${SE_SERVER_PROTOCOL}://${DISPLAY_CONTAINER_NAME}:${SE_NODE_PORT}/status" + ;; +esac Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion to use a case statement improves code readability and maintainability by reducing the complexity of multiple if-else conditions. This change is beneficial for future modifications and understanding of the code structure.	8
	✅ Simplify the logic for setting the grid_url variable using parameter expansion with default values Consider using parameter expansion with default values to simplify the logic for setting the grid_url variable, reducing the number of conditional statements. Video/video_gridUrl.sh [12-24] -if [ -z "${SE_HUB_HOST:-$SE_ROUTER_HOST}" ] || [ -z "${SE_HUB_PORT:-$SE_ROUTER_PORT}" ]; then - grid_url="" -else +grid_url=${SE_NODE_GRID_URL} +if [ -n "${SE_HUB_HOST:-$SE_ROUTER_HOST}" ] && [ -n "${SE_HUB_PORT:-$SE_ROUTER_PORT}" ]; then grid_url=${SE_SERVER_PROTOCOL}://${BASIC_AUTH}${SE_HUB_HOST:-$SE_ROUTER_HOST}:${SE_HUB_PORT:-$SE_ROUTER_PORT}${SE_SUB_PATH} +elif [ -n "${DISPLAY_CONTAINER_NAME}" ] && [ "${SE_VIDEO_RECORD_STANDALONE}" = "true" ]; then + grid_url="${SE_SERVER_PROTOCOL}://${BASIC_AUTH}${DISPLAY_CONTAINER_NAME}:${SE_NODE_PORT:-4444}${SE_SUB_PATH}" fi -if [ -z "${grid_url}" ] && [ -n "${DISPLAY_CONTAINER_NAME}" ] && [ "${SE_VIDEO_RECORD_STANDALONE}" = "true" ]; then - grid_url="${SE_SERVER_PROTOCOL}://${BASIC_AUTH}${DISPLAY_CONTAINER_NAME}:${SE_NODE_PORT:-4444}${SE_SUB_PATH}" # For standalone mode -fi - -if [ -z "${grid_url}" ]; then - grid_url="${SE_NODE_GRID_URL}" -fi - [Suggestion has been applied] Suggestion importance[1-10]: 8 Why: Simplifying the logic for setting the grid_url variable using parameter expansion with default values reduces the number of conditional statements, making the code cleaner and easier to maintain. This is a good enhancement for code clarity.	8
	Add a health check for the FTP server to ensure it's ready before dependent services start Consider adding a health check for the FTP server to ensure it's ready before other services that depend on it start. docker-compose-v3-video-upload-standalone.yml [8-15] ftp_server: image: delfer/alpine-ftp-server:latest environment: - USERS=seluser|selenium.dev volumes: # Mount the local directory `/tmp/upload` to the FTP server's `/ftp/seluser` directory to check out the uploaded videos - /tmp/upload:/ftp/seluser stop_grace_period: 30s + healthcheck: + test: ["CMD", "nc", "-z", "localhost", "21"] + interval: 10s + timeout: 5s + retries: 3 Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding a health check is a good practice that ensures the FTP server is ready before dependent services start, improving reliability and stability of the system.	7
Best practice	Add resource limits to containers to prevent potential resource exhaustion Consider adding resource limits to the containers to prevent potential resource exhaustion issues. docker-compose-v3-video-upload-dynamic-grid.yml [17-29] node-docker: image: selenium/node-docker:latest volumes: - ./assets:/opt/selenium/assets - ./NodeDocker/config.toml:/opt/selenium/config.toml - /var/run/docker.sock:/var/run/docker.sock depends_on: - selenium-hub environment: - SE_EVENT_BUS_HOST=selenium-hub - SE_EVENT_BUS_PUBLISH_PORT=4442 - SE_EVENT_BUS_SUBSCRIBE_PORT=4443 + deploy: + resources: + limits: + cpus: '0.5' + memory: 512M Apply this suggestion Suggestion importance[1-10]: 8 Why: Setting resource limits is a best practice that helps prevent resource exhaustion, ensuring that the system remains stable and responsive under load.	8
Performance	Implement batch processing for more efficient file uploads Consider using a more efficient approach for batch processing of rclone uploads by accumulating a list of files to upload and then processing them in batches, rather than checking the batch size after each file. Video/upload.sh [77-82] -function rclone_upload() { - local source=$1 - local target=$2 - echo "$(date +%FT%T%Z) [${process_name}] - Uploading ${source} to ${target}" - exec rclone --config ${UPLOAD_CONFIG_DIRECTORY}/${UPLOAD_CONFIG_FILE_NAME} ${UPLOAD_COMMAND} --cutoff-mode SOFT --metadata ${UPLOAD_OPTS} "${source}" "${target}" & +function rclone_upload_batch() { + local -a sources=("$@") + local -a targets=("${sources[@]/#/${UPLOAD_DESTINATION_PREFIX}/}") + echo "$(date +%FT%T%Z) [${process_name}] - Uploading batch of ${#sources[@]} files" + exec rclone --config ${UPLOAD_CONFIG_DIRECTORY}/${UPLOAD_CONFIG_FILE_NAME} ${UPLOAD_COMMAND} --cutoff-mode SOFT --metadata ${UPLOAD_OPTS} "${sources[@]}" "${targets[@]}" & } Apply this suggestion Suggestion importance[1-10]: 7 Why: Implementing batch processing for rclone uploads can enhance performance by reducing the overhead of frequent checks and process initiations. This suggestion is valid and can lead to more efficient resource utilization.	7
	Cache the result of process check to improve performance for multiple requests Consider caching the result of the process check to avoid repeated iterations over all processes for each request, especially if the script handles multiple requests in quick succession. Video/video_ready.py [11-14] -if environ.get('SE_VIDEO_UPLOAD_ENABLED', 'false').lower() != 'true' and environ.get('SE_VIDEO_FILE_NAME', 'video.mp4').lower() != 'auto': - video_ready = "ffmpeg" in (p.name().lower() for p in psutil.process_iter()) -else: - video_ready = True +def is_ffmpeg_running(): + return any(p.name().lower() == "ffmpeg" for p in psutil.process_iter()) +ffmpeg_running = is_ffmpeg_running() + +class Handler(BaseHTTPRequestHandler): + def do_GET(self): + global ffmpeg_running + if environ.get('SE_VIDEO_UPLOAD_ENABLED', 'false').lower() != 'true' and environ.get('SE_VIDEO_FILE_NAME', 'video.mp4').lower() != 'auto': + video_ready = ffmpeg_running + else: + video_ready = True + Apply this suggestion Suggestion importance[1-10]: 6 Why: Caching the result of the process check can improve performance by reducing the need to repeatedly iterate over processes. However, the suggestion may require careful consideration of when to update the cache to ensure accuracy.	6