Moving the Agent locking code to a module.

Also cleaning up the lock usage by yielding to a block
when a lock is attained.

Signed-off-by: Luke Kanies <luke@madstop.com>

2 files changed, 94 insertions, 28 deletions

diff --git a/spec/unit/agent.rb b/spec/unit/agent.rb
index 922577cb6..e0ae3689e 100755
--- a/spec/unit/agent.rb
+++ b/spec/unit/agent.rb
@@ -14,6 +14,10 @@ describe Puppet::Agent do
     it "should include the Fact Handler module" do
         Puppet::Agent.ancestors.should be_include(Puppet::Agent::FactHandler)
     end
+
+    it "should include the Locker module" do
+        Puppet::Agent.ancestors.should be_include(Puppet::Agent::Locker)
+    end
 end
 
 describe Puppet::Agent, "when executing a catalog run" do
@@ -21,10 +25,7 @@ describe Puppet::Agent, "when executing a catalog run" do
         Puppet.settings.stubs(:use).returns(true)
         @agent = Puppet::Agent.new
         @agent.stubs(:splay)
-
-        @lockfile = stub 'lockfile', :lock => true, :locked? => false, :lockfile => "/my/lock/file", :unlock => true
-
-        @agent.stubs(:lockfile).returns @lockfile
+        @agent.stubs(:lock).yields.then.returns true
     end
 
     it "should splay" do
@@ -43,8 +44,8 @@ describe Puppet::Agent, "when executing a catalog run" do
         @agent.run
     end
 
-    it "should use a lockfile to make sure no other process is executing the catalog" do
-        @lockfile.expects(:lock).returns true
+    it "should retrieve the catalog if a lock is attained" do
+        @agent.expects(:lock).yields.then.returns true
 
         @agent.expects(:retrieve_catalog)
 
@@ -52,7 +53,7 @@ describe Puppet::Agent, "when executing a catalog run" do
     end
 
     it "should log and do nothing if the lock cannot be acquired" do
-        @lockfile.expects(:lock).returns false
+        @agent.expects(:lock).returns false
 
         @agent.expects(:retrieve_catalog).never
 
@@ -93,27 +94,6 @@ describe Puppet::Agent, "when executing a catalog run" do
         @agent.run
     end
 
-    it "should remove the lock file when done applying the catalog" do
-        catalog = stub 'catalog', :retrieval_duration= => nil, :apply => nil
-        @agent.expects(:retrieve_catalog).returns catalog
-
-        @lockfile.expects(:lock).returns true
-
-        @lockfile.expects(:unlock)
-
-        @agent.run
-    end
-
-    it "should remove the lock file even if there was an exception during the run" do
-        catalog = stub 'catalog', :retrieval_duration= => nil
-        @agent.expects(:retrieve_catalog).returns catalog
-
-        catalog.expects(:apply).raises "eh"
-
-        @lockfile.expects(:unlock)
-        @agent.run
-    end
-
     it "should HUP itself if it should be restarted" do
         catalog = stub 'catalog', :retrieval_duration= => nil, :apply => nil
         @agent.expects(:retrieve_catalog).returns catalog
diff --git a/spec/unit/agent/locker.rb b/spec/unit/agent/locker.rb
new file mode 100755
index 000000000..aae7c0c7b
--- /dev/null
+++ b/spec/unit/agent/locker.rb
@@ -0,0 +1,86 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+require 'puppet/agent'
+require 'puppet/agent/locker'
+
+class LockerTester
+    include Puppet::Agent::Locker
+end
+
+describe Puppet::Agent::Locker do
+    before do
+        @locker = LockerTester.new
+    end
+
+    it "should use a Pidlock instance as its lockfile" do
+        @locker.lockfile.should be_instance_of(Puppet::Util::Pidlock)
+    end
+
+    it "should reuse the same lock file each time" do
+        @locker.lockfile.should equal(@locker.lockfile)
+    end
+
+    it "should use the lock file to anonymously lock the process when disabled" do
+        @locker.lockfile.expects(:lock).with(:anonymous => true)
+
+        @locker.disable
+    end
+
+    it "should use the lock file to anonymously unlock the process when enabled" do
+        @locker.lockfile.expects(:unlock).with(:anonymous => true)
+
+        @locker.enable
+    end
+
+    it "should have a method that yields when a lock is attained" do
+        @locker.lockfile.expects(:lock).returns true
+
+        yielded = false
+        @locker.lock do
+            yielded = true
+        end
+        yielded.should be_true
+    end
+
+    it "should return true when the lock method successfully locked" do
+        @locker.lockfile.expects(:lock).returns true
+
+        @locker.lock {}.should be_true
+    end
+
+    it "should return true when the lock method does not receive the lock" do
+        @locker.lockfile.expects(:lock).returns false
+
+        @locker.lock {}.should be_false
+    end
+
+    it "should not yield when the lock method does not receive the lock" do
+        @locker.lockfile.expects(:lock).returns false
+
+        yielded = false
+        @locker.lock { yielded = true }
+        yielded.should be_false
+    end
+
+    it "should not unlock when a lock was not received" do
+        @locker.lockfile.expects(:lock).returns false
+        @locker.lockfile.expects(:unlock).never
+
+        @locker.lock {}
+    end
+
+    it "should unlock after yielding upon obtaining a lock" do
+        @locker.lockfile.stubs(:lock).returns true
+        @locker.lockfile.expects(:unlock)
+
+        @locker.lock {}
+    end
+
+    it "should unlock after yielding upon obtaining a lock, even if the block throws an exception" do
+        @locker.lockfile.stubs(:lock).returns true
+        @locker.lockfile.expects(:unlock)
+
+        lambda { @locker.lock { raise "foo" } }.should raise_error(RuntimeError)
+    end
+end