Skip to content

Commit ba2ab2c

Browse files
diemoldiemol
committed
[grid] Simplifying code in DockerSessionFactory by adding defaults
1 parent 58f7a9b commit ba2ab2c

File tree

3 files changed

+108
-117
lines changed

3 files changed

+108
-117
lines changed

‎java/server/src/org/openqa/selenium/grid/docker/DockerSessionAssetsPath.java renamed to ‎java/server/src/org/openqa/selenium/grid/docker/DockerAssetsPath.java

+11-21
Original file line numberDiff line numberDiff line change
@@ -28,33 +28,23 @@
2828
import java.util.logging.Level;
2929
import java.util.logging.Logger;
3030

31-
public class DockerSessionAssetsPath {
31+
public class DockerAssetsPath {
3232

33-
private static final Logger LOG = Logger.getLogger(DockerSessionAssetsPath.class.getName());
33+
private static final Logger LOG = Logger.getLogger(DockerAssetsPath.class.getName());
3434

35-
private final String hostAssetsPath;
36-
private final String containerAssetsPath;
35+
private final String hostPath;
36+
private final String containerPath;
3737

38-
public DockerSessionAssetsPath(String hostAssetsPath, String containerAssetsPath) {
39-
this.hostAssetsPath = hostAssetsPath;
40-
this.containerAssetsPath = containerAssetsPath;
38+
public DockerAssetsPath(String hostPath, String containerPath) {
39+
this.hostPath = hostPath;
40+
this.containerPath = containerPath;
4141
}
4242

43-
public String getHostSessionAssetsPath(SessionId id) {
44-
return this.hostAssetsPath + File.separator + id;
43+
public String getHostPath(SessionId id) {
44+
return this.hostPath + File.separator + id;
4545
}
4646

47-
public Optional<Path> createContainerSessionAssetsPath(SessionId id) {
48-
if (this.containerAssetsPath == null || this.containerAssetsPath.isEmpty()) {
49-
return Optional.empty();
50-
}
51-
try {
52-
return Optional.of(Files.createDirectories(Paths.get(this.containerAssetsPath, id.toString())));
53-
} catch (IOException e) {
54-
LOG.log(Level.WARNING,
55-
"Failed to create path to store session assets, no assets will be stored: " +
56-
this.containerAssetsPath, e);
57-
}
58-
return Optional.empty();
47+
public String getContainerPath(SessionId id) {
48+
return this.containerPath + File.separator + id;
5949
}
6050
}

‎java/server/src/org/openqa/selenium/grid/docker/DockerOptions.java

+46-45
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import com.google.common.collect.Multimap;
2424
import org.openqa.selenium.Capabilities;
2525
import org.openqa.selenium.Platform;
26+
import org.openqa.selenium.docker.ContainerId;
27+
import org.openqa.selenium.docker.ContainerInfo;
2628
import org.openqa.selenium.docker.Docker;
2729
import org.openqa.selenium.docker.DockerException;
2830
import org.openqa.selenium.docker.Image;
@@ -51,6 +53,8 @@
5153
public class DockerOptions {
5254

5355
private static final String DOCKER_SECTION = "docker";
56+
private static final String CONTAINER_ASSETS_PATH = "/opt/selenium/assets";
57+
private static final String DEFAULT_VIDEO_IMAGE = "selenium/video:latest";
5458

5559
private static final Logger LOG = Logger.getLogger(DockerOptions.class.getName());
5660
private static final Json JSON = new Json();
@@ -94,27 +98,29 @@ private URI getDockerUri() {
9498
}
9599
}
96100

97-
private boolean isEnabled(HttpClient.Factory clientFactory) {
101+
private boolean isEnabled(Docker docker) {
98102
if (!config.getAll(DOCKER_SECTION, "configs").isPresent()) {
99103
return false;
100104
}
101105

102106
// Is the daemon up and running?
103-
URI uri = getDockerUri();
104-
HttpClient client = clientFactory.createClient(ClientConfig.defaultConfig().baseUri(uri));
105-
106-
return new Docker(client).isSupported();
107+
return docker.isSupported();
107108
}
108109

109110
public Map<Capabilities, Collection<SessionFactory>> getDockerSessionFactories(
110111
Tracer tracer,
111112
HttpClient.Factory clientFactory) {
112113

113-
if (!isEnabled(clientFactory)) {
114+
HttpClient client = clientFactory.createClient(ClientConfig.defaultConfig().baseUri(getDockerUri()));
115+
Docker docker = new Docker(client);
116+
117+
if (!isEnabled(docker)) {
114118
LOG.warning("Unable to reach the Docker daemon.");
115119
return ImmutableMap.of();
116120
}
117121

122+
DockerAssetsPath assetsPath = getAssetsPath(docker);
123+
118124
List<String> allConfigs = config.getAll(DOCKER_SECTION, "configs")
119125
.orElseThrow(() -> new DockerException("Unable to find docker configs"));
120126

@@ -130,37 +136,26 @@ public Map<Capabilities, Collection<SessionFactory>> getDockerSessionFactories(
130136
kinds.put(imageName, stereotype);
131137
}
132138

133-
HttpClient client = clientFactory.createClient(ClientConfig.defaultConfig().baseUri(getDockerUri()));
134-
Docker docker = new Docker(client);
135-
136139
loadImages(docker, kinds.keySet().toArray(new String[0]));
137140
Image videoImage = getVideoImage(docker);
138-
if (isVideoRecordingAvailable()) {
139-
loadImages(docker, videoImage.getName());
140-
}
141+
loadImages(docker, videoImage.getName());
141142

142143
int maxContainerCount = Runtime.getRuntime().availableProcessors();
143144
ImmutableMultimap.Builder<Capabilities, SessionFactory> factories = ImmutableMultimap.builder();
144145
kinds.forEach((name, caps) -> {
145146
Image image = docker.getImage(name);
146147
for (int i = 0; i < maxContainerCount; i++) {
147-
if (isVideoRecordingAvailable()) {
148-
factories.put(
149-
caps,
150-
new DockerSessionFactory(
151-
tracer,
152-
clientFactory,
153-
docker,
154-
getDockerUri(),
155-
image,
156-
caps,
157-
videoImage,
158-
getAssetsPath()));
159-
} else {
160-
factories.put(
148+
factories.put(
149+
caps,
150+
new DockerSessionFactory(
151+
tracer,
152+
clientFactory,
153+
docker,
154+
getDockerUri(),
155+
image,
161156
caps,
162-
new DockerSessionFactory(tracer, clientFactory, docker, getDockerUri(), image, caps));
163-
}
157+
videoImage,
158+
assetsPath));
164159
}
165160
LOG.info(String.format(
166161
"Mapping %s to docker image %s %d times",
@@ -171,27 +166,33 @@ public Map<Capabilities, Collection<SessionFactory>> getDockerSessionFactories(
171166
return factories.build().asMap();
172167
}
173168

174-
private boolean isVideoRecordingAvailable() {
175-
return config.get(DOCKER_SECTION, "video-image").isPresent()
176-
&& config.get(DOCKER_SECTION, "assets-path").isPresent();
177-
}
178-
179169
private Image getVideoImage(Docker docker) {
180-
Optional<String> videoImage = config.get(DOCKER_SECTION, "video-image");
181-
return videoImage.map(docker::getImage).orElse(null);
170+
String videoImage = config.get(DOCKER_SECTION, "video-image").orElse(DEFAULT_VIDEO_IMAGE);
171+
return docker.getImage(videoImage);
182172
}
183173

184-
private DockerSessionAssetsPath getAssetsPath() {
185-
Optional<String> hostAssetsPath = config.get(DOCKER_SECTION, "assets-path");
186-
Optional<String> containerAssetsPath = config.get(DOCKER_SECTION, "container-assets-path");
187-
if (hostAssetsPath.isPresent() && containerAssetsPath.isPresent()) {
188-
return new DockerSessionAssetsPath(hostAssetsPath.get(), containerAssetsPath.get());
189-
} else if (hostAssetsPath.isPresent()) {
190-
// If only the host assets path is present, we assume this is not running inside a container.
191-
return new DockerSessionAssetsPath(hostAssetsPath.get(), hostAssetsPath.get());
174+
private DockerAssetsPath getAssetsPath(Docker docker) {
175+
Optional<String> assetsPath = config.get(DOCKER_SECTION, "assets-path");
176+
if (assetsPath.isPresent()) {
177+
// We assume the user is not running the Selenium Server inside a Docker container
178+
// Therefore, we have access to the assets path on the host
179+
return new DockerAssetsPath(assetsPath.get(), assetsPath.get());
180+
}
181+
// Selenium Server is running inside a Docker container, we will inspect that container
182+
// to get the mounted volume and use that. If no volume was mounted, no assets will be saved.
183+
// Since Docker 1.12, the env var HOSTNAME has the container id (unless the user overwrites it)
184+
String hostname = System.getenv("HOSTNAME");
185+
ContainerInfo info = docker.inspect(new ContainerId(hostname));
186+
Optional<Map<String, Object>> mountedVolume = info.getMountedVolumes()
187+
.stream()
188+
.filter(
189+
mounted ->
190+
CONTAINER_ASSETS_PATH.equalsIgnoreCase(String.valueOf(mounted.get("Destination"))))
191+
.findFirst();
192+
if (mountedVolume.isPresent()) {
193+
String hostPath = String.valueOf(mountedVolume.get().get("Source"));
194+
return new DockerAssetsPath(hostPath, CONTAINER_ASSETS_PATH);
192195
}
193-
// We should not reach this point because the invocation to this method is
194-
// guarded by `isVideoRecordingAvailable()`
195196
return null;
196197
}
197198

‎java/server/src/org/openqa/selenium/grid/docker/DockerSessionFactory.java

+51-51
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
import java.net.URL;
5959
import java.nio.charset.Charset;
6060
import java.nio.file.Files;
61-
import java.nio.file.Path;
6261
import java.nio.file.Paths;
6362
import java.time.Duration;
6463
import java.time.Instant;
@@ -89,26 +88,8 @@ public class DockerSessionFactory implements SessionFactory {
8988
private final URI dockerUri;
9089
private final Image browserImage;
9190
private final Capabilities stereotype;
92-
private boolean isVideoRecordingAvailable;
93-
private Image videoImage;
94-
private DockerSessionAssetsPath assetsPath;
95-
96-
public DockerSessionFactory(
97-
Tracer tracer,
98-
HttpClient.Factory clientFactory,
99-
Docker docker,
100-
URI dockerUri,
101-
Image browserImage,
102-
Capabilities stereotype) {
103-
this.tracer = Require.nonNull("Tracer", tracer);
104-
this.clientFactory = Require.nonNull("HTTP client", clientFactory);
105-
this.docker = Require.nonNull("Docker command", docker);
106-
this.dockerUri = Require.nonNull("Docker URI", dockerUri);
107-
this.browserImage = Require.nonNull("Docker browser image", browserImage);
108-
this.stereotype = ImmutableCapabilities.copyOf(
109-
Require.nonNull("Stereotype", stereotype));
110-
this.isVideoRecordingAvailable = false;
111-
}
91+
private final Image videoImage;
92+
private final DockerAssetsPath assetsPath;
11293

11394
public DockerSessionFactory(
11495
Tracer tracer,
@@ -118,9 +99,14 @@ public DockerSessionFactory(
11899
Image browserImage,
119100
Capabilities stereotype,
120101
Image videoImage,
121-
DockerSessionAssetsPath assetsPath) {
122-
this(tracer, clientFactory, docker, dockerUri, browserImage, stereotype);
123-
this.isVideoRecordingAvailable = true;
102+
DockerAssetsPath assetsPath) {
103+
this.tracer = Require.nonNull("Tracer", tracer);
104+
this.clientFactory = Require.nonNull("HTTP client", clientFactory);
105+
this.docker = Require.nonNull("Docker command", docker);
106+
this.dockerUri = Require.nonNull("Docker URI", dockerUri);
107+
this.browserImage = Require.nonNull("Docker browser image", browserImage);
108+
this.stereotype = ImmutableCapabilities.copyOf(
109+
Require.nonNull("Stereotype", stereotype));
124110
this.videoImage = videoImage;
125111
this.assetsPath = assetsPath;
126112
}
@@ -145,15 +131,7 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
145131
attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(),
146132
EventAttribute.setValue(this.getClass().getName()));
147133
LOG.info("Creating container, mapping container port 4444 to " + port);
148-
Map<String, String> browserContainerEnvVars =
149-
getBrowserContainerEnvVars(sessionRequest.getCapabilities());
150-
Map<String, String> devShmMount =
151-
Collections.singletonMap("/dev/shm", "/dev/shm");
152-
Container container = docker.create(
153-
image(browserImage)
154-
.env(browserContainerEnvVars)
155-
.bind(devShmMount)
156-
.map(Port.tcp(4444), Port.tcp(port)));
134+
Container container = createBrowserContainer(port, sessionRequest.getCapabilities());
157135
container.start();
158136
ContainerInfo containerInfo = container.inspect();
159137

@@ -211,22 +189,16 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
211189

212190
SessionId id = new SessionId(response.getSessionId());
213191
Capabilities capabilities = new ImmutableCapabilities((Map<?, ?>) response.getValue());
192+
Capabilities mergedCapabilities = capabilities.merge(sessionRequest.getCapabilities());
193+
214194
Container videoContainer = null;
215-
if (isVideoRecordingAvailable) {
216-
Capabilities mergedCapabilities = capabilities.merge(sessionRequest.getCapabilities());
217-
Optional<Path> containerAssetsPath = assetsPath.createContainerSessionAssetsPath(id);
218-
containerAssetsPath.ifPresent(path -> saveSessionCapabilities(mergedCapabilities, path));
219-
if (containerAssetsPath.isPresent() && recordVideoForSession(mergedCapabilities)) {
220-
Map<String, String> envVars = getVideoContainerEnvVars(
221-
mergedCapabilities,
222-
containerInfo.getIp());
223-
String hostAssetsPath = assetsPath.getHostSessionAssetsPath(id);
224-
Map<String, String> volumeBinds =
225-
Collections.singletonMap(hostAssetsPath, "/videos");
226-
videoContainer = docker.create(image(videoImage).env(envVars).bind(volumeBinds));
227-
videoContainer.start();
228-
LOG.info(String.format("Video container started (id: %s)", videoContainer.getId()));
229-
}
195+
Optional<DockerAssetsPath> path = ofNullable(this.assetsPath);
196+
if (path.isPresent()) {
197+
// Seems we can store session assets
198+
String containerPath = path.get().getContainerPath(id);
199+
saveSessionCapabilities(mergedCapabilities, containerPath);
200+
String hostPath = path.get().getHostPath(id);
201+
videoContainer = startVideoContainer(mergedCapabilities, containerInfo.getIp(), hostPath);
230202
}
231203

232204
Dialect downstream = sessionRequest.getDownstreamDialects().contains(result.getDialect()) ?
@@ -249,13 +221,25 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
249221
id,
250222
remoteAddress,
251223
stereotype,
252-
capabilities,
224+
mergedCapabilities,
253225
downstream,
254226
result.getDialect(),
255227
Instant.now()));
256228
}
257229
}
258230

231+
private Container createBrowserContainer(int port, Capabilities sessionCapabilities) {
232+
Map<String, String> browserContainerEnvVars =
233+
getBrowserContainerEnvVars(sessionCapabilities);
234+
Map<String, String> devShmMount =
235+
Collections.singletonMap("/dev/shm", "/dev/shm");
236+
return docker.create(
237+
image(browserImage)
238+
.env(browserContainerEnvVars)
239+
.bind(devShmMount)
240+
.map(Port.tcp(4444), Port.tcp(port)));
241+
}
242+
259243
private Map<String, String> getBrowserContainerEnvVars(Capabilities sessionRequestCapabilities) {
260244
Optional<Dimension> screenResolution =
261245
ofNullable(getScreenResolution(sessionRequestCapabilities));
@@ -269,6 +253,21 @@ private Map<String, String> getBrowserContainerEnvVars(Capabilities sessionReque
269253
return envVars;
270254
}
271255

256+
private Container startVideoContainer(Capabilities sessionCapabilities,
257+
String browserContainerIp, String hostPath) {
258+
if (!recordVideoForSession(sessionCapabilities)) {
259+
return null;
260+
}
261+
Map<String, String> envVars = getVideoContainerEnvVars(
262+
sessionCapabilities,
263+
browserContainerIp);
264+
Map<String, String> volumeBinds = Collections.singletonMap(hostPath, "/videos");
265+
Container videoContainer = docker.create(image(videoImage).env(envVars).bind(volumeBinds));
266+
videoContainer.start();
267+
LOG.info(String.format("Video container started (id: %s)", videoContainer.getId()));
268+
return videoContainer;
269+
}
270+
272271
private Map<String, String> getVideoContainerEnvVars(Capabilities sessionRequestCapabilities,
273272
String containerIp) {
274273
Map<String, String> envVars = new HashMap<>();
@@ -332,11 +331,12 @@ private Object getCapability(Capabilities sessionRequestCapabilities, String cap
332331
return null;
333332
}
334333

335-
private void saveSessionCapabilities(Capabilities sessionRequestCapabilities, Path assetsPath) {
334+
private void saveSessionCapabilities(Capabilities sessionRequestCapabilities, String path) {
336335
String capsToJson = new Json().toJson(sessionRequestCapabilities);
337336
try {
337+
Files.createDirectories(Paths.get(path));
338338
Files.write(
339-
Paths.get(assetsPath.toString(), "sessionCapabilities.json"),
339+
Paths.get(path, "sessionCapabilities.json"),
340340
capsToJson.getBytes(Charset.defaultCharset()));
341341
} catch (IOException e) {
342342
LOG.log(Level.WARNING,

0 commit comments

Comments
 (0)