Removed the code from the client that tries to avoid recompiling

the catalog. The client will now always recompile, assuming it
can reach the server.  It will still use the cached config if
there's a failure.

4 files changed, 7 insertions, 191 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 80fa83c6f..2e1a3d709 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,8 @@
+    Removed the code from the client that tries to avoid recompiling
+    the catalog. The client will now always recompile, assuming it
+    can reach the server.  It will still use the cached config if
+    there's a failure.
+
     Fixing #1173 -- classes and definitions can now have the same
     name as a directory with no failures.
 
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index 955acbfb5..22bb3fa4e 100644
--- a/lib/puppet/network/client/master.rb
+++ b/lib/puppet/network/client/master.rb
@@ -49,6 +49,8 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
     end
 
     # Return the list of dynamic facts as an array of symbols
+    # NOTE:LAK(2008/04/10): This code is currently unused, since we now always
+    # recompile.
     def self.dynamic_facts
         # LAK:NOTE See http://snurl.com/21zf8  [groups_google_com] 
         x = Puppet.settings[:dynamicfacts].split(/\s*,\s*/).collect { |fact| fact.downcase }
@@ -97,31 +99,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         end
     end
 
-    # Check whether our catalog is up to date
-    def fresh?(facts)
-        if Puppet[:ignorecache]
-            Puppet.notice "Ignoring cache"
-            return false
-        end
-        unless self.compile_time
-            Puppet.debug "No cached compile time"
-            return false
-        end
-        if facts_changed?(facts)
-            Puppet.info "Facts have changed; recompiling" unless local?
-            return false
-        end
-
-        newcompile = @driver.freshness
-        # We're willing to give a 2 second drift
-        if newcompile - @compile_time.to_i < 1
-            return true
-        else
-            Puppet.debug "Server compile time is %s vs %s" % [newcompile, @compile_time.to_i]
-            return false
-        end
-    end
-
     # Let the daemon run again, freely in the filesystem.  Frolick, little
     # daemon!
     def enable
@@ -148,11 +125,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         # Retrieve the plugins.
         getplugins() if Puppet[:pluginsync]
 
-        if (self.catalog or FileTest.exist?(self.cachefile)) and self.fresh?(facts)
-            Puppet.info "Configuration is up to date"
-            return if use_cached_config
-        end
-
         Puppet.debug("Retrieving catalog")
 
         # If we can't retrieve the catalog, just return, which will either
@@ -451,32 +423,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
 
     loadfacts()
 
-    # Have the facts changed since we last compiled?
-    def facts_changed?(facts)
-        oldfacts = (Puppet::Util::Storage.cache(:configuration)[:facts] || {}).dup
-        newfacts = facts.dup
-        self.class.dynamic_facts.each do |fact|
-            [oldfacts, newfacts].each do |facthash|
-                facthash.delete(fact) if facthash.include?(fact)
-            end
-        end
-
-        if oldfacts == newfacts
-            return false
-        else
-#            unless oldfacts
-#                puts "no old facts"
-#                return true
-#            end
-#            newfacts.keys.each do |k|
-#                unless newfacts[k] == oldfacts[k]
-#                    puts "%s: %s vs %s" % [k, newfacts[k], oldfacts[k]]
-#                end
-#            end
-            return true
-        end
-    end
-    
     # Actually retrieve the catalog, either from the server or from a
     # local master.
     def get_actual_config(facts)
diff --git a/spec/unit/network/client/master.rb b/spec/unit/network/client/master.rb
index 7bf755d88..c0ad7562f 100755
--- a/spec/unit/network/client/master.rb
+++ b/spec/unit/network/client/master.rb
@@ -36,30 +36,6 @@ describe Puppet::Network::Client::Master, " when retrieving the catalog" do
         proc { @client.getconfig }.should raise_error(Puppet::Network::ClientError)
     end
 
-    it "should use the cached catalog if it is up to date" do
-        file = "/path/to/cachefile"
-        @client.stubs(:cachefile).returns(file)
-        FileTest.expects(:exist?).with(file).returns(true)
-        @client.expects(:fresh?).with(@facts).returns true
-        @client.class.stubs(:facts).returns(@facts)
-        @client.expects(:use_cached_config).returns(true)
-        Puppet.stubs(:info)
-
-        @client.getconfig
-    end
-
-    it "should log that the catalog does not need a recompile" do
-        file = "/path/to/cachefile"
-        @client.stubs(:cachefile).returns(file)
-        FileTest.stubs(:exist?).with(file).returns(true)
-        @client.stubs(:fresh?).with(@facts).returns true
-        @client.stubs(:use_cached_config).returns(true)
-        @client.class.stubs(:facts).returns(@facts)
-        Puppet.expects(:info).with { |m| m.include?("up to date") }
-
-        @client.getconfig
-    end
-
     it "should retrieve plugins if :pluginsync is enabled" do
         file = "/path/to/cachefile"
         @client.stubs(:cachefile).returns(file)
