Docker: Process of xvfb_vnc_novnc start by env variable

[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

Fixes #2500
To prevent confusion messages from PID spawn again, even already set it as false to disable

2024-12-10 19:14:29,438 INFO spawned: 'vnc' with pid 1412
2024-12-10 19:14:29,439 INFO spawned: 'novnc' with pid 1413
2024-12-10 19:14:29,441 WARN exited: vnc (exit status 0; not expected)
2024-12-10 19:14:29,442 WARN exited: novnc (exit status 0; not expected)
2024-12-10 19:14:32,446 INFO spawned: 'vnc' with pid 1425
2024-12-10 19:14:32,447 INFO spawned: 'novnc' with pid 1426
2024-12-10 19:14:32,449 WARN exited: vnc (exit status 0; not expected)
2024-12-10 19:14:32,449 INFO gave up: vnc entered FATAL state, too many start retries too quickly
2024-12-10 19:14:32,450 WARN exited: novnc (exit status 0; not expected)
2024-12-10 19:14:33,451 INFO gave up: novnc entered FATAL state, too many start retries too quickly

Motivation and Context

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

Bug fix

---

Description

• Fixed issue where VNC and noVNC processes would attempt to start even when disabled via environment variables
• Modified supervisor configuration to properly respect SE_START_VNC, SE_START_NO_VNC, and SE_START_XVFB environment variables
• Prevents unnecessary process spawning and error messages when services are intentionally disabled
• Improves resource usage by properly disabling unused display and remote access services

---

Changes walkthrough 📝

	Relevant files
Configuration changes	selenium.conf Enable environment variable control for VNC and display services NodeBase/selenium.conf Modified supervisor configuration to respect environment variables for process control Changed autostart and autorestart settings for xvfb, vnc, and novnc services to use environment variables Added dynamic control through SE_START_XVFB, SE_START_VNC, and SE_START_NO_VNC variables +6/-6

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2500 - Partially compliant Compliant requirements: Fix issue where VNC services still spawn processes even when disabled via environment variables Non-compliant requirements: Remove confusing log messages about VNC process spawning when VNC is disabled Requires further human verification: Verify that VNC services are completely disabled when environment variables are set to false Check if the Grid UI properly reflects VNC disabled state

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Configuration Validation Verify that the supervisor configuration properly handles the case when environment variables are not set or have invalid values. Current implementation assumes variables will always be valid boolean values. autostart=%(ENV_SE_START_XVFB)s autorestart=%(ENV_SE_START_XVFB)s

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Ensure environment variables are properly typed for supervisor configuration Add environment variable type validation to ensure boolean values, as supervisor expects true/false for autostart and autorestart settings. NodeBase/selenium.conf [8-9] -autostart=%(ENV_SE_START_XVFB)s -autorestart=%(ENV_SE_START_XVFB)s +autostart=%(ENV_SE_START_XVFB:bool)s +autorestart=%(ENV_SE_START_XVFB:bool)s Apply this suggestion Suggestion importance[1-10]: 9 Why: Type validation for boolean environment variables is critical to prevent runtime errors, as supervisor strictly expects true/false values for autostart/autorestart settings. Without type validation, invalid values could cause service startup failures.	9
	Add fallback values for environment variables to prevent service startup failures Add default values for the environment variables using the environment= directive to ensure the services start correctly even if the environment variables are not set. NodeBase/selenium.conf [8-9] +environment=SE_START_XVFB=true autostart=%(ENV_SE_START_XVFB)s autorestart=%(ENV_SE_START_XVFB)s Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding default values for environment variables is crucial for service reliability, preventing startup failures when environment variables are not explicitly set. This is particularly important for core services like XVFB.	8

[qodo-merge-pro]

CI Failure Feedback 🧐

(Checks updated until commit 326703b)

Action: Rerun workflow when failure

Failed stage: Authenticate GitHub CLI for PR [❌]

Failure summary: The action failed because the GitHub authentication token lacks the required 'read:org' permission scope. The error occurred during the gh auth login command, which requires proper authorization to access organization-related information.

Relevant error logs: 1: ##[group]Operating System 2: Ubuntu ... 28: SecurityEvents: write 29: Statuses: write 30: ##[endgroup] 31: Secret source: Actions 32: Prepare workflow directory 33: Prepare all required actions 34: Getting action download info 35: Download action repository 'actions/checkout@main' (SHA:cbb722410c2e876e24abbe8de2cc27693e501dcb) 36: Complete job name: Rerun workflow when failure ... 48: show-progress: true 49: lfs: false 50: submodules: false 51: set-safe-directory: true 52: env: 53: GH_CLI_TOKEN: *** 54: GH_CLI_TOKEN_PR: *** 55: RUN_ID: 12488767879 56: RERUN_FAILED_ONLY: true ... 119: ##[group]Run sudo apt update 120: �[36;1msudo apt update�[0m 121: �[36;1msudo apt install gh�[0m 122: shell: /usr/bin/bash -e {0} 123: env: 124: GH_CLI_TOKEN: *** 125: GH_CLI_TOKEN_PR: *** 126: RUN_ID: 12488767879 127: RERUN_FAILED_ONLY: true ... 164: 0 upgraded, 0 newly installed, 0 to remove and 48 not upgraded. 165: ##[group]Run echo "$GH_CLI_TOKEN_PR" | gh auth login --with-token 166: �[36;1mecho "$GH_CLI_TOKEN_PR" | gh auth login --with-token�[0m 167: shell: /usr/bin/bash -e {0} 168: env: 169: GH_CLI_TOKEN: *** 170: GH_CLI_TOKEN_PR: *** 171: RUN_ID: 12488767879 172: RERUN_FAILED_ONLY: true 173: RUN_ATTEMPT: 1 174: ##[endgroup] 175: error validating token: missing required scope 'read:org' 176: ##[error]Process completed with exit code 1.

---

✨ CI feedback usage guide:

---

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

• Failed stage
• Failed test name
• Failure summary
• Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

• enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
• excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
• enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
• persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
• final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.