From 74d77f76a012d523430e43f1092609a4ca584cc7 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 12 Oct 2007 18:00:10 -0500 Subject: Adding version handling through most of the indirection work. This counts as the first commit where configuration compiling actually uses the caching correctly according to the application model. --- lib/puppet/indirector.rb | 4 ++ lib/puppet/indirector/code/configuration.rb | 20 +++++---- lib/puppet/indirector/indirection.rb | 6 +++ lib/puppet/node/configuration.rb | 6 ++- spec/unit/indirector/code/configuration.rb | 68 ++++++++++++++++++++++++----- spec/unit/indirector/indirection.rb | 5 +++ spec/unit/indirector/indirector.rb | 5 +++ 7 files changed, 94 insertions(+), 20 deletions(-) diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index a2eb41763..6009a7ba7 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -59,6 +59,10 @@ module Puppet::Indirector def search(*args) indirection.search(*args) end + + def version(*args) + indirection.version(*args) + end end module InstanceMethods diff --git a/lib/puppet/indirector/code/configuration.rb b/lib/puppet/indirector/code/configuration.rb index 949926a3c..50728757c 100644 --- a/lib/puppet/indirector/code/configuration.rb +++ b/lib/puppet/indirector/code/configuration.rb @@ -47,15 +47,19 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code $0 =~ /puppetmasterd/ end - # Return the configuration version. - def version(client = nil, clientip = nil) - if client and node = Puppet::Node.search(client) - update_node_check(node) - return interpreter.configuration_version(node) + # Return the configuration version. Here we're returning the + # latest of the node, fact, or parse date. These are the + # three things that go into compiling a client configuration, + # so changes in any of them result in changes. + # LAK:FIXME Note that this only works when all three sources + # use timestamps; once one of them moves to using real versions, + # the comparison stops working. + def version(key) + if node = Puppet::Node.search(key) + return [Puppet::Node.version(key).to_f, Puppet::Node::Facts.version(key).to_f, interpreter.configuration_version(node).to_f].sort[-1] else - # Just return something that will always result in a recompile, because - # this is local. - return (Time.now + 1000).to_i + # This is the standard for "got nothing for ya". + 0 end end diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 5f7baa3da..efa8819bb 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -92,6 +92,7 @@ class Puppet::Indirector::Indirection def find(key, *args) if cache? and cache.has_most_recent?(key, terminus.version(key)) + Puppet.info "Using cached %s %s" % [self.name, key] return cache.find(key, *args) end if result = terminus.find(key, *args) @@ -117,10 +118,15 @@ class Puppet::Indirector::Indirection instance.version ||= Time.now.utc dest = cache? ? cache : terminus return if dest.has_most_recent?(instance.name, instance.version) + Puppet.info "Caching %s %s" % [self.name, instance.name] if cache? cache.save(instance, *args) if cache? terminus.save(instance, *args) end + def version(*args) + terminus.version(*args) + end + private # Create a new terminus instance. diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index da8dc3a9a..9adf9aea3 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -388,8 +388,12 @@ class Puppet::Node::Configuration < Puppet::PGraph # dumped by default, nor does yaml-dumping # the edge-labels work at this point (I don't # know why). # Neither of these matters right now, but I suppose it could at some point. + # We also have to have the vertex_dict dumped after the resource table, because yaml can't + # seem to handle the output of yaml-dumping the vertex_dict. def to_yaml_properties - instance_variables.reject { |v| %w{@edgelist_class @edge_labels}.include?(v) } + props = instance_variables.reject { |v| %w{@edgelist_class @edge_labels @vertex_dict}.include?(v) } + props << "@vertex_dict" + props end private diff --git a/spec/unit/indirector/code/configuration.rb b/spec/unit/indirector/code/configuration.rb index 0038a038e..b2a23ac19 100755 --- a/spec/unit/indirector/code/configuration.rb +++ b/spec/unit/indirector/code/configuration.rb @@ -7,16 +7,6 @@ require File.dirname(__FILE__) + '/../../../spec_helper' require 'puppet/indirector/code/configuration' -describe Puppet::Indirector::Code::Configuration do - # LAK:TODO I have no idea how to do this, or even if it should be in this class or test or what. - # This is used for determining if the client should recompile its configuration, so it's not sufficient - # to recompile and compare versions. - # It might be that the right solution is to require configuration caching, and then compare the cached - # configuration version to the current version, via some querying mechanism (i.e., the client asks for just - # the configuration's 'up-to-date' attribute, rather than the whole configuration). - it "should provide a mechanism for determining if the client's configuration is up to date" -end - describe Puppet::Indirector::Code::Configuration do before do Puppet.expects(:version).returns(1) @@ -48,6 +38,8 @@ end describe Puppet::Indirector::Code::Configuration, " when creating the interpreter" do before do + # This gets pretty annoying on a plane where we have no IP address + Facter.stubs(:value).returns("whatever") @compiler = Puppet::Indirector::Code::Configuration.new end @@ -67,6 +59,7 @@ end describe Puppet::Indirector::Code::Configuration, " when finding nodes" do before do + Facter.stubs(:value).returns("whatever") @compiler = Puppet::Indirector::Code::Configuration.new @name = "me" @node = mock 'node' @@ -117,11 +110,14 @@ describe Puppet::Indirector::Code::Configuration, " after finding nodes" do # LAK:TODO This is going to be difficult, because this whole process is so # far removed from the actual connection that the certificate information # will be quite hard to come by, dum by, gum by. - it "should search for the name using the client certificate's DN if the :node_name setting is set to 'cert'" + it "should search for the name using the client certificate's DN if the :node_name setting is set to 'cert'" do + pending "Probably will end up in the REST work" + end end describe Puppet::Indirector::Code::Configuration, " when creating configurations" do before do + Facter.stubs(:value).returns("whatever") @compiler = Puppet::Indirector::Code::Configuration.new @name = "me" @node = Puppet::Node.new @name, :environment => "yay" @@ -158,3 +154,53 @@ describe Puppet::Indirector::Code::Configuration, " when creating configurations @compiler.find(@name) end end + +describe Puppet::Indirector::Code::Configuration, " when determining a client's available configuration version" do + before do + Puppet::Node::Facts.stubs(:find).returns(nil) + Facter.stubs(:value).returns("whatever") + @configuration = Puppet::Indirector::Code::Configuration.new + @name = "johnny" + end + + it "should provide a mechanism for providing the version of a given client's configuration" do + @configuration.should respond_to(:version) + end + + it "should use the client's Facts version as the available configuration version if it is the most recent" do + Puppet::Node::Facts.expects(:version).with(@name).returns(5) + Puppet::Node.expects(:version).with(@name).returns(3) + @configuration.interpreter.stubs(:configuration_version).returns(4) + + @configuration.version(@name).should == 5 + end + + it "should use the client's Node version as the available configuration version if it is the most recent" do + Puppet::Node::Facts.expects(:version).with(@name).returns(3) + Puppet::Node.expects(:version).with(@name).returns(5) + @configuration.interpreter.stubs(:configuration_version).returns(4) + + @configuration.version(@name).should == 5 + end + + it "should use the last parse date as the available configuration version if it is the most recent" do + Puppet::Node::Facts.expects(:version).with(@name).returns(3) + Puppet::Node.expects(:version).with(@name).returns(4) + @configuration.interpreter.stubs(:configuration_version).returns(5) + + @configuration.version(@name).should == 5 + end + + it "should return a version of 0 if no information on the node can be found" do + Puppet::Node.stubs(:search).returns(nil) + @configuration.version(@name).should == 0 + end + + it "should indicate when an update is available even if an input has clock skew" do + pending "Unclear how to implement this" + end + + it "should not indicate an available update when apparent updates are a result of clock skew" do + pending "Unclear how to implement this" + end +end diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb index 7a1c4531c..25152d123 100755 --- a/spec/unit/indirector/indirection.rb +++ b/spec/unit/indirector/indirection.rb @@ -37,6 +37,11 @@ describe Puppet::Indirector::Indirection do @terminus.expects(:save).with(@instance).returns(@instance) @indirection.save(@instance).should == @instance end + + it "should handle version lookups by letting the appropriate terminus perform the lookup" do + @terminus.expects(:version).with(@name).returns(5) + @indirection.version(@name).should == 5 + end it "should add versions to found instances that do not already have them" do @terminus.expects(:find).with(@name).returns(@instance) diff --git a/spec/unit/indirector/indirector.rb b/spec/unit/indirector/indirector.rb index 78c8c614a..1f63da30d 100755 --- a/spec/unit/indirector/indirector.rb +++ b/spec/unit/indirector/indirector.rb @@ -95,6 +95,11 @@ describe Puppet::Indirector, " when redirecting a model" do thing.save end + it "should give the model the ability to look up an instance's version by letting the indirection perform the lookup" do + @indirection.expects(:version).with(:thing) + @thingie.version(:thing) + end + it "should give the model the ability to set the indirection terminus class" do @indirection.expects(:terminus_class=).with(:myterm) @thingie.terminus_class = :myterm -- cgit