@@ -69,7 +45,6 @@ describe Puppet::Network::Client::Master, " when retrieving the catalog" do
         @client.expects(:getplugins)
         @client.stubs(:get_actual_config).returns(nil)
         FileTest.stubs(:exist?).with(file).returns(true)
-        @client.stubs(:fresh?).with(@facts).returns true
         @client.stubs(:use_cached_config).returns(true)
         @client.class.stubs(:facts).returns(@facts)
         @client.getconfig
diff --git a/test/network/client/master.rb b/test/network/client/master.rb
index 14c7dd852..dc2140e62 100755
--- a/test/network/client/master.rb
+++ b/test/network/client/master.rb
@@ -305,44 +305,6 @@ end
         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 }" }
-        
-        Puppet::Node::Facts.indirection.stubs(:save)
-        
-        driver = mkmaster(:Manifest => 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?(master.class.facts),
-            "Considered fresh with no compile at all")
-
-        assert_nothing_raised { master.run }
-        assert(master.fresh?(master.class.facts),
-            "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::Util::Storage.cache(:configuration)[:compile_time], "times did not match")
-        time = master.compile_time
-        master.clear
-        File.unlink(file)
-        Puppet::Util::Storage.store
-        
-        # Now make a new master
-        Puppet::Util::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
-
     # #540 - make sure downloads aren't affected by noop
     def test_download_in_noop
         source = tempfile
@@ -384,44 +346,6 @@ end
             "Tried to load cache when it is non-existent")
     end
 
-    # #519 - cache the facts so that we notice if they change.
-    def test_factchanges_cause_recompile
-        $value = "one"
-        Facter.add(:testfact) do
-            setcode { $value }
-        end
-        assert_equal("one", Facter.value(:testfact), "fact was not set correctly")
-        master = mkclient
-        master.local = false
-        driver = master.send(:instance_variable_get, "@driver")
-        driver.local = false
-
-        Puppet::Node::Facts.indirection.stubs(:save)
-
-        assert_nothing_raised("Could not compile config") do
-            master.getconfig
-        end
-
-        $value = "two"
-        Facter.clear
-        Facter.loadfacts
-        Facter.add(:testfact) do
-            setcode { $value }
-        end
-        facts = master.class.facts
-        assert_equal("two", Facter.value(:testfact), "fact did not change")
-
-        assert(master.send(:facts_changed?, facts),
-            "master does not think facts changed")
-        assert(! master.fresh?(facts),
-            "master is considered fresh after facts changed")
-
-        assert_nothing_raised("Could not recompile when facts changed") do
-            master.getconfig
-        end
-
-    end
-
     def test_locking
         master = mkclient
 
@@ -455,40 +379,6 @@ end
         assert_equal(100, master.timeout, "Did not get changed integer default value for timeout on second run")
     end
 
-    # #569 -- Make sure we can ignore dynamic facts.
-    def test_dynamic_facts
-        client = mkclient 
-
-        assert_equal(%w{memorysize memoryfree swapsize swapfree}, client.class.dynamic_facts,
-            "Did not get correct defaults for dynamic facts")
-
-        # Cache some values for comparison
-        cached = {"one" => "yep", "two" => "nope"}
-        Puppet::Util::Storage.cache(:configuration)[:facts] = cached
-
-        assert(! client.send(:facts_changed?, cached), "Facts incorrectly considered to be changed")
-
-        # Now add some values to the passed result and make sure we get a positive
-        newfacts = cached.dup
-        newfacts["changed"] = "something"
-
-        assert(client.send(:facts_changed?, newfacts), "Did not catch changed fact")
-
-        # Now add a dynamic fact and make sure it's ignored
-        newfacts = cached.dup
-        newfacts["memorysize"] = "something"
-
-        assert(! client.send(:facts_changed?, newfacts), "Dynamic facts resulted in a false positive")
-
-        # And try it with both
-        cached["memorysize"] = "something else"
-        assert(! client.send(:facts_changed?, newfacts), "Dynamic facts resulted in a false positive")
-
-        # And finally, with only in the cache
-        newfacts.delete("memorysize")
-        assert(! client.send(:facts_changed?, newfacts), "Dynamic facts resulted in a false positive")
-    end
-
     def test_splay
         client = mkclient