Fixing #424. The configuration compile time is now cached in the yaml cache file.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2081 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 87 insertions, 18 deletions

diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index 972fcb19c..35368de99 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -69,6 +69,7 @@ class Puppet::Client::MasterClient < Puppet::Client
     @drivername = :Master
 
     attr_accessor :objects
+    attr_reader :compile_time
 
     class << self
         # Puppetd should only have one instance running, and we need a way
@@ -103,7 +104,6 @@ class Puppet::Client::MasterClient < Puppet::Client
 
     # This method actually applies the configuration.
     def apply(tags = nil, ignoreschedules = false)
-        dostorage()
         unless defined? @objects
             raise Puppet::Error, "Cannot apply; objects not defined"
         end
@@ -172,6 +172,7 @@ class Puppet::Client::MasterClient < Puppet::Client
     def dostorage
         begin
             Puppet::Storage.load
+            @compile_time ||= Puppet::Storage.cache(:configuration)[:compile_time]
         rescue => detail
             if Puppet[:trace]
                 puts detail.backtrace
@@ -189,14 +190,16 @@ class Puppet::Client::MasterClient < Puppet::Client
 
     # Check whether our configuration is up to date
     def fresh?
-        unless defined? @configstamp
+        unless self.compile_time
+            Puppet.warning "No compile time"
             return false
         end
 
         # We're willing to give a 2 second drift
-        if @driver.freshness - @configstamp < 1
+        if @driver.freshness - @compile_time < 1
             return true
         else
+            Puppet.warning "%s vs %s" % [@driver.freshness, @compile_time]
             return false
         end
     end
@@ -215,12 +218,19 @@ class Puppet::Client::MasterClient < Puppet::Client
     # Retrieve the config from a remote server.  If this fails, then
     # use the cached copy.
     def getconfig
+        dostorage()
         if self.fresh?
             Puppet.info "Config is up to date"
+            unless defined? @objects
+                begin
+                    @objects = YAML.load(self.retrievecache).to_type
+                rescue => detail
+                    Puppet.warning "Could not load cached configuration: %s" % detail
+                end
+            end
             return
         end
         Puppet.debug("getting config")
-        dostorage()
 
         # Retrieve the plugins.
         if Puppet[:pluginsync]
@@ -282,7 +292,8 @@ class Puppet::Client::MasterClient < Puppet::Client
         Puppet.config.use(:puppet, :sslcertificates, :puppetd)
         super
 
-        @configtime = Time.now
+        # This might be nil
+        @configtime = 0
 
         self.class.instance = self
         @running = false
@@ -537,7 +548,7 @@ class Puppet::Client::MasterClient < Puppet::Client
         # If we're local, we don't have to do any of the conversion
         # stuff.
         objects = @driver.getconfig(facts, "yaml")
-        @configstamp = Time.now.to_i
+        @compile_time = Time.now.to_i
 
         if objects == ""
             raise Puppet::Error, "Could not retrieve configuration"
@@ -578,7 +589,9 @@ class Puppet::Client::MasterClient < Puppet::Client
             Puppet.warning "Could not get config; using cached copy"
             fromcache = true
         else
-            @configstamp = Time.now.to_i
+            @compile_time = Time.now.to_i
+            Puppet.warning "Caching compile time %s" % @compile_time.inspect
+            Puppet::Storage.cache(:configuration)[:compile_time] = @compile_time
         end
 
         begin
diff --git a/lib/puppet/storage.rb b/lib/puppet/storage.rb
index e6c92cc65..ebebb8ee5 100644
--- a/lib/puppet/storage.rb
+++ b/lib/puppet/storage.rb
@@ -18,18 +18,21 @@ module Puppet
         # types like exec, but it also means that if an object changes locations
         # in the configuration it will lose its cache.
         def self.cache(object)
-            unless object.is_a? Puppet::Type
-                raise Puppet::DevFail, "Must pass a Type instance to Storage.cache"
-            end
-
-            # We used to store things by path, now we store them by ref.
-            # In oscar(0.20.0) this changed to using the ref.
-            if @@state.include?(object.path)
-                @@state[object.ref] = @@state[object.path]
-                @@state.delete(object.path)
+            if object.is_a? Puppet::Type
+                # We used to store things by path, now we store them by ref.
+                # In oscar(0.20.0) this changed to using the ref.
+                if @@state.include?(object.path)
+                    @@state[object.ref] = @@state[object.path]
+                    @@state.delete(object.path)
+                end
+                name = object.ref
+            elsif object.is_a?(Symbol)
+                name = object
+            else
+                raise ArgumentError, "You can only cache information for Types and symbols"
             end
 
-            return @@state[object.ref] ||= {}
+            return @@state[name] ||= {}
         end
 
         def self.clear
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 33874f83e..5659d3760 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -38,7 +38,6 @@ class Transaction
     # Apply all changes for a resource, returning a list of the events
     # generated.
     def apply(resource)
-        resource.info "Applying"
         begin
             changes = resource.evaluate
         rescue => detail
diff --git a/test/client/master.rb b/test/client/master.rb
index 48a8dc80a..41e2df111 100755
--- a/test/client/master.rb
+++ b/test/client/master.rb
@@ -464,6 +464,40 @@ end
         # And make sure the ruby version is in there
         assert_equal(RUBY_VERSION, facts["rubyversion"], "ruby version did not get added")
     end
+    
+    # #424
+    def test_caching_of_compile_time
+        file = tempfile()
+        manifest = tempfile()
+        File.open(manifest, "w") { |f| f.puts "file { '#{file}': content => yay }" }
+        
+        driver = mkmaster(manifest)
+        driver.local = false
+        master = mkclient(driver)
+        
+        # We have to make everything thinks it's remote, because there's no local caching info
+        master.local = false
+        
+        assert(! master.fresh?, "Considered fresh with no compile at all")
+        
+        assert_nothing_raised { master.run }
+        assert(master.fresh?, "not considered fresh after compile")
+        
+        # Now make sure the config time is cached
+        assert(master.compile_time, "No stored config time")
+        assert_equal(master.compile_time, Puppet::Storage.cache(:configuration)[:compile_time], "times did not match")
+        time = master.compile_time
+        master.clear
+        File.unlink(file)
+        Puppet::Storage.store
+        
+        # Now make a new master
+        Puppet::Storage.clear
+        master = mkclient(driver)
+        master.run
+        assert_equal(time, master.compile_time, "time was not retrieved from cache")
+        assert(FileTest.exists?(file), "file was not created on second run")
+    end
 end
 
 # $Id$
diff --git a/test/other/storage.rb b/test/other/storage.rb
index 90b39df20..1ad51e4ee 100755
--- a/test/other/storage.rb
+++ b/test/other/storage.rb
@@ -98,6 +98,26 @@ class TestStorage < Test::Unit::TestCase
         assert_same Hash, state.class
         assert_equal 0, state.size
     end
+    
+    def test_caching
+        hash = nil
+        one = Puppet::Type.type(:exec).create :title => "/bin/echo one"
+        [one, :yayness].each do |object|
+            assert_nothing_raised do
+                hash = Puppet::Storage.cache(object)
+            end
+            assert_equal({}, hash, "Did not get empty hash back for %s" % object)
+        
+            hash[:testing] = true
+            assert_nothing_raised do
+                hash = Puppet::Storage.cache(object)
+            end
+            assert_equal({:testing => true}, hash, "Did not get hash back for %s" % object)
+        end
+        assert_raise(ArgumentError, "was able to cache from string") do
+            Puppet::Storage.cache("somethingelse")
+        end
+    end
 end
 
 # $Id$