Merge branch '0.24.x' of ssh://reductivelabs.com/opt/rl/git/puppet into 0.24.x

10 files changed, 102 insertions, 47 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a4068cb8e..e17c1737a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,16 @@
+    Modified the ldap node terminus to also use the facts version
+    as the version for a node, which should similarly encourage the
+    use of the yaml cache.  (Related to #1130)
+
+    Caching node information in yaml (I figured caching in memory will
+    cause ever-larger memory growth), and changing the external node
+    terminus to use the version of the facts as their version.  This
+    will usually result in the cached node information being used,
+    instead of always hitting the external node app during file
+    serving. Note that if the facts aren't changed by the client,
+    then this will result in the cached node being used, but at this
+    point, the client always updates its facts.  (#1130)
+
     Fixing #1132 -- host names can now have dashes anywhere.
     (Patch by freiheit.)
 
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 129676e9c..d47433c60 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -162,7 +162,7 @@ class Puppet::Indirector::Indirection
 
         # See if our instance is in the cache and up to date.
         if cache? and cache.has_most_recent?(key, terminus(terminus_name).version(key))
-            Puppet.info "Using cached %s %s" % [self.name, key]
+            Puppet.debug "Using cached %s %s" % [self.name, key]
             return cache.find(key, *args)
         end
 
diff --git a/lib/puppet/indirector/node/exec.rb b/lib/puppet/indirector/node/exec.rb
index 71f4fa8e1..dcfc625b2 100644
--- a/lib/puppet/indirector/node/exec.rb
+++ b/lib/puppet/indirector/node/exec.rb
@@ -24,6 +24,13 @@ class Puppet::Node::Exec < Puppet::Indirector::Exec
         return create_node(name, result)
     end
 
+    # Use the version of the facts, since we assume that's the main thing
+    # that changes.  If someone wants their own way of defining version,
+    # they can easily provide their own, um, version of this class.
+    def version(name)
+        Puppet::Node::Facts.version(name)
+    end
+
     private
 
     # Turn our outputted objects into a Puppet::Node instance.
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 9320f3ba1..8537e1cf3 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -122,4 +122,8 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         end
         filter
     end
+
+    def version(name)
+        Puppet::Node::Facts.version(name)
+    end
 end
diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb
index 16816d941..4baeb38db 100644
--- a/lib/puppet/indirector/yaml.rb
+++ b/lib/puppet/indirector/yaml.rb
@@ -40,6 +40,11 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
         end
     end
 
+    def version(name)
+        return nil unless FileTest.exist?(path(name))
+        return File.stat(path(name)).mtime
+    end
+
     private
 
     def from_yaml(text)
diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb
index c0628ecdc..c39f364bc 100644
--- a/lib/puppet/node.rb
+++ b/lib/puppet/node.rb
@@ -10,7 +10,7 @@ class Puppet::Node
     extend Puppet::Indirector
 
     # Use the node source as the indirection terminus.
-    indirects :node, :terminus_setting => :node_terminus, :doc => "Where to find node information.
+    indirects :node, :terminus_setting => :node_terminus, :cache_class => :yaml, :doc => "Where to find node information.
         A node is composed of its name, its facts, and its environment."
 
     # Retrieve a node from the node source, with some additional munging
diff --git a/spec/unit/indirector/node/exec.rb b/spec/unit/indirector/node/exec.rb
index 5e6a9b1da..b67e0fe97 100755
--- a/spec/unit/indirector/node/exec.rb
+++ b/spec/unit/indirector/node/exec.rb
@@ -4,65 +4,70 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/indirector/node/exec'
 
-describe Puppet::Node::Exec, " when constructing the command to run" do
+describe Puppet::Node::Exec do
     before do
         @indirection = mock 'indirection'
         Puppet.settings.stubs(:value).with(:external_nodes).returns("/echo")
         @searcher = Puppet::Node::Exec.new
     end
 
-    it "should use the external_node script as the command" do
-        Puppet.expects(:[]).with(:external_nodes).returns("/bin/echo")
-        @searcher.command.should == %w{/bin/echo}
+    it "should use the version of the facts as its version" do
+        version = mock 'version'
+        Puppet::Node::Facts.expects(:version).with("me").returns version
+        @searcher.version("me").should equal(version)
     end
 
-    it "should throw an exception if no external node command is set" do
-        Puppet.expects(:[]).with(:external_nodes).returns("none")
-        proc { @searcher.find("foo") }.should raise_error(ArgumentError)
-    end
-end
+    describe "when constructing the command to run" do
+        it "should use the external_node script as the command" do
+            Puppet.expects(:[]).with(:external_nodes).returns("/bin/echo")
+            @searcher.command.should == %w{/bin/echo}
+        end
 
-describe Puppet::Node::Exec, " when handling the results of the command" do
-    before do
-        @indirection = mock 'indirection'
-        Puppet.settings.stubs(:value).with(:external_nodes).returns("/echo")
-        @searcher = Puppet::Node::Exec.new
-        @node = stub 'node', :fact_merge => nil
-        @name = "yay"
-        Puppet::Node.expects(:new).with(@name).returns(@node)
-        @result = {}
-        # Use a local variable so the reference is usable in the execute() definition.
-        result = @result
-        @searcher.meta_def(:execute) do |command|
-            return YAML.dump(result)
+        it "should throw an exception if no external node command is set" do
+            Puppet.expects(:[]).with(:external_nodes).returns("none")
+            proc { @searcher.find("foo") }.should raise_error(ArgumentError)
         end
     end
 
-    it "should translate the YAML into a Node instance" do
-        # Use an empty hash
-        @searcher.find(@name).should equal(@node)
-    end
+    describe "when handling the results of the command" do
+        before do
+            @node = stub 'node', :fact_merge => nil
+            @name = "yay"
+            Puppet::Node.expects(:new).with(@name).returns(@node)
+            @result = {}
+            # Use a local variable so the reference is usable in the execute() definition.
+            result = @result
+            @searcher.meta_def(:execute) do |command|
+                return YAML.dump(result)
+            end
+        end
 
-    it "should set the resulting parameters as the node parameters" do
-        @result[:parameters] = {"a" => "b", "c" => "d"}
-        @node.expects(:parameters=).with "a" => "b", "c" => "d"
-        @searcher.find(@name)
-    end
+        it "should translate the YAML into a Node instance" do
+            # Use an empty hash
+            @searcher.find(@name).should equal(@node)
+        end
 
-    it "should set the resulting classes as the node classes" do
-        @result[:classes] = %w{one two}
-        @node.expects(:classes=).with %w{one two}
-        @searcher.find(@name)
-    end
+        it "should set the resulting parameters as the node parameters" do
+            @result[:parameters] = {"a" => "b", "c" => "d"}
+            @node.expects(:parameters=).with "a" => "b", "c" => "d"
+            @searcher.find(@name)
+        end
 
-    it "should merge the node's facts with its parameters" do
-        @node.expects(:fact_merge)
-        @searcher.find(@name)
-    end
+        it "should set the resulting classes as the node classes" do
+            @result[:classes] = %w{one two}
+            @node.expects(:classes=).with %w{one two}
+            @searcher.find(@name)
+        end
+
+        it "should merge the node's facts with its parameters" do
+            @node.expects(:fact_merge)
+            @searcher.find(@name)
+        end
 
-    it "should set the node's environment if one is provided" do
-        @result[:environment] = "yay"
-        @node.expects(:environment=).with "yay"
-        @searcher.find(@name)
+        it "should set the node's environment if one is provided" do
+            @result[:environment] = "yay"
+            @node.expects(:environment=).with "yay"
+            @searcher.find(@name)
+        end
     end
 end
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index 2a4f240ed..34456703d 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -5,6 +5,13 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/indirector/node/ldap'
 
 describe Puppet::Node::Ldap do
+    it "should use the version of the facts as its version" do
+        @searcher = Puppet::Node::Ldap.new
+        version = mock 'version'
+        Puppet::Node::Facts.expects(:version).with("me").returns version
+        @searcher.version("me").should equal(version)
+    end
+
     describe "when searching for nodes" do
         before :each do
             @searcher = Puppet::Node::Ldap.new
diff --git a/spec/unit/indirector/yaml.rb b/spec/unit/indirector/yaml.rb
index 2e0b453d7..b61332485 100755
--- a/spec/unit/indirector/yaml.rb
+++ b/spec/unit/indirector/yaml.rb
@@ -24,6 +24,16 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do
         Puppet.settings.stubs(:value).with(:yamldir).returns(@dir)
     end
 
+    it "should use the mtime of the written file as the version" do
+        stat = mock 'stat'
+        FileTest.stubs(:exist?).returns true
+        File.expects(:stat).returns stat
+        time = Time.now
+        stat.expects(:mtime).returns time
+
+        @store.version(:me).should equal(time)
+    end
+
     describe Puppet::Indirector::Yaml, " when choosing file location" do
 
         it "should store all files in a single file root set in the Puppet defaults" do
@@ -98,4 +108,4 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do
             proc { @store.find(@subject.name) }.should raise_error(Puppet::Error)
         end
     end
-end
\ No newline at end of file
+end
diff --git a/spec/unit/node.rb b/spec/unit/node.rb
index 0ce702936..4861cb9e3 100755
--- a/spec/unit/node.rb
+++ b/spec/unit/node.rb
@@ -127,6 +127,10 @@ describe Puppet::Node, " when indirecting" do
         Puppet::Node.indirection.terminus_class.should == :plain
     end
 
+    it "should use yaml for caching" do
+        Puppet::Node.indirection.cache_class.should == :yaml
+    end
+
     after do
         Puppet::Indirector::Indirection.clear_cache
     end