serve: Ensure redis-server saves after signal

This is part one of improved shutdown handling; part two will come in a
future commit involving a more graceful shutdown of the main node
process itself.

I began to notice that running `./go serve` to launch the redis-server,
using the app, and then killing the script quickly resulted in data
loss. This didn't happen when running redis-server manually, as a signal
sent to that process would result in it saving its data before exiting.
The problem appeared to be that signals sent to the `./go serve` script
would be sent simultaneously to the redis-server process, under which
circumstances it wouldn't save its data automatically.

The script now launches redis-server in a separate process group so that
any signals sent to the script aren't sent to the redis-server process.
The custom-links server itself now runs as a background process so that
the it can be managed explicitly via a trap on the TERM, INT, and HUP
signals. Finally, an EXIT trap sends the `shutdown save` command to the
redis-server and waits for the process to exit to complete a graceful
shutdown with all data saved.

The redis-server also now runs with `--appendonly yes` specified,
providing more durability against data loss.

This also marks the first time Bats tests have been added for any of the
scripts. These are now included as part of `./go test`.

The `./go serve` script is pretty well-tested at this point, but a few
notes:

* `scripts/lib/config-json` needs tests specific to it
* The new helper functions added to `tests/scripts/serve.bats`, namely
  `run_in_background`, `wait_for_background_output`, and
  `stop_background_run`, should get moved into the `go-script-bash`
  framework.
* `scripts/lib/config-json` and parts of `scripts/serve` might make
  sense to move into `go-script-bash` as well.
* Coverage for `./go test scripts` isn't yet enabled.

.gitignore

@@ -3,6 +3,7 @@
 .config/env.local
 .coverage/
 coverage/
+appendonly.aof
 dump.rdb
 node_modules/
 npm.log

scripts/lib/config-json

@@ -0,0 +1,36 @@
+#! /usr/local/env bash
+#
+# Access to configuration values in JSON files
+#
+# Requires that `jq` is installed on the system.
+
+# Sets a variable based on an environment variable or configuration file entry
+#
+# Arguments:
+#   config_file_path:   Path to the configuration file
+#   config_entry_name:  Name of the configuration file entry, e.g. `REDIS_HOST`
+#   result_var_name:    Name of caller's variable into which to store the value 
+#   default_value:      Value to assign if neither env or config var is set
+#
+# Returns:
+#   Zero if `result_var_name` has been set with either an env or config var
+#     value; nonzero if it's been set using the default value
+cl.get_config_variable() {
+  local config_file_path="$1"
+  local config_entry_name="$2"
+  local result_var_name="$3"
+  local default_value="$4"
+  local env_var_name="CUSTOM_LINKS_${config_entry_name}"
+  local config_value="${!env_var_name:-null}"
+  local result='0'
+
+  if [[ "$config_value" == 'null' ]]; then
+    config_value="$(jq -r ".${config_entry_name}" "$config_file_path")"
+  fi
+  if [[ "$config_value" == 'null' ]]; then
+    config_value="$default_value"
+    result='1'
+  fi
+  printf -v "$result_var_name" '%s' "$config_value"
+  return "$result"
+}

scripts/serve

@@ -10,7 +10,9 @@
 #
 # Environment variables:
 #   CUSTOM_LINKS_REDIS_PORT      Port on which redis-server will run/is running
-#   CUSTOM_LINKS_REDIS_LOG_FILE  Log file for automatically-started redis-server
+#   CUSTOM_LINKS_REDIS_DIR       Directory in which redis-server will save files
+#   CUSTOM_LINKS_REDIS_LOG_PATH  Log file for automatically-started redis-server
+#   CUSTOM_LINKS_REDIS_TIMEOUT   Max seconds to wait for redis to start
 #
 # If redis-server is already running, it must be running on either the default
 # port or on the port specified by CUSTOM_LINKS_REDIS_PORT (or by REDIS_PORT in
@@ -22,10 +24,10 @@
 # CUSTOM_LINKS_REDIS_LOG_PATH or {{root}}/redis.log by default. (This variable
 # cannot be defined in the <config-file>.)
 
