Merge pull request #1035 from opscode/CHEF-4556

CHEF-4556: chef-client service starts at every run of chef-client::service recipe

3 files changed, 38 insertions, 75 deletions

diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 6863dc7691..04d6799988 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -475,6 +475,7 @@ class Chef
       runlock.acquire
       # don't add code that may fail before entering this section to be sure to release lock
       begin
+        runlock.save_pid
         run_context = nil
         @events.run_start(Chef::VERSION)
         Chef::Log.info("*** Chef #{Chef::VERSION} ***")
diff --git a/lib/chef/daemon.rb b/lib/chef/daemon.rb
index e5479905af..e5abca29d8 100644
--- a/lib/chef/daemon.rb
+++ b/lib/chef/daemon.rb
@@ -18,12 +18,14 @@
 # I love you Merb (lib/merb-core/server.rb)
 
 require 'chef/config'
+require 'chef/run_lock'
 require 'etc'
 
 class Chef
   class Daemon
     class << self
       attr_accessor :name
+      attr_accessor :runlock
 
       # Daemonize the current process, managing pidfiles and process uid/gid
       #
@@ -32,9 +34,9 @@ class Chef
       #
       def daemonize(name)
         @name = name
-        pid = pid_from_file
-        unless running?
-          remove_pid_file()
+        @runlock = RunLock.new(pid_file)
+        if runlock.test
+          # We've acquired the daemon lock. Now daemonize.
           Chef::Log.info("Daemonizing..")
           begin
             exit if fork
@@ -45,50 +47,12 @@ class Chef
             $stdin.reopen("/dev/null")
             $stdout.reopen("/dev/null", "a")
             $stderr.reopen($stdout)
-            save_pid_file
-            at_exit { remove_pid_file }
+            runlock.save_pid
           rescue NotImplementedError => e
             Chef::Application.fatal!("There is no fork: #{e.message}")
           end
         else
-          Chef::Application.fatal!("Chef is already running pid #{pid}")
-        end
-      end
-
-      # Check if Chef is running based on the pid_file
-      # ==== Returns
-      # Boolean::
-      # True if Chef is running
-      # False if Chef is not running
-      #
-      def running?
-        if pid_from_file.nil?
-          false
-        else
-          Process.kill(0, pid_from_file)
-          true
-        end
-      rescue Errno::ESRCH, Errno::ENOENT
-        false
-      rescue Errno::EACCES => e
-        Chef::Application.fatal!("You don't have access to the PID file at #{pid_file}: #{e.message}")
-      end
-
-      # Check if this process if forked from a Chef daemon
-      # ==== Returns
-      # Boolean::
-      # True if this process is forked
-      # False if this process is not forked
-      #
-      def forked?
-        if running? and Process.ppid == pid_from_file.to_i
-          # chef daemon is running and this process is a child of it
-          true
-        elsif not running? and Process.ppid == 1
-          # an orphaned fork, its parent becomes init, launchd, etc. after chef daemon dies
-          true
-        else
-          false
+          Chef::Application.fatal!("Chef is already running pid #{pid_from_file}")
         end
       end
 
@@ -113,31 +77,6 @@ class Chef
         nil
       end
 
-      # Store the PID on the filesystem
-      # This uses the Chef::Config[:pid_file] option, or "/tmp/name.pid" otherwise
-      #
-      def save_pid_file
-        file = pid_file
-        begin
-          FileUtils.mkdir_p(File.dirname(file))
-        rescue Errno::EACCES => e
-          Chef::Application.fatal!("Failed store pid in #{File.dirname(file)}, permission denied: #{e.message}")
-        end
-
-        begin
-          File.open(file, "w") { |f| f.write(Process.pid.to_s) }
-        rescue Errno::EACCES => e
-          Chef::Application.fatal!("Couldn't write to pidfile #{file}, permission denied: #{e.message}")
-        end
-      end
-
-      # Delete the PID from the filesystem
-      def remove_pid_file
-        if not forked? then
-          FileUtils.rm(pid_file) if File.exists?(pid_file)
-        end
-      end
-
       # Change process user/group to those specified in Chef::Config
       #
       def change_privilege
diff --git a/lib/chef/run_lock.rb b/lib/chef/run_lock.rb
index 4bde09de54..2ddf02973a 100644
--- a/lib/chef/run_lock.rb
+++ b/lib/chef/run_lock.rb
@@ -47,24 +47,47 @@ class Chef
     #
     # The implementation is based on File#flock (see also: flock(2)).
     def acquire
+      wait unless test
+    end
+
+    #
+    # Tests and if successful acquires the system-wide lock.
+    # Returns true if the lock is acquired, false otherwise.
+    #
+    def test
       # ensure the runlock_file path exists
       create_path(File.dirname(runlock_file))
-      @runlock = File.open(runlock_file,'w+')
+      @runlock = File.open(runlock_file,'a+')
       # if we support FD_CLOEXEC (linux, !windows), then use it.
       # NB: ruby-2.0.0-p195 sets FD_CLOEXEC by default, but not ruby-1.8.7/1.9.3
       if Fcntl.const_defined?('F_SETFD') && Fcntl.const_defined?('FD_CLOEXEC')
         runlock.fcntl(Fcntl::F_SETFD, runlock.fcntl(Fcntl::F_GETFD, 0) | Fcntl::FD_CLOEXEC)
       end
-      unless runlock.flock(File::LOCK_EX|File::LOCK_NB)
-        # Another chef client running...
-        runpid = runlock.read.strip.chomp
-        Chef::Log.warn("Chef client #{runpid} is running, will wait for it to finish and then run.")
-        runlock.flock(File::LOCK_EX)
+      # Flock will return 0 if it can acquire the lock otherwise it
+      # will return false
+      if runlock.flock(File::LOCK_NB|File::LOCK_EX) == 0
+        true
+      else
+        false
       end
-      # We grabbed the run lock.  Save the pid.
+    end
+
+    #
+    # Waits until acquiring the system-wide lock.
+    #
+    def wait
+      runpid = runlock.read.strip.chomp
+      Chef::Log.warn("Chef client #{runpid} is running, will wait for it to finish and then run.")
+      runlock.flock(File::LOCK_EX)
+    end
+
+    def save_pid
       runlock.truncate(0)
       runlock.rewind # truncate doesn't reset position to 0.
       runlock.write(Process.pid.to_s)
+      # flush the file fsync flushes the system buffers
+      # in addition to ruby buffers
+      runlock.fsync
     end
 
     # Release the system-wide lock.