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)

5 files changed, 70 insertions, 45 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a4068cb8e..99b5124f2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,12 @@
+    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/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/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/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