+export CUSTOM_LINKS_CONFIG_PATH="${CUSTOM_LINKS_CONFIG_PATH:-test-config.json}"
 export CUSTOM_LINKS_REDIS_LOG_PATH="${CUSTOM_LINKS_REDIS_LOG_PATH:-redis.log}"
-export CUSTOM_LINKS_REDIS_PID=''
 
-. "$_GO_USE_MODULES" 'log'
+. "$_GO_USE_MODULES" 'log' 'config-json'
 
 cl.serve_tab_completion() {
   local word_index="$1"
@@ -38,40 +40,93 @@ cl.serve_tab_completion() {
   fi
 }
 
-cl.serve_launch_redis() {
-  local server_config="$1"
-  local args=('redis-server')
-  local redis_port="${CUSTOM_LINKS_REDIS_PORT:-null}"
-  local redis_regex='[r]edis-server .*:'
+cl.serve_ensure_redis_is_running() {
+  local server_config="${1:-$CUSTOM_LINKS_CONFIG_PATH}"
+  local redis_host
+  local redis_port
 
-  if [[ -n "${CUSTOM_LINKS_REDIS_HOST}" ]]; then
-    return
-  fi
-  if [[ "$redis_port" == 'null' ]] && command -v jq >/dev/null; then
-    redis_port="$(jq '.REDIS_PORT' "$server_config")"
-  fi
-  if [[ "$redis_port" == 'null' ]]; then
-    redis_port='6379'
-  fi
-  args+=('--port' "$redis_port")
-  redis_regex+="$redis_port"
+  cl.get_config_variable "$server_config" 'REDIS_HOST' 'redis_host'
+  cl.get_config_variable "$server_config" 'REDIS_PORT' 'redis_port' '6379'
 
-  if grep -q -- "$redis_regex" <(ps aux); then
-    return
+  if [[ -n "$redis_host" ]] || pgrep -fq "redis-server .*:$redis_port"; then
+    cl.serve_wait_for_redis "$redis_host" "$redis_port"
+  else
+    cl.serve_launch_redis "$redis_port"
   fi
+}
+
+cl.serve_launch_redis() {
+  local redis_port="$1"
+  local redis_pid
+
   @go.log RUN Launching redis-server on port "$redis_port"
-  "${args[@]}" >> "$CUSTOM_LINKS_REDIS_LOG_PATH" 2>&1 &
-  CUSTOM_LINKS_REDIS_PID="$!"
+  args=('redis-server' '--port' "$redis_port" '--appendonly' 'yes')
+  args+=('--dir' "${CUSTOM_LINKS_REDIS_DIR:-$_GO_ROOTDIR}")
+
+  # Setting monitor mode launches redis-server in a separate process group,
+  # preventing signals sent to this process from terminating it.
+  set -m
+  "${args[@]}" >>"$CUSTOM_LINKS_REDIS_LOG_PATH" 2>&1 &
+  set +m
+  redis_pid="$!"
 
-  if ! kill -0 "$CUSTOM_LINKS_REDIS_PID" 2>/dev/null; then
+  if ! cl.serve_wait_for_redis '' "$redis_pid"; then
+    kill "$redis_pid" >/dev/null 2>&1
     @go.log FATAL Failed to launch redis-server
   fi
-  trap "redis-cli -p $redis_port shutdown save" EXIT
-  @go.log INFO redis-server running as PID "$CUSTOM_LINKS_REDIS_PID"
+  trap "cl.serve_exit_trap $redis_pid $redis_port" EXIT
+  @go.log INFO redis-server running as PID "$redis_pid"
+}
+
+cl.serve_wait_for_redis() {
+  local redis_host="$1"
+  local redis_pid="$2"
+  local timeout="${CUSTOM_LINKS_REDIS_TIMEOUT:-5}"
+
+  @go.log INFO "Waiting for Redis server at ${redis_host:-*}:${redis_port}"
+  # Sleep for a quarter-second as this is enough for small installations.
+  sleep 0.25
+
+  while ! nc -z "$redis_host" "$redis_port" >/dev/null 2>&1; do
+    if [[ "$((--timeout))" -lt '0' ]]; then
+      return 1
+    fi
+    sleep 1
+  done
+}
+
+cl.serve_exit_trap() {
+  local redis_pid="$1"
+  local redis_port="$2"
+
+  @go.log RUN Shutting down redis-server
+  redis-cli -p "$redis_port" shutdown save
+  wait "$redis_pid"
+  @go.log INFO redis-server shutdown complete
+}
+
+cl.serve_run_custom_links_server() {
+  local server_pid
+
+  @go.log RUN Launching custom-links server
+  node "$_GO_ROOTDIR/index.js" "$server_config" &
+  server_pid="$!"
+
+  if ! kill -0 "$server_pid" 2>/dev/null; then
+    @go.log FATAL Failed to launch custom-links server
+  fi
+  @go.log INFO custom-links server running as pid "$server_pid"
+
+  # Inspired by: https://veithen.github.io/2014/11/16/sigterm-propagation.html
+  trap "kill -TERM $server_pid" TERM INT HUP
+  wait "$server_pid"
+  trap - TERM INT HUP
+  wait "$server_pid"
+  @go.log INFO custom-links server shutdown complete
 }
 
 cl.serve() {
-  local server_config="${1:-$_GO_ROOTDIR/test-config.json}"
+  local server_config="$1"
 
   case "$1" in
   --complete)
@@ -82,8 +137,8 @@ cl.serve() {
     ;;
   esac
 
-  cl.serve_launch_redis "$server_config"
-  node "$_GO_ROOTDIR/index.js" "$server_config"
+  cl.serve_ensure_redis_is_running "$server_config"
+  cl.serve_run_custom_links_server "$server_config"
 }
 
 cl.serve "$@"

scripts/setup

@@ -6,12 +6,26 @@ export CL_PLATFORM="${CL_PLATFORM:-$(node -e \
   'console.log(process.platform)')}"
 
 cl.check_for_prerequisite_tools() {
+  local required=('redis-server' 'jq' 'nc')
+  local cmd_name
+  local missing=()
+
   if ! command -v node >/dev/null; then
     @go.printf 'Please install Node.js before continuing.\n' >&2
     return 1
-  elif [[ "$CL_PLATFORM" != 'win32' ]] &&
-    ! command -v redis-server >/dev/null; then
-    @go.printf 'Please install redis-server before continuing.\n' >&2
+  elif [[ "$CL_PLATFORM" == 'win32' ]]; then
+    return
+  fi
+
+  for cmd_name in "${required[@]}"; do
+    if ! command -v "$cmd_name" >/dev/null; then
+      missing+=("$cmd_name")
+    fi
+  done
+
+  if [[ "${#missing[@]}" -ne '0' ]]; then
+    @go.printf 'Please install the following programs before continuing:\n' >&2
+    printf '  %s\n' "${missing[@]}" >&2
     return 1
   fi
 }

scripts/test

@@ -81,6 +81,9 @@ _test() {
   if ! @go.log_command @go test end-to-end; then
     result='1'
   fi
+  if ! @go.log_command @go test scripts; then
+    result='1'
+  fi
   if [[ -n "$coverage_run" ]] && ! cl.generate_coverage_report "$result"; then
     result='1'
   fi

scripts/test.d/scripts

@@ -0,0 +1,18 @@
+#! /usr/bin/env bash
+#
+# Tests `./go` scripts
+#
+# 
+
+# Passes all arguments through to `@go.bats_main` from `lib/bats-main`.
+_test_main() {
+  local _GO_BATS_COVERAGE_INCLUDE
+  _GO_BATS_COVERAGE_INCLUDE=('scripts/')
+  local _GO_COVERALLS_URL='https://coveralls.io/github/mbland/custom-links'
+
+  . "$_GO_USE_MODULES" 'bats-main'
+  # Tab completions
+  @go.bats_main "$@"
+}
+
+_test_main "$@"

tests/scripts/environment.bash

@@ -0,0 +1,5 @@
+. "$_GO_CORE_DIR/lib/testing/environment"
+. "$_GO_CORE_DIR/lib/bats/assertions"
+
+set_bats_test_suite_name "${BASH_SOURCE[0]%/*}"
+remove_bats_test_dirs