Format errors and warnings with regard to config files in a manner similar to lager

Use exit code 64 when configuration is invalid

Correctly deal with RABBITMQ_PID_FILE

Fix bug in setting up RABBITMQ_PID_FILE as well as printing multi-line warnings and errors

Calculate indent

5 files changed, 97 insertions, 47 deletions

diff --git a/scripts/rabbitmq-env b/scripts/rabbitmq-env
index 3181c1276a..d3348620ed 100755
--- a/scripts/rabbitmq-env
+++ b/scripts/rabbitmq-env
@@ -45,6 +45,40 @@ if [ -z "$RABBITMQ_SCRIPTS_DIR" ]; then
     RABBITMQ_SCRIPTS_DIR=`dirname $SCRIPT_PATH`
 fi
 
+_rmq_env_now()
+{
+    date '+%Y-%m-%d %H:%M:%S'
+}
+
+_rmq_env_print()
+{
+    _rmq_env_tmp="$1"
+    _rmq_env_tmp_len="${#_rmq_env_tmp}"
+    shift
+    printf '%s %s %s\n' "$(_rmq_env_now)" "$_rmq_env_tmp" "$1" 1>&2
+    shift
+    _rmq_env_print_line=''
+    _rmq_env_indent="$((_rmq_env_tmp_len + 21))"
+    for _rmq_env_print_line in "$@"
+    do
+        printf "%${_rmq_env_indent}s%s\n" ' ' "$_rmq_env_print_line" 1>&2
+    done
+    unset _rmq_env_print_line
+    unset _rmq_env_indent
+    unset _rmq_env_tmp_len
+    unset _rmq_env_tmp
+}
+
+_rmq_env_perr()
+{
+    _rmq_env_print '[error]' "$@"
+}
+
+_rmq_env_pwarn()
+{
+    _rmq_env_print '[warning]' "$@"
+}
+
 rmq_realpath() {
     local path=$1
 
@@ -77,6 +111,11 @@ ESCRIPT_DIR="${RABBITMQ_HOME}/escript"
 ## Set defaults
 . ${RABBITMQ_SCRIPTS_DIR}/rabbitmq-defaults
 
+# We save the current value of $RABBITMQ_PID_FILE in case it was set by
+# an init script. If $CONF_ENV_FILE overrides it again, we must ignore
+# it and warn the user.
+saved_RABBITMQ_PID_FILE="$RABBITMQ_PID_FILE"
+
 ## Get configuration variables from the configure environment file
 [ "x" = "x$RABBITMQ_CONF_ENV_FILE" ] && RABBITMQ_CONF_ENV_FILE=${CONF_ENV_FILE}
 [ -f ${RABBITMQ_CONF_ENV_FILE} ] && . ${RABBITMQ_CONF_ENV_FILE} || true
@@ -100,21 +139,6 @@ DEFAULT_MAX_NUMBER_OF_ATOMS=5000000
 ## Common server defaults
 SERVER_ERL_ARGS=" +P $RABBITMQ_MAX_NUMBER_OF_PROCESSES +t $RABBITMQ_MAX_NUMBER_OF_ATOMS +stbt $RABBITMQ_SCHEDULER_BIND_TYPE +zdbbl $RABBITMQ_DISTRIBUTION_BUFFER_SIZE "
 
-# We save the current value of $RABBITMQ_PID_FILE in case it was set by
-# an init script. If $CONF_ENV_FILE overrides it again, we must ignore
-# it and warn the user.
-saved_RABBITMQ_PID_FILE=$RABBITMQ_PID_FILE
-
-if [ "$saved_RABBITMQ_PID_FILE" -a \
-     "$saved_RABBITMQ_PID_FILE" != "$RABBITMQ_PID_FILE" ]; then
-    echo "WARNING: RABBITMQ_PID_FILE was already set by the init script to:" 1>&2
-    echo "           $saved_RABBITMQ_PID_FILE" 1>&2
-    echo "         The value set in rabbitmq-env.conf is ignored because it" 1>&2
-    echo "         would break the init script." 1>&2
-
-    RABBITMQ_PID_FILE="$saved_RABBITMQ_PID_FILE"
-fi
-
 [ "x" = "x$RABBITMQ_USE_LONGNAME" ] && RABBITMQ_USE_LONGNAME=${USE_LONGNAME}
 if [ "xtrue" = "x$RABBITMQ_USE_LONGNAME" ] ; then
     RABBITMQ_NAME_TYPE=-name
@@ -230,8 +254,21 @@ rmq_normalize_path_var \
     RABBITMQ_MNESIA_BASE \
     RABBITMQ_MNESIA_DIR
 
-[ "x" = "x$RABBITMQ_PID_FILE" ] && RABBITMQ_PID_FILE=${PID_FILE}
-[ "x" = "x$RABBITMQ_PID_FILE" ] && RABBITMQ_PID_FILE=${RABBITMQ_MNESIA_DIR}.pid
+[ "x" = "x$RABBITMQ_PID_FILE" ] && RABBITMQ_PID_FILE="$PID_FILE"
+
+if [ -n "$saved_RABBITMQ_PID_FILE" ] && \
+    [ "$saved_RABBITMQ_PID_FILE" != "$RABBITMQ_PID_FILE" ]
+then
+    _rmq_env_pwarn 'RABBITMQ_PID_FILE was already set by the init script to:' \
+        "$saved_RABBITMQ_PID_FILE" \
+        'The value set in rabbitmq-env.conf is ignored because it would break the init script.'
+
+    RABBITMQ_PID_FILE="$saved_RABBITMQ_PID_FILE"
+fi
+
+# Note: at this point, no RABBITMQ_PID_FILE is set so we use the mnesia dir value
+[ "x" = "x$RABBITMQ_PID_FILE" ] && RABBITMQ_PID_FILE="${RABBITMQ_MNESIA_DIR}.pid"
+
 rmq_normalize_path_var RABBITMQ_PID_FILE
 
 [ "x" = "x$RABBITMQ_BOOT_MODULE" ] && RABBITMQ_BOOT_MODULE=${BOOT_MODULE}
diff --git a/scripts/rabbitmq-server b/scripts/rabbitmq-server
index 86f213b3de..36dca38882 100755
--- a/scripts/rabbitmq-server
+++ b/scripts/rabbitmq-server
@@ -93,17 +93,17 @@ RABBITMQ_CONFIG_FILE_NOEX=$(get_noex ${RABBITMQ_CONFIG_FILE})
 if [ "${RABBITMQ_CONFIG_FILE_NOEX}" = "${RABBITMQ_CONFIG_FILE}" ]; then
     if [ -f "${RABBITMQ_CONFIG_FILE_NOEX}.config" ]; then
         if [ -f "${RABBITMQ_CONFIG_FILE_NOEX}.conf" ]; then
-            # Both files exist. Print a warning
-            echo "WARNING: Both old (.config) and new (.conf) format config files exist."
-            echo "WARNING: Using the old format config file: ${RABBITMQ_CONFIG_FILE_NOEX}.config"
-            echo "WARNING: Please update your config files to the new format and remove the old file"
+            # Both files exist. Print a warning.
+            _rmq_env_pwarn 'Both old (.config) and new (.conf) format config files exist.' \
+                "Using the old format config file: ${RABBITMQ_CONFIG_FILE_NOEX}.config" \
+                'Please update your config files to the new format and remove the old file.'
         fi
         RABBITMQ_CONFIG_FILE="${RABBITMQ_CONFIG_FILE_NOEX}.config"
     elif [ -f "${RABBITMQ_CONFIG_FILE_NOEX}.conf" ]; then
         RABBITMQ_CONFIG_FILE="${RABBITMQ_CONFIG_FILE_NOEX}.conf"
     else
         if [ -f ${RABBITMQ_ADVANCED_CONFIG_FILE} ]; then
-            echo "WARNING: Using RABBITMQ_ADVANCED_CONFIG_FILE: ${RABBITMQ_ADVANCED_CONFIG_FILE}"
+            _rmq_env_pwarn "Using RABBITMQ_ADVANCED_CONFIG_FILE: ${RABBITMQ_ADVANCED_CONFIG_FILE}"
         fi
         # No config file exist. Use advanced config for -config arg.
         RABBITMQ_CONFIG_ARG_FILE="${RABBITMQ_ADVANCED_CONFIG_FILE}"
@@ -125,23 +125,23 @@ elif [ "${RABBITMQ_CONFIG_FILE_NOEX}.conf" = "${RABBITMQ_CONFIG_FILE}" ]; then
 elif [ "x" != "x${RABBITMQ_CONFIG_FILE}" \
        -a  "${RABBITMQ_CONFIG_FILE_NOEX}" != "${RABBITMQ_CONFIG_FILE}" ]; then
     # Config file has an extension, but it's neither .conf or .config
-    echo "ERROR: Wrong extension for RABBITMQ_CONFIG_FILE: ${RABBITMQ_CONFIG_FILE}"
-    echo "ERROR: extension should be either .conf or .config"
-    exit 1
+    _rmq_env_perr "Wrong extension for RABBITMQ_CONFIG_FILE: ${RABBITMQ_CONFIG_FILE}" \
+        'The extension should be either .conf or .config'
+    exit 64 # EX_USAGE
 fi
 
 RABBITMQ_CONFIG_ARG_FILE_NOEX=$(get_noex ${RABBITMQ_CONFIG_ARG_FILE})
 
 if [ "${RABBITMQ_CONFIG_ARG_FILE_NOEX}.config" != "${RABBITMQ_CONFIG_ARG_FILE}" ]; then
     if [ "${RABBITMQ_CONFIG_ARG_FILE}" = "${RABBITMQ_ADVANCED_CONFIG_FILE}" ]; then
-        echo "ERROR: Wrong extension for RABBITMQ_ADVANCED_CONFIG_FILE: ${RABBITMQ_ADVANCED_CONFIG_FILE}"
-        echo "ERROR: extension should be .config"
-        exit 1
+        _rmq_env_perr "Wrong extension for RABBITMQ_ADVANCED_CONFIG_FILE: ${RABBITMQ_ADVANCED_CONFIG_FILE}" \
+            'The extension should be .config'
+        exit 64 # EX_USAGE
     else
         # We should never got here, but still there should be some explanation
-        echo "ERROR: Wrong extension for ${RABBITMQ_CONFIG_ARG_FILE}"
-        echo "ERROR: extension should be .config"
-        exit 1
+        _rmq_env_perr "Wrong extension for ${RABBITMQ_CONFIG_ARG_FILE}"
+            'The extension should be .config'
+        exit 64 # EX_USAGE
     fi
 fi
 
diff --git a/src/rabbit.erl b/src/rabbit.erl
index 6f36de0aa5..f90e42454f 100644
--- a/src/rabbit.erl
+++ b/src/rabbit.erl
@@ -880,7 +880,7 @@ boot_error(Class, Reason) ->
 
 log_boot_error_and_exit(Reason, Format, Args) ->
     rabbit_log:error(Format, Args),
-    io:format("~nBOOT FAILED~n===========~n" ++ Format ++ "~n", Args),
+    io:format(standard_error, "~nBOOT FAILED~n===========~n" ++ Format ++ "~n", Args),
     timer:sleep(1000),
     exit(Reason).
 
diff --git a/src/rabbit_config.erl b/src/rabbit_config.erl
index 43248df60e..a18e3888c5 100644
--- a/src/rabbit_config.erl
+++ b/src/rabbit_config.erl
@@ -255,30 +255,38 @@ assert_config(none, _) -> ok;
 assert_config(Filename, Env) ->
     assert_config(filename:extension(Filename), Filename, Env).
 
+-define(ERRMSG_INDENT, "                                ").
+
 assert_config(".config", Filename, Env) ->
     case filelib:is_regular(Filename) of
         true ->
             case file:consult(Filename) of
                 {ok, []}    -> {error,
-                                {"ERROR: Config file ~s should not be empty: ~s",
+                                {"Config file ~s should not be empty: ~s",
                                  [Env, Filename]}};
                 {ok, [_]}   -> ok;
                 {ok, [_|_]} -> {error,
-                                {"ERROR: Config file ~s must contain ONE list ended by <dot>: ~s",
+                                {"Config file ~s must contain ONE list ended by <dot>: ~s",
                                  [Env, Filename]}};
                 {error, {1, erl_parse, Err}} ->
-                    {error, {"ERROR: Unable to parse erlang terms from ~s file: ~s~n"
-                             "ERROR: Reason: ~p~n"
-                             "ERROR: Check that the file is in the erlang terms format. " ++
+                    % Note: the sequence of spaces is to indent from the [error] prefix, like this:
+                    %
+                    % 2018-09-06 07:05:40.225 [error] Unable to parse erlang terms from RABBITMQ_ADVANCED_CONFIG_FILE...
+                    %                                 Reason: ["syntax error before: ",[]]
+                    {error, {"Unable to parse erlang terms from ~s file: ~s~n"
+                             ?ERRMSG_INDENT
+                             "Reason: ~p~n"
+                             ?ERRMSG_INDENT
+                             "Check that the file is in erlang term format. " ++
                              case Env of
                                 "RABBITMQ_CONFIG_FILE" ->
-                                    "If you are using the new ini-style format, the file "
-                                    "extension should be '.conf'~n";
+                                    "If you are using the new ini-style format, the file extension should be '.conf'~n";
                                 _ -> ""
                              end,
                              [Env, Filename, Err]}};
                 {error, Err} ->
-                    {error, {"ERROR Unable to parse erlang terms from  ~s file: ~s~n"
+                    {error, {"Unable to parse erlang terms from  ~s file: ~s~n"
+                             ?ERRMSG_INDENT
                              "Error: ~p~n",
                              [Env, Filename, Err]}}
             end;
@@ -286,7 +294,7 @@ assert_config(".config", Filename, Env) ->
             ok
     end;
 assert_config(BadExt, Filename, Env) ->
-    {error, {"ERROR: '~s': Expected extension '.config', got extension '~s' for file '~s'~n", [Env, BadExt, Filename]}}.
+    {error, {"'~s': Expected extension '.config', got extension '~s' for file '~s'~n", [Env, BadExt, Filename]}}.
 
 assert_conf("", _) -> ok;
 assert_conf(Filename, Env) ->
@@ -298,8 +306,9 @@ assert_conf(".conf", Filename, Env) ->
             case file:consult(Filename) of
                 {ok, []} -> ok;
                 {ok, _}  ->
-                    {error, {"ERROR: Wrong format of the config file ~s: ~s~n"
-                             "ERROR: Check that the file is in the new ini-style config format "
+                    {error, {"Wrong format of the config file ~s: ~s~n"
+                             ?ERRMSG_INDENT
+                             "Check that the file is in the new ini-style config format. "
                              "If you are using the old format the file extension should "
                              "be .config~n",
                              [Env, Filename]}};
@@ -310,4 +319,4 @@ assert_conf(".conf", Filename, Env) ->
             ok
     end;
 assert_conf(BadExt, Filename, Env) ->
-    {error, {"ERROR: '~s': Expected extension '.config', got extension '~s' for file '~s'~n", [Env, BadExt, Filename]}}.
+    {error, {"'~s': Expected extension '.config', got extension '~s' for file '~s'~n", [Env, BadExt, Filename]}}.
diff --git a/src/rabbit_prelaunch.erl b/src/rabbit_prelaunch.erl
index 1231f0090e..e43cba728a 100644
--- a/src/rabbit_prelaunch.erl
+++ b/src/rabbit_prelaunch.erl
@@ -26,6 +26,7 @@
 -define(SET_DIST_PORT, 0).
 -define(ERROR_CODE, 1).
 -define(DO_NOT_SET_DIST_PORT, 2).
+-define(EX_USAGE, 64).
 
 %%----------------------------------------------------------------------------
 %% Specs
@@ -62,9 +63,12 @@ stop() ->
 config_file_check() ->
     case rabbit_config:validate_config_files() of
         ok -> ok;
-        {error, {Err, Args}} ->
-            io:format(Err, Args),
-            rabbit_misc:quit(?ERROR_CODE)
+        {error, {ErrFmt, ErrArgs}} ->
+            ErrMsg = io_lib:format(ErrFmt, ErrArgs),
+            {{Year, Month, Day}, {Hour, Minute, Second, Milli}} = lager_util:localtime_ms(),
+            io:format(standard_error, "~b-~2..0b-~2..0b ~2..0b:~2..0b:~2..0b.~b [error] ~s",
+                      [Year, Month, Day, Hour, Minute, Second, Milli, ErrMsg]),
+            rabbit_misc:quit(?EX_USAGE)
     end.
 
 %% Check whether a node with the same name is already running