Skip to content

Conversation

@VietND96
Copy link
Member

@VietND96 VietND96 commented Nov 19, 2025

User description

🔗 Related Issues

💥 What does this PR do?

In previous PR #16599 - Improve browser container labels and naming in Dynamic Grid.

If Docker Compose starts with --exit-code-from, the compose stack will exit when any of the dynamic containers (Grid browser or recorder) terminate.

The com.docker.compose.oneoff=False label marks containers as service containers rather than one-off containers. When Docker Compose runs with --exit-code-from, it only monitors containers that are marked as one-off (or the specific container named in the flag). By setting this label to False, these dynamically created browser and video containers will:

  • Still be part of the compose stack (grouped by the compose labels)
  • Not trigger the compose stack to exit when they terminate
  • Allow the main services to continue running independently

This is particularly useful when running Dynamic Grid in Docker Compose, where browser container and test container are in same stack, and only listen to the status of test container for exit code.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Enhancement


Description

  • Add com.docker.compose.oneoff=False label to dynamic containers

  • Prevents compose stack exit when browser containers terminate

  • Allows main services to continue running independently

  • Ensures proper container lifecycle management in Docker Compose


Diagram Walkthrough

flowchart LR
  A["Docker Compose Stack"] -->|"--exit-code-from flag"| B["Monitor Container Status"]
  B -->|"oneoff=False label"| C["Browser Containers"]
  C -->|"Continue running"| D["Main Services Unaffected"]
Loading

File Walkthrough

Relevant files
Enhancement
DockerSessionFactory.java
Add oneoff label to dynamic container configuration           

java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java

  • Add com.docker.compose.oneoff=False label to compose labels map
  • Label prevents dynamic containers from triggering stack exit
  • Ensures browser and video containers remain part of compose stack
  • Allows independent lifecycle management of test containers
+2/-0     

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@selenium-ci selenium-ci added B-grid Everything grid and server related C-java Java Bindings labels Nov 19, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 19, 2025

PR Compliance Guide 🔍

(Compliance updated until commit f7e0af3)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit fd8260c
Security Compliance
Mutable shared input

Description: The code mutates the injected compose labels map in-place by adding
com.docker.compose.oneoff=False, which can have unintended side effects or label collision
if the input map is shared or untrusted; defensive copying should be considered.
DockerSessionFactory.java [145-146]

Referred Code
// Merge compose labels with oneoff=False to prevent triggering --exit-code-from dynamic grid
this.composeLabels.put("com.docker.compose.oneoff", "False");
Ticket Compliance
🟡
🎫 #5678
🔴 Investigate and resolve repeated "Error: ConnectFailure (Connection refused)" when
instantiating multiple ChromeDriver instances on Ubuntu with specified versions.
Ensure subsequent ChromeDriver instantiations do not log ConnectFailure errors while the
first works.
Provide a reliable fix or configuration change within Selenium/Grid that prevents these
connection failures.
🟡
🎫 #1234
🔴 Clicking links with javascript in href should trigger JS in Selenium 2.48.x on Firefox 42
similar to 2.47.1 behavior.
Provide fix or regression resolution for click() not triggering javascript href.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The new code updates compose labels but does not log this critical configuration change,
making it unclear who/when/what changed for audit purposes.

Referred Code
// Merge compose labels with oneoff=False to prevent triggering --exit-code-from dynamic grid
this.composeLabels.put("com.docker.compose.oneoff", "False");

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null checks: The code mutates composeLabels without verifying mutability or handling potential nulls or
unsupported operations, which could throw at runtime.

Referred Code
this.composeLabels = Require.nonNull("Docker Compose labels", composeLabels);
// Merge compose labels with oneoff=False to prevent triggering --exit-code-from dynamic grid
this.composeLabels.put("com.docker.compose.oneoff", "False");

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 19, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid mutating a constructor parameter
Suggestion Impact:The commit changes replaced direct mutation of composeLabels with creating a new HashMap copy, adding the label, and assigning an unmodifiable map, preventing mutation of the input parameter.

code diff:

-    this.composeLabels = Require.nonNull("Docker Compose labels", composeLabels);
     // Merge compose labels with oneoff=False to prevent triggering --exit-code-from dynamic grid
-    this.composeLabels.put("com.docker.compose.oneoff", "False");
+    Map<String, String> allLabels =
+        new HashMap<>(Require.nonNull("Docker Compose labels", composeLabels));
+    allLabels.put("com.docker.compose.oneoff", "False");
+    this.composeLabels = Collections.unmodifiableMap(allLabels);

To avoid mutating a constructor parameter and prevent potential runtime
exceptions, create a new mutable copy of the composeLabels map before adding the
new label.

java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [144-146]

-this.composeLabels = Require.nonNull("Docker Compose labels", composeLabels);
 // Merge compose labels with oneoff=False to prevent triggering --exit-code-from dynamic grid
-this.composeLabels.put("com.docker.compose.oneoff", "False");
+Map<String, String> allLabels = new HashMap<>(Require.nonNull("Docker Compose labels", composeLabels));
+allLabels.put("com.docker.compose.oneoff", "False");
+this.composeLabels = Collections.unmodifiableMap(allLabels);

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential UnsupportedOperationException if an immutable map is passed to the constructor and avoids a side effect by not mutating the input parameter, which significantly improves the code's robustness.

Medium
  • Update

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 merged commit e0afdd3 into trunk Nov 19, 2025
46 checks passed
@VietND96 VietND96 deleted the dynamic-grid-exit-code branch November 19, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-grid Everything grid and server related C-java Java Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants