Merge pull request #2986 from chef/jdm/win-service

Chef client running as a windows service should have a configurable timeout

3 files changed, 41 insertions, 12 deletions

diff --git a/lib/chef/application/windows_service.rb b/lib/chef/application/windows_service.rb
index ba7c6dab5a..b42a01cfdb 100644
--- a/lib/chef/application/windows_service.rb
+++ b/lib/chef/application/windows_service.rb
@@ -189,9 +189,15 @@ class Chef
           config_params += " -c #{Chef::Config[:config_file]}" unless  Chef::Config[:config_file].nil?
           config_params += " -L #{Chef::Config[:log_location]}" unless Chef::Config[:log_location] == STDOUT
           # Starts a new process and waits till the process exits
-          result = shell_out("chef-client #{config_params}")
+          result = shell_out("chef-client #{config_params}", :timeout => Chef::Config[:windows_service][:watchdog_timeout])
           Chef::Log.debug "#{result.stdout}"
           Chef::Log.debug "#{result.stderr}"
+        rescue Mixlib::ShellOut::CommandTimeout => e
+          Chef::Log.error "chef-client timed out\n(#{e})"
+          Chef::Log.error(<<-EOF) 
+            Your chef-client run timed out. You can increase the time chef-client is given 
+            to complete by configuring windows_service.watchdog_timeout in your client.rb.
+          EOF
         rescue Mixlib::ShellOut::ShellCommandFailed => e
           Chef::Log.warn "Not able to start chef-client in new process (#{e})"
         rescue => e
diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index c9e0914c0b..2eb9870a64 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -643,6 +643,12 @@ class Chef
     default :normal_attribute_whitelist, nil
     default :override_attribute_whitelist, nil
 
+    config_context :windows_service do
+      # Set `watchdog_timeout` to the number of seconds to wait for a chef-client run
+      # to finish
+      default :watchdog_timeout, 2 * (60 * 60) # 2 hours
+    end
+
     # Chef requires an English-language UTF-8 locale to function properly.  We attempt
     # to use the 'locale -a' command and search through a list of preferences until we
     # find one that we can use.  On Ubuntu systems we should find 'C.UTF-8' and be
diff --git a/spec/unit/windows_service_spec.rb b/spec/unit/windows_service_spec.rb
index cf933a9ab2..bc5e781c03 100644
--- a/spec/unit/windows_service_spec.rb
+++ b/spec/unit/windows_service_spec.rb
@@ -39,16 +39,33 @@ describe "Chef::Application::WindowsService", :windows_only do
     allow(instance).to receive(:state).and_return(4)
     instance.service_main
   end
-  it "passes config params to new process" do
-    Chef::Config.merge!({:log_location => tempfile.path, :config_file => "test_config_file", :log_level => :info})
-    expect(instance).to receive(:configure_chef).twice
-    instance.service_init
-    allow(instance).to receive(:running?).and_return(true, false)
-    allow(instance.instance_variable_get(:@service_signal)).to receive(:wait)
-    allow(instance).to receive(:state).and_return(4)
-    expect(instance).to receive(:run_chef_client).and_call_original
-    expect(instance).to receive(:shell_out).with("chef-client  --no-fork -c test_config_file -L #{tempfile.path}").and_return(shell_out_result)
-    instance.service_main
-    tempfile.unlink
+
+  context 'when running chef-client' do
+    it "passes config params to new process with a default timeout of 2 hours (7200 seconds)" do
+      Chef::Config.merge!({:log_location => tempfile.path, :config_file => "test_config_file", :log_level => :info})
+      expect(instance).to receive(:configure_chef).twice
+      instance.service_init
+      allow(instance).to receive(:running?).and_return(true, false)
+      allow(instance.instance_variable_get(:@service_signal)).to receive(:wait)
+      allow(instance).to receive(:state).and_return(4)
+      expect(instance).to receive(:run_chef_client).and_call_original
+      expect(instance).to receive(:shell_out).with("chef-client  --no-fork -c test_config_file -L #{tempfile.path}", {:timeout => 7200}).and_return(shell_out_result)
+      instance.service_main
+      tempfile.unlink
+    end
+
+    it "passes config params to new process with a the timeout specified in the config" do
+      Chef::Config.merge!({:log_location => tempfile.path, :config_file => "test_config_file", :log_level => :info})
+      Chef::Config[:windows_service][:watchdog_timeout] = 10
+      expect(instance).to receive(:configure_chef).twice
+      instance.service_init
+      allow(instance).to receive(:running?).and_return(true, false)
+      allow(instance.instance_variable_get(:@service_signal)).to receive(:wait)
+      allow(instance).to receive(:state).and_return(4)
+      expect(instance).to receive(:run_chef_client).and_call_original
+      expect(instance).to receive(:shell_out).with("chef-client  --no-fork -c test_config_file -L #{tempfile.path}", {:timeout => 10}).and_return(shell_out_result)
+      instance.service_main
+      tempfile.unlink
+    end
   end
 end