merge default into bug24332

4 files changed, 56 insertions, 43 deletions

diff --git a/src/rabbit.erl b/src/rabbit.erl
index 8cae7fde86..26d13288da 100644
--- a/src/rabbit.erl
+++ b/src/rabbit.erl
@@ -245,12 +245,14 @@ environment() ->
 
 rotate_logs(BinarySuffix) ->
     Suffix = binary_to_list(BinarySuffix),
-    log_rotation_result(rotate_logs(log_location(kernel),
+    R = log_rotation_result(rotate_logs(log_location(kernel),
                                     Suffix,
                                     rabbit_error_logger_file_h),
                         rotate_logs(log_location(sasl),
                                     Suffix,
-                                    rabbit_sasl_report_file_h)).
+                                    rabbit_sasl_report_file_h)),
+    io:format("Rot Handlers: ~p~n", [gen_event:which_handlers(error_logger)]),
+    R.
 
 %%--------------------------------------------------------------------
 
@@ -447,16 +449,19 @@ ensure_working_log_handlers() ->
                                     error_logger_tty_h,
                                     log_location(kernel),
                                     Handlers),
-
     ok = ensure_working_log_handler(sasl_report_file_h,
                                     rabbit_sasl_report_file_h,
                                     sasl_report_tty_h,
                                     log_location(sasl),
                                     Handlers),
+    case log_location(kernel) of
+        tty -> ok;
+        _   -> error_logger:delete_report_handler(error_logger_tty_h)
+    end,
     ok.
 
-ensure_working_log_handler(OldFHandler, NewFHandler, TTYHandler,
-                           LogLocation, Handlers) ->
+ensure_working_log_handler(OldFHandler, NewFHandler, TTYHandler, LogLocation,
+                           Handlers) ->
     case LogLocation of
         undefined -> ok;
         tty       -> case lists:member(TTYHandler, Handlers) of
