From 72562d8d88e9f2e2492c461910e6dd6124e0e954 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 6 May 2024 12:39:29 +0000 Subject: [PATCH] [grid][java]: session-timeout set connection timeout in RemoteNode (#13854) * [grid][java]: add session-timeout to all kind of Nodes Signed-off-by: Viet Nguyen Duc * [grid][java]: fix code as suggestions Signed-off-by: Viet Nguyen Duc * [java][grid]: update test CustomNode can get/set sessionTimeout Signed-off-by: Viet Nguyen Duc --------- Signed-off-by: Viet Nguyen Duc --- .../openqa/selenium/grid/data/NodeStatus.java | 23 ++++++++++++++++++- .../selenium/grid/distributor/AddNode.java | 1 + .../selenium/grid/distributor/GridModel.java | 2 ++ .../distributor/local/LocalDistributor.java | 1 + .../org/openqa/selenium/grid/node/Node.java | 10 +++++++- .../selenium/grid/node/k8s/OneShotNode.java | 5 +++- .../selenium/grid/node/local/LocalNode.java | 8 ++++++- .../selenium/grid/node/remote/RemoteNode.java | 11 ++++++--- .../selenium/redis/GridRedisClient.java | 1 + .../selenium/grid/data/NodeStatusTest.java | 1 + .../grid/distributor/AddingNodesTest.java | 12 ++++++++-- .../selector/DefaultSlotSelectorTest.java | 1 + .../openqa/selenium/grid/node/NodeTest.java | 4 ++++ 13 files changed, 71 insertions(+), 9 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/data/NodeStatus.java b/java/src/org/openqa/selenium/grid/data/NodeStatus.java index d2f31d6a858ff..47753585ec989 100644 --- a/java/src/org/openqa/selenium/grid/data/NodeStatus.java +++ b/java/src/org/openqa/selenium/grid/data/NodeStatus.java @@ -42,6 +42,7 @@ public class NodeStatus { private final Set slots; private final Availability availability; private final Duration heartbeatPeriod; + private final Duration sessionTimeout; private final String version; private final Map osInfo; @@ -52,6 +53,7 @@ public NodeStatus( Set slots, Availability availability, Duration heartbeatPeriod, + Duration sessionTimeout, String version, Map osInfo) { this.nodeId = Require.nonNull("Node id", nodeId); @@ -62,6 +64,7 @@ public NodeStatus( this.slots = unmodifiableSet(new HashSet<>(Require.nonNull("Slots", slots))); this.availability = Require.nonNull("Availability", availability); this.heartbeatPeriod = heartbeatPeriod; + this.sessionTimeout = sessionTimeout; this.version = Require.nonNull("Grid Node version", version); this.osInfo = Require.nonNull("Node host OS info", osInfo); } @@ -73,6 +76,7 @@ public static NodeStatus fromJson(JsonInput input) { Set slots = null; Availability availability = null; Duration heartbeatPeriod = null; + Duration sessionTimeout = null; String version = null; Map osInfo = null; @@ -87,6 +91,10 @@ public static NodeStatus fromJson(JsonInput input) { heartbeatPeriod = Duration.ofMillis(input.read(Long.class)); break; + case "sessionTimeout": + sessionTimeout = Duration.ofMillis(input.read(Long.class)); + break; + case "nodeId": nodeId = input.read(NodeId.class); break; @@ -119,7 +127,15 @@ public static NodeStatus fromJson(JsonInput input) { input.endObject(); return new NodeStatus( - nodeId, externalUri, maxSessions, slots, availability, heartbeatPeriod, version, osInfo); + nodeId, + externalUri, + maxSessions, + slots, + availability, + heartbeatPeriod, + sessionTimeout, + version, + osInfo); } public boolean hasCapability(Capabilities caps, SlotMatcher slotMatcher) { @@ -162,6 +178,10 @@ public Duration getHeartbeatPeriod() { return heartbeatPeriod; } + public Duration getSessionTimeout() { + return sessionTimeout; + } + public String getVersion() { return version; } @@ -212,6 +232,7 @@ private Map toJson() { toReturn.put("slots", slots); toReturn.put("availability", availability); toReturn.put("heartbeatPeriod", heartbeatPeriod.toMillis()); + toReturn.put("sessionTimeout", sessionTimeout.toMillis()); toReturn.put("version", version); toReturn.put("osInfo", osInfo); diff --git a/java/src/org/openqa/selenium/grid/distributor/AddNode.java b/java/src/org/openqa/selenium/grid/distributor/AddNode.java index 95e8a33c304ee..904faaab6f1ec 100644 --- a/java/src/org/openqa/selenium/grid/distributor/AddNode.java +++ b/java/src/org/openqa/selenium/grid/distributor/AddNode.java @@ -65,6 +65,7 @@ public HttpResponse execute(HttpRequest req) { status.getNodeId(), status.getExternalUri(), registrationSecret, + status.getSessionTimeout(), status.getSlots().stream().map(Slot::getStereotype).collect(Collectors.toSet())); distributor.add(node); diff --git a/java/src/org/openqa/selenium/grid/distributor/GridModel.java b/java/src/org/openqa/selenium/grid/distributor/GridModel.java index c2b3d05898702..3aa27085e1f81 100644 --- a/java/src/org/openqa/selenium/grid/distributor/GridModel.java +++ b/java/src/org/openqa/selenium/grid/distributor/GridModel.java @@ -367,6 +367,7 @@ private NodeStatus rewrite(NodeStatus status, Availability availability) { status.getSlots(), availability, status.getHeartbeatPeriod(), + status.getSessionTimeout(), status.getVersion(), status.getOsInfo()); } @@ -508,6 +509,7 @@ private void amend(Availability availability, NodeStatus status, Slot slot) { newSlots, availability, status.getHeartbeatPeriod(), + status.getSessionTimeout(), status.getVersion(), status.getOsInfo())); } finally { diff --git a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java index 332ea7ef6a695..f9581b1fff016 100644 --- a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java +++ b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java @@ -314,6 +314,7 @@ private void register(NodeStatus status) { status.getNodeId(), status.getExternalUri(), registrationSecret, + status.getSessionTimeout(), capabilities); add(remoteNode); diff --git a/java/src/org/openqa/selenium/grid/node/Node.java b/java/src/org/openqa/selenium/grid/node/Node.java index 5fb0877d8fcc7..5d54bb3e2981d 100644 --- a/java/src/org/openqa/selenium/grid/node/Node.java +++ b/java/src/org/openqa/selenium/grid/node/Node.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableMap; import java.io.IOException; import java.net.URI; +import java.time.Duration; import java.util.Map; import java.util.ServiceLoader; import java.util.Set; @@ -116,13 +117,16 @@ public abstract class Node implements HasReadyState, Routable { protected final Tracer tracer; private final NodeId id; private final URI uri; + private final Duration sessionTimeout; private final Route routes; protected boolean draining; - protected Node(Tracer tracer, NodeId id, URI uri, Secret registrationSecret) { + protected Node( + Tracer tracer, NodeId id, URI uri, Secret registrationSecret, Duration sessionTimeout) { this.tracer = Require.nonNull("Tracer", tracer); this.id = Require.nonNull("Node id", id); this.uri = Require.nonNull("URI", uri); + this.sessionTimeout = Require.positive("Session timeout", sessionTimeout); Require.nonNull("Registration secret", registrationSecret); RequiresSecretFilter requiresSecret = new RequiresSecretFilter(registrationSecret); @@ -246,6 +250,10 @@ public TemporaryFilesystem getDownloadsFilesystem(UUID uuid) throws IOException public abstract HealthCheck getHealthCheck(); + public Duration getSessionTimeout() { + return sessionTimeout; + } + public boolean isDraining() { return draining; } diff --git a/java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java b/java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java index 22b79969eefd1..d293d1c6ba78c 100644 --- a/java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java +++ b/java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java @@ -109,12 +109,13 @@ private OneShotNode( EventBus events, Secret registrationSecret, Duration heartbeatPeriod, + Duration sessionTimeout, NodeId id, URI uri, URI gridUri, Capabilities stereotype, WebDriverInfo driverInfo) { - super(tracer, id, uri, registrationSecret); + super(tracer, id, uri, registrationSecret, Require.positive(sessionTimeout)); this.heartbeatPeriod = heartbeatPeriod; this.events = Require.nonNull("Event bus", events); @@ -169,6 +170,7 @@ public static Node create(Config config) { eventOptions.getEventBus(), secretOptions.getRegistrationSecret(), nodeOptions.getHeartbeatPeriod(), + nodeOptions.getSessionTimeout(), new NodeId(UUID.randomUUID()), serverOptions.getExternalUri(), nodeOptions @@ -376,6 +378,7 @@ public NodeStatus getStatus() { : new Session(sessionId, getUri(), stereotype, capabilities, Instant.now()))), isDraining() ? DRAINING : UP, heartbeatPeriod, + getSessionTimeout(), getNodeVersion(), getOsInfo()); } diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java index 79f687448cf91..2283e8573042d 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -154,7 +154,12 @@ protected LocalNode( List factories, Secret registrationSecret, boolean managedDownloadsEnabled) { - super(tracer, new NodeId(UUID.randomUUID()), uri, registrationSecret); + super( + tracer, + new NodeId(UUID.randomUUID()), + uri, + registrationSecret, + Require.positive(sessionTimeout)); this.bus = Require.nonNull("Event bus", bus); @@ -906,6 +911,7 @@ public NodeStatus getStatus() { slots, availability, heartbeatPeriod, + getSessionTimeout(), getNodeVersion(), getOsInfo()); } diff --git a/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java b/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java index 913909b9a93ff..ae7cc8e1af9fb 100644 --- a/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java +++ b/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java @@ -22,6 +22,7 @@ import static org.openqa.selenium.grid.data.Availability.DRAINING; import static org.openqa.selenium.grid.data.Availability.UP; import static org.openqa.selenium.net.Urls.fromUri; +import static org.openqa.selenium.remote.http.ClientConfig.defaultConfig; import static org.openqa.selenium.remote.http.Contents.asJson; import static org.openqa.selenium.remote.http.Contents.reader; import static org.openqa.selenium.remote.http.HttpMethod.DELETE; @@ -35,6 +36,7 @@ import java.io.Reader; import java.io.UncheckedIOException; import java.net.URI; +import java.time.Duration; import java.util.Collection; import java.util.Map; import java.util.Objects; @@ -60,6 +62,7 @@ import org.openqa.selenium.json.Json; import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.remote.SessionId; +import org.openqa.selenium.remote.http.ClientConfig; import org.openqa.selenium.remote.http.Filter; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.HttpHandler; @@ -83,13 +86,15 @@ public RemoteNode( NodeId id, URI externalUri, Secret registrationSecret, + Duration sessionTimeout, Collection capabilities) { - super(tracer, id, externalUri, registrationSecret); + super(tracer, id, externalUri, registrationSecret, sessionTimeout); this.externalUri = Require.nonNull("External URI", externalUri); this.capabilities = ImmutableSet.copyOf(capabilities); - this.client = - Require.nonNull("HTTP client factory", clientFactory).createClient(fromUri(externalUri)); + ClientConfig clientConfig = + defaultConfig().readTimeout(this.getSessionTimeout()).baseUrl(fromUri(externalUri)); + this.client = Require.nonNull("HTTP client factory", clientFactory).createClient(clientConfig); this.healthCheck = new RemoteCheck(); diff --git a/java/src/org/openqa/selenium/redis/GridRedisClient.java b/java/src/org/openqa/selenium/redis/GridRedisClient.java index 3a8011d7d5a94..045d25e0e4a23 100644 --- a/java/src/org/openqa/selenium/redis/GridRedisClient.java +++ b/java/src/org/openqa/selenium/redis/GridRedisClient.java @@ -144,6 +144,7 @@ public Optional getNode(NodeId id) { node.getSlots(), node.getAvailability(), node.getHeartbeatPeriod(), + node.getSessionTimeout(), node.getVersion(), node.getOsInfo()); return Optional.of(resultNode); diff --git a/java/test/org/openqa/selenium/grid/data/NodeStatusTest.java b/java/test/org/openqa/selenium/grid/data/NodeStatusTest.java index f5ed2ca76e2fa..4926f08e02c9e 100644 --- a/java/test/org/openqa/selenium/grid/data/NodeStatusTest.java +++ b/java/test/org/openqa/selenium/grid/data/NodeStatusTest.java @@ -56,6 +56,7 @@ void ensureRoundTripWorks() throws URISyntaxException { Instant.now()))), UP, Duration.ofSeconds(10), + Duration.ofSeconds(300), "4.0.0", ImmutableMap.of( "name", "Max OS X", diff --git a/java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java b/java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java index 482f577d05d22..0649e1bbee235 100644 --- a/java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java +++ b/java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java @@ -164,6 +164,7 @@ void shouldBeAbleToRegisterACustomNode() throws URISyntaxException { bus, new NodeId(UUID.randomUUID()), externalUrl.toURI(), + Duration.ofSeconds(300), c -> new Session( new SessionId(UUID.randomUUID()), sessionUri, stereotype, c, Instant.now())); @@ -193,6 +194,7 @@ void shouldBeAbleToRegisterACustomNode() throws URISyntaxException { NodeStatus status = getOnlyElement(distributor.getStatus().getNodes()); assertEquals(1, getStereotypes(status).get(CAPS)); + assertEquals(Duration.ofSeconds(300), status.getSessionTimeout()); } } @@ -345,6 +347,7 @@ void distributorShouldUpdateStateOfExistingNodeWhenNodePublishesStateChange() Instant.now()))), UP, Duration.ofSeconds(10), + status.getSessionTimeout(), status.getVersion(), status.getOsInfo()); @@ -374,8 +377,12 @@ static class CustomNode extends Node { private Session running; protected CustomNode( - EventBus bus, NodeId nodeId, URI uri, Function factory) { - super(DefaultTestTracer.createTracer(), nodeId, uri, registrationSecret); + EventBus bus, + NodeId nodeId, + URI uri, + Duration sessionTimeout, + Function factory) { + super(DefaultTestTracer.createTracer(), nodeId, uri, registrationSecret, sessionTimeout); this.bus = bus; this.factory = Objects.requireNonNull(factory); @@ -468,6 +475,7 @@ public NodeStatus getStatus() { new Slot(new SlotId(getId(), UUID.randomUUID()), CAPS, Instant.now(), sess)), UP, Duration.ofSeconds(10), + getSessionTimeout(), getNodeVersion(), getOsInfo()); } diff --git a/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java b/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java index 9cec975cb7361..d05e9d1c5f81b 100644 --- a/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java +++ b/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java @@ -229,6 +229,7 @@ private NodeStatus createNode(List stereotypes, int count, int cur ImmutableSet.copyOf(slots), UP, Duration.ofSeconds(10), + Duration.ofSeconds(300), "4.0.0", ImmutableMap.of( "name", "Max OS X", diff --git a/java/test/org/openqa/selenium/grid/node/NodeTest.java b/java/test/org/openqa/selenium/grid/node/NodeTest.java index 9f6d4c50b404a..b1c50ed0dc65a 100644 --- a/java/test/org/openqa/selenium/grid/node/NodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/NodeTest.java @@ -158,6 +158,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { new NodeId(UUID.randomUUID()), uri, registrationSecret, + local.getSessionTimeout(), ImmutableSet.of(caps)); } @@ -172,6 +173,7 @@ void shouldRefuseToCreateASessionIfNoFactoriesAttached() { new NodeId(UUID.randomUUID()), uri, registrationSecret, + local.getSessionTimeout(), ImmutableSet.of()); Either response = @@ -223,6 +225,7 @@ public boolean test(Capabilities capabilities) { new NodeId(UUID.randomUUID()), uri, registrationSecret, + local.getSessionTimeout(), ImmutableSet.of(caps)); ImmutableCapabilities wrongCaps = new ImmutableCapabilities("browserName", "burger"); @@ -346,6 +349,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { new NodeId(UUID.randomUUID()), uri, registrationSecret, + local.getSessionTimeout(), ImmutableSet.of(caps)); Either response =