GEODE-10327: Overhaul GfshRule to kill processes and save artifacts (a…

…pache#7731)

PROBLEM

Tests that use GfshRule leave behind orphaned processes and do not save
artifacts for debugging failures.

SOLUTION

GfshRule needs to cleanup all processes it forks. It also needs to save
off all runtime artifacts such as logging, stats, pid files, diskstores
to enable debugging of test failures.

DETAILS

Enhance GfshRule and modify all tests using it for proper debugging and
to prevent test pollution.

Overhaul of GfshRule:

* kill ALL geode processes during cleanup
* use FolderRule to ensure all logs and files are properly saved off
  when a test fails
* extract GfshExecutor from JUnit rule code
* GfshExecutor allows a test to use any number of Geode versions with
  just one GfshRule
* add Gfsh log level support for easier debugging
* add support for new VmConfiguration to allow control over Geode and
  Java versions
* overhaul API of GfshRule and companion classes for better consistency
  and design

New FolderRule:

* replaces TemporaryFolder and saves off all content when a test fails
* creates root directory under the gradle worker instead of under temp

Update HTTP session caching module tests:

* use new FolderRule to save all artifacts when a test fails
* use nio Paths for filesystem variables

Update acceptance and upgrade tests that use GfshRule:

* use new improved GfshRule and GfshExecutor
* use new FolderRule instead of TemporaryFolder to save all artifacts
  when a test fails
* use --disable-default-server in tests with no clients
* fix flakiness of many tests by using random ports instead of default
  or hardcoded port values
* reformat GfshRule API usage in tests to improve readability and
  consistency
* add GfshStopper to provide common place to await process stop (stop
  locator/server is async so restarting with same ports is very prone
  to hitting BindExceptions)

Update ProcessUtils:

* extract NativeProcessUtils and make it public for direct use
* rename InternalProcessUtils as ProcessUtilsProvider and move to its
  own class
* rethrow IOExceptions as UncheckedIOExceptions
* fix flakiness in NativeProcessUtilsTest by moving findAvailablePid
  into test method

Minor changes:

* improve code formatting and readability
* convert from old io File to nio Path APIs as much as possible
* close output streams to fix filesystem issues on Windows

Fixes flaky test tickets:

* DeployJarAcceptanceTest GEODE-9615
* possibly other tests that uses GfshRule

Changes for resubmit:

* log error message if unable to delete folder

NOTES

The jdk8, jdk17 and windows labels were used to run tests on more
environments.

This PR contains mostly test and framework changes. The only product
code altered is ServerLauncher and several classes in
org.apache.geode.internal.process, all of which is in geode-core.

geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/ServerContainer.java

@@ -14,8 +14,8 @@
  */
 package org.apache.geode.session.tests;
 
+import static java.nio.file.Files.copy;
 import static java.util.stream.Collectors.joining;
-import static org.apache.geode.session.tests.ContainerInstall.TMP_DIR;
 import static org.apache.geode.test.process.JavaModuleHelper.getJvmModuleOptions;
 
 import java.io.File;
@@ -33,6 +33,7 @@
 import org.codehaus.cargo.container.ContainerType;
 import org.codehaus.cargo.container.InstalledLocalContainer;
 import org.codehaus.cargo.container.State;
+import org.codehaus.cargo.container.configuration.Configuration;
 import org.codehaus.cargo.container.configuration.ConfigurationType;
 import org.codehaus.cargo.container.configuration.LocalConfiguration;
 import org.codehaus.cargo.container.deployable.WAR;
@@ -55,32 +56,30 @@
  * Subclasses provide setup and configuration of specific containers.
  */
 public abstract class ServerContainer {
-  private final File containerConfigHome;
+
+  protected static final Logger logger = LogService.getLogger();
+
+  private static final String DEFAULT_LOGGING_LEVEL = LoggingLevel.LOW.getLevel();
+
+  private final Path containerConfigHome;
   private final IntSupplier portSupplier;
   private final InstalledLocalContainer container;
   private final ContainerInstall install;
 
   private String locatorAddress;
   private int locatorPort;
-  private File warFile;
+  private Path warFile;
 
   public String description;
-  public File gemfireLogFile;
-  public File cacheXMLFile;
-  public File cargoLogDir;
-
-  public String loggingLevel;
+  public Path cacheXMLFile;
+  public Path cargoLogDir;
 
-  public HashMap<String, String> cacheProperties;
-  public HashMap<String, String> systemProperties;
+  private String loggingLevel;
 
-  public final String DEFAULT_CONF_DIR;
+  protected HashMap<String, String> cacheProperties;
+  protected HashMap<String, String> systemProperties;
 
-  public static final String DEFAULT_LOGGING_LEVEL = LoggingLevel.LOW.getLevel();
-  public static final String DEFAULT_LOG_DIR = "cargo_logs/";
-  public static final String DEFAULT_CONFIG_DIR = TMP_DIR + "/cargo_configs/";
-
-  public static final Logger logger = LogService.getLogger();
+  final Path defaultConfigDir;
 
   /**
    * Sets up the container using the given installation
@@ -91,85 +90,91 @@ public abstract class ServerContainer {
    * variable.
    *
    * @param install the installation with which to set up the container
+   * @param rootDir The root folder used by default for cargo logs, container configs and other
+   *        files and directories
    * @param containerConfigHome The folder that the container configuration folder should be setup
    *        in
    * @param containerDescriptors A string of extra descriptors for the container used in the
    *        containers {@link #description}
    * @param portSupplier allocates ports for use by the container
    * @throws IOException if an exception is encountered
    */
-  public ServerContainer(ContainerInstall install, File containerConfigHome,
+  public ServerContainer(ContainerInstall install, Path rootDir, Path containerConfigHome,
       String containerDescriptors, IntSupplier portSupplier) throws IOException {
     this.install = install;
     this.portSupplier = portSupplier;
     // Get a container description for logging and output
     description = generateUniqueContainerDescription(containerDescriptors);
     // Setup logging
     loggingLevel = DEFAULT_LOGGING_LEVEL;
-    cargoLogDir = new File(DEFAULT_LOG_DIR + description);
-    cargoLogDir.mkdirs();
+    cargoLogDir = rootDir.resolve("cargo_logs").resolve(description);
+    Files.createDirectories(cargoLogDir);
 
     logger.info("Creating new container {}", description);
 
-    DEFAULT_CONF_DIR = install.getHome() + "/conf/";
+    defaultConfigDir = install.getHome().resolve("conf");
+
     // Use the default configuration home path if not passed a config home
+    Path defaultConfigDir = rootDir.resolve("cargo_configs");
+
     this.containerConfigHome = containerConfigHome == null
-        ? new File(DEFAULT_CONFIG_DIR + description) : containerConfigHome;
+        ? defaultConfigDir.resolve(description) : containerConfigHome;
 
     // Init the property lists
     cacheProperties = new HashMap<>();
     systemProperties = new HashMap<>();
     // Set WAR file to session testing war
-    warFile = new File(install.getWarFilePath());
+    warFile = install.getWarFilePath();
 
     // Create the Cargo Container instance wrapping our physical container
-    LocalConfiguration configuration = (LocalConfiguration) new DefaultConfigurationFactory()
+    Configuration configuration = new DefaultConfigurationFactory()
         .createConfiguration(install.getInstallId(), ContainerType.INSTALLED,
-            ConfigurationType.STANDALONE, this.containerConfigHome.getAbsolutePath());
+            ConfigurationType.STANDALONE, this.containerConfigHome.toString());
+
     // Set configuration/container logging level
     configuration.setProperty(GeneralPropertySet.LOGGING, loggingLevel);
     // Removes secureRandom generation so that container startup is much faster
     configuration.setProperty(GeneralPropertySet.JVMARGS,
         "-Djava.security.egd=file:/dev/./urandom -Xmx256m -Xms64m");
 
     // Setup the gemfire log file for this container
-    gemfireLogFile = new File(cargoLogDir.getAbsolutePath() + "/gemfire.log");
-    gemfireLogFile.getParentFile().mkdirs();
-    setSystemProperty("log-file", gemfireLogFile.getAbsolutePath());
+    Path gemfireLogFile = cargoLogDir.resolve("gemfire.log");
+    Files.createDirectories(cargoLogDir);
+    setSystemProperty("log-file", gemfireLogFile.toString());
 
-    logger.info("Gemfire logs can be found in {}", gemfireLogFile.getAbsolutePath());
+    logger.info("Gemfire logs can be found in {}", gemfireLogFile);
 
     // Create the container
-    container = (InstalledLocalContainer) (new DefaultContainerFactory())
+    container = (InstalledLocalContainer) new DefaultContainerFactory()
         .createContainer(install.getInstallId(), ContainerType.INSTALLED, configuration);
     // Set container's home dir to where it was installed
-    container.setHome(install.getHome());
+    container.setHome(install.getHome().toString());
     // Set container output log to directory setup for it
-    container.setOutput(cargoLogDir.getAbsolutePath() + "/container.log");
+    container.setOutput(cargoLogDir.resolve("container.log").toString());
 
     // Set cacheXML file
-    File installXMLFile = install.getCacheXMLFile();
+    Path installXMLFile = install.getCacheXMLFile();
     // Sets the cacheXMLFile variable and adds the cache XML file server system property map
-    setCacheXMLFile(new File(cargoLogDir.getAbsolutePath() + "/" + installXMLFile.getName()));
+    setCacheXMLFile(cargoLogDir.resolve(installXMLFile.getFileName()));
     // Copy the cacheXML file to a new, unique location for this container
-    FileUtils.copyFile(installXMLFile, cacheXMLFile);
+    copy(installXMLFile, cacheXMLFile);
   }
 
   /*
    * Generates a unique, mostly human readable, description string of the container using the
    * installation's description, extraIdentifiers, and the current system nano time
    */
-  public String generateUniqueContainerDescription(String extraIdentifiers) {
+  private String generateUniqueContainerDescription(String extraIdentifiers) {
     return String.join("_", Arrays.asList(install.getInstallDescription(), extraIdentifiers,
         UUID.randomUUID().toString()));
   }
 
   /**
    * Deploys the {@link #warFile} to the cargo container ({@link #container}).
    */
-  public void deployWar() {
+  protected void deployWar() {
     // Get the cargo war from the war file
-    WAR war = new WAR(warFile.getAbsolutePath());
+    WAR war = new WAR(warFile.toString());
     // Set context access to nothing
     war.setContext("");
     // Deploy the war the container's configuration
@@ -232,8 +237,8 @@ public void stop() {
 
   public void dumpLogs() {
     System.out.println("Logs for container " + this);
-    dumpLogsInDir(cargoLogDir.toPath());
-    dumpLogsInDir(containerConfigHome.toPath().resolve("logs"));
+    dumpLogsInDir(cargoLogDir);
+    dumpLogsInDir(containerConfigHome.resolve("logs"));
     dumpConfiguration();
   }
 
@@ -254,7 +259,7 @@ private static void dumpToStdOut(Path path) {
     System.out.println(path.toAbsolutePath());
     System.out.println("-------------------------------------------");
     try {
-      Files.copy(path, System.out);
+      copy(path, System.out);
     } catch (IOException thrown) {
       System.out.println("Exception while dumping log file to stdout.");
       System.out.println("   File: " + path.toAbsolutePath());
@@ -302,7 +307,7 @@ public void cleanUp() throws IOException {
    * @param port the port that the locator is listening on
    * @throws IOException if an exception is encountered when updating the locator property file
    */
-  public void setLocator(String address, int port) throws IOException {
+  protected void setLocator(String address, int port) throws IOException {
     locatorAddress = address;
     locatorPort = port;
     updateLocator();
@@ -311,29 +316,29 @@ public void setLocator(String address, int port) throws IOException {
   /*
    * Sets the container's cache XML file
    */
-  public void setCacheXMLFile(File cacheXMLFile) throws IOException {
-    setSystemProperty("cache-xml-file", cacheXMLFile.getAbsolutePath());
+  private void setCacheXMLFile(Path cacheXMLFile) throws IOException {
+    setSystemProperty("cache-xml-file", cacheXMLFile.toString());
     this.cacheXMLFile = cacheXMLFile;
   }
 
   /*
    * Set a geode session replication property
    */
-  public String setCacheProperty(String name, String value) throws IOException {
+  protected String setCacheProperty(String name, String value) throws IOException {
     return cacheProperties.put(name, value);
   }
 
   /*
    * Set geode distributed system property
    */
-  public String setSystemProperty(String name, String value) throws IOException {
+  private String setSystemProperty(String name, String value) throws IOException {
     return systemProperties.put(name, value);
   }
 
   /*
    * Sets the war file for this container to deploy and use
    */
-  public void setWarFile(File warFile) {
+  void setWarFile(Path warFile) {
     this.warFile = warFile;
   }
 
@@ -356,7 +361,7 @@ public ContainerInstall getInstall() {
     return install;
   }
 
-  public File getWarFile() {
+  public Path getWarFile() {
     return warFile;
   }
 
@@ -372,11 +377,11 @@ public State getState() {
     return container.getState();
   }
 
-  public String getCacheProperty(String name) {
+  protected String getCacheProperty(String name) {
     return cacheProperties.get(name);
   }
 
-  public String getSystemProperty(String name) {
+  protected String getSystemProperty(String name) {
     return systemProperties.get(name);
   }
 
@@ -452,8 +457,7 @@ private void updateLocator() throws IOException {
       attributes.put("host", locatorAddress);
       attributes.put("port", Integer.toString(locatorPort));
 
-      ContainerInstall.editXMLFile(cacheXMLFile.getAbsolutePath(), "locator", "pool",
-          attributes, true);
+      ContainerInstall.editXMLFile(cacheXMLFile, "locator", "pool", attributes, true);
     } else {
       setSystemProperty("locators", locatorAddress + "[" + locatorPort + "]");
     }

geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/session/tests/TomcatContainer.java

@@ -14,12 +14,13 @@
  */
 package org.apache.geode.session.tests;
 
-import java.io.File;
+import static java.nio.file.Files.copy;
+
 import java.io.IOException;
+import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.function.IntSupplier;
 
-import org.apache.commons.io.FileUtils;
 import org.codehaus.cargo.container.configuration.FileConfig;
 import org.codehaus.cargo.container.configuration.StandaloneLocalConfiguration;
 import org.codehaus.cargo.container.tomcat.TomcatPropertySet;
@@ -34,14 +35,15 @@
  */
 public class TomcatContainer extends ServerContainer {
 
-  private final File contextXMLFile;
-  private final File serverXMLFile;
+  private final Path contextXMLFile;
+  private final Path serverXMLFile;
 
-  public static final String DEFAULT_TOMCAT_CONFIG_XML_DIR = "conf/";
+  private static final String DEFAULT_TOMCAT_CONFIG_XML_DIR = "conf/";
 
-  public static final String DEFAULT_TOMCAT_XML_REPLACEMENT_DIR =
+  private static final String DEFAULT_TOMCAT_XML_REPLACEMENT_DIR =
       DEFAULT_TOMCAT_CONFIG_XML_DIR + "Catalina/localhost/";
-  public static final String DEFAULT_TOMCAT_CONTEXT_XML_REPLACEMENT_NAME = "context.xml.default";
+
+  private static final String DEFAULT_TOMCAT_CONTEXT_XML_REPLACEMENT_NAME = "context.xml.default";
 
   /*
    * Setup the Tomcat container
@@ -51,18 +53,19 @@ public class TomcatContainer extends ServerContainer {
    * properties, deploys the session testing WAR file to the Cargo container, and sets various
    * container properties (i.e. locator, local cache, etc.)
    */
-  public TomcatContainer(TomcatInstall install, File containerConfigHome,
+  TomcatContainer(TomcatInstall install, Path rootDir, Path containerConfigHome,
       String containerDescriptors, IntSupplier portSupplier) throws IOException {
-    super(install, containerConfigHome, containerDescriptors, portSupplier);
+    super(install, rootDir, containerConfigHome, containerDescriptors, portSupplier);
 
     // Setup container specific XML files
-    contextXMLFile = new File(cargoLogDir.getAbsolutePath() + "/context.xml");
-    serverXMLFile = new File(DEFAULT_CONF_DIR + "server.xml");
+    contextXMLFile = cargoLogDir.resolve("context.xml");
+    serverXMLFile = defaultConfigDir.resolve("server.xml");
 
     // Copy the default container context XML file from the install to the specified path
-    FileUtils.copyFile(new File(DEFAULT_CONF_DIR + "context.xml"), contextXMLFile);
+    copy(defaultConfigDir.resolve("context.xml"), contextXMLFile);
     // Set the container context XML file to the new location copied to above
-    setConfigFile(contextXMLFile.getAbsolutePath(), DEFAULT_TOMCAT_XML_REPLACEMENT_DIR,
+    setConfigFile(contextXMLFile,
+        DEFAULT_TOMCAT_XML_REPLACEMENT_DIR,
         DEFAULT_TOMCAT_CONTEXT_XML_REPLACEMENT_NAME);
 
     if (install.getConnectionType() == ContainerInstall.ConnectionType.CLIENT_SERVER ||
@@ -94,23 +97,25 @@ public String getAJPPort() {
   }
 
   /**
-   * Implements the {@link ServerContainer#writeSettings()} function in order to write the proper
+   * Implements the {@code ServerContainer#writeSettings()} function in order to write the proper
    * settings to the container
    *
-   * Method uses the {@link ContainerInstall#editXMLFile(String, String, String, String, HashMap)}
+   * <p>
+   * Method uses the {@link ContainerInstall#editXMLFile(Path, String, String, String, HashMap)}
    * to edit the {@link #contextXMLFile} with the {@link #cacheProperties}. Method uses
    * {@link #writePropertiesToConfig(StandaloneLocalConfiguration, String, String, HashMap)} to
    * write the {@link #systemProperties} to the {@link #serverXMLFile} using the container's
    * configuration (obtained from {@link #getConfiguration()}).
    */
   @Override
-  public void writeSettings() throws IOException {
+  public void writeSettings() {
     StandaloneLocalConfiguration config = (StandaloneLocalConfiguration) getConfiguration();
 
     // Edit the context XML file
-    ContainerInstall.editXMLFile(contextXMLFile.getAbsolutePath(), "Tomcat", "Manager", "Context",
+    ContainerInstall.editXMLFile(contextXMLFile, "Tomcat", "Manager", "Context",
         cacheProperties);
-    writePropertiesToConfig(config, DEFAULT_TOMCAT_CONFIG_XML_DIR + "/" + serverXMLFile.getName(),
+    writePropertiesToConfig(config,
+        DEFAULT_TOMCAT_CONFIG_XML_DIR + "/" + serverXMLFile.toFile().getName(),
         "//Server/Listener[@className='"
             + ((TomcatInstall) getInstall()).getServerLifeCycleListenerClass() + "']",
         systemProperties);
@@ -156,10 +161,10 @@ private void writePropertiesToConfig(StandaloneLocalConfiguration config, String
    * @param configDirDest The name of the directory that the configuration file be placed in
    * @param configFileDestName The name of destination file for the new configuration file
    */
-  private void setConfigFile(String filePath, String configDirDest, String configFileDestName) {
+  private void setConfigFile(Path filePath, String configDirDest, String configFileDestName) {
     FileConfig configFile = new FileConfig();
 
-    configFile.setFile(filePath);
+    configFile.setFile(filePath.toString());
     configFile.setToDir(configDirDest);
     configFile.setToFile(configFileDestName);
     getConfiguration().setConfigFileProperty(configFile);