Fixing a small checksumming bug, reorganizing the client stuff a bit, and adding freshness checking for the configuration, so the config is recompiled every time nor is it downloaded unless it has been recompiled

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

4 files changed, 151 insertions, 39 deletions

diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index adc6b09ba..9023a2529 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -276,6 +276,26 @@ module Puppet
                 return transaction
             end
 
+            # Cache the config
+            def cache(text)
+                Puppet.info "Caching configuration at %s" % self.cachefile
+                confdir = File.dirname(Puppet[:localconfig])
+                unless FileTest.exists?(confdir)
+                    Puppet.recmkdir(confdir, 0770)
+                end
+                File.open(self.cachefile + ".tmp", "w", 0660) { |f|
+                    f.print text
+                }
+                File.rename(self.cachefile + ".tmp", self.cachefile)
+            end
+
+            def cachefile
+                unless defined? @cachefile
+                    @cachefile = Puppet[:localconfig] + ".yaml"
+                end
+                @cachefile
+            end
+
             # Initialize and load storage
             def dostorage
                 begin
@@ -293,9 +313,27 @@ module Puppet
                 end
             end
 
+            # Check whether our configuration is up to date
+            def fresh?
+                unless defined? @configstamp
+                    return false
+                end
+
+                # We're willing to give a 2 second drift
+                if @driver.freshness - @configstamp < 1
+                    return true
+                else
+                    return false
+                end
+            end
+
             # Retrieve the config from a remote server.  If this fails, then
             # use the cached copy.
             def getconfig
+                if self.fresh?
+                    Puppet.info "Config is up to date"
+                    return
+                end
                 Puppet.debug("getting config")
                 dostorage()
 
@@ -309,50 +347,47 @@ module Puppet
 
                 objects = nil
                 if @local
+                    # 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
 
                     if objects == ""
                         raise Puppet::Error, "Could not retrieve configuration"
                     end
                 else
+                    textobjects = ""
+
                     textfacts = CGI.escape(YAML.dump(facts))
 
                     # error handling for this is done in the network client
-                    textobjects = @driver.getconfig(textfacts, "yaml")
-
-                    unless textobjects == ""
-                        begin
-                            textobjects = CGI.unescape(textobjects)
-                        rescue => detail
-                            raise Puppet::Error, "Could not CGI.unescape configuration"
-                        end
+                    begin
+                        textobjects = @driver.getconfig(textfacts, "yaml")
+                    rescue => detail
+                        Puppet.err "Could not retrieve configuration: %s" % detail
                     end
 
-                    cachefile = Puppet[:localconfig] + ".yaml"
-                    if @cache
+                    fromcache = false
+                    if textobjects == ""
+                        textobjects = self.retrievecache
                         if textobjects == ""
-                            if FileTest.exists?(cachefile)
-                                textobjects = File.read(cachefile)
-                            else
-                                raise Puppet::Error.new(
-                                    "Cannot connect to server and there is no cached configuration"
-                                )
-                            end
-                        else
-                            # We store the config so that if we can't connect
-                            # next time, we can just run against the most
-                            # recently acquired copy.
-                            Puppet.info "Caching configuration at %s" % cachefile
-                            confdir = File.dirname(Puppet[:localconfig])
-                            unless FileTest.exists?(confdir)
-                                Puppet.recmkdir(confdir, 0770)
-                            end
-                            File.open(cachefile, "w", 0660) { |f|
-                                f.print textobjects
-                            }
+                            raise Puppet::Error.new(
+                                "Cannot connect to server and there is no cached configuration"
+                            )
                         end
-                    elsif textobjects == ""
-                        raise Puppet::Error, "Could not retrieve configuration"
+                        Puppet.notice "Could not get config; using cached copy"
+                        fromcache = true
+                    end
+
+                    begin
+                        textobjects = CGI.unescape(textobjects)
+                        @configstamp = Time.now.to_i
+                    rescue => detail
+                        raise Puppet::Error, "Could not CGI.unescape configuration"
+                    end
+
+                    if @cache and ! fromcache
+                        self.cache(textobjects)
                     end
 
                     begin
@@ -372,14 +407,14 @@ module Puppet
                 if classes = objects.classes
                     self.setclasses(classes)
                 else
-                    Puppet.info "No classes"
+                    Puppet.info "No classes to store"
                 end
 
                 # Clear all existing objects, so we can recreate our stack.
-                @objects = nil
                 if defined? @objects
                     Puppet::Type.allclear
                 end
+                @objects = nil
 
                 # Now convert the objects to real Puppet objects
                 @objects = objects.to_type
@@ -395,6 +430,46 @@ module Puppet
                 return @objects
             end
 
+            # Retrieve the cached config
+            def retrievecache
+                if FileTest.exists?(self.cachefile)
+                    return File.read(self.cachefile)
+                else
+                    return ""
+                end
+            end
+
+            # The code that actually runs the configuration.  For now, just
+            # ignore the onetime thing.
+            def run(onetime = false)
+                #if onetime
+                    begin
+                        self.getconfig
+                        self.apply
+                    rescue => detail
+                        Puppet.err detail.to_s
+                        if Puppet[:debug]
+                            puts detail.backtrace
+                        end
+                        exit(13)
+                    end
+                    return
+                #end
+
+#                Puppet.newthread do
+#                    begin
+#                        self.getconfig
+#                        self.apply
+#                    rescue => detail
+#                        Puppet.err detail.to_s
+#                        if Puppet[:debug]
+#                            puts detail.backtrace
+#                        end
+#                        exit(13)
+#                    end
+#                end
+            end
+
             def setclasses(ary)
                 begin
                     File.open(Puppet[:classfile], "w") { |f|
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index b1cf4207c..942c417a2 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -10,7 +10,7 @@ require 'puppet/parser/scope'
 module Puppet
     module Parser
         class Interpreter
-            attr_accessor :ast
+            attr_accessor :ast, :filetimeout
             # just shorten the constant path a bit, using what amounts to an alias
             AST = Puppet::Parser::AST
 
@@ -21,6 +21,9 @@ module Puppet
                 end
 
                 @file = hash[:Manifest]
+                @filetimeout = hash[:ParseCheck] || 15
+
+                @lastchecked = 0
 
                 if hash.include?(:UseNodes)
                     @usenodes = hash[:UseNodes]
@@ -38,6 +41,11 @@ module Puppet
                 evaluate
             end
 
+            def parsedate
+                parsefiles()
+                @parsedate
+            end
+
             # evaluate our whole tree
             def run(client, facts)
                 parsefiles()
@@ -124,14 +132,20 @@ module Puppet
 
             def parsefiles
                 if defined? @parser
-                    unless @parser.reparse?
-                        return false
+                    # Only check the files every 15 seconds or so, not on
+                    # every single connection
+                    if (Time.now - @lastchecked).to_i >= @filetimeout.to_i
+                        unless @parser.reparse?
+                            @lastchecked = Time.now
+                            return false
+                        end
+                    else
+                        return
                     end
                 end
 
                 unless FileTest.exists?(@file)
                     if @ast
-                        Puppet.warning "Manifest %s has disappeared" % @file
                         return
                     else
                         raise Puppet::Error, "Manifest %s must exist" % @file
@@ -146,6 +160,10 @@ module Puppet
                 @parser.file = @file
                 @ast = @parser.parse
 
+                # Mark when we parsed, so we can check freshness
+                @parsedate = Time.now.to_i
+                @lastchecked = Time.now
+
                 # Reevaluate the config.  This is what actually replaces the
                 # existing scope.
                 evaluate
diff --git a/lib/puppet/server/master.rb b/lib/puppet/server/master.rb
index 8d4ddbfde..1b9883ead 100644
--- a/lib/puppet/server/master.rb
+++ b/lib/puppet/server/master.rb
@@ -14,8 +14,26 @@ class Server
 
         @interface = XMLRPC::Service::Interface.new("puppetmaster") { |iface|
                 iface.add_method("string getconfig(string)")
+                iface.add_method("int freshness()")
         }
 
+        def filetimeout
+            @interpreter.filetimeout
+        end
+
+        def filetimeout=(int)
+            @interpreter.filetimeout = int
+        end
+
+        # Tell a client whether there's a fresh config for it
+        def freshness(client = nil, clientip = nil)
+            if defined? @interpreter
+                return @interpreter.parsedate
+            else
+                return 0
+            end
+        end
+
         def initialize(hash = {})
 
             # FIXME this should all be s/:File/:Manifest/g or something
@@ -35,9 +53,11 @@ class Server
                 @ca = nil
             end
 
+            @parsecheck = hash[:FileTimeout] || 15
+
             Puppet.debug("Creating interpreter")
 
-            args = {:Manifest => @file}
+            args = {:Manifest => @file, :ParseCheck => @parsecheck}
 
             if hash.include?(:UseNodes)
                 args[:UseNodes] = hash[:UseNodes]
diff --git a/lib/puppet/type/pfile/checksum.rb b/lib/puppet/type/pfile/checksum.rb
index 2f71a1a30..61d8fea80 100755
--- a/lib/puppet/type/pfile/checksum.rb
+++ b/lib/puppet/type/pfile/checksum.rb
@@ -134,7 +134,6 @@ module Puppet
             # out of sync.  We don't want to generate an event the first
             # time we get a sum.
             if ! defined? @should or @should == [:nosum]
-                self.info "@should is %s" % @should.inspect
                 @should = [@is]
                 # FIXME we should support an updatechecksums-like mechanism
                 self.updatesum