New include_java_xrs config option; systemd init scripts now use --ex…

…ec option
fizzed · Sep 8, 2017 · b60cbbd · b60cbbd
1 parent dcc6c19
commit b60cbbd
Show file tree

Hide file tree

Showing 18 changed files with 161 additions and 78 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,22 +3,30 @@ Stork by Fizzed
 
 #### 2.6.x - In Progress
 
- - stork-launcher: New `--start-run` option for Linux daemon launchers that
+ - stork-launcher: New `include_java_xrs` configuration option to instruct
+   launcher to prepend the `-Xrs` flag for Java.  This is `true` by default.
+   The `-Xrs` flag is critical to avoid Java exiting with a non-zero error code
+   if you `kill` the process to stop it.  This allows inits like SYSTEMD to
+   report the service stopped correctly.
+ - stork-launcher: New `--exec` option for Linux daemon launchers that
    combines features from the `--start` and `--run` actions.  This action will
-   start your Java app in the background, but will not use `nohup`.  Your apps
-   `stdin`, `stdout`, and `stderr` streams will be passed thru.  Unlike `--run`
-   the system property `launcher.type` will be DAEMON -- so you can decided in 
-   your app if you need to stop logging to STDOUT at runtime.
- - stork-launcher: SYSTEMD init scripts now use the `--start-run` action. NOTE
-   that your apps `stdout` will go to the attached `tty` -- which for SYSTEMD
-   means it will go to `journald`.
+   start your Java app using `exec`.  That will allow `stdin`, `stdout`, and
+   `stderr` streams to be unprocessed by the launcher script.
+ - stork-launcher: SYSTEMD init scripts now use the `--exec` action to start
+   your app and use the `--stop` action to gracefully stop it.  Please note
+   that `stdout` will not be redirected to a file (like it was in previous
+   versions) -- so you should be mindful that SYSTEMD will now log anything
+   to `stdout` to `journald`.
  - stork-launcher: Windows launchers now match Linux by setting system properties
    of `launcher.name`, `launcher.type`, and `launcher.app.dir`.
  - stork-launcher: Linux daemon launcher correctly sets `launcher.type` to
-   CONSOLE if `--run` is used.
+   CONSOLE if `--run` is used.  You can now safely check if the `launcher.type`
+   property is `DAEMON` to determine if you are really running as a service (e.g.
+   you were started with SYSV or SYSTEMD).
  - Deprecated and removed stork-bootstrap. The EXTRA_JAVA_ARGS feature from v2.5.0
    mostly addresses what it was trying to accomplish.
- - Significant enhancements to all unit tests across the board.
+ - Significant enhancements to all unit tests across the board.  See `docs/DEV.md`
+   for info about contributing changes to the project.
 
 #### 2.5.1 - 2017-09-06
 

diff --git a/Vagrantfile b/Vagrantfile
@@ -47,6 +47,7 @@ Vagrant.configure(2) do |config|
     guest.vm.box_version = "7.0"
     guest.vm.provision :shell, inline: "yum install -y java-1.7.0-openjdk-headless unzip curl"
     # unit tests required passing along env vars in ssh commands
+    guest.vm.provision :shell, inline: "echo 'forcing sshd to accept env'; echo 'UseDNS no' >> /etc/ssh/sshd_config"
     guest.vm.provision :shell, inline: "echo 'forcing sshd to accept env'; echo 'AcceptEnv *' >> /etc/ssh/sshd_config"
     guest.vm.provision :shell, inline: "systemctl restart sshd.service"
   end

diff --git a/docs/DEV.md b/docs/DEV.md
@@ -1,53 +1,65 @@
 Stork by Fizzed
 ===============
 
+## Overview
+
+Stork heavily interacts with various operating systems.  While many unit tests
+are designed to run locally (and not in a virtual machine), to truly do development
+with Stork you'll want to verify what you changed works across a number of
+operating systems.
+
 ## Vagrant
 
 Make sure you have at least Vagrant v1.9.8 installed as well as the extension pack. 
 You can verify your version by running:
 
     vagrant -v
 
-Stork interacts with various operating systems.  While many unit tests are designed
-to run locally (and not in a virtual machine), the rest of them require 1 or
-more virtual machines to run. Vagrant is used to help setup virtual machines running
-various operating systems so we can run unit tests against them.  The unit tests
-are designed to detect what's virtual machines are running and then run tests
-against them -- so you only need to spin up what you'd like to test against. 
-Let's run thru an "ubuntu14" example:
+Vagrant is used to help setup virtual machines running various operating systems
+so we can run unit tests against them.  The unit tests are designed to detect what's
+virtual machines are running and then run tests against them -- so you only need to
+spin up what you'd like to test against. Let's run thru an "ubuntu14" example:
 
 To spin up an Ubuntu 14.04 virtual machine:
 
     vagrant up ubuntu14
 
-To run unit tests against a specific virtual machine:
+## Running Unit Tests
+
+To run all unit tests against a specific virtual machine:
 
     mvn test -Dhost=ubuntu14
 
-To run just the launchers test:
+To test only the stork-launcher module on that host:
+
+    mvn -am -pl stork-launcher test -DfailIfNoTests=false -Dtest=com.fizzed.stork.launcher.*Test -Dhost=ubuntu14 
+
+To test only the stork-deploy module on that host:
 
-     mvn -am -pl stork-launcher-test test-compile test -Dhost=ubuntu14 -DfailIfNoTests=false -Dtest=com.fizzed.stork.test.LauncherTest
+    mvn -am -pl stork-deploy test -DfailIfNoTests=false -Dtest=com.fizzed.stork.deploy.*Test -Dhost=ubuntu14 
+
+To run your tests locally, just use the host of `local`:
+
+    mvn test -Dhost=local
+
+## Testing for Windows on Linux/Mac
 
 To run tests against Windows (if on Linux or OSX):
 
     vagrant up windows10
 
 Be patient as the image is ~6GB to download.  Once ready, you'll need to make
-sure Java 8 is on it.  Open up VirtualBox, double click the vm, then open powershell
+sure Java 8 is on it.  We didn't have time to make the vagrant install do all
+the prep, so there's a couple manual steps.  Open up VirtualBox, double click
+the vm, then open powershell:
 
     choco install -y jdk8
 
-Then run your tests
+Then you can run any of the tests above against the host `windows10`:
 
     mvn test -Dhost=windows10
 
-If you change how the launchers work, please look at creating a unit test for
-them in `stork-launcher-test`.  Validating your change doesn't break across
-numerous operating systems is important.
-
-## Windows
-
-Its incredibly important your windows scripts have correct line endings or
+NOTE: its incredibly important your windows scripts have correct line endings or
 the Windows cmd.exe interpreter will give you strange results.
 
-    unix2dos stork-launcher/src/main/resources/com/fizzed/stork/launcher/windows/*
+    unix2dos stork-launcher/src/main/resources/com/fizzed/stork/launcher/windows/*
diff --git a/stork-deploy/src/main/java/com/fizzed/stork/deploy/SystemdHelper.java b/stork-deploy/src/main/java/com/fizzed/stork/deploy/SystemdHelper.java
@@ -50,18 +50,23 @@ static public void modifyForInstall(Logger log, Assembly assembly, Deployment in
                 List<String> modifiedSystemdLines
                     = Files.lines(systemdServiceFile)
                         .map((line) -> {
-                            if (line.startsWith("ExecStart=") || line.startsWith("PIDFile=")) {
-                                int equalsPos = line.indexOf("=");
+                            int equalsPos = line.indexOf("=");
+                            if (equalsPos > 0) {
+                                String key = line.substring(0, equalsPos);
+                                String value = line.substring(equalsPos+1).trim();
+
+                                // does the value reference a /bin or /run?
                                 int dirPos = indexOfAny(line, new String[] { "/bin", "/run" });
+
                                 if (dirPos > 0) {
                                     return line.substring(0, equalsPos+1)
                                         + install.getCurrentDir()
                                         + line.substring(dirPos);
+                                } else if (key.equalsIgnoreCase("user")) {
+                                    return "User=" + install.getUser().orElse("");
+                                } else if (key.equalsIgnoreCase("group")) {
+                                    return "Group=" + install.getGroup().orElse("");
                                 }
-                            } else if (line.startsWith("User=")) {
-                                return "User=" + install.getUser().orElse("");
-                            } else if (line.startsWith("Group=")) {
-                                return "Group=" + install.getGroup().orElse("");
                             }
                             return line;
                         })

diff --git a/stork-deploy/src/test/java/com/fizzed/stork/deploy/DeployerDaemonTest.java b/stork-deploy/src/test/java/com/fizzed/stork/deploy/DeployerDaemonTest.java
@@ -52,7 +52,6 @@ public void deploy() throws Exception {
         // assume its unix for now
         UnixTarget target = (UnixTarget)Targets.connect(getHostUri());
 
-        // make sure app does not exist on host
         target.sshExec(true, true, "kill $(ps aux | grep java | grep -v grep | awk \"{print \\$2}\")")
             .exitValues(0, 1, 2)
             .run();
@@ -66,18 +65,19 @@ public void deploy() throws Exception {
             new Deployer().deploy(assembly, options, target);
         }
 
-        // is the server running on port
+        // is the server running on port?
         String output
             = target.sshExec(false, false, "curl", "http://localhost:18745")
                 .exitValues(0)
-                .pipeOutput(Streamables.captureOutput())
-                .runResult()
-                .map(Actions::toCaptureOutput)
+                .runCaptureOutput()
                 .asString();
 
         assertThat(output, containsString("Hello World!"));
 
 
+
+
+
         //
         // verify upgrade works too
         //
@@ -89,9 +89,7 @@ public void deploy() throws Exception {
         output
             = target.sshExec(false, false, "curl", "http://localhost:18745")
                 .exitValues(0)
-                .pipeOutput(Streamables.captureOutput())
-                .runResult()
-                .map(Actions::toCaptureOutput)
+                .runCaptureOutput()
                 .asString();
 
         assertThat(output, containsString("Hello World!")); 

diff --git a/stork-deploy/src/test/launchers/stork-daemon1.yml b/stork-deploy/src/test/launchers/stork-daemon1.yml
@@ -9,4 +9,4 @@ working_dir_mode: APP_HOME
 min_java_version: "1.7"
 min_java_memory: 32
 max_java_memory: 64
-symlink_java: false
+symlink_java: true
diff --git a/stork-launcher/src/main/java/com/fizzed/stork/launcher/Configuration.java b/stork-launcher/src/main/java/com/fizzed/stork/launcher/Configuration.java
@@ -94,6 +94,11 @@ static public enum DaemonMethod {
     private Integer minJavaMemoryPct = null;
     private Integer maxJavaMemoryPct = null;
 
+    // java processes that are daemons should always add the -Xrs option
+    // (especially if you are running via systemd) - otherwise systemd will
+    // report the process exited with status code of 143...
+    private boolean includeJavaXrs = true;
+
     // best effort to symlink java binary so process is named something more
     // friendly for users (only safe for daemons with unique names)
     // default name is "<app name>-java"
@@ -281,6 +286,14 @@ public void setLibDir(String libDir) {
         this.libDir = libDir;
     }
 
+    public boolean isIncludeJavaXrs() {
+        return includeJavaXrs;
+    }
+
+    public void setIncludeJavaXrs(boolean includeJavaXrs) {
+        this.includeJavaXrs = includeJavaXrs;
+    }
+
     public String getMinJavaVersion() {
         return minJavaVersion;
     }

diff --git a/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-daemon-nohup.ftl b/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-daemon-nohup.ftl
@@ -5,7 +5,7 @@
 
 usage()
 {
-    echo "Usage: $0 [--start|--start-run|--stop|--run|--status}"
+    echo "Usage: $0 [--start|--stop|--exec|--run|--status}"
     exit 1
 }
 
@@ -41,7 +41,6 @@ if [ -f "$NOHUP_OUT" ] && [ ! -w "$NOHUP_OUT" ]; then
     exit 1
 fi
 
-
 case "$APP_ACTION_ARG" in
 
   --start)
@@ -82,19 +81,24 @@ case "$APP_ACTION_ARG" in
     fi
     ;;
 
-  --start-run)
+  --exec)
     # best choice for running from systemd
-    printf "Starting $NAME: "
+    echo "Starting $NAME: ..."
 
     # some launcher frameworks manage the PID (this skips the check entirely)
     # only enable this env var if you know what you're doing
     if [ "$SKIP_PID_CHECK" = "0" ]; then
         verifyNotRunning $APP_PID_FILE
     fi
 
-    "$JAVA_BIN" $RUN_ARGS &
-    PID=$!
-    echo $PID > $APP_PID_FILE
+    # take pid of shell for pid lock
+    echo $$ > $APP_PID_FILE
+
+    # best effort to remove pid file upon exit via trap
+    trap 'echo "Removing pid file $APP_PID_FILE"; rm -f "$APP_PID_FILE"' 2 3 6 15
+
+    # shell will now become the java process :-)
+    exec $JAVA_BIN $RUN_ARGS
     ;;
 
   --stop)
@@ -116,9 +120,12 @@ case "$APP_ACTION_ARG" in
 
     # take pid of shell for pid lock
     echo $$ > $APP_PID_FILE
+
     # best effort to remove pid file upon exit via trap
-    trap 'echo "cleaning up pid file: $APP_PID_FILE"; rm -f "$APP_PID_FILE"' 2 3 6 15
-    eval $RUN_CMD
+    trap 'echo "Removing pid file $APP_PID_FILE"; rm -f "$APP_PID_FILE"' 2 3 6 15
+
+    # eval will passthru SIGHUP and allows you to CTRL-C an app in foreground
+    eval \"$JAVA_BIN\" $RUN_ARGS
     ;;
 
   --status)

diff --git a/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-functions.sh b/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-functions.sh
@@ -365,7 +365,8 @@ verifyNotRunning()
   then
     if running $TMPPID
     then
-      echo "Already running!!"
+      PID=`cat $TMPPID 2>/dev/null`
+      echo "$NAME is currently running with pid $PID"
       exit 1
     else
       # dead pid file - remove
@@ -387,7 +388,7 @@ checkRunning()
       rm -f $TMPPID
     fi
   else
-    echo "Warning: app is not running!"
+    echo "$NAME is not currently running!"
   fi
 }
 
@@ -405,7 +406,7 @@ stopJavaApp()
   done
   if [ ! $timeout -gt 0 ]
   then 
-      echo "Unable to kill app within timeout; may need to kill it manually [pid: $TMPPID]"
+      echo "Unable to kill $NAME within timeout; may need to kill it manually [pid: $TMPPID]"
   else
       rm -f $TMPPID
   fi

diff --git a/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-header.ftl b/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-header.ftl
@@ -21,6 +21,7 @@ MAIN_CLASS="${config.mainClass}"
 [ -z "$WORKING_DIR_MODE" ] && WORKING_DIR_MODE="${config.workingDirMode}"
 [ -z "$MIN_JAVA_VERSION" ] && MIN_JAVA_VERSION="${config.minJavaVersion}"
 [ -z "$SYMLINK_JAVA" ] && SYMLINK_JAVA="${config.symlinkJava?string("1", "0")}"
+[ -z "$INCLUDE_JAVA_XRS" ] && INCLUDE_JAVA_XRS="${config.includeJavaXrs?string("1", "0")}"
 
 #
 # working directory

diff --git a/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-java.ftl b/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/script-java.ftl
@@ -54,10 +54,8 @@ APP_ACTION_ARG=
 
 # first arg for a daemon is the action to do such as start vs. stop
 if [ "$TYPE" = "DAEMON" ] && [ $# -gt 0 ]; then
-  APP_ACTION_ARG=$1
-  shift
-  # append system property
-  JAVA_ARGS="$JAVA_ARGS -Dlauncher.action=$APP_ACTION_ARG"
+    APP_ACTION_ARG=$1
+    shift
 fi
 
 # append extra app and java args
@@ -93,6 +91,15 @@ done
 SYS_MEM_MB=`getSystemMemoryMB`
 logLauncherDebug "detected system memory: $SYS_MEM_MB MB"
 
+#
+# include -Xrs flag?
+#
+if [ "$INCLUDE_JAVA_XRS" = "1" ]; then
+    if [ ! `echo "$JAVA_ARGS" | grep -q '\-Xrs '` ]; then
+        # prepend the flag on
+        JAVA_ARGS="-Xrs $JAVA_ARGS"
+    fi
+fi
 
 #
 # add max memory java option (if specified)
@@ -164,7 +171,7 @@ fi
 
 # NOTE: placing double/single quotes around classpath causes an issues using
 # --start with a small number of systemd versions
-RUN_ARGS="-Dlauncher.name=$NAME -Dlauncher.type=$RUN_TYPE -Dlauncher.app.dir=$APP_HOME -classpath $APP_JAVA_CLASSPATH $JAVA_ARGS $MAIN_CLASS $APP_ARGS"
+RUN_ARGS="-Dlauncher.name=$NAME -Dlauncher.type=$RUN_TYPE -Dlauncher.app.dir=$APP_HOME $JAVA_ARGS -classpath $APP_JAVA_CLASSPATH $MAIN_CLASS $APP_ARGS"
 RUN_CMD="\"$JAVA_BIN\" $RUN_ARGS"
 
 #

diff --git a/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/systemd-daemon.ftl b/stork-launcher/src/main/resources/com/fizzed/stork/launcher/linux/systemd-daemon.ftl
@@ -4,7 +4,8 @@ After=network.target
 
 [Service]
 EnvironmentFile=/etc/default/${config.name}
-ExecStart=${config.getPlatformPrefixDir("LINUX")}/${config.name}/bin/${config.name} --start-run
+ExecStart=${config.getPlatformPrefixDir("LINUX")}/${config.name}/bin/${config.name} --exec
+ExecStop=${config.getPlatformPrefixDir("LINUX")}/${config.name}/bin/${config.name} --stop
 PIDFile=${config.getPlatformPrefixDir("LINUX")}/${config.name}/run/${config.name}.pid
 KillMode=process
 Restart=on-failure

diff --git a/stork-launcher/src/main/resources/com/fizzed/stork/launcher/windows/batch-header.ftl b/stork-launcher/src/main/resources/com/fizzed/stork/launcher/windows/batch-header.ftl
@@ -35,6 +35,7 @@ set TYPE=${config.type}
 set MAIN_CLASS=${config.mainClass}
 if "%MIN_JAVA_VERSION%"=="" set MIN_JAVA_VERSION=${config.minJavaVersion}
 if "%WORKING_DIR_MODE%"=="" set WORKING_DIR_MODE=${config.workingDirMode}
+if "%INCLUDE_JAVA_XRS%"=="" set INCLUDE_JAVA_XRS=${config.includeJavaXrs?string("1", "0")}
 
 @REM echo temporarily change working directory to get good abs path for home
 pushd %APP_HOME_REL%