Skip to content

Commit a807625

Browse files
gaussianrecurrenceMario Salazar de Torres
gaussianrecurrence
and
Mario Salazar de Torres
authored
GEODE-9478: Fix status --dir to use file controller (#6737)
* 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 * GEODE-9478: Revision 1 - Removed throws in javadoc given new constructors doesn't have any PID --------- Co-authored-by: Mario Salazar de Torres <mario.salazar.de.torres@est.tech>
1 parent 7ec0626 commit a807625

10 files changed

Lines changed: 112 additions & 256 deletions

File tree

‎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;
@@ -217,6 +215,13 @@ public void statusWithPidReturnsOnlineWithDetails() throws Exception {
217215
public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws Exception {
218216
givenRunningLocator();
219217

218+
// Keep it as we are going to overwrite it
219+
int pid = readPidFile();
220+
221+
// Sets a fake PID to ensure that the request is going through FileProcessController,
222+
// because if not, the request would fail due to the PID not existing
223+
givenPidFile(fakePid);
224+
220225
LocatorState locatorState = new Builder()
221226
.setWorkingDirectory(getWorkingDirectoryPath())
222227
.build()
@@ -230,62 +235,11 @@ public void statusWithWorkingDirectoryReturnsOnlineWithDetails() throws Exceptio
230235
assertThat(locatorState.getJvmArguments()).isEqualTo(getJvmArguments());
231236
assertThat(locatorState.getLogFile()).isEqualTo(getLogFilePath());
232237
assertThat(locatorState.getMemberName()).isEqualTo(getUniqueName());
233-
assertThat(locatorState.getPid().intValue()).isEqualTo(readPidFile());
238+
assertThat(locatorState.getPid().intValue()).isEqualTo(pid);
234239
assertThat(locatorState.getUptime()).isGreaterThan(0);
235240
assertThat(locatorState.getWorkingDirectory()).isEqualTo(new File(".").getCanonicalPath());
236241
}
237242

238-
@Test
239-
public void statusWithEmptyPidFileThrowsIllegalArgumentException() {
240-
givenEmptyPidFile();
241-
LocatorLauncher launcher = new Builder()
242-
.setWorkingDirectory(getWorkingDirectoryPath())
243-
.build();
244-
245-
Throwable thrown = catchThrowable(launcher::status);
246-
247-
assertThat(thrown)
248-
.isInstanceOf(IllegalArgumentException.class)
249-
.hasMessageContaining("Invalid pid 'null' found in");
250-
}
251-
252-
@Test
253-
public void statusWithEmptyWorkingDirectoryReturnsNotRespondingWithDetails() throws Exception {
254-
givenEmptyWorkingDirectory();
255-
256-
LocatorState locatorState = new Builder()
257-
.setWorkingDirectory(getWorkingDirectoryPath())
258-
.build()
259-
.status();
260-
261-
assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
262-
assertThat(locatorState.getClasspath()).isNull();
263-
assertThat(locatorState.getGemFireVersion()).isEqualTo(GemFireVersion.getGemFireVersion());
264-
assertThat(locatorState.getHost()).isEqualTo(getLocalHost().getCanonicalHostName());
265-
assertThat(locatorState.getJavaVersion()).isEqualTo(System.getProperty("java.version"));
266-
assertThat(locatorState.getJvmArguments()).isEqualTo(getRuntimeMXBean().getInputArguments());
267-
assertThat(locatorState.getLogFile()).isNull();
268-
assertThat(locatorState.getMemberName()).isNull();
269-
assertThat(locatorState.getPid()).isNull();
270-
assertThat(locatorState.getUptime().intValue()).isEqualTo(0);
271-
assertThat(locatorState.getWorkingDirectory()).isEqualTo(getWorkingDirectoryPath());
272-
}
273-
274-
/**
275-
* This test takes > 1 minute to run in {@link LocatorLauncherLocalFileIntegrationTest}.
276-
*/
277-
@Test
278-
public void statusWithStalePidFileReturnsNotResponding() {
279-
givenPidFile(fakePid);
280-
281-
LocatorState locatorState = new Builder()
282-
.setWorkingDirectory(getWorkingDirectoryPath())
283-
.build()
284-
.status();
285-
286-
assertThat(locatorState.getStatus()).isEqualTo(NOT_RESPONDING);
287-
}
288-
289243
@Test
290244
public void stopWithPidReturnsStopped() {
291245
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)