From 5b72948bf65c59b9dd504453c1010474b7c0adb3 Mon Sep 17 00:00:00 2001 From: Florian Kilz Date: Thu, 27 Feb 2025 12:52:36 +0100 Subject: [PATCH 1/3] 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 #1 --- .../org/openqa/selenium/grid/node/docker/DockerFlags.java | 6 ++++++ .../openqa/selenium/grid/node/docker/DockerOptions.java | 7 +++++++ .../selenium/grid/node/docker/DockerSessionFactory.java | 7 +++++-- scripts/dev-image/Dockerfile | 1 + 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java b/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java index 02af7f7c2dd41..982cae6982b15 100644 --- a/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java +++ b/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java @@ -54,6 +54,12 @@ public class DockerFlags implements HasRoles { @ConfigValue(section = DockerOptions.DOCKER_SECTION, name = "port", example = "2375") private Integer dockerPort; + @Parameter( + names = {"--docker-server-start-timeout"}, + description = "Max time (in seconds) to wait for the server to successfully start up, before cancelling the process.") + @ConfigValue(section = DockerOptions.DOCKER_SECTION, name = "server-start-timeout", example = "55") + private Integer serverStartTimeout; + @Parameter( names = {"--docker", "-D"}, description = diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java b/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java index 5866375988ddb..f628eae87241d 100644 --- a/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java +++ b/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java @@ -25,6 +25,7 @@ import com.google.common.collect.Multimap; import java.net.URI; import java.net.URISyntaxException; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -63,6 +64,7 @@ public class DockerOptions { static final String DEFAULT_DOCKER_URL = "unix:/var/run/docker.sock"; static final String DEFAULT_VIDEO_IMAGE = "false"; static final int DEFAULT_MAX_SESSIONS = Runtime.getRuntime().availableProcessors(); + static final int DEFAULT_SERVER_START_TIMEOUT = 60; private static final String DEFAULT_DOCKER_NETWORK = "bridge"; private static final Logger LOG = Logger.getLogger(DockerOptions.class.getName()); private static final Json JSON = new Json(); @@ -104,6 +106,10 @@ private URI getDockerUri() { } } + private Duration getServerStartTimeout() { + return Duration.ofSeconds(config.getInt(DOCKER_SECTION, "server-start-timeout").orElse(DEFAULT_SERVER_START_TIMEOUT)); + } + private boolean isEnabled(Docker docker) { if (!config.getAll(DOCKER_SECTION, "configs").isPresent()) { return false; @@ -179,6 +185,7 @@ public Map> getDockerSessionFactories( tracer, clientFactory, options.getSessionTimeout(), + getServerStartTimeout(), docker, getDockerUri(), image, diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java b/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java index 65b322674662f..6f6ab71caef05 100644 --- a/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java +++ b/java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java @@ -91,6 +91,7 @@ public class DockerSessionFactory implements SessionFactory { private final Tracer tracer; private final HttpClient.Factory clientFactory; private final Duration sessionTimeout; + private final Duration serverStartTimeout; private final Docker docker; private final URI dockerUri; private final Image browserImage; @@ -108,6 +109,7 @@ public DockerSessionFactory( Tracer tracer, HttpClient.Factory clientFactory, Duration sessionTimeout, + Duration serverStartTimeout, Docker docker, URI dockerUri, Image browserImage, @@ -123,6 +125,7 @@ public DockerSessionFactory( this.tracer = Require.nonNull("Tracer", tracer); this.clientFactory = Require.nonNull("HTTP client", clientFactory); this.sessionTimeout = Require.nonNull("Session timeout", sessionTimeout); + this.serverStartTimeout = Require.nonNull("Server start timeout", serverStartTimeout); this.docker = Require.nonNull("Docker command", docker); this.dockerUri = Require.nonNull("Docker URI", dockerUri); this.browserImage = Require.nonNull("Docker browser image", browserImage); @@ -181,7 +184,7 @@ public Either apply(CreateSessionRequest sess "Waiting for server to start (container id: %s, url %s)", container.getId(), remoteAddress)); try { - waitForServerToStart(client, Duration.ofMinutes(1)); + waitForServerToStart(client, serverStartTimeout); } catch (TimeoutException e) { span.setAttribute(AttributeKey.ERROR.getKey(), true); span.setStatus(Status.CANCELLED); @@ -367,7 +370,7 @@ private Container startVideoContainer( clientFactory.createClient(ClientConfig.defaultConfig().baseUri(videoContainerUrl)); try { LOG.fine(String.format("Waiting for video recording... (id: %s)", videoContainer.getId())); - waitForServerToStart(videoClient, Duration.ofMinutes(1)); + waitForServerToStart(videoClient, serverStartTimeout); } catch (Exception e) { videoContainer.stop(Duration.ofSeconds(10)); String message = diff --git a/scripts/dev-image/Dockerfile b/scripts/dev-image/Dockerfile index f47b7d351e1bc..d3a77df8c3312 100644 --- a/scripts/dev-image/Dockerfile +++ b/scripts/dev-image/Dockerfile @@ -3,6 +3,7 @@ FROM shs96c/selenium-remote-build:latest USER root +RUN rm -f /etc/apt/sources.list.d/google-chrome.list RUN apt-get update -qqy && apt-get install -y wget curl gnupg2 # So we can install browsers later From 9b49122ce0a805e72bf2edf367fecef9ce8e1f19 Mon Sep 17 00:00:00 2001 From: Florian Kilz Date: Tue, 4 Mar 2025 10:06:12 +0100 Subject: [PATCH 2/3] Removed line from Dockerfile Removed an added line from the Dockerfile to be able to contribute it in a separate PR. --- scripts/dev-image/Dockerfile | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/dev-image/Dockerfile b/scripts/dev-image/Dockerfile index d3a77df8c3312..f47b7d351e1bc 100644 --- a/scripts/dev-image/Dockerfile +++ b/scripts/dev-image/Dockerfile @@ -3,7 +3,6 @@ FROM shs96c/selenium-remote-build:latest USER root -RUN rm -f /etc/apt/sources.list.d/google-chrome.list RUN apt-get update -qqy && apt-get install -y wget curl gnupg2 # So we can install browsers later From 37805484e957cafec9fbc7b2e6b40247257a19dc Mon Sep 17 00:00:00 2001 From: Florian Kilz Date: Wed, 5 Mar 2025 15:58:54 +0100 Subject: [PATCH 3/3] Ran ./scripts/format.sh to fix the linting issues. --- .../openqa/selenium/grid/node/docker/DockerFlags.java | 9 +++++++-- .../openqa/selenium/grid/node/docker/DockerOptions.java | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java b/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java index 982cae6982b15..827f1deab4074 100644 --- a/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java +++ b/java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java @@ -56,8 +56,13 @@ public class DockerFlags implements HasRoles { @Parameter( names = {"--docker-server-start-timeout"}, - description = "Max time (in seconds) to wait for the server to successfully start up, before cancelling the process.") - @ConfigValue(section = DockerOptions.DOCKER_SECTION, name = "server-start-timeout", example = "55") + description = + "Max time (in seconds) to wait for the server to successfully start up, before cancelling" + + " the process.") + @ConfigValue( + section = DockerOptions.DOCKER_SECTION, + name = "server-start-timeout", + example = "55") private Integer serverStartTimeout; @Parameter( diff --git a/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java b/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java index f628eae87241d..c714373eeb7af 100644 --- a/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java +++ b/java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java @@ -107,7 +107,8 @@ private URI getDockerUri() { } private Duration getServerStartTimeout() { - return Duration.ofSeconds(config.getInt(DOCKER_SECTION, "server-start-timeout").orElse(DEFAULT_SERVER_START_TIMEOUT)); + return Duration.ofSeconds( + config.getInt(DOCKER_SECTION, "server-start-timeout").orElse(DEFAULT_SERVER_START_TIMEOUT)); } private boolean isEnabled(Docker docker) {