Skip to content

Commit 07af603

Browse files
shs96cshs96c
committed
[grid] Reimplement readiness checks
Make the `/readyz` endpoint return a value based on the dependencies of each component. That is, the Distributor will only claim to be ready if the EventBus and the SessionMap are both also ready.
1 parent 769b24f commit 07af603

28 files changed

Lines changed: 190 additions & 24 deletions

‎java/server/src/org/openqa/selenium/events/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ java_library(
99
deps = [
1010
"//java/client/src/org/openqa/selenium:core",
1111
"//java/client/src/org/openqa/selenium/json",
12+
"//java/server/src/org/openqa/selenium/status",
1213
],
1314
)

‎java/server/src/org/openqa/selenium/events/EventBus.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
package org.openqa.selenium.events;
1919

20+
import org.openqa.selenium.status.HasReadyState;
21+
2022
import java.io.Closeable;
2123
import java.util.function.Consumer;
2224

23-
public interface EventBus extends Closeable {
25+
public interface EventBus extends Closeable, HasReadyState {
2426

2527
void addListener(Type type, Consumer<Event> onType);
2628

‎java/server/src/org/openqa/selenium/events/local/GuavaEventBus.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public GuavaEventBus() {
3838
guavaBus = new EventBus();
3939
}
4040

41+
@Override
42+
public boolean isReady() {
43+
return true;
44+
}
45+
4146
@Override
4247
public void addListener(Type type, Consumer<Event> onType) {
4348
Listener listener = new Listener(type, onType);

‎java/server/src/org/openqa/selenium/events/zeromq/BoundZmqEventBus.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ class BoundZmqEventBus implements EventBus {
7171
LOG.info("Event bus ready");
7272
}
7373

74-
74+
@Override
75+
public boolean isReady() {
76+
return !executor.isShutdown();
77+
}
7578

7679
@Override
7780
public void addListener(Type type, Consumer<Event> onType) {

‎java/server/src/org/openqa/selenium/events/zeromq/UnboundZmqEventBus.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717

1818
package org.openqa.selenium.events.zeromq;
1919

20-
import static java.nio.charset.StandardCharsets.UTF_8;
21-
2220
import com.google.common.collect.EvictingQueue;
23-
2421
import org.openqa.selenium.events.Event;
2522
import org.openqa.selenium.events.EventBus;
2623
import org.openqa.selenium.events.Type;
@@ -48,6 +45,8 @@
4845
import java.util.logging.Level;
4946
import java.util.logging.Logger;
5047

48+
import static java.nio.charset.StandardCharsets.UTF_8;
49+
5150
class UnboundZmqEventBus implements EventBus {
5251

5352
private static final Logger LOG = Logger.getLogger(EventBus.class.getName());
@@ -137,6 +136,11 @@ class UnboundZmqEventBus implements EventBus {
137136
}
138137
}
139138

139+
@Override
140+
public boolean isReady() {
141+
return !executor.isShutdown();
142+
}
143+
140144
private boolean isSubAddressIPv6(String connection) {
141145
try {
142146
return InetAddress.getByName(new URI(connection).getHost()) instanceof Inet6Address;

‎java/server/src/org/openqa/selenium/grid/commands/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ java_library(
3434
"//java/server/src/org/openqa/selenium/grid/sessionmap/local",
3535
"//java/server/src/org/openqa/selenium/grid/web",
3636
"//java/server/src/org/openqa/selenium/netty/server",
37+
"//java/server/src/org/openqa/selenium/status",
3738
artifact("com.beust:jcommander"),
3839
artifact("com.google.guava:guava"),
3940
],

‎java/server/src/org/openqa/selenium/grid/commands/Hub.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,11 @@
3939
import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
4040
import org.openqa.selenium.grid.web.CombinedHandler;
4141
import org.openqa.selenium.grid.web.RoutableHttpClientFactory;
42-
import org.openqa.selenium.grid.web.StatusBasedReadinessCheck;
4342
import org.openqa.selenium.netty.server.NettyServer;
43+
import org.openqa.selenium.remote.http.Contents;
4444
import org.openqa.selenium.remote.http.HttpClient;
4545
import org.openqa.selenium.remote.http.HttpHandler;
46+
import org.openqa.selenium.remote.http.HttpResponse;
4647
import org.openqa.selenium.remote.http.Route;
4748
import org.openqa.selenium.remote.tracing.Tracer;
4849

@@ -52,9 +53,10 @@
5253
import java.util.Set;
5354
import java.util.logging.Logger;
5455

56+
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
57+
import static java.net.HttpURLConnection.HTTP_OK;
5558
import static org.openqa.selenium.grid.config.StandardGridRoles.EVENT_BUS_ROLE;
5659
import static org.openqa.selenium.grid.config.StandardGridRoles.HTTPD_ROLE;
57-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
5860
import static org.openqa.selenium.remote.http.Route.combine;
5961

6062
@AutoService(CliCommand.class)
@@ -130,7 +132,12 @@ protected void execute(Config config) {
130132

131133
Router router = new Router(tracer, clientFactory, sessions, distributor);
132134
GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri().toString());
133-
StatusBasedReadinessCheck readinessCheck = new StatusBasedReadinessCheck(router, GET, "/status");
135+
HttpHandler readinessCheck = req -> {
136+
boolean ready = router.isReady() && bus.isReady();
137+
return new HttpResponse()
138+
.setStatus(ready ? HTTP_OK : HTTP_INTERNAL_ERROR)
139+
.setContent(Contents.utf8String("Router is " + ready));
140+
};
134141

135142
HttpHandler httpHandler = combine(
136143
router,

‎java/server/src/org/openqa/selenium/grid/commands/Standalone.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,12 @@
4242
import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
4343
import org.openqa.selenium.grid.web.CombinedHandler;
4444
import org.openqa.selenium.grid.web.RoutableHttpClientFactory;
45-
import org.openqa.selenium.grid.web.StatusBasedReadinessCheck;
4645
import org.openqa.selenium.net.NetworkUtils;
4746
import org.openqa.selenium.netty.server.NettyServer;
47+
import org.openqa.selenium.remote.http.Contents;
4848
import org.openqa.selenium.remote.http.HttpClient;
4949
import org.openqa.selenium.remote.http.HttpHandler;
50+
import org.openqa.selenium.remote.http.HttpResponse;
5051
import org.openqa.selenium.remote.http.Route;
5152
import org.openqa.selenium.remote.tracing.Tracer;
5253

@@ -58,9 +59,10 @@
5859
import java.util.Set;
5960
import java.util.logging.Logger;
6061

62+
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
63+
import static java.net.HttpURLConnection.HTTP_OK;
6164
import static org.openqa.selenium.grid.config.StandardGridRoles.HTTPD_ROLE;
6265
import static org.openqa.selenium.grid.config.StandardGridRoles.NODE_ROLE;
63-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
6466
import static org.openqa.selenium.remote.http.Route.combine;
6567

6668
@AutoService(CliCommand.class)
@@ -136,7 +138,12 @@ protected void execute(Config config) {
136138
Distributor distributor = new LocalDistributor(tracer, bus, clientFactory, sessions, null);
137139
combinedHandler.addHandler(distributor);
138140
Router router = new Router(tracer, clientFactory, sessions, distributor);
139-
StatusBasedReadinessCheck readinessCheck = new StatusBasedReadinessCheck(router, GET, "/status");
141+
HttpHandler readinessCheck = req -> {
142+
boolean ready = sessions.isReady() && distributor.isReady() && bus.isReady();
143+
return new HttpResponse()
144+
.setStatus(ready ? HTTP_OK : HTTP_INTERNAL_ERROR)
145+
.setContent(Contents.utf8String("Standalone is " + ready));
146+
};
140147

141148
BaseServerOptions serverOptions = new BaseServerOptions(config);
142149
GraphqlHandler graphqlHandler = new GraphqlHandler(distributor, serverOptions.getExternalUri().toString());

‎java/server/src/org/openqa/selenium/grid/distributor/BUILD.bazel‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ java_library(
77
"//java/server/src/org/openqa/selenium/grid:__subpackages__",
88
"//java/server/test/org/openqa/selenium/grid:__subpackages__",
99
],
10+
exports = [
11+
"//java/server/src/org/openqa/selenium/status",
12+
],
1013
deps = [
1114
"//java/client/src/org/openqa/selenium:core",
1215
"//java/client/src/org/openqa/selenium/json",
@@ -16,6 +19,7 @@ java_library(
1619
"//java/server/src/org/openqa/selenium/grid/node/remote",
1720
"//java/server/src/org/openqa/selenium/grid/sessionmap/remote",
1821
"//java/server/src/org/openqa/selenium/grid/web",
22+
"//java/server/src/org/openqa/selenium/status",
1923
artifact("com.google.guava:guava"),
2024
],
2125
)

‎java/server/src/org/openqa/selenium/grid/distributor/Distributor.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.openqa.selenium.remote.http.Route;
3333
import org.openqa.selenium.remote.tracing.SpanDecorator;
3434
import org.openqa.selenium.remote.tracing.Tracer;
35+
import org.openqa.selenium.status.HasReadyState;
3536

3637
import java.io.UncheckedIOException;
3738
import java.util.UUID;
@@ -73,7 +74,7 @@
7374
* </tr>
7475
* </table>
7576
*/
76-
public abstract class Distributor implements Predicate<HttpRequest>, Routable, HttpHandler {
77+
public abstract class Distributor implements HasReadyState, Predicate<HttpRequest>, Routable {
7778

7879
private final Route routes;
7980
protected final Tracer tracer;

0 commit comments

Comments
 (0)