Start chef-client in new process when it is run as a service on windows

5 files changed, 134 insertions, 9 deletions

diff --git a/lib/chef/application/windows_service.rb b/lib/chef/application/windows_service.rb
index e8e2760c1c..08403c7aa2 100644
--- a/lib/chef/application/windows_service.rb
+++ b/lib/chef/application/windows_service.rb
@@ -27,11 +27,13 @@ require 'chef/rest'
 require 'mixlib/cli'
 require 'socket'
 require 'win32/daemon'
+require 'chef/mixin/shell_out'
 
 class Chef
   class Application
     class WindowsService < ::Win32::Daemon
       include Mixlib::CLI
+      include Chef::Mixin::ShellOut
 
       option :config_file,
         :short => "-c CONFIG",
@@ -160,17 +162,29 @@ class Chef
 
       # Initializes Chef::Client instance and runs it
       def run_chef_client
-        @chef_client = Chef::Client.new(
-          @chef_client_json,
-          :override_runlist => config[:override_runlist]
-        )
-        @chef_client_json = nil
-
-        @chef_client.run
-        @chef_client = nil
+        # The chef client will be started in a new process. We have used shell_out to start the chef-client.
+        # The log_location and config_file of the parent process is passed to the new chef-client process.
+        # We need to add the --no-fork, as by default it is set to fork=true.
+        begin
+          Chef::Log.info "Starting chef-client in a new process"
+          # Pass config params to the new process
+          config_params = " --no-fork"
+          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}")
+          Chef::Log.debug "#{result.stdout}"
+          Chef::Log.debug "#{result.stderr}"
+        rescue Mixlib::ShellOut::ShellCommandFailed => e
+          Chef::Log.warn "Not able to start chef-client in new process (#{e})"
+        rescue => e
+          Chef::Log.error e
+        ensure
+          # Once process exits, we log the current process' pid
+          Chef::Log.info "Child process exited (pid: #{Process.pid})"
+        end
       end
 
-
       def apply_config(config_file_path)
         Chef::Config.from_file(config_file_path)
         Chef::Config.merge!(config)
diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index c71452d0e3..6863dc7691 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -478,6 +478,7 @@ class Chef
         run_context = nil
         @events.run_start(Chef::VERSION)
         Chef::Log.info("*** Chef #{Chef::VERSION} ***")
+        Chef::Log.info "Chef-client pid: #{Process.pid}"
         enforce_path_sanity
         run_ohai
         @events.ohai_completed(node)
diff --git a/spec/functional/win32/windows_service_spec.rb b/spec/functional/win32/windows_service_spec.rb
new file mode 100644
index 0000000000..f8da0adeee
--- /dev/null
+++ b/spec/functional/win32/windows_service_spec.rb
@@ -0,0 +1,55 @@
+#
+# Author:: Mukta Aphale (<mukta.aphale@clogeny.com>)
+# Copyright:: Copyright (c) 2013 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'spec_helper'
+if Chef::Platform.windows?
+  require 'chef/application/windows_service'
+  include Chef::Mixin::ShellOut
+end
+
+def get_pid(result)
+  result = result.split
+  result = result[result.length-1]
+  result = result.split(")")
+  result[0]
+end
+
+describe "Chef::Application::WindowsService", :windows_only do
+  let (:instance) {Chef::Application::WindowsService.new}
+
+  it "runs chef-client in new process" do
+    tempfilename = Tempfile.new("log")
+    Chef::Config.merge!({:log_location => tempfilename.path, :log_level => :info})
+    instance.stub(:parse_options)
+    instance.should_receive(:configure_chef).twice
+    instance.service_init
+    instance.stub(:running?).and_return(true, false)
+    instance.instance_variable_get(:@service_signal).stub(:wait)
+    instance.stub(:state).and_return(4)
+    instance.should_receive(:run_chef_client).and_call_original
+    instance.should_receive(:shell_out).and_call_original
+    instance.service_main
+    result1 = shell_out("grep 'Chef-client pid:' #{tempfilename.path}")
+    result2 = shell_out("grep 'Child process exited' #{tempfilename.path}")
+    result1.stdout.should_not == ""
+    result2.stdout.should_not == ""
+    pid_child = get_pid(result1.stdout)
+    pid_parent = get_pid(result2.stdout)
+    tempfilename.unlink
+    pid_child.should_not == pid_parent
+  end
+end
diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb
index 1b7a292f18..a3b5c6ec08 100644
--- a/spec/unit/client_spec.rb
+++ b/spec/unit/client_spec.rb
@@ -450,6 +450,7 @@ shared_examples_for Chef::Client do
 end
 
 describe Chef::Client do
+  Chef::Config[:client_fork] = false
   it_behaves_like Chef::Client
 end
 
diff --git a/spec/unit/windows_service_spec.rb b/spec/unit/windows_service_spec.rb
new file mode 100644
index 0000000000..ba3d2980df
--- /dev/null
+++ b/spec/unit/windows_service_spec.rb
@@ -0,0 +1,54 @@
+#
+# Author:: Mukta Aphale (<mukta.aphale@clogeny.com>)
+# Copyright:: Copyright (c) 2013 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'spec_helper'
+if Chef::Platform.windows?
+  require 'chef/application/windows_service'
+end
+
+describe "Chef::Application::WindowsService", :windows_only do
+  let (:instance) {Chef::Application::WindowsService.new}
+  let (:shell_out_result) {Object.new}
+  let (:tempfile) {Tempfile.new "log_file"}
+  before do
+    instance.stub(:parse_options)
+    shell_out_result.stub(:stdout)
+    shell_out_result.stub(:stderr)
+  end
+  it "runs chef-client in new process" do
+    instance.should_receive(:configure_chef).twice
+    instance.service_init
+    instance.should_receive(:run_chef_client).and_call_original
+    instance.should_receive(:shell_out).and_return(shell_out_result)
+    instance.stub(:running?).and_return(true, false)
+    instance.instance_variable_get(:@service_signal).stub(:wait)
+    instance.stub(: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})
+    instance.should_receive(:configure_chef).twice
+    instance.service_init
+    instance.stub(:running?).and_return(true, false)
+    instance.instance_variable_get(:@service_signal).stub(:wait)
+    instance.stub(:state).and_return(4)
+    instance.should_receive(:run_chef_client).and_call_original
+    instance.should_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
+  end
+end