Skip to content

Commit 9eeb75c

Browse files
author
Mario Salazar de Torres
committed
GEODE-9478: Fix status --dir to use file controller
 - Previously when you only specified --dir option the PID was read from the member workDir and the status request was attempted to solved by using the attachment API, and after that JMX interface. But given only --dir was specified the controller resolving the request should be FileProcessController instead. - Logic has been changed for both servers and locators to always use FileProcessConroller whenever only --dir flag is specified. - Added an UT to verify new code. - Modified several ITs to verify the new behaviour. - Deleted the following ITs which no longer apply with the new logic: * statusWithEmptyPidFileThrowsIllegalArgumentException * statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails * statusWithStalePidFileReturnsNotResponding
1 parent f7cc081 commit 9eeb75c

File tree

10 files changed

+107
-257
lines changed

10 files changed

+107
-257
lines changed

‎geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java‎

Lines changed: 8 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
*/
1515
package org.apache.geode.distributed;
1616

17-
import static java.lang.management.ManagementFactory.getRuntimeMXBean;
18-
import static org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
1917
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
2018
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
2119
import static org.apache.geode.distributed.ConfigurationProperties.DISABLE_AUTO_RECONNECT;
@@ -195,6 +193,13 @@ public void statusWithPidReturnsOnlineWithDetails() throws Exception {
195193
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws Exception {
196194
givenRunningLocator();
197195

196+
// Keep it as we are going to overwrite it
197+
int pid = readPidFile();
198+
199+
// Sets a fake PID to ensure that the request is going through FileProcessController,
200+
// because if not, the request would fail due to the PID not existing
201+
givenPidFile(fakePid);
202+
198203
LocatorState locatorState = new Builder()
199204
.setWorkingDirectory(getWorkingDirectoryPath())
200205
.build()
@@ -208,62 +213,11 @@ public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws Exceptio
208213
assertThat(locatorState.getJvmArguments()).isEqualTo(getJvmArguments());
209214
assertThat(locatorState.getLogFile()).isEqualTo(getLogFilePath());
210215
assertThat(locatorState.getMemberName()).isEqualTo(getUniqueName());
211-
assertThat(locatorState.getPid().intValue()).isEqualTo(readPidFile());
216+
assertThat(locatorState.getPid().intValue()).isEqualTo(pid);
212217
assertThat(locatorState.getUptime()).isGreaterThan(0);
213218
assertThat(locatorState.getWorkingDirectory()).isEqualTo(new File(".").getCanonicalPath());
214219
}
215220

216-
@Test
217-
public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
218-
givenEmptyPidFile();
219-
LocatorLauncher launcher = new Builder()
220-
.setWorkingDirectory(getWorkingDirectoryPath())
221-
.build();
222-
223-
Throwable thrown = catchThrowable(() -> launcher.status());
224-
225-
assertThat(thrown)
226-
.isInstanceOf(IllegalArgumentException.class)
227-
.hasMessageContaining("Invalid pid 'null' found in");
228-
}
229-
230-
@Test
231-
public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails() throws Exception {
232-
givenEmptyWorkingDirectory();
233-
234-
LocatorState locatorState = new Builder()
235-
.setWorkingDirectory(getWorkingDirectoryPath())
236-
.build()
237-
.status();
238-
239-
assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
240-
assertThat(locatorState.getClasspath()).isNull();
241-
assertThat(locatorState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
242-
assertThat(locatorState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
243-
assertThat(locatorState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
244-
assertThat(locatorState.getJvmArguments()).isEqualTo(getRuntimeMXBean().getInputArguments());
245-
assertThat(locatorState.getLogFile()).isNull();
246-
assertThat(locatorState.getMemberName()).isNull();
247-
assertThat(locatorState.getPid()).isNull();
248-
assertThat(locatorState.getUptime().intValue()).isEqualTo(0);
249-
assertThat(locatorState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
250-
}
251-
252-
/**
253-
* This test takes > 1 minute to run in {@link LocatorLauncherLocalFileIntegrationTest}.
254-
*/
255-
@Test
256-
public void statusWithStalePidFileReturnsNotResponding() {
257-
givenPidFile(fakePid);
258-
259-
LocatorState locatorState = new Builder()
260-
.setWorkingDirectory(getWorkingDirectoryPath())
261-
.build()
262-
.status();
263-
264-
assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
265-
}
266-
267221
@Test
268222
public void stopWithPidReturnsStopped() {
269223
givenRunningLocator();

‎geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java‎

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,10 @@
1414
*/
1515
package org.apache.geode.distributed;
1616

17-
import static org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
1817
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
1918
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
2019
import static org.apache.geode.internal.inet.LocalHostUtil.getLocalHost;
2120
import static org.assertj.core.api.Assertions.assertThat;
22-
import static org.assertj.core.api.Assertions.catchThrowable;
2321

2422
import java.io.File;
2523
import java.net.BindException;
@@ -149,6 +147,13 @@ public void statusWithPidReturnsOnlineWithDetails() throws Exception {
149147
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws Exception {
150148
givenRunningLocator(withPort(0));
151149

150+
// Keep it as we are going to overwrite it
151+
int pid = readPidFile();
152+
153+
// Sets a fake PID to ensure that the request is going through FileProcessController,
154+
// because if not, the request would fail due to the PID not existing
155+
givenPidFile(fakePid);
156+
152157
LocatorState locatorState = new Builder()
153158
.setWorkingDirectory(getWorkingDirectoryPath())
154159
.build()
@@ -161,62 +166,12 @@ public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws Exceptio
161166
assertThat(locatorState.getJvmArguments()).isEqualTo(getJvmArguments());
162167
assertThat(locatorState.getLogFile()).isEqualTo(getLogFile().getCanonicalPath());
163168
assertThat(locatorState.getMemberName()).isEqualTo(getUniqueName());
164-
assertThat(locatorState.getPid().intValue()).isEqualTo(readPidFile());
169+
assertThat(locatorState.getPid().intValue()).isEqualTo(pid);
165170
assertThat(locatorState.getStatus()).isEqualTo(ONLINE);
166171
assertThat(locatorState.getUptime()).isGreaterThan(0);
167172
assertThat(locatorState.getWorkingDirectory()).isEqualToIgnoringCase(getWorkingDirectoryPath());
168173
}
169174

170-
@Test
171-
public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
172-
givenEmptyPidFile();
173-
LocatorLauncher launcher = new Builder()
174-
.setWorkingDirectory(getWorkingDirectoryPath())
175-
.build();
176-
177-
Throwable thrown = catchThrowable(() -> launcher.status());
178-
179-
assertThat(thrown)
180-
.isInstanceOf(IllegalArgumentException.class)
181-
.hasMessageContaining("Invalid pid 'null' found in");
182-
}
183-
184-
@Test
185-
public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails() throws Exception {
186-
givenEmptyWorkingDirectory();
187-
188-
LocatorState locatorState = new Builder()
189-
.setWorkingDirectory(getWorkingDirectoryPath())
190-
.build()
191-
.status();
192-
193-
assertThat(locatorState.getClasspath()).isNull();
194-
assertThat(locatorState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
195-
assertThat(locatorState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
196-
assertThat(locatorState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
197-
assertThat(locatorState.getLogFile()).isNull();
198-
assertThat(locatorState.getMemberName()).isNull();
199-
assertThat(locatorState.getPid()).isNull();
200-
assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
201-
assertThat(locatorState.getUptime().intValue()).isEqualTo(0);
202-
assertThat(locatorState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
203-
}
204-
205-
/**
206-
* This test takes > 1 minute to run in {@link LocatorLauncherRemoteFileIntegrationTest}.
207-
*/
208-
@Test
209-
public void statusWithStalePidFileReturnsNotResponding() {
210-
givenPidFile(fakePid);
211-
212-
LocatorState locatorState = new Builder()
213-
.setWorkingDirectory(getWorkingDirectoryPath())
214-
.build()
215-
.status();
216-
217-
assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
218-
}
219-
220175
@Test
221176
public void stopWithPidReturnsStopped() {
222177
givenRunningLocator(withPort(0));

‎geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherLocalIntegrationTest.java‎

Lines changed: 8 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
*/
1515
package org.apache.geode.distributed;
1616

17-
import static org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
1817
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
1918
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
2019
import static org.apache.geode.distributed.ConfigurationProperties.DISABLE_AUTO_RECONNECT;
@@ -308,6 +307,13 @@ public void statusWithPidReturnsOnlineWithDetails() throws UnknownHostException
308307
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws UnknownHostException {
309308
givenRunningServer();
310309

310+
// Keep it as we are going to overwrite it
311+
int pid = readPidFile();
312+
313+
// Sets a fake PID to ensure that the request is going through FileProcessController,
314+
// because if not, the request would fail due to the PID not existing
315+
givenPidFile(fakePid);
316+
311317
ServerState serverState = new Builder()
312318
.setWorkingDirectory(getWorkingDirectoryPath())
313319
.build()
@@ -320,64 +326,12 @@ public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws UnknownH
320326
assertThat(serverState.getJvmArguments()).isEqualTo(getJvmArguments());
321327
assertThat(serverState.getLogFile()).isEqualTo(getLogFilePath());
322328
assertThat(serverState.getMemberName()).isEqualTo(getUniqueName());
323-
assertThat(serverState.getPid().intValue()).isEqualTo(readPidFile());
329+
assertThat(serverState.getPid().intValue()).isEqualTo(pid);
324330
assertThat(serverState.getStatus()).isEqualTo(ONLINE);
325331
assertThat(serverState.getUptime()).isGreaterThan(0);
326332
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
327333
}
328334

329-
@Test
330-
public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
331-
givenEmptyPidFile();
332-
ServerLauncher launcher = new Builder()
333-
.setWorkingDirectory(getWorkingDirectoryPath())
334-
.build();
335-
336-
Throwable thrown = catchThrowable(() -> launcher.status());
337-
338-
assertThat(thrown)
339-
.isInstanceOf(IllegalArgumentException.class)
340-
.hasMessageContaining("Invalid pid 'null' found in");
341-
}
342-
343-
@Test
344-
public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails()
345-
throws UnknownHostException {
346-
givenEmptyWorkingDirectory();
347-
348-
ServerState serverState = new Builder()
349-
.setWorkingDirectory(getWorkingDirectoryPath())
350-
.build()
351-
.status();
352-
353-
assertThat(serverState.getClasspath()).isNull();
354-
assertThat(serverState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
355-
assertThat(serverState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
356-
assertThat(serverState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
357-
assertThat(serverState.getJvmArguments()).isEqualTo(getJvmArguments());
358-
assertThat(serverState.getLogFile()).isNull();
359-
assertThat(serverState.getMemberName()).isNull();
360-
assertThat(serverState.getPid()).isNull();
361-
assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
362-
assertThat(serverState.getUptime().intValue()).isEqualTo(0);
363-
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
364-
}
365-
366-
/**
367-
* This test takes > 1 minute to run in {@link ServerLauncherLocalFileIntegrationTest}.
368-
*/
369-
@Test
370-
public void statusWithStalePidFileReturnsNotResponding() {
371-
givenPidFile(fakePid);
372-
373-
ServerState serverState = new Builder()
374-
.setWorkingDirectory(getWorkingDirectoryPath())
375-
.build()
376-
.status();
377-
378-
assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
379-
}
380-
381335
@Test
382336
public void stopWithPidReturnsStopped() {
383337
givenRunningServer();

‎geode-core/src/integrationTest/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java‎

Lines changed: 8 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,12 @@
1414
*/
1515
package org.apache.geode.distributed;
1616

17-
import static org.apache.geode.distributed.AbstractLauncher.Status.NOT_RESPONDING;
1817
import static org.apache.geode.distributed.AbstractLauncher.Status.ONLINE;
1918
import static org.apache.geode.distributed.AbstractLauncher.Status.STOPPED;
2019
import static org.apache.geode.distributed.ConfigurationProperties.CACHE_XML_FILE;
2120
import static org.apache.geode.internal.inet.LocalHostUtil.getLocalHost;
2221
import static org.apache.geode.util.internal.GeodeGlossary.GEMFIRE_PREFIX;
2322
import static org.assertj.core.api.Assertions.assertThat;
24-
import static org.assertj.core.api.Assertions.catchThrowable;
2523

2624
import java.io.File;
2725
import java.io.IOException;
@@ -210,13 +208,20 @@ public void statusWithPidReturnsOnlineWithDetails() throws IOException {
210208
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws IOException {
211209
givenRunningServer();
212210

211+
// Keep it as we are going to overwrite it
212+
int pid = readPidFile();
213+
214+
// Sets a fake PID to ensure that the request is going through FileProcessController,
215+
// because if not, the request would fail due to the PID not existing
216+
givenPidFile(fakePid);
217+
213218
ServerState serverState = new Builder()
214219
.setWorkingDirectory(getWorkingDirectoryPath())
215220
.build()
216221
.status();
217222

218223
assertThat(serverState.getStatus()).isEqualTo(ONLINE);
219-
assertThat(serverState.getPid().intValue()).isEqualTo(readPidFile());
224+
assertThat(serverState.getPid().intValue()).isEqualTo(pid);
220225
assertThat(serverState.getUptime()).isGreaterThan(0);
221226
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
222227
assertThat(serverState.getJvmArguments()).isEqualTo(getJvmArguments());
@@ -228,54 +233,6 @@ public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws IOExcept
228233
assertThat(serverState.getMemberName()).isEqualTo(getUniqueName());
229234
}
230235

231-
@Test
232-
public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
233-
givenEmptyPidFile();
234-
235-
ServerLauncher launcher = new Builder()
236-
.setWorkingDirectory(getWorkingDirectoryPath())
237-
.build();
238-
239-
Throwable thrown = catchThrowable(() -> launcher.status());
240-
241-
assertThat(thrown)
242-
.isInstanceOf(IllegalArgumentException.class)
243-
.hasMessageContaining("Invalid pid 'null' found in");
244-
}
245-
246-
@Test
247-
public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails() throws IOException {
248-
givenEmptyWorkingDirectory();
249-
250-
ServerState serverState = new Builder()
251-
.setWorkingDirectory(getWorkingDirectoryPath())
252-
.build()
253-
.status();
254-
255-
assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
256-
assertThat(serverState.getPid()).isNull();
257-
assertThat(serverState.getUptime().intValue()).isEqualTo(0);
258-
assertThat(serverState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
259-
assertThat(serverState.getClasspath()).isNull();
260-
assertThat(serverState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
261-
assertThat(serverState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
262-
assertThat(serverState.getLogFile()).isNull();
263-
assertThat(serverState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
264-
assertThat(serverState.getMemberName()).isNull();
265-
}
266-
267-
@Test
268-
public void statusWithStalePidFileReturnsNotResponding() {
269-
givenPidFile(fakePid);
270-
271-
ServerState serverState = new Builder()
272-
.setWorkingDirectory(getWorkingDirectoryPath())
273-
.build()
274-
.status();
275-
276-
assertThat(serverState.getStatus()).isEqualTo(NOT_RESPONDING);
277-
}
278-
279236
@Test
280237
public void stopWithPidReturnsStopped() {
281238
givenRunningServer();

‎geode-core/src/integrationTest/java/org/apache/geode/internal/process/ProcessControllerFactoryIntegrationTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2121
import static org.mockito.Mockito.mock;
22+
import static org.mockito.Mockito.when;
2223

2324
import java.io.File;
2425
import java.io.FileNotFoundException;
@@ -83,6 +84,19 @@ public void createProcessController_returnsMBeanProcessController() throws Excep
8384
assertThat(controller).isInstanceOf(MBeanOrFileProcessController.class);
8485
}
8586

87+
@Test
88+
public void createProcessController_returnsFileProcessController() throws Exception {
89+
// arrange
90+
when(parameters.getDirectory()).thenReturn(new File("./"));
91+
92+
// act
93+
ProcessController controller =
94+
factory.createProcessController(parameters);
95+
96+
// assert
97+
assertThat(controller).isInstanceOf(FileProcessController.class);
98+
}
99+
86100
@Test
87101
public void createProcessController_withoutAttachAPI_returnsFileProcessController()
88102
throws Exception {

0 commit comments

Comments
 (0)