[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, master, updated. debian/0.24.6-1-356-g5718585

[James Turnbull]

The following commit has been merged in the master branch:
commit 45144a1b9da2839fd9f8a182a8f82ecb06e17292
Author: Luke Kanies <luke at madstop.com>
Date:   Wed Dec 10 23:29:39 2008 -0600

    Fixing #1812 (hopefully) - adding read and write locks to yaml.
    
    It's obviously not really possible to test that this fixes it,
    but I'm confident that the locks work, and now we're using them,
    so it *should*.
    
    Signed-off-by: Luke Kanies <luke at madstop.com>

diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb
index 3f05ce6..5b53a76 100644
--- a/lib/puppet/indirector/yaml.rb
+++ b/lib/puppet/indirector/yaml.rb
@@ -1,17 +1,26 @@
 require 'puppet/indirector/terminus'
+require 'puppet/util/file_locking'
 
 # The base class for YAML indirection termini.
 class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
+    include Puppet::Util::FileLocking
+
     # Read a given name's file in and convert it from YAML.
     def find(request)
         file = path(request.key)
         return nil unless FileTest.exist?(file)
 
+        yaml = nil
         begin
-            return from_yaml(File.read(file))
+            readlock(file) { |fh| yaml = fh.read }
         rescue => detail
             raise Puppet::Error, "Could not read YAML data for %s %s: %s" % [indirection.name, request.key, detail]
         end
+        begin
+            return from_yaml(yaml)
+        rescue => detail
+            raise Puppet::Error, "Could not parse YAML data for %s %s: %s" % [indirection.name, request.key, detail]
+        end
     end
 
     # Convert our object to YAML and store it to the disk.
@@ -28,7 +37,7 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
         end
 
         begin
-            File.open(file, "w", 0660) { |f| f.print to_yaml(request.instance) }
+            writelock(file, 0660) { |f| f.print to_yaml(request.instance) }
         rescue TypeError => detail
             Puppet.err "Could not save %s %s: %s" % [self.name, request.key, detail]
         end
@@ -36,7 +45,7 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
 
     # Get the yaml directory
     def base
-      (Puppet[:name] == "puppetmasterd") ? Puppet[:yamldir] : Puppet[:clientyamldir]
+        (Puppet[:name] == "puppetmasterd") ? Puppet[:yamldir] : Puppet[:clientyamldir]
     end
 
     # Return the path to a given node's file.
diff --git a/spec/unit/indirector/yaml.rb b/spec/unit/indirector/yaml.rb
index 081ae96..7536fbc 100755
--- a/spec/unit/indirector/yaml.rb
+++ b/spec/unit/indirector/yaml.rb
@@ -58,12 +58,12 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do
             proc { @store.save(@request) }.should raise_error(ArgumentError)
         end
 
-        it "should convert Ruby objects to YAML and write them to disk" do
+        it "should convert Ruby objects to YAML and write them to disk using a write lock" do
             yaml = @subject.to_yaml
             file = mock 'file'
             path = @store.send(:path, @subject.name)
             FileTest.expects(:exist?).with(File.dirname(path)).returns(true)
-            File.expects(:open).with(path, "w", 0660).yields(file)
+            @store.expects(:writelock).with(path, 0660).yields(file)
             file.expects(:print).with(yaml)
 
             @store.save(@request)
@@ -74,9 +74,11 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do
             file = mock 'file'
             path = @store.send(:path, @subject.name)
             dir = File.dirname(path)
+
             FileTest.expects(:exist?).with(dir).returns(false)
             Dir.expects(:mkdir).with(dir)
-            File.expects(:open).with(path, "w", 0660).yields(file)
+
+            @store.expects(:writelock).yields(file)
             file.expects(:print).with(yaml)
 
             @store.save(@request)
@@ -84,24 +86,29 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do
     end
 
     describe Puppet::Indirector::Yaml, " when retrieving YAML" do
-        it "should read YAML in from disk and convert it to Ruby objects" do
+        it "should read YAML in from disk using a read lock and convert it to Ruby objects" do
             path = @store.send(:path, @subject.name)
 
             yaml = @subject.to_yaml
             FileTest.expects(:exist?).with(path).returns(true)
-            File.expects(:read).with(path).returns(yaml)
+
+            fh = mock 'filehandle'
+            @store.expects(:readlock).with(path).yields fh
+            fh.expects(:read).returns yaml
 
             @store.find(@request).instance_variable_get("@name").should == :me
         end
 
         it "should fail coherently when the stored YAML is invalid" do
             path = @store.send(:path, @subject.name)
+            FileTest.expects(:exist?).with(path).returns(true)
 
             # Something that will fail in yaml
             yaml = "--- !ruby/object:Hash"
 
-            FileTest.expects(:exist?).with(path).returns(true)
-            File.expects(:read).with(path).returns(yaml)
+            fh = mock 'filehandle'
+            @store.expects(:readlock).yields fh
+            fh.expects(:read).returns yaml
 
             proc { @store.find(@request) }.should raise_error(Puppet::Error)
         end

-- 
Puppet packaging for Debian