[
https://issues.apache.org/jira/browse/ARTEMIS-3808?focusedWorklogId=768503&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768503
]
ASF GitHub Bot logged work on ARTEMIS-3808:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 10/May/22 13:33
Start Date: 10/May/22 13:33
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4061:
URL: https://github.com/apache/activemq-artemis/pull/4061#discussion_r869187012
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java:
##########
@@ -4479,5 +4482,39 @@ public void replay(String startScan, String endScan,
String address, String targ
server.replay(startScanDate, endScanDate, address, target, filter);
}
+
+ @Override
+ public void stopEmbeddedWebServer() throws Exception {
+ for (ActiveMQComponent component : server.getExternalComponents()) {
+ if (component instanceof WebServerComponentMarker) {
+ ((ServiceComponent) component).stop(true);
+ }
+ }
+ }
+
+ @Override
+ public void startEmbeddedWebServer() throws Exception {
+ for (ActiveMQComponent component : server.getExternalComponents()) {
+ if (component instanceof WebServerComponentMarker) {
+ component.start();
+ }
+ }
+ }
+
+ @Override
+ public void restartEmbeddedWebServer() throws Exception {
+ /*
+ * This needs to be run in its own thread managed by the broker because
if it is run on a thread managed by Jetty
+ * (e.g. if it is invoked from the web console) then the thread will die
before Jetty can be restarted.
+ */
+ server.getThreadPool().execute(() -> {
+ try {
+ stopEmbeddedWebServer();
+ startEmbeddedWebServer();
Review Comment:
Its actually the bits around here calling start/stop or in this case
stop+start that I thought should be coordinated for consistency, but changing
the component itself was a good change too and takes care of some of the
problems.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4290,6 +4292,65 @@ public boolean isStarted() {
}
}
+ @Test
+ public void testManualStopStartEmbeddedWebServer() throws Exception {
+ testStartStopEmbeddedWebServer(true);
+ }
+
+ @Test
+ public void testRestartEmbeddedWebServer() throws Exception {
+ testStartStopEmbeddedWebServer(false);
+ }
+
+ private void testStartStopEmbeddedWebServer(boolean manual) throws
Exception {
+ class Fake implements ServiceComponent, WebServerComponentMarker {
+ boolean started = false;
+ long startTime = 0;
+
+ @Override
+ public void start() throws Exception {
+ started = true;
+ startTime = System.currentTimeMillis();
Review Comment:
Should probably gate this update on it not already being started, for a
better validation it was first stopped.
##########
artemis-web/src/test/java/org/apache/activemq/cli/test/WebServerComponentTest.java:
##########
@@ -175,31 +166,31 @@ public void testComponentStopBehavior() throws Exception {
Assert.assertFalse(webServerComponent.isStarted());
webServerComponent.configure(webServerDTO, "./src/test/resources/",
"./src/test/resources/");
webServerComponent.start();
- final int port = webServerComponent.getPort();
// Make the connection attempt.
- CountDownLatch latch = new CountDownLatch(1);
- final ClientHandler clientHandler = new ClientHandler(latch);
- bootstrap.group(group).channel(NioSocketChannel.class).handler(new
ChannelInitializer() {
- @Override
- protected void initChannel(Channel ch) throws Exception {
- ch.pipeline().addLast(new HttpClientCodec());
- ch.pipeline().addLast(clientHandler);
- }
- });
- Channel ch = bootstrap.connect("localhost", port).sync().channel();
+ verifyConnection(webServerComponent.getPort());
+ Assert.assertTrue(webServerComponent.isStarted());
- URI uri = new URI(URL);
- // Prepare the HTTP request.
- HttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
HttpMethod.GET, uri.getRawPath());
- request.headers().set(HttpHeaderNames.HOST, "localhost");
+ //usual stop won't actually stop it
+ webServerComponent.stop();
+ assertTrue(webServerComponent.isStarted());
- // Send the HTTP request.
- ch.writeAndFlush(request);
- assertTrue(latch.await(5, TimeUnit.SECONDS));
- assertEquals("12345", clientHandler.body.toString());
- // Wait for the server to close the connection.
- ch.close();
- ch.eventLoop().shutdownNow();
+ webServerComponent.stop(true);
+ Assert.assertFalse(webServerComponent.isStarted());
+ }
+
+ @Test
+ public void testComponentStopStartBehavior() throws Exception {
Review Comment:
Thats partly my point. None of that stuff or it related docs seems to be
changing here, i.e everythings just now magically 'covering this' powerful new
restart-the-management stuff. Anyone not seeing the JIRA go past would be
likely to not even know it [now] exists, which seems off. I'd guess some folks
will want to restrict the stop/start/restart ability differently than some
other operations if they knew it exists. Seems reasonable to test and
especially document.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4290,6 +4292,65 @@ public boolean isStarted() {
}
}
+ @Test
+ public void testManualStopStartEmbeddedWebServer() throws Exception {
+ testStartStopEmbeddedWebServer(true);
+ }
+
+ @Test
+ public void testRestartEmbeddedWebServer() throws Exception {
+ testStartStopEmbeddedWebServer(false);
+ }
+
+ private void testStartStopEmbeddedWebServer(boolean manual) throws
Exception {
+ class Fake implements ServiceComponent, WebServerComponentMarker {
+ boolean started = false;
+ long startTime = 0;
+
+ @Override
+ public void start() throws Exception {
+ started = true;
+ startTime = System.currentTimeMillis();
+ }
+
+ @Override
+ public void stop() throws Exception {
+ }
+
+ @Override
+ public void stop(boolean shutdown) throws Exception {
+ started = false;
+ }
Review Comment:
Since its value is criticial to what happens in the real case, this Fake
component should also ensure the boolean arg was as expected (i.e true) rather
than assuming.
(Using e.g Mockito would probably have been smaller and allowed more
granular verification of the calls made).
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4290,6 +4292,65 @@ public boolean isStarted() {
}
}
+ @Test
+ public void testManualStopStartEmbeddedWebServer() throws Exception {
+ testStartStopEmbeddedWebServer(true);
+ }
+
+ @Test
+ public void testRestartEmbeddedWebServer() throws Exception {
+ testStartStopEmbeddedWebServer(false);
+ }
+
+ private void testStartStopEmbeddedWebServer(boolean manual) throws
Exception {
+ class Fake implements ServiceComponent, WebServerComponentMarker {
+ boolean started = false;
+ long startTime = 0;
+
+ @Override
+ public void start() throws Exception {
+ started = true;
+ startTime = System.currentTimeMillis();
+ }
+
+ @Override
+ public void stop() throws Exception {
+ }
+
+ @Override
+ public void stop(boolean shutdown) throws Exception {
+ started = false;
+ }
+
+ @Override
+ public boolean isStarted() {
+ return started;
+ }
+
+ public long getStartTime() {
+ return startTime;
+ }
+ }
+ Fake fake = new Fake();
+ server.addExternalComponent(fake, true);
+ Assert.assertTrue(fake.isStarted());
+
+ ActiveMQServerControl serverControl = createManagementControl();
+ if (manual) {
+ serverControl.stopEmbeddedWebServer();
+ Assert.assertFalse(fake.isStarted());
+ serverControl.startEmbeddedWebServer();
+ Assert.assertTrue(fake.isStarted());
+ } else {
+ Assert.assertTrue(System.currentTimeMillis() > fake.getStartTime());
+ long time = System.currentTimeMillis();
+ Thread.sleep(1);
+ serverControl.restartEmbeddedWebServer();
+ Assert.assertTrue(time < fake.getStartTime());
Review Comment:
Isnt this racey if restart uses the executor, perhaps need a short Wait?
Issue Time Tracking
-------------------
Worklog Id: (was: 768503)
Time Spent: 2h 40m (was: 2.5h)
> Support starting/stopping the embedded web server via mangement
> ---------------------------------------------------------------
>
> Key: ARTEMIS-3808
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3808
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 2h 40m
> Remaining Estimate: 0h
>
> It would be useful to be able to cycle the embedded web server if, for
> example, one needed to renew the SSL certificates.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
