You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jj...@apache.org on 2019/05/22 15:57:58 UTC
[geode] branch develop updated: GEODE-5731: Add tests for
hostname-for-clients
This is an automated email from the ASF dual-hosted git repository.
jjramos pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 0430a53 GEODE-5731: Add tests for hostname-for-clients
0430a53 is described below
commit 0430a538f133ea9f450234256f980fcba9356e1c
Author: Juan José Ramos <ju...@users.noreply.github.com>
AuthorDate: Wed May 22 16:57:46 2019 +0100
GEODE-5731: Add tests for hostname-for-clients
- Fixed minor warnings.
- Added some tests to check that the `hostname-for-clients` flag is
propagated across the stack while starting locators and servers.
- Replaced usage of class `org.apache.geode.internal.lang.StringUtils`
(deprecated) by `org.apache.commons.lang3.StringUtils`.
---
.../cli/commands/StartLocatorCommandDUnitTest.java | 38 ++++++++++++++++++++++
.../cli/commands/StartServerCommandDUnitTest.java | 37 +++++++++++++++++++++
.../StartLocatorCommandIntegrationTest.java | 16 +++++++--
.../StartServerCommandIntegrationTest.java | 18 +++++++---
.../internal/cli/commands/StartLocatorCommand.java | 15 +++------
.../internal/cli/commands/StartServerCommand.java | 26 +++++++--------
6 files changed, 120 insertions(+), 30 deletions(-)
diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandDUnitTest.java b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandDUnitTest.java
index 9fbedcf..0d004cb 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandDUnitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandDUnitTest.java
@@ -16,7 +16,9 @@ package org.apache.geode.management.internal.cli.commands;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.GEODE_0_PROPERTIES_1_NOT_FOUND_MESSAGE;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR__CONNECT;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR__DIR;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR__HOSTNAME_FOR_CLIENTS;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR__LOCATORS;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR__MEMBER_NAME;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_LOCATOR__PORT;
@@ -31,6 +33,7 @@ import java.net.InetSocketAddress;
import java.net.Socket;
import java.nio.file.Paths;
import java.text.MessageFormat;
+import java.util.List;
import org.junit.AfterClass;
import org.junit.Before;
@@ -41,6 +44,11 @@ import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.rules.TestName;
+import org.apache.geode.cache.client.internal.locator.LocatorListRequest;
+import org.apache.geode.cache.client.internal.locator.LocatorListResponse;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.ServerLocator;
import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.internal.process.ProcessType;
import org.apache.geode.internal.process.ProcessUtils;
@@ -223,4 +231,34 @@ public class StartLocatorCommandDUnitTest {
.hasOutput()
.doesNotContain(unexpectedMessage).containsPattern(expectedMessage);
}
+
+ @Test
+ public void startLocatorRespectsHostnameForClients() throws IOException {
+ File workingDir = temporaryFolder.newFolder();
+ String expectedMessagePattern = "Locator (.*) is currently online";
+
+ String command = new CommandStringBuilder(START_LOCATOR)
+ .addOption(START_LOCATOR__MEMBER_NAME, memberName)
+ .addOption(START_LOCATOR__LOCATORS, locatorConnectionString)
+ .addOption(START_LOCATOR__DIR, workingDir.getAbsolutePath())
+ .addOption(START_LOCATOR__PORT, "10339")
+ .addOption(START_LOCATOR__HOSTNAME_FOR_CLIENTS, "fakeLocatorName")
+ .addOption(START_LOCATOR__CONNECT, "true").getCommandString();
+
+ gfsh.executeAndAssertThat(command).statusIsSuccess()
+ .hasOutput().containsPattern(expectedMessagePattern);
+
+ // Verify that the ServerLocation contains the specified hostname-for-clients.
+ locator.invoke(() -> {
+ InternalLocator internalLocator = InternalLocator.getLocator();
+ final ServerLocator serverLocator = internalLocator.getServerLocatorAdvisee();
+ LocatorListResponse locatorListResponse =
+ (LocatorListResponse) serverLocator.processRequest(new LocatorListRequest());
+
+ List<ServerLocation> locators = locatorListResponse.getLocators();
+ assertThat(locators.stream().filter(
+ serverLocation -> "fakeLocatorName".equals(serverLocation.getHostName())))
+ .isNotEmpty().hasSize(1);
+ });
+ }
}
diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandDUnitTest.java b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandDUnitTest.java
index 502f303..83070ec 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandDUnitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandDUnitTest.java
@@ -24,6 +24,7 @@ import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SER
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__CACHE_XML_FILE;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__DIR;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__FORCE;
+import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__HOSTNAME__FOR__CLIENTS;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__LOCATORS;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__MAXHEAP;
import static org.apache.geode.management.internal.cli.i18n.CliStrings.START_SERVER__NAME;
@@ -42,6 +43,7 @@ import java.io.Serializable;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.nio.file.Paths;
+import java.util.List;
import org.junit.AfterClass;
import org.junit.Before;
@@ -50,10 +52,15 @@ import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
+import org.apache.geode.cache.client.internal.locator.GetAllServersRequest;
+import org.apache.geode.cache.client.internal.locator.GetAllServersResponse;
import org.apache.geode.cache.execute.FunctionException;
import org.apache.geode.cache.execute.FunctionService;
import org.apache.geode.distributed.AbstractLauncher;
import org.apache.geode.distributed.ServerLauncher;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.ServerLocator;
import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.internal.process.ProcessType;
import org.apache.geode.internal.process.ProcessUtils;
@@ -370,4 +377,34 @@ public class StartServerCommandDUnitTest implements Serializable {
assertThat(ProcessUtils.isProcessAlive(serverPid)).isFalse();
}
+
+
+ @Test
+ public void startServerRespectsHostnameForClients() throws IOException {
+ String expectedMessagePattern = "Server (.*) is currently online";
+
+ String command = new CommandStringBuilder(START_SERVER)
+ .addOption(START_SERVER__NAME, memberName)
+ .addOption(START_SERVER__LOCATORS, locatorConnectionString)
+ .addOption(START_SERVER__DIR, workingDir.getCanonicalPath())
+ .addOption(START_SERVER__SERVER_PORT, String.valueOf(serverPort))
+ .addOption(START_SERVER__HOSTNAME__FOR__CLIENTS, "fakeServerName")
+ .getCommandString();
+
+ gfsh.executeAndAssertThat(command).statusIsSuccess()
+ .hasOutput().containsPattern(expectedMessagePattern);
+
+ // Verify that the ServerLocation contains the specified hostname-for-clients.
+ locator.invoke(() -> {
+ InternalLocator internalLocator = InternalLocator.getLocator();
+ final ServerLocator serverLocator = internalLocator.getServerLocatorAdvisee();
+ GetAllServersResponse serversResponse =
+ (GetAllServersResponse) serverLocator.processRequest(new GetAllServersRequest());
+ List<ServerLocation> locators = serversResponse.getServers();
+
+ assertThat(locators.stream().filter(
+ serverLocation -> "fakeServerName".equals(serverLocation.getHostName())))
+ .isNotEmpty().hasSize(1);
+ });
+ }
}
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
index 7818c0e..59aaa80 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandIntegrationTest.java
@@ -12,7 +12,6 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-
package org.apache.geode.management.internal.cli.commands;
import static org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
@@ -44,7 +43,7 @@ public class StartLocatorCommandIntegrationTest {
private StartLocatorCommand spy;
@Before
- public void before() throws Exception {
+ public void before() {
spy = Mockito.spy(StartLocatorCommand.class);
doReturn(mock(Gfsh.class)).when(spy).getGfsh();
}
@@ -89,4 +88,17 @@ public class StartLocatorCommandIntegrationTest {
String[] lines = commandLines.getValue();
assertThat(lines[12]).isEqualTo("--bind-address=127.0.0.1");
}
+
+ @Test
+ public void startLocatorRespectsHostnameForClients() throws Exception {
+ doReturn(mock(Process.class)).when(spy).getProcess(any(), any());
+ String startLocatorCommand = new CommandStringBuilder("start locator")
+ .addOption("hostname-for-clients", FAKE_HOSTNAME).toString();
+
+ commandRule.executeAndAssertThat(spy, startLocatorCommand);
+ ArgumentCaptor<String[]> commandLines = ArgumentCaptor.forClass(String[].class);
+ verify(spy).getProcess(any(), commandLines.capture());
+ String[] lines = commandLines.getValue();
+ assertThat(lines).containsOnlyOnce("--hostname-for-clients=" + FAKE_HOSTNAME);
+ }
}
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
index 82a7f80..7f6271e 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/StartServerCommandIntegrationTest.java
@@ -12,7 +12,6 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-
package org.apache.geode.management.internal.cli.commands;
import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_HOSTNAME_FOR_CLIENTS;
@@ -44,14 +43,14 @@ public class StartServerCommandIntegrationTest {
private StartServerCommand spy;
@Before
- public void before() throws Exception {
+ public void before() {
spy = Mockito.spy(StartServerCommand.class);
doReturn(mock(Gfsh.class)).when(spy).getGfsh();
}
@Test
public void startServerWorksWithNoOptions() throws Exception {
- commandRule.executeCommandWithInstance(spy, "start server");
+ commandRule.executeAndAssertThat(spy, "start server");
ArgumentCaptor<Properties> gemfirePropertiesCaptor = ArgumentCaptor.forClass(Properties.class);
verify(spy).createStartServerCommandLine(any(), any(), any(), gemfirePropertiesCaptor.capture(),
@@ -67,7 +66,7 @@ public class StartServerCommandIntegrationTest {
String startServerCommand = new CommandStringBuilder("start server")
.addOption(JMX_MANAGER_HOSTNAME_FOR_CLIENTS, FAKE_HOSTNAME).toString();
- commandRule.executeCommandWithInstance(spy, startServerCommand);
+ commandRule.executeAndAssertThat(spy, startServerCommand);
ArgumentCaptor<Properties> gemfirePropertiesCaptor = ArgumentCaptor.forClass(Properties.class);
verify(spy).createStartServerCommandLine(any(), any(), any(), gemfirePropertiesCaptor.capture(),
@@ -78,4 +77,15 @@ public class StartServerCommandIntegrationTest {
assertThat(gemfireProperties.get(JMX_MANAGER_HOSTNAME_FOR_CLIENTS)).isEqualTo(FAKE_HOSTNAME);
}
+ @Test
+ public void startServerRespectsHostnameForClients() throws Exception {
+ String startServerCommand = new CommandStringBuilder("start server")
+ .addOption("hostname-for-clients", FAKE_HOSTNAME).toString();
+
+ commandRule.executeAndAssertThat(spy, startServerCommand);
+ ArgumentCaptor<String[]> commandLines = ArgumentCaptor.forClass(String[].class);
+ verify(spy).getProcess(any(), commandLines.capture());
+ String[] lines = commandLines.getValue();
+ assertThat(lines).containsOnlyOnce("--hostname-for-clients=" + FAKE_HOSTNAME);
+ }
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
index 90e9f81..863f7d1 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
@@ -28,6 +28,7 @@ import javax.management.MalformedObjectNameException;
import javax.net.ssl.SSLException;
import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
@@ -36,10 +37,8 @@ import org.apache.geode.distributed.ConfigurationProperties;
import org.apache.geode.distributed.LocatorLauncher;
import org.apache.geode.distributed.ServerLauncher;
import org.apache.geode.internal.OSProcess;
-import org.apache.geode.internal.lang.StringUtils;
import org.apache.geode.internal.lang.SystemUtils;
import org.apache.geode.internal.process.ProcessStreamReader;
-import org.apache.geode.internal.process.ProcessType;
import org.apache.geode.internal.util.IOUtils;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
@@ -181,10 +180,6 @@ public class StartLocatorCommand extends OfflineGfshCommand {
gemfireSecurityPropertiesFile.getAbsolutePath()));
}
- File locatorPidFile = new File(workingDirectory, ProcessType.LOCATOR.getPidFileName());
-
- final int oldPid = StartMemberUtils.readPid(locatorPidFile);
-
Properties gemfireProperties = new Properties();
StartMemberUtils.setPropertyIfNotNull(gemfireProperties, ConfigurationProperties.GROUPS, group);
@@ -239,7 +234,7 @@ public class StartLocatorCommand extends OfflineGfshCommand {
ProcessStreamReader.InputListener inputListener = line -> {
message.append(line);
if (readingMode == ProcessStreamReader.ReadingMode.BLOCKING) {
- message.append(StringUtils.LINE_SEPARATOR);
+ message.append(SystemUtils.getLineSeparator());
}
};
@@ -514,7 +509,7 @@ public class StartLocatorCommand extends OfflineGfshCommand {
commandLine.add("--redirect-output");
}
- return commandLine.toArray(new String[commandLine.size()]);
+ return commandLine.toArray(new String[] {});
}
String getLocatorClasspath(final boolean includeSystemClasspath, final String userClasspath) {
@@ -528,7 +523,7 @@ public class StartLocatorCommand extends OfflineGfshCommand {
}
return StartMemberUtils.toClasspath(includeSystemClasspath,
- jarFilePathnames.toArray(new String[jarFilePathnames.size()]), userClasspath);
+ jarFilePathnames.toArray(new String[] {}), userClasspath);
}
private String[] getExtensionsJars() {
@@ -537,7 +532,7 @@ public class StartLocatorCommand extends OfflineGfshCommand {
if (extensionsJars != null) {
// assume `extensions` directory does not contain any subdirectories. It only contains jars.
- return Arrays.stream(extensionsJars).filter(file -> file.isFile()).map(
+ return Arrays.stream(extensionsJars).filter(File::isFile).map(
file -> IOUtils.appendToPath(StartMemberUtils.GEODE_HOME, "extensions", file.getName()))
.toArray(String[]::new);
} else {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java
index 5f7ce8d..df1fe40 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartServerCommand.java
@@ -12,7 +12,6 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
-
package org.apache.geode.management.internal.cli.commands;
import java.io.File;
@@ -26,6 +25,7 @@ import java.util.concurrent.TimeUnit;
import javax.management.MalformedObjectNameException;
import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
@@ -34,10 +34,8 @@ import org.apache.geode.distributed.AbstractLauncher;
import org.apache.geode.distributed.ConfigurationProperties;
import org.apache.geode.distributed.ServerLauncher;
import org.apache.geode.internal.OSProcess;
-import org.apache.geode.internal.lang.StringUtils;
import org.apache.geode.internal.lang.SystemUtils;
import org.apache.geode.internal.process.ProcessStreamReader;
-import org.apache.geode.internal.process.ProcessType;
import org.apache.geode.internal.util.IOUtils;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
@@ -256,10 +254,6 @@ public class StartServerCommand extends OfflineGfshCommand {
gemfireSecurityPropertiesFile.getAbsolutePath()));
}
- File serverPidFile = new File(workingDirectory, ProcessType.SERVER.getPidFileName());
-
- final int oldPid = StartMemberUtils.readPid(serverPidFile);
-
Properties gemfireProperties = new Properties();
StartMemberUtils.setPropertyIfNotNull(gemfireProperties, ConfigurationProperties.BIND_ADDRESS,
@@ -341,8 +335,8 @@ public class StartServerCommand extends OfflineGfshCommand {
getGfsh().logInfo(StringUtils.join(serverCommandLine, StringUtils.SPACE), null);
}
- Process serverProcess = new ProcessBuilder(serverCommandLine)
- .directory(new File(serverLauncher.getWorkingDirectory())).start();
+ Process serverProcess =
+ getProcess(serverLauncher.getWorkingDirectory(), serverCommandLine);
serverProcess.getInputStream().close();
serverProcess.getOutputStream().close();
@@ -355,7 +349,7 @@ public class StartServerCommand extends OfflineGfshCommand {
ProcessStreamReader.InputListener inputListener = line -> {
message.append(line);
if (readingMode == ProcessStreamReader.ReadingMode.BLOCKING) {
- message.append(StringUtils.LINE_SEPARATOR);
+ message.append(SystemUtils.getLineSeparator());
}
};
@@ -377,7 +371,6 @@ public class StartServerCommand extends OfflineGfshCommand {
.tryGetCanonicalPathElseGetAbsolutePath(new File(serverLauncher.getWorkingDirectory()))),
null);
- serverState = ServerLauncher.ServerState.fromDirectory(workingDirectory, memberName);
do {
if (serverProcess.isAlive()) {
Gfsh.print(".");
@@ -427,6 +420,11 @@ public class StartServerCommand extends OfflineGfshCommand {
}
}
+ Process getProcess(String workingDir, String[] serverCommandLine) throws IOException {
+ return new ProcessBuilder(serverCommandLine)
+ .directory(new File(workingDir)).start();
+ }
+
String[] createStartServerCommandLine(final ServerLauncher launcher,
final File gemfirePropertiesFile, final File gemfireSecurityPropertiesFile,
final Properties gemfireProperties, final String userClasspath,
@@ -559,7 +557,7 @@ public class StartServerCommand extends OfflineGfshCommand {
+ launcher.getHostNameForClients());
}
- return commandLine.toArray(new String[commandLine.size()]);
+ return commandLine.toArray(new String[] {});
}
String getServerClasspath(final boolean includeSystemClasspath, final String userClasspath) {
@@ -574,7 +572,7 @@ public class StartServerCommand extends OfflineGfshCommand {
}
return StartMemberUtils.toClasspath(includeSystemClasspath,
- jarFilePathnames.toArray(new String[jarFilePathnames.size()]), userClasspath);
+ jarFilePathnames.toArray(new String[] {}), userClasspath);
}
private String[] getExtensionsJars() {
@@ -583,7 +581,7 @@ public class StartServerCommand extends OfflineGfshCommand {
if (extensionsJars != null) {
// assume `extensions` directory does not contain any subdirectories. It only contains jars.
- return Arrays.stream(extensionsJars).filter(file -> file.isFile()).map(
+ return Arrays.stream(extensionsJars).filter(File::isFile).map(
file -> IOUtils.appendToPath(StartMemberUtils.GEODE_HOME, "extensions", file.getName()))
.toArray(String[]::new);
} else {