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.

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
@@ -8,16 +8,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)
         Facter.expects(:value).with('fqdn').returns("my.server.com")
@@ -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