@@ -478,7 +483,7 @@ ensure_working_log_handler(OldFHandler, NewFHandler, TTYHandler,
     end.
 
 log_location(Type) ->
-    case application:get_env(Type, case Type of
+    case application:get_env(rabbit, case Type of
                                        kernel -> error_logger;
                                        sasl   -> sasl_error_logger
                                    end) of
diff --git a/src/rabbit_error_logger_file_h.erl b/src/rabbit_error_logger_file_h.erl
index 7e9ebc4fa2..519aac10ad 100644
--- a/src/rabbit_error_logger_file_h.erl
+++ b/src/rabbit_error_logger_file_h.erl
@@ -30,7 +30,8 @@
 %% Used only when swapping handlers in log rotation
 init({{File, Suffix}, []}) ->
     case rabbit_misc:append_file(File, Suffix) of
-        ok  -> ok;
+        ok -> file:delete(File),
+              ok;
         {error, Error} ->
             rabbit_log:error("Failed to append contents of "
                              "log file '~s' to '~s':~n~p~n",
@@ -45,12 +46,21 @@ init({{File, _}, error}) ->
 %% log rotation
 init({File, []}) ->
     init(File);
-init({File, _Type} = FileInfo) ->
+init({File, {error_logger, []}}) ->
     rabbit_misc:ensure_parent_dirs_exist(File),
-    error_logger_file_h:init(FileInfo);
+    init_file(File, error_logger);
 init(File) ->
     rabbit_misc:ensure_parent_dirs_exist(File),
-    error_logger_file_h:init(File).
+    init_file(File, []).
+
+init_file(File, PrevHandler) ->
+    process_flag(trap_exit, true),
+    case file:open(File, [append]) of
+	{ok,Fd} ->
+	    {ok, {Fd, File, PrevHandler}};
+	Error ->
+	    Error
+    end.
 
 handle_event(Event, State) ->
     error_logger_file_h:handle_event(Event, State).
diff --git a/src/rabbit_sasl_report_file_h.erl b/src/rabbit_sasl_report_file_h.erl
index 6f3c5c75bc..d45ed3d33d 100644
--- a/src/rabbit_sasl_report_file_h.erl
+++ b/src/rabbit_sasl_report_file_h.erl
@@ -31,7 +31,8 @@
 %% log rotation
 init({{File, Suffix}, []}) ->
     case rabbit_misc:append_file(File, Suffix) of
-        ok -> ok;
+        ok -> file:delete(File),
+              ok;
         {error, Error} ->
             rabbit_log:error("Failed to append contents of "
                              "sasl log file '~s' to '~s':~n~p~n",
@@ -48,10 +49,19 @@ init({File, []}) ->
     init(File);
 init({File, _Type} = FileInfo) ->
     rabbit_misc:ensure_parent_dirs_exist(File),
-    sasl_report_file_h:init(FileInfo);
+    init_file(FileInfo);
 init(File) ->
     rabbit_misc:ensure_parent_dirs_exist(File),
-    sasl_report_file_h:init({File, sasl_error_logger_type()}).
+    init_file({File, sasl_error_logger_type()}).
+
+init_file({File, Type}) ->
+    process_flag(trap_exit, true),
+    case file:open(File, [append]) of
+	{ok,Fd} ->
+	    {ok, {Fd, File, Type}};
+	What ->
+	    What
+    end.
 
 handle_event(Event, State) ->
     sasl_report_file_h:handle_event(Event, State).
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index bbca55b433..275aca8319 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -801,47 +801,32 @@ test_log_management() ->
     ok = control_action(rotate_logs, [Suffix]),
     [true, true] = empty_files([[MainLog, Suffix], [SaslLog, Suffix]]),
 
-    %% original main log file is not writable
-    ok = make_files_non_writable([MainLog]),
-    {error, {cannot_rotate_main_logs, _}} = control_action(rotate_logs, []),
-    ok = clean_logs([MainLog], Suffix),
-    ok = add_log_handlers([{rabbit_error_logger_file_h, MainLog}]),
-
-    %% original sasl log file is not writable
-    ok = make_files_non_writable([SaslLog]),
-    {error, {cannot_rotate_sasl_logs, _}} = control_action(rotate_logs, []),
-    ok = clean_logs([SaslLog], Suffix),
-    ok = add_log_handlers([{rabbit_sasl_report_file_h, SaslLog}]),
-
     %% logs with suffix are not writable
     ok = control_action(rotate_logs, [Suffix]),
     ok = make_files_non_writable([[MainLog, Suffix], [SaslLog, Suffix]]),
     ok = control_action(rotate_logs, [Suffix]),
     ok = test_logs_working(MainLog, SaslLog),
 
-    %% original log files are not writable
+    %% rotate when original log files are not writable
     ok = make_files_non_writable([MainLog, SaslLog]),
-    {error, {{cannot_rotate_main_logs, _},
-             {cannot_rotate_sasl_logs, _}}} = control_action(rotate_logs, []),
+    ok = control_action(rotate_logs, []),
 
     %% logging directed to tty (handlers were removed in last test)
     ok = clean_logs([MainLog, SaslLog], Suffix),
-    ok = application:set_env(sasl, sasl_error_logger, tty),
-    ok = application:set_env(kernel, error_logger, tty),
+    ok = application:set_env(rabbit, sasl_error_logger, tty),
+    ok = application:set_env(rabbit, error_logger, tty),
     ok = control_action(rotate_logs, []),
     [{error, enoent}, {error, enoent}] = empty_files([MainLog, SaslLog]),
 
     %% rotate logs when logging is turned off
-    ok = application:set_env(sasl, sasl_error_logger, false),
-    ok = application:set_env(kernel, error_logger, silent),
+    ok = application:set_env(rabbit, sasl_error_logger, false),
+    ok = application:set_env(rabbit, error_logger, silent),
     ok = control_action(rotate_logs, []),
     [{error, enoent}, {error, enoent}] = empty_files([MainLog, SaslLog]),
 
     %% cleanup
-    ok = application:set_env(sasl, sasl_error_logger, {file, SaslLog}),
-    ok = application:set_env(kernel, error_logger, {file, MainLog}),
-    ok = add_log_handlers([{rabbit_error_logger_file_h, MainLog},
-                           {rabbit_sasl_report_file_h, SaslLog}]),
+    ok = application:set_env(rabbit, sasl_error_logger, {file, SaslLog}),
+    ok = application:set_env(rabbit, error_logger, {file, MainLog}),
     passed.
 
 test_log_management_during_startup() ->
@@ -850,8 +835,8 @@ test_log_management_during_startup() ->
 
     %% start application with simple tty logging
     ok = control_action(stop_app, []),
-    ok = application:set_env(kernel, error_logger, tty),
-    ok = application:set_env(sasl, sasl_error_logger, tty),
+    ok = application:set_env(rabbit, error_logger, tty),
+    ok = application:set_env(rabbit, sasl_error_logger, tty),
     ok = add_log_handlers([{error_logger_tty_h, []},
                            {sasl_report_tty_h, []}]),
     ok = control_action(start_app, []),
@@ -868,16 +853,18 @@ test_log_management_during_startup() ->
          end,
 
     %% fix sasl logging
-    ok = application:set_env(sasl, sasl_error_logger,
-                             {file, SaslLog}),
+    ok = application:unset_env(sasl, sasl_error_logger),
+    ok = application:set_env(rabbit, sasl_error_logger, {file, SaslLog}),
 
     %% start application with logging to non-existing directory
     TmpLog = "/tmp/rabbit-tests/test.log",
     delete_file(TmpLog),
-    ok = application:set_env(kernel, error_logger, {file, TmpLog}),
+    ok = application:set_env(rabbit, error_logger, {file, TmpLog}),
+    ok = application:set_env(sasl, error_logger, {file, TmpLog}),
 
     ok = delete_log_handlers([rabbit_error_logger_file_h]),
     ok = add_log_handlers([{error_logger_file_h, MainLog}]),
+
     ok = control_action(start_app, []),
 
     %% start application with logging to directory with no
@@ -895,7 +882,7 @@ test_log_management_during_startup() ->
     %% start application with logging to a subdirectory which
     %% parent directory has no write permissions
     TmpTestDir = "/tmp/rabbit-tests/no-permission/test/log",
-    ok = application:set_env(kernel, error_logger, {file, TmpTestDir}),
+    ok = application:set_env(rabbit, error_logger, {file, TmpTestDir}),
     ok = add_log_handlers([{error_logger_file_h, MainLog}]),
     ok = case control_action(start_app, []) of
              ok -> exit({got_success_but_expected_failure,
@@ -910,7 +897,8 @@ test_log_management_during_startup() ->
 
     %% start application with standard error_logger_file_h
     %% handler not installed
-    ok = application:set_env(kernel, error_logger, {file, MainLog}),
+    ok = application:unset_env(kernel, error_logger),
+    ok = application:set_env(rabbit, error_logger, {file, MainLog}),
     ok = control_action(start_app, []),
     ok = control_action(stop_app, []),