diff options
| -rw-r--r-- | lib/chef/application.rb | 76 | ||||
| -rw-r--r-- | lib/chef/application/client.rb | 38 | ||||
| -rw-r--r-- | spec/unit/application/client_spec.rb | 79 |
3 files changed, 133 insertions, 60 deletions
diff --git a/lib/chef/application.rb b/lib/chef/application.rb index c007867f2b..8a0c82d39d 100644 --- a/lib/chef/application.rb +++ b/lib/chef/application.rb @@ -62,6 +62,10 @@ class Chef::Application Chef::Application.fatal!("SIGINT received, stopping", 2) end + trap("TERM") do + Chef::Application.fatal!("SIGTERM received, stopping", 3) + end + unless Chef::Platform.windows? trap("QUIT") do Chef::Log.info("SIGQUIT received, call stack:\n " + caller.join("\n ")) @@ -206,44 +210,66 @@ class Chef::Application ) @chef_client_json = nil - if Chef::Config[:client_fork] + if can_fork? fork_chef_client # allowed to run client in forked process else - @chef_client.run + # Unforked interval runs are disabled, so this runs chef-client + # once and then exits. If TERM signal is received, will "ignore" + # the signal to finish converge, then exit with exitstatus 3. + run_with_graceful_exit_option end @chef_client = nil end end private - def fork_chef_client + def can_fork? # win32-process gem exposes some form of :fork for Process # class. So we are seperately ensuring that the platform we're # running on is not windows before forking. - if(Process.respond_to?(:fork) && !Chef::Platform.windows?) - Chef::Log.info "Forking chef instance to converge..." - pid = fork do - [:INT, :TERM].each {|s| trap(s, "EXIT") } - client_solo = Chef::Config[:solo] ? "chef-solo" : "chef-client" - $0 = "#{client_solo} worker: ppid=#{Process.ppid};start=#{Time.new.strftime("%R:%S")};" - begin - Chef::Log.debug "Forked instance now converging" - @chef_client.run - rescue Exception => e - Chef::Log.error(e.to_s) - exit 1 - else - exit 0 - end + Chef::Config[:client_fork] && Process.respond_to?(:fork) && !Chef::Platform.windows? + end + + # Run chef-client once and then exit. If TERM signal is received, ignores the + # signal to finish the converge and exists. + def run_with_graceful_exit_option + # Override the TERM signal. + trap('TERM') do + Chef::Log.debug("SIGTERM received during converge," + + " finishing converge to exit normally (send SIGINT to terminate immediately)") + end + + @chef_client.run + true + end + + def fork_chef_client + Chef::Log.info "Forking chef instance to converge..." + pid = fork do + # Want to allow forked processes to finish converging when + # TERM singal is received (exit gracefully) + trap('TERM') do + Chef::Log.debug("SIGTERM received during converge," + + " finishing converge to exit normally (send SIGINT to terminate immediately)") + end + + client_solo = Chef::Config[:solo] ? "chef-solo" : "chef-client" + $0 = "#{client_solo} worker: ppid=#{Process.ppid};start=#{Time.new.strftime("%R:%S")};" + begin + Chef::Log.debug "Forked instance now converging" + @chef_client.run + rescue Exception => e + Chef::Log.error(e.to_s) + exit 1 + else + exit 0 end - Chef::Log.debug "Fork successful. Waiting for new chef pid: #{pid}" - result = Process.waitpid2(pid) - handle_child_exit(result) - Chef::Log.debug "Forked instance successfully reaped (pid: #{pid})" - true - else - @chef_client.run end + Chef::Log.debug "Fork successful. Waiting for new chef pid: #{pid}" + result = Process.waitpid2(pid) + handle_child_exit(result) + Chef::Log.debug "Forked instance successfully reaped (pid: #{pid})" + true end def handle_child_exit(pid_and_status) diff --git a/lib/chef/application/client.rb b/lib/chef/application/client.rb index aa394cb911..098804739e 100644 --- a/lib/chef/application/client.rb +++ b/lib/chef/application/client.rb @@ -239,7 +239,6 @@ class Chef::Application::Client < Chef::Application end IMMEDIATE_RUN_SIGNAL = "1".freeze - GRACEFUL_EXIT_SIGNAL = "2".freeze attr_reader :chef_client_json @@ -298,8 +297,9 @@ class Chef::Application::Client < Chef::Application Chef::Daemon.change_privilege end - # Run the chef client, optionally daemonizing or looping at intervals. - def run_application + def setup_signal_handlers + super + unless Chef::Platform.windows? SELF_PIPE.replace IO.pipe @@ -307,14 +307,6 @@ class Chef::Application::Client < Chef::Application Chef::Log.info("SIGUSR1 received, waking up") SELF_PIPE[1].putc(IMMEDIATE_RUN_SIGNAL) # wakeup master process from select end - - # see CHEF-5172 - if Chef::Config[:daemonize] || Chef::Config[:interval] - trap("TERM") do - Chef::Log.info("SIGTERM received, exiting gracefully") - SELF_PIPE[1].putc(GRACEFUL_EXIT_SIGNAL) - end - end end end @@ -327,7 +319,7 @@ class Chef::Application::Client < Chef::Application if !Chef::Config[:client_fork] || Chef::Config[:once] begin # run immediately without interval sleep, or splay - run_chef_client(Chef::Config[:specific_recipes]) + run_chef_client(Chef::Config[:specific_recipes] || []) rescue SystemExit raise rescue Exception => e @@ -344,18 +336,17 @@ class Chef::Application::Client < Chef::Application Chef::Daemon.daemonize("chef-client") end - signal = nil - loop do begin - Chef::Application.exit!("Exiting", 0) if signal == GRACEFUL_EXIT_SIGNAL - - if signal != IMMEDIATE_RUN_SIGNAL - signal = interval_sleep(time_to_sleep) + @signal = test_signal + if @signal != IMMEDIATE_RUN_SIGNAL + sleep_sec = time_to_sleep + Chef::Log.debug("Sleeping for #{sleep_sec} seconds") + interval_sleep(sleep_sec) end - signal = nil - run_chef_client(Chef::Config[:specific_recipes]) + @signal = nil + run_chef_client(Chef::Config[:specific_recipes] || []) Chef::Application.exit!("Exiting", 0) if !Chef::Config[:interval] rescue SystemExit => e @@ -372,6 +363,10 @@ class Chef::Application::Client < Chef::Application end end + def test_signal + client_sleep(0) + end + def time_to_sleep duration = 0 duration += rand(Chef::Config[:splay]) if Chef::Config[:splay] @@ -380,7 +375,6 @@ class Chef::Application::Client < Chef::Application end def interval_sleep(sec) - Chef::Log.debug("Sleeping for #{sec} seconds") unless SELF_PIPE.empty? client_sleep(sec) else @@ -391,7 +385,7 @@ class Chef::Application::Client < Chef::Application def client_sleep(sec) IO.select([ SELF_PIPE[0] ], nil, nil, sec) or return - SELF_PIPE[0].getc.chr + @signal = SELF_PIPE[0].getc.chr end def unforked_interval_error_message diff --git a/spec/unit/application/client_spec.rb b/spec/unit/application/client_spec.rb index f0b4f3bda8..a3599e2ac0 100644 --- a/spec/unit/application/client_spec.rb +++ b/spec/unit/application/client_spec.rb @@ -168,28 +168,78 @@ end describe Chef::Application::Client, "run_application", :unix_only do before(:each) do - Chef::Config[:daemonize] = true - @pipe = IO.pipe @app = Chef::Application::Client.new + @app.setup_signal_handlers # Default logger doesn't work correctly when logging from a trap handler. @app.configure_logging - Chef::Daemon.stub(:daemonize).and_return(true) - @app.stub(:run_chef_client) do + + @pipe = IO.pipe + @client = Chef::Client.new + Chef::Client.stub(:new).and_return(@client) + @client.stub(:run) do @pipe[1].puts 'started' sleep 1 @pipe[1].puts 'finished' end end - it "should exit gracefully when sent SIGTERM", :volatile_on_solaris do - pid = fork do - @app.run_application + context "when sent SIGTERM", :volatile_on_solaris do + context "when converging in forked process" do + before do + Chef::Config[:daemonize] = true + Chef::Daemon.stub(:daemonize).and_return(true) + end + + it "should exit hard with exitstatus 3" do + pid = fork do + @app.run_application + end + Process.kill("TERM", pid) + _pid, result = Process.waitpid2(pid) + result.exitstatus.should == 3 + end + + it "should allow child to finish converging" do + pid = fork do + @app.run_application + end + @pipe[0].gets.should == "started\n" + Process.kill("TERM", pid) + Process.wait + sleep 1 # Make sure we give the converging child process enough time to finish + IO.select([@pipe[0]], nil, nil, 0).should_not be_nil + @pipe[0].gets.should == "finished\n" + end + end + + context "when running unforked" do + before(:each) do + Chef::Config[:client_fork] = false + Chef::Config[:daemonize] = false + end + + it "should exit gracefully when sent during converge" do + pid = fork do + @app.run_application + end + @pipe[0].gets.should == "started\n" + Process.kill("TERM", pid) + _pid, result = Process.waitpid2(pid) + result.exitstatus.should == 0 + IO.select([@pipe[0]], nil, nil, 0).should_not be_nil + @pipe[0].gets.should == "finished\n" + end + + it "should exit hard when sent before converge" do + pid = fork do + sleep 3 + @app.run_application + end + Process.kill("TERM", pid) + _pid, result = Process.waitpid2(pid) + result.exitstatus.should == 3 + end end - @pipe[0].gets.should == "started\n" - Process.kill("TERM", pid) - Process.wait - IO.select([@pipe[0]], nil, nil, 0).should_not be_nil - @pipe[0].gets.should == "finished\n" end describe "when splay is set" do @@ -213,6 +263,9 @@ describe Chef::Application::Client, "run_application", :unix_only do # If everything is fine, sending USR1 to self should prevent # app to go into splay sleep forever. Process.kill("USR1", Process.pid) + # On Ruby < 2.1, we need to give the signal handlers a little + # more time, otherwise the test will fail because interleavings. + sleep 1 end number_of_sleep_calls = 0 @@ -222,7 +275,7 @@ describe Chef::Application::Client, "run_application", :unix_only do # We have to do it this way because the main loop of # Chef::Application::Client swallows most exceptions, and we need to be # able to expose our expectation failures to the parent process in the test. - @app.stub(:sleep) do |arg| + @app.stub(:interval_sleep) do |arg| number_of_sleep_calls += 1 if number_of_sleep_calls > 1 exit 127 |