Skip to content
Browse files

(#13584) in daemon mode, master can swallow errors during startup

This commit delays the closing of stdin/stdout/stderr to a time
a little later in the daemon startup process, in hopes of providing
a larger window in which the console is available as a fallback for
logging errors.
  • Loading branch information...
1 parent ba112e9 commit c2c082381895954c25357b1ca87ad86ab3532c5c @cprice404 cprice404 committed Apr 3, 2012
Showing with 40 additions and 13 deletions.
  1. +18 −0 lib/puppet/daemon.rb
  2. +9 −11 lib/puppet/network/server.rb
  3. +11 −2 lib/puppet/util.rb
  4. +1 −0 spec/unit/daemon_spec.rb
  5. +1 −0 spec/unit/network/server_spec.rb
View
18 lib/puppet/daemon.rb
@@ -25,11 +25,18 @@ def daemonize
Process.setsid
Dir.chdir("/")
+ end
+
+ # Close stdin/stdout/stderr so that we can finish our transition into 'daemon' mode.
+ # @return nil
+ def self.close_streams()
+ Puppet.debug("Closing streams for daemon mode")
begin
$stdin.reopen "/dev/null"
$stdout.reopen "/dev/null", "a"
$stderr.reopen $stdout
Puppet::Util::Log.reopen
+ Puppet.debug("Finished closing streams for daemon mode")
rescue => detail
Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
Puppet::Util::replace_file("/tmp/daemonout", 0644) do |f|
@@ -39,6 +46,11 @@ def daemonize
end
end
+ # Convenience signature for calling Puppet::Daemon.close_streams
+ def close_streams()
+ Puppet::Daemon.close_streams
+ end
+
# Create a pidfile for our daemon, so we can be stopped and others
# don't try to start.
def create_pidfile
@@ -123,6 +135,12 @@ def start
# Start the listening server, if required.
server.start if server
+ # now that the server has started, we've waited just about as long as possible to close
+ # our streams and become a "real" daemon process. This is in hopes of allowing
+ # errors to have the console available as a fallback for logging for as long as
+ # possible.
+ close_streams
+
# Finally, loop forever running events - or, at least, until we exit.
run_event_loop
end
View
20 lib/puppet/network/server.rb
@@ -4,6 +4,10 @@
class Puppet::Network::Server
attr_reader :server_type, :address, :port
+
+ # TODO: does anything actually call this? It seems like it's a duplicate of the code in Puppet::Daemon, but that
+ # it's not actually called anywhere.
+
# Put the daemon into the background.
def daemonize
if pid = fork
@@ -16,17 +20,10 @@ def daemonize
Process.setsid
Dir.chdir("/")
- begin
- $stdin.reopen "/dev/null"
- $stdout.reopen "/dev/null", "a"
- $stderr.reopen $stdout
- Puppet::Util::Log.reopen
- rescue => detail
- Puppet::Util.replace_file("/tmp/daemonout", 0644) { |f|
- f.puts "Could not start #{Puppet[:name]}: #{detail}"
- }
- raise "Could not start #{Puppet[:name]}: #{detail}"
- end
+ end
+
+ def close_streams()
+ Puppet::Daemon.close_streams()
end
# Create a pidfile for our daemon, so we can be stopped and others
@@ -113,6 +110,7 @@ def http_server_class
def start
create_pidfile
+ close_streams
listen
end
View
13 lib/puppet/util.rb
@@ -476,13 +476,22 @@ def replace_file(file, default_mode, &block)
# @yield
def exit_on_fail(message, code = 1)
yield
- rescue ArgumentError, RuntimeError, NotImplementedError => detail
- Puppet.log_exception(detail, "Could not #{message}: #{detail}")
+ # First, we need to check and see if we are catching a SystemExit error. These will be raised
+ # when we daemonize/fork, and they do not necessarily indicate a failure case.
+ rescue SystemExit => err
+ raise err
+
+ # Now we need to catch *any* other kind of exception, because we may be calling third-party
+ # code (e.g. webrick), and we have no idea what they might throw.
+ rescue Exception => err
+ Puppet.log_exception(err, "Could not #{message}: #{err}")
exit(code)
end
module_function :exit_on_fail
+
+
#######################################################################################################
# Deprecated methods relating to process execution; these have been moved to Puppet::Util::Execution
#######################################################################################################
View
1 spec/unit/daemon_spec.rb
@@ -22,6 +22,7 @@ def lockfile_path
before do
@agent = Puppet::Agent.new(TestClient.new)
@daemon = Puppet::Daemon.new
+ @daemon.stubs(:close_streams).returns nil
end
it "should be able to manage an agent" do
View
1 spec/unit/network/server_spec.rb
@@ -13,6 +13,7 @@
Puppet::Network::HTTP.stubs(:server_class_by_type).returns(@mock_http_server_class)
Puppet.settings.stubs(:value).with(:servertype).returns(:suparserver)
@server = Puppet::Network::Server.new(:port => 31337)
+ @server.stubs(:close_streams).returns(nil)
end
describe "when initializing" do

0 comments on commit c2c0823

Please sign in to comment.
Something went wrong with that request. Please try again.