GEODE-5731: Add tests for hostname-for-clients

geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommandDUnitTest.java

@@ -16,7 +16,9 @@
 
 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.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.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 void testWithDefaultLocatorPort() throws IOException {
         .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);
+    });
+  }
 }

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_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.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.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 void testServerJVMTerminatesOnOutOfMemoryError() throws IOException {
 
     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);
+    });
+  }
 }

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 void startWithBindAddress() throws Exception {
     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);
+  }
 }

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 void startServerRespectsJmxManagerHostnameForClients() throws Exception {
     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 void startServerRespectsJmxManagerHostnameForClients() throws Exception {
     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);
+  }
 }

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java

@@ -28,6 +28,7 @@
 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.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 @@ ResultModel doStartLocator(
               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 @@ ResultModel doStartLocator(
     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 @@ String[] createStartLocatorCommandLine(final LocatorLauncher launcher,
       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 @@ String getLocatorClasspath(final boolean includeSystemClasspath, final String us
     }
 
     return StartMemberUtils.toClasspath(includeSystemClasspath,
-        jarFilePathnames.toArray(new String[jarFilePathnames.size()]), userClasspath);
+        jarFilePathnames.toArray(new String[] {}), userClasspath);
   }
 
   private String[] getExtensionsJars() {
@@ -537,7 +532,7 @@ private String[] getExtensionsJars() {
 
     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 {

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 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.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 @@ ResultModel doStartServer(String memberName, Boolean assignBuckets, String bindA
               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 @@ ResultModel doStartServer(String memberName, Boolean assignBuckets, String bindA
       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 @@ ResultModel doStartServer(String memberName, Boolean assignBuckets, String bindA
     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 @@ ResultModel doStartServer(String memberName, Boolean assignBuckets, String bindA
           .tryGetCanonicalPathElseGetAbsolutePath(new File(serverLauncher.getWorkingDirectory()))),
           null);
 
-      serverState = ServerLauncher.ServerState.fromDirectory(workingDirectory, memberName);
       do {
         if (serverProcess.isAlive()) {
           Gfsh.print(".");
@@ -427,6 +420,11 @@ ResultModel doStartServer(String memberName, Boolean assignBuckets, String bindA
     }
   }
 
+  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 @@ String[] createStartServerCommandLine(final ServerLauncher launcher,
           + 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 @@ String getServerClasspath(final boolean includeSystemClasspath, final String use
     }
 
     return StartMemberUtils.toClasspath(includeSystemClasspath,
-        jarFilePathnames.toArray(new String[jarFilePathnames.size()]), userClasspath);
+        jarFilePathnames.toArray(new String[] {}), userClasspath);
   }
 
   private String[] getExtensionsJars() {
@@ -583,7 +581,7 @@ private String[] getExtensionsJars() {
 
     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 {