From f3a304c557c096b5dbf319671cb84e5611eee30f Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 16 Mar 2008 17:37:12 -0500 Subject: Modifying the yaml terminus base class to use the timestamp of the yaml file as the version of the object. --- lib/puppet/indirector/yaml.rb | 5 +++++ spec/unit/indirector/yaml.rb | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) 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/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 -- cgit From 4a45a1da0653f18df6bd41b009a30387df697909 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 16 Mar 2008 17:52:46 -0500 Subject: 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) --- CHANGELOG | 9 ++++ lib/puppet/indirector/node/exec.rb | 7 +++ lib/puppet/node.rb | 2 +- spec/unit/indirector/node/exec.rb | 93 ++++++++++++++++++++------------------ spec/unit/node.rb | 4 ++ 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 -- cgit From 1dc0e24a4ff565486c00c542f78ab45e23a64ef9 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 16 Mar 2008 17:58:16 -0500 Subject: 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) --- CHANGELOG | 4 ++++ lib/puppet/indirector/node/ldap.rb | 4 ++++ spec/unit/indirector/node/ldap.rb | 7 +++++++ 3 files changed, 15 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 99b5124f2..e17c1737a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ + 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 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/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 -- cgit From bba0b43a594273b82721b6e272aa5d95ab4e89f1 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 16 Mar 2008 17:58:57 -0500 Subject: Downgrading the "Using cache" message from the indirection to debug --- lib/puppet/indirector/indirection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 -- cgit