diff options
| -rw-r--r-- | RELEASE_NOTES.md | 8 | ||||
| -rw-r--r-- | lib/chef/application/client.rb | 59 | ||||
| -rw-r--r-- | spec/unit/application/client_spec.rb | 188 |
3 files changed, 165 insertions, 90 deletions
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 0d7ce17261..c25d6396c2 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -45,3 +45,11 @@ and `:win_evt` is shorthand for `Chef::Log::WinEvt.new`. All previously valid options are still valid, including Logger or Logger-like instances, e.g. `Chef::Log::Syslog.new` with other args than the defaults. + +## chef-client `--daemonize` option now takes an optional integer argument + +Optional integer argument (.e.g `chef-client --daemonize 5`) is the +number of seconds to wait before the first daemonized run. See +[#3305] for background. + +[#3305]: https://github.com/chef/chef/issues/3305 diff --git a/lib/chef/application/client.rb b/lib/chef/application/client.rb index 500aa8ac59..ac46e533dd 100644 --- a/lib/chef/application/client.rb +++ b/lib/chef/application/client.rb @@ -105,10 +105,12 @@ class Chef::Application::Client < Chef::Application unless Chef::Platform.windows? option :daemonize, - :short => "-d", - :long => "--daemonize", - :description => "Daemonize the process", - :proc => lambda { |p| true } + :short => "-d [WAIT]", + :long => "--daemonize [WAIT]", + :description => + "Daemonize the process. Accepts an optional integer which is the " \ + "number of seconds to wait before the first daemonized run.", + :proc => lambda { |wait| wait =~ /^\d+$/ ? wait.to_i : true } end option :pid_file, @@ -430,33 +432,38 @@ class Chef::Application::Client < Chef::Application def interval_run_chef_client if Chef::Config[:daemonize] Chef::Daemon.daemonize("chef-client") + + # Start first daemonized run after configured number of seconds + if Chef::Config[:daemonize].is_a?(Integer) + sleep_then_run_chef_client(Chef::Config[:daemonize]) + end end loop do - begin - @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]) + sleep_then_run_chef_client(time_to_sleep) + Chef::Application.exit!("Exiting", 0) if !Chef::Config[:interval] + end + end - Chef::Application.exit!("Exiting", 0) if !Chef::Config[:interval] - rescue SystemExit => e - raise - rescue Exception => e - if Chef::Config[:interval] - Chef::Log.error("#{e.class}: #{e}") - Chef::Log.debug("#{e.class}: #{e}\n#{e.backtrace.join("\n")}") - retry - else - Chef::Application.fatal!("#{e.class}: #{e.message}", 1) - end - end + def sleep_then_run_chef_client(sleep_sec) + @signal = test_signal + unless @signal == IMMEDIATE_RUN_SIGNAL + Chef::Log.debug("Sleeping for #{sleep_sec} seconds") + interval_sleep(sleep_sec) + end + @signal = nil + + run_chef_client(Chef::Config[:specific_recipes]) + rescue SystemExit => e + raise + rescue Exception => e + if Chef::Config[:interval] + Chef::Log.error("#{e.class}: #{e}") + Chef::Log.debug("#{e.class}: #{e}\n#{e.backtrace.join("\n")}") + retry end + + Chef::Application.fatal!("#{e.class}: #{e.message}", 1) end def test_signal diff --git a/spec/unit/application/client_spec.rb b/spec/unit/application/client_spec.rb index 97a297ccb5..6765ca93ae 100644 --- a/spec/unit/application/client_spec.rb +++ b/spec/unit/application/client_spec.rb @@ -17,6 +17,55 @@ require "spec_helper" +shared_context "with signal handlers" do + before do + Chef::Config[:specific_recipes] = [] # normally gets set in @app.reconfigure + + @app = Chef::Application::Client.new + @app.setup_signal_handlers + # Default logger doesn't work correctly when logging from a trap handler. + @app.configure_logging + end +end + +shared_context "with interval_sleep" do + before do + run_count = 0 + + # uncomment to debug failures... + # Chef::Log.init($stderr) + # Chef::Log.level = :debug + + allow(@app).to receive(:run_chef_client) do + run_count += 1 + if run_count > 3 + exit 0 + end + + # 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 + + # This is a very complicated way of writing + # @app.should_receive(:sleep).once. + # 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. + allow(@app).to receive(:interval_sleep) do |arg| + number_of_sleep_calls += 1 + if number_of_sleep_calls > 1 + exit 127 + end + end + end +end + describe Chef::Application::Client, "reconfigure" do let(:app) do a = described_class.new @@ -26,6 +75,7 @@ describe Chef::Application::Client, "reconfigure" do before do allow(Kernel).to receive(:trap).and_return(:ok) + allow(::File).to receive(:read).and_call_original allow(::File).to receive(:read).with(Chef::Config.platform_specific_path("/etc/chef/client.rb")).and_return("") @original_argv = ARGV.dup @@ -52,17 +102,44 @@ describe Chef::Application::Client, "reconfigure" do app.reconfigure end - context "when given a named_run_list" do + shared_examples "sets the configuration" do |cli_arguments, expected_config| + describe cli_arguments do + before do + ARGV.replace(cli_arguments.split) + app.reconfigure + end - before do - ARGV.replace( %w{ --named-run-list arglebargle-example } ) - app.reconfigure + it "sets #{expected_config}" do + expect(Chef::Config.configuration).to include expected_config + end + end + end + + describe "--named-run-list" do + it_behaves_like "sets the configuration", + "--named-run-list arglebargle-example", + :named_run_list => "arglebargle-example" + end + + describe "--no-listen" do + it_behaves_like "sets the configuration", "--no-listen", :listen => false + end + + describe "--daemonize", :unix_only do + context "with no value" do + it_behaves_like "sets the configuration", "--daemonize", + :daemonize => true end - it "sets named_run_list in Chef::Config" do - expect(Chef::Config[:named_run_list]).to eq("arglebargle-example") + context "with an integer value" do + it_behaves_like "sets the configuration", "--daemonize 5", + :daemonize => 5 end + context "with a non-integer value" do + it_behaves_like "sets the configuration", "--daemonize foo", + :daemonize => true + end end end @@ -116,15 +193,46 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config end end - describe "when in daemonized mode and no interval has been set" do + describe "daemonized mode", :unix_only do + let(:daemonize) { true } + before do - Chef::Config[:daemonize] = true - Chef::Config[:interval] = nil + Chef::Config[:daemonize] = daemonize + allow(Chef::Daemon).to receive(:daemonize) end - it "should set the interval to 1800" do - app.reconfigure - expect(Chef::Config.interval).to eq(1800) + context "when no interval has been set" do + before do + Chef::Config[:interval] = nil + end + + it "should set the interval to 1800" do + app.reconfigure + expect(Chef::Config.interval).to eq(1800) + end + end + + context "when the daemonize option is an integer" do + include_context "with signal handlers" + include_context "with interval_sleep" + + let(:wait_secs) { 1 } + let(:daemonize) { wait_secs } + + before do + allow(@app).to receive(:interval_sleep).with(wait_secs).and_return true + allow(@app).to receive(:interval_sleep).with(0).and_call_original + end + + it "sleeps for the amount of time passed" do + pid = fork do + expect(@app).to receive(:interval_sleep).with(wait_secs) + @app.run_application + end + _pid, result = Process.waitpid2(pid) + + expect(result.exitstatus).to eq 0 + end end end @@ -148,16 +256,6 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config end - describe "when --no-listen is set" do - - it "configures listen = false" do - app.config[:listen] = false - app.reconfigure - expect(Chef::Config[:listen]).to eq(false) - end - - end - describe "when the json_attribs configuration option is specified" do let(:json_attribs) { { "a" => "b" } } @@ -305,14 +403,9 @@ describe Chef::Application::Client, "configure_chef" do end describe Chef::Application::Client, "run_application", :unix_only do - before(:each) do - Chef::Config[:specific_recipes] = [] # normally gets set in @app.reconfigure - - @app = Chef::Application::Client.new - @app.setup_signal_handlers - # Default logger doesn't work correctly when logging from a trap handler. - @app.configure_logging + include_context "with signal handlers" + before(:each) do @pipe = IO.pipe @client = Chef::Client.new allow(Chef::Client).to receive(:new).and_return(@client) @@ -383,44 +476,11 @@ describe Chef::Application::Client, "run_application", :unix_only do end describe "when splay is set" do + include_context "with interval_sleep" + before do Chef::Config[:splay] = 10 Chef::Config[:interval] = 10 - - run_count = 0 - - # uncomment to debug failures... - # Chef::Log.init($stderr) - # Chef::Log.level = :debug - - allow(@app).to receive(:run_chef_client) do - - run_count += 1 - if run_count > 3 - exit 0 - end - - # 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 - - # This is a very complicated way of writing - # @app.should_receive(:sleep).once. - # 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. - allow(@app).to receive(:interval_sleep) do |arg| - number_of_sleep_calls += 1 - if number_of_sleep_calls > 1 - exit 127 - end - end end it "shouldn't sleep when sent USR1" do |