Skip to content

Commit 5b72948

Browse files
committed
Introduced new variable for server start timeout
Added a new variable in DockerSessionFactory that can be used as a maximum to wait for the server/node to start. Made it adjustable via a DockerFlag and set the default to 60 seconds, which was previously the hardcoded value in that field. Also added a line to Dockerfile in dev-image, because dev container was not working as is. Fixes SeleniumHQ#1
1 parent 040420d commit 5b72948

File tree

4 files changed

+19
-2
lines changed

4 files changed

+19
-2
lines changed

Diff for: java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java

+6
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ public class DockerFlags implements HasRoles {
5454
@ConfigValue(section = DockerOptions.DOCKER_SECTION, name = "port", example = "2375")
5555
private Integer dockerPort;
5656

57+
@Parameter(
58+
names = {"--docker-server-start-timeout"},
59+
description = "Max time (in seconds) to wait for the server to successfully start up, before cancelling the process.")
60+
@ConfigValue(section = DockerOptions.DOCKER_SECTION, name = "server-start-timeout", example = "55")
61+
private Integer serverStartTimeout;
62+
5763
@Parameter(
5864
names = {"--docker", "-D"},
5965
description =

Diff for: java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java

+7
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.common.collect.Multimap;
2626
import java.net.URI;
2727
import java.net.URISyntaxException;
28+
import java.time.Duration;
2829
import java.util.ArrayList;
2930
import java.util.Arrays;
3031
import java.util.Collection;
@@ -63,6 +64,7 @@ public class DockerOptions {
6364
static final String DEFAULT_DOCKER_URL = "unix:/var/run/docker.sock";
6465
static final String DEFAULT_VIDEO_IMAGE = "false";
6566
static final int DEFAULT_MAX_SESSIONS = Runtime.getRuntime().availableProcessors();
67+
static final int DEFAULT_SERVER_START_TIMEOUT = 60;
6668
private static final String DEFAULT_DOCKER_NETWORK = "bridge";
6769
private static final Logger LOG = Logger.getLogger(DockerOptions.class.getName());
6870
private static final Json JSON = new Json();
@@ -104,6 +106,10 @@ private URI getDockerUri() {
104106
}
105107
}
106108

109+
private Duration getServerStartTimeout() {
110+
return Duration.ofSeconds(config.getInt(DOCKER_SECTION, "server-start-timeout").orElse(DEFAULT_SERVER_START_TIMEOUT));
111+
}
112+
107113
private boolean isEnabled(Docker docker) {
108114
if (!config.getAll(DOCKER_SECTION, "configs").isPresent()) {
109115
return false;
@@ -179,6 +185,7 @@ public Map<Capabilities, Collection<SessionFactory>> getDockerSessionFactories(
179185
tracer,
180186
clientFactory,
181187
options.getSessionTimeout(),
188+
getServerStartTimeout(),
182189
docker,
183190
getDockerUri(),
184191
image,

Diff for: java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public class DockerSessionFactory implements SessionFactory {
9191
private final Tracer tracer;
9292
private final HttpClient.Factory clientFactory;
9393
private final Duration sessionTimeout;
94+
private final Duration serverStartTimeout;
9495
private final Docker docker;
9596
private final URI dockerUri;
9697
private final Image browserImage;
@@ -108,6 +109,7 @@ public DockerSessionFactory(
108109
Tracer tracer,
109110
HttpClient.Factory clientFactory,
110111
Duration sessionTimeout,
112+
Duration serverStartTimeout,
111113
Docker docker,
112114
URI dockerUri,
113115
Image browserImage,
@@ -123,6 +125,7 @@ public DockerSessionFactory(
123125
this.tracer = Require.nonNull("Tracer", tracer);
124126
this.clientFactory = Require.nonNull("HTTP client", clientFactory);
125127
this.sessionTimeout = Require.nonNull("Session timeout", sessionTimeout);
128+
this.serverStartTimeout = Require.nonNull("Server start timeout", serverStartTimeout);
126129
this.docker = Require.nonNull("Docker command", docker);
127130
this.dockerUri = Require.nonNull("Docker URI", dockerUri);
128131
this.browserImage = Require.nonNull("Docker browser image", browserImage);
@@ -181,7 +184,7 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
181184
"Waiting for server to start (container id: %s, url %s)",
182185
container.getId(), remoteAddress));
183186
try {
184-
waitForServerToStart(client, Duration.ofMinutes(1));
187+
waitForServerToStart(client, serverStartTimeout);
185188
} catch (TimeoutException e) {
186189
span.setAttribute(AttributeKey.ERROR.getKey(), true);
187190
span.setStatus(Status.CANCELLED);
@@ -367,7 +370,7 @@ private Container startVideoContainer(
367370
clientFactory.createClient(ClientConfig.defaultConfig().baseUri(videoContainerUrl));
368371
try {
369372
LOG.fine(String.format("Waiting for video recording... (id: %s)", videoContainer.getId()));
370-
waitForServerToStart(videoClient, Duration.ofMinutes(1));
373+
waitForServerToStart(videoClient, serverStartTimeout);
371374
} catch (Exception e) {
372375
videoContainer.stop(Duration.ofSeconds(10));
373376
String message =

Diff for: scripts/dev-image/Dockerfile

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ FROM shs96c/selenium-remote-build:latest
33

44
USER root
55

6+
RUN rm -f /etc/apt/sources.list.d/google-chrome.list
67
RUN apt-get update -qqy && apt-get install -y wget curl gnupg2
78

89
# So we can install browsers later

0 commit comments

Comments
 (0)