Merge pull request #304 from rbrw/pdb-5215-dont-crash-on-resources-during-sighup

(PDB-5215) Don't crash on resources during sighup

Author: austb

ext/test/custom-exit-behavior

@@ -4,17 +4,17 @@ set -uexo pipefail
 
 usage() { echo "Usage: $(basename "$0")"; }
 misuse() { usage 1>&2; exit 2; }
-tk() { lein trampoline run "$@"; }
 
 test $# -eq 0 || misuse
 
-tmpdir="$(mktemp -d "test-top-level-cli-XXXXXX")"
+tmpdir="$(mktemp -d "test-custom-exit-behavior-XXXXXX")"
 tmpdir="$(cd "$tmpdir" && pwd)"
 trap "$(printf 'rm -rf %q' "$tmpdir")" EXIT
 
 rc=0
-tk -d -b ext/test/lib/custom-exit-behavior/bootstrap.cfg \
-   1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
+./tk -cp "$(pwd)/test" -- \
+     -d -b <(echo puppetlabs.trapperkeeper.custom-exit-behavior-test/custom-exit-behavior-test-service) \
+     1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
 cat "$tmpdir/out" "$tmpdir/err"
 test "$rc" -eq 7
 grep -F 'Some excitement!' "$tmpdir/out"

ext/test/lib/custom-exit-behavior/bootstrap.cfg

@@ -9,3 +9,4 @@ test $# -eq 0 || misuse
 
 ext/test/top-level-cli
 ext/test/custom-exit-behavior
+ext/test/signal-handling

ext/test/run-all

@@ -0,0 +1,62 @@
+#!/usr/bin/env bash
+
+set -uexo pipefail
+
+usage() { echo "Usage: $(basename "$0")"; }
+misuse() { usage 1>&2; exit 2; }
+
+await-file()
+(
+    local target="$1"
+    set +x
+    while ! test -e "$target"; do sleep 0.1; done
+)
+
+tk_pid=''
+tmpdir=''
+on-exit()
+{
+    if test "$tk_pid"; then
+        kill "$tk_pid"
+        status=0
+        wait "$tk_pid" || status=$?
+        set +x
+        echo tk exited with status "$status (143 is likely)" 1>&2
+        set -x
+    fi
+    rm -rf "$tmpdir"
+}
+trap on-exit EXIT
+
+test $# -eq 0 || misuse
+
+tmpdir="$(mktemp -d "test-signal-handling-XXXXXX")"
+tmpdir="$(cd "$tmpdir" && pwd)"
+
+# Start the test server, which repeatedly writes to the configured
+# target file, and make sure the target changes after a config file
+# change and signal.
+
+target_1="$tmpdir/target-1"
+target_2="$tmpdir/target-2"
+
+cat > "$tmpdir/config.json" <<EOS
+{"signal-test-target": "$target_1"}
+EOS
+
+./tk -cp "$(pwd)/test" -- \
+     --config "$tmpdir/config.json" \
+     -b <(echo puppetlabs.trapperkeeper.signal-handling-test/signal-handling-test-service) &
+tk_pid=$!
+
+await-file "$target_1"
+test exciting = "$(< "$target_1")"
+
+test ! -e "$target_2"
+cat > "$tmpdir/config.json" <<EOS
+{"signal-test-target": "$target_2"}
+EOS
+
+kill -HUP "$tk_pid"
+await-file "$target_2"
+test exciting = "$(< "$target_2")"

ext/test/signal-handling

@@ -14,15 +14,15 @@ trap "$(printf 'rm -rf %q' "$tmpdir")" EXIT
 
 ## Test handling an unknown option
 rc=0
-./tk --invalid-option 1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
+./tk -- --invalid-option 1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
 cat "$tmpdir/out" "$tmpdir/err"
 test "$rc" -eq 1
 grep -F 'Unknown option: "--invalid-option"' "$tmpdir/err"
 
 
 ## Test --help
 rc=0
-./tk --help  1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
+./tk -- --help  1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
 cat "$tmpdir/out" "$tmpdir/err"
 test "$rc" -eq 0
 grep -F 'Path to bootstrap config file' "$tmpdir/out"
@@ -32,7 +32,7 @@ test $(wc -c < "$tmpdir/out") -eq 650
 
 ## Test handling a missing bootstrap file
 rc=0
-./tk frobnicate ... 1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
+./tk -- frobnicate ... 1>"$tmpdir/out" 2>"$tmpdir/err" || rc=$?
 cat "$tmpdir/out" "$tmpdir/err"
 test "$rc" -eq 1
 grep -F 'Unable to find bootstrap.cfg file via --bootstrap-config' "$tmpdir/err"

ext/test/top-level-cli

@@ -93,11 +93,18 @@
         (log/debug (i18n/trs "Loading bootstrap config from current working directory: ''{0}''" config-file-path))
         [config-file-path]))))
 
+(defn- resource [name]
+  ;; Fixes io/resource crashes when context class loader is nil.
+  ;; https://clojure.atlassian.net/browse/CLJ-2431
+  (.getResource (or (.getContextClassLoader (Thread/currentThread))
+                    (ClassLoader/getSystemClassLoader))
+                name))
+
 (schema/defn config-from-classpath :- [(schema/maybe schema/Str)]
   "Check to see if there is a bootstrap config file available on the classpath;
   if so, return it."
   []
-  (when-let [classpath-config (io/resource bootstrap-config-file-name)]
+  (when-let [classpath-config (resource bootstrap-config-file-name)]
     (log/debug (i18n/trs "Loading bootstrap config from classpath: ''{0}''" classpath-config))
     [(str classpath-config)]))
 
@@ -133,7 +140,7 @@
   the config. Throws an exception if there is a problem reading the file or
   the URI can't be loaded"
   [config-path :- schema/Str]
-  (if (fs/file? config-path)
+  (if (fs/readable? config-path)
     (line-seq (io/reader (fs/file config-path)))
     (try
       ; If it's not a file, attempt to read it as a URI

src/puppetlabs/trapperkeeper/bootstrap.clj

@@ -58,7 +58,11 @@
     (edn/read (PushbackReader. (io/reader file)))
 
     #{".yaml" ".yml"}
-    (yaml/parse-string (slurp file))))
+    (yaml/parse-string (slurp file))
+
+    (throw (IllegalArgumentException.
+            (i18n/trs "Config file {0} must end in .conf or other recognized extension"
+                      (-> file str pr-str))))))
 
 (defn override-restart-file-from-cli-data
   [config-data cli-data]

src/puppetlabs/trapperkeeper/config.clj

@@ -210,7 +210,7 @@
 (defn main
   "Launches the trapperkeeper framework. This function blocks until
   trapperkeeper is shut down. This may be called directly, but is also
-  called by `puppetlabs.trapperkeeper.core/-main` if you use
+  called by `puppetlabs.trapperkeeper.main/-main` if you use
   `puppetlabs.trapperkeeper.core` as the `:main` namespace in your
   leinengen project.  Never returns (calls System/exit) after argument
   processing errors, `--help` requests, or calls to `request-shutdown`

src/puppetlabs/trapperkeeper/core.clj

@@ -0,0 +1,34 @@
+(ns puppetlabs.trapperkeeper.signal-handling-test
+  (:require
+   [puppetlabs.trapperkeeper.core :as core]))
+
+(defn- start-test [context get-in-config]
+  (let [continue? (atom true)
+        thread (future
+                 (try ;; future just discards top-level exceptions
+                   (while @continue?
+                     (let [target (get-in-config [:signal-test-target])]
+                       (assert target)
+                       (Thread/sleep 200)
+                       (spit target "exciting")))
+                   (catch Throwable ex
+                     (prn ex)
+                     (throw ex))))]
+    (assoc context
+           :finish-signal-test
+           (fn exit-signal-test []
+             (reset! continue? false)
+             @thread))))
+
+(defn- stop-test [{:keys [finish-signal-test] :as context}]
+  (finish-signal-test)
+  context)
+
+(defprotocol SignalHandlingTestService)
+
+(core/defservice signal-handling-test-service
+  SignalHandlingTestService
+  [[:ConfigService get-in-config]]
+  (init [this context] context)
+  (start [this context] (start-test context get-in-config))
+  (stop [this context] (stop-test context)))

test/puppetlabs/trapperkeeper/signal_handling_test.clj

@@ -1,9 +1,40 @@
 #!/usr/bin/env bash
 
-set -e
+set -ueo pipefail
+
+usage() { echo "Usage: tk JVM_ARG ... -- TK_ARG ..."; }
+misuse() { usage 1>&2; exit 2; }
 
 jar_glob='trapperkeeper-*-SNAPSHOT-standalone.jar'
 
+# Believe last -cp wins for java, and here, any final -cp path will be
+# placed in front of the jar.
+
+cp=''
+jvm_args=()
+while test $# -gt 0; do
+    case "$1" in
+        -h|--help)
+            usage
+            exit 0
+            ;;
+        -cp)
+            shift
+            test $# -gt 0 || misuse
+            cp="$1"
+            shift
+            ;;
+        --)
+            shift
+            break
+            ;;
+        *)
+            shift
+            jvm_args+=("$1")
+            ;;
+    esac
+done
+
 if test "${TRAPPERKEEPER_JAR:-}"; then
     jar="$TRAPPERKEEPER_JAR"
 else
@@ -28,4 +59,11 @@ if ! test -e "$jar"; then
     exit 2
 fi
 
-exec java -jar "$jar" "$@"
+set -x
+if test "$cp"; then
+    cp="$cp:$jar"
+else
+    cp="$jar"
+fi
+
+exec java -cp "$cp" clojure.main -m puppetlabs.trapperkeeper.main "$@"