Running puppetd with --use_cached_catalog you would see messages like:
info: Not using expired catalog for mattmac.local from cache
notice: Using cached catalog
Both Puppet::Util::Cacher, which extends catalogs, and
Puppet::Indirector::Envelope,
which extends all Indirection objects including catalogs, have their own cache
expiring mechanisms. The Envelope mechanism was declining to use cached
catalogs without taking into account what the --use_cached_catalog
options said. This patch fixes so it uses the cached catalog and just logs:
debug: Using cached catalog for mattmac.local
This commit also renames a method that makes requests from request
to instantiate request, and gets rid of the extender hook on Cacher
since it was only being used on one test and probably shouldn't be used
in general.
Reviewed by: Nick Lewis
Signed-off-by: Matt Robinson <[email protected]>
---
lib/puppet/configurer.rb | 1 -
lib/puppet/indirector/indirection.rb | 20 +++++++-------
lib/puppet/resource/catalog.rb | 5 +++
lib/puppet/util/cacher.rb | 9 ------
spec/integration/indirector/direct_file_server.rb | 6 ++--
spec/unit/indirector/indirection.rb | 28 ++++++++++----------
spec/unit/resource/catalog.rb | 7 +++++
spec/unit/util/cacher.rb | 6 ++--
8 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 6aeac74..551d996 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -194,7 +194,6 @@ class Puppet::Configurer
@duration = thinmark do
result = Puppet::Resource::Catalog.find(Puppet[:certname],
fact_options.merge(:ignore_terminus => true))
end
- Puppet.notice "Using cached catalog"
result
rescue => detail
puts detail.backtrace if Puppet[:trace]
diff --git a/lib/puppet/indirector/indirection.rb
b/lib/puppet/indirector/indirection.rb
index 7c31833..97fd03a 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -116,7 +116,7 @@ class Puppet::Indirector::Indirection
end
# Set up our request object.
- def request(method, key, arguments = nil)
+ def instantiate_request(method, key, arguments = nil)
Puppet::Indirector::Request.new(self.name, method, key, arguments)
end
@@ -169,24 +169,24 @@ class Puppet::Indirector::Indirection
# remove it, we expire it and write it back out to disk. This way people
# can still use the expired object if they want.
def expire(key, *args)
- request = request(:expire, key, *args)
+ request = instantiate_request(:expire, key, *args)
return nil unless cache?
- return nil unless instance = cache.find(request(:find, key, *args))
+ return nil unless instance = cache.find(instantiate_request(:find,
key, *args))
Puppet.info "Expiring the %s cache of %s" % [self.name, instance.name]
# Set an expiration date in the past
instance.expiration = Time.now - 60
- cache.save(request(:save, instance, *args))
+ cache.save(instantiate_request(:save, instance, *args))
end
# Search for an instance in the appropriate terminus, caching the
# results if caching is configured..
def find(key, *args)
- request = request(:find, key, *args)
+ request = instantiate_request(:find, key, *args)
terminus = prepare(request)
begin
@@ -203,7 +203,7 @@ class Puppet::Indirector::Indirection
result.expiration ||= self.expiration
if cache? and request.use_cache?
Puppet.info "Caching %s for %s" % [self.name, request.key]
- cache.save request(:save, result, *args)
+ cache.save instantiate_request(:save, result, *args)
end
return terminus.respond_to?(:filter) ? terminus.filter(result) :
result
@@ -226,12 +226,12 @@ class Puppet::Indirector::Indirection
# Remove something via the terminus.
def destroy(key, *args)
- request = request(:destroy, key, *args)
+ request = instantiate_request(:destroy, key, *args)
terminus = prepare(request)
result = terminus.destroy(request)
- if cache? and cached = cache.find(request(:find, key, *args))
+ if cache? and cached = cache.find(instantiate_request(:find, key,
*args))
# Reuse the existing request, since it's equivalent.
cache.destroy(request)
end
@@ -241,7 +241,7 @@ class Puppet::Indirector::Indirection
# Search for more than one instance. Should always return an array.
def search(key, *args)
- request = request(:search, key, *args)
+ request = instantiate_request(:search, key, *args)
terminus = prepare(request)
if result = terminus.search(request)
@@ -256,7 +256,7 @@ class Puppet::Indirector::Indirection
# Save the instance in the appropriate terminus. This method is
# normally an instance method on the indirected class.
def save(instance, *args)
- request = request(:save, instance, *args)
+ request = instantiate_request(:save, instance, *args)
terminus = prepare(request)
result = terminus.save(request)
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 6f58908..a0a8711 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -49,6 +49,11 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
# Some metadata to help us compile and generally respond to the current
state.
attr_accessor :client_version, :server_version
+ def expired?
+ return false if Puppet[:use_cached_catalog]
+ super
+ end
+
# Add classes to our class list.
def add_class(*classes)
classes.each do |klass|
diff --git a/lib/puppet/util/cacher.rb b/lib/puppet/util/cacher.rb
index 28786ab..3c9d24d 100644
--- a/lib/puppet/util/cacher.rb
+++ b/lib/puppet/util/cacher.rb
@@ -18,15 +18,6 @@ module Puppet::Util::Cacher
extend Expirer
- # Our module has been extended in a class; we can only add the Instance
methods,
- # which become *class* methods in the class.
- def self.extended(other)
- class << other
- extend ClassMethods
- include InstanceMethods
- end
- end
-
# Our module has been included in a class, which means the class gets the
class methods
# and all of its instances get the instance methods.
def self.included(other)
diff --git a/spec/integration/indirector/direct_file_server.rb
b/spec/integration/indirector/direct_file_server.rb
index 9843c7e..c33d23e 100755
--- a/spec/integration/indirector/direct_file_server.rb
+++ b/spec/integration/indirector/direct_file_server.rb
@@ -18,7 +18,7 @@ describe Puppet::Indirector::DirectFileServer, " when
interacting with the files
it "should return an instance of the model" do
FileTest.expects(:exists?).with(@filepath).returns(true)
- @terminus.find(@terminus.indirection.request(:find,
"file://hos...@filepath}")).should be_instance_of(Puppet::FileServing::Content)
+ @terminus.find(@terminus.indirection.instantiate_request(:find,
"file://hos...@filepath}")).should be_instance_of(Puppet::FileServing::Content)
end
it "should return an instance capable of returning its content" do
@@ -26,7 +26,7 @@ describe Puppet::Indirector::DirectFileServer, " when
interacting with the files
File.stubs(:lstat).with(@filepath).returns(stub("stat", :ftype =>
"file"))
File.expects(:read).with(@filepath).returns("my content")
- instance = @terminus.find(@terminus.indirection.request(:find,
"file://hos...@filepath}"))
+ instance =
@terminus.find(@terminus.indirection.instantiate_request(:find,
"file://hos...@filepath}"))
instance.content.should == "my content"
end
@@ -45,7 +45,7 @@ describe Puppet::Indirector::DirectFileServer, " when
interacting with FileServi
File.open(File.join(@path, "one"), "w") { |f| f.print "one content" }
File.open(File.join(@path, "two"), "w") { |f| f.print "two content" }
- @request = @terminus.indirection.request(:search, "file:///%s" %
@path, :recurse => true)
+ @request = @terminus.indirection.instantiate_request(:search,
"file:///%s" % @path, :recurse => true)
end
after do
diff --git a/spec/unit/indirector/indirection.rb
b/spec/unit/indirector/indirection.rb
index 2ed51d7..8049d77 100755
--- a/spec/unit/indirector/indirection.rb
+++ b/spec/unit/indirector/indirection.rb
@@ -8,7 +8,7 @@ describe "Indirection Delegator", :shared => true do
it "should create a request object with the appropriate method name and
all of the passed arguments" do
request = Puppet::Indirector::Request.new(:indirection, :find, "me")
- @indirection.expects(:request).with(@method, "mystuff", :one =>
:two).returns request
+ @indirection.expects(:instantiate_request).with(@method, "mystuff",
:one => :two).returns request
@terminus.stubs(@method)
@@ -24,7 +24,7 @@ describe "Indirection Delegator", :shared => true do
request = Puppet::Indirector::Request.new(:indirection, :find, "me")
- @indirection.stubs(:request).returns request
+ @indirection.stubs(:instantiate_request).returns request
@indirection.expects(:select_terminus).with(request).returns
:test_terminus
@@ -43,7 +43,7 @@ describe "Indirection Delegator", :shared => true do
request = stub 'request', :key => "me", :options => {}
- @indirection.stubs(:request).returns request
+ @indirection.stubs(:instantiate_request).returns request
@indirection.expects(:select_terminus).with(request).returns nil
@@ -179,34 +179,34 @@ describe Puppet::Indirector::Indirection do
end
it "should have a method for creating an indirection request instance"
do
- @indirection.should respond_to(:request)
+ @indirection.should respond_to(:instantiate_request)
end
describe "creates a request" do
it "should create it with its name as the request's indirection
name" do
Puppet::Indirector::Request.expects(:new).with { |name,
*other| @indirection.name == name }
- @indirection.request(:funtest, "yayness")
+ @indirection.instantiate_request(:funtest, "yayness")
end
it "should require a method and key" do
Puppet::Indirector::Request.expects(:new).with { |name,
method, key, *other| method == :funtest and key == "yayness" }
- @indirection.request(:funtest, "yayness")
+ @indirection.instantiate_request(:funtest, "yayness")
end
it "should support optional arguments" do
Puppet::Indirector::Request.expects(:new).with { |name,
method, key, other| other == {:one => :two} }
- @indirection.request(:funtest, "yayness", :one => :two)
+ @indirection.instantiate_request(:funtest, "yayness", :one =>
:two)
end
it "should default to the arguments being nil" do
Puppet::Indirector::Request.expects(:new).with { |name,
method, key, args| args.nil? }
- @indirection.request(:funtest, "yayness")
+ @indirection.instantiate_request(:funtest, "yayness")
end
it "should return the request" do
request = mock 'request'
Puppet::Indirector::Request.expects(:new).returns request
- @indirection.request(:funtest, "yayness").should equal(request)
+ @indirection.instantiate_request(:funtest, "yayness").should
equal(request)
end
end
@@ -405,7 +405,7 @@ describe Puppet::Indirector::Indirection do
it "should return the result of saving to the terminus" do
request = stub 'request', :instance => @instance, :node =>
nil
- @indirection.expects(:request).returns request
+ @indirection.expects(:instantiate_request).returns request
@cache.stubs(:save)
@terminus.stubs(:save).returns @instance
@@ -415,7 +415,7 @@ describe Puppet::Indirector::Indirection do
it "should use a request to save the object to the cache" do
request = stub 'request', :instance => @instance, :node =>
nil
- @indirection.expects(:request).returns request
+ @indirection.expects(:instantiate_request).returns request
@cache.expects(:save).with(request)
@terminus.stubs(:save)
@@ -425,7 +425,7 @@ describe Puppet::Indirector::Indirection do
it "should not save to the cache if the normal save fails" do
request = stub 'request', :instance => @instance, :node =>
nil
- @indirection.expects(:request).returns request
+ @indirection.expects(:instantiate_request).returns request
@cache.expects(:save).never
@terminus.expects(:save).raises "eh"
@@ -457,8 +457,8 @@ describe Puppet::Indirector::Indirection do
destroy = stub 'destroy_request', :key => "/my/key", :node
=> nil
find = stub 'destroy_request', :key => "/my/key", :node =>
nil
- @indirection.expects(:request).with(:destroy,
"/my/key").returns destroy
- @indirection.expects(:request).with(:find,
"/my/key").returns find
+ @indirection.expects(:instantiate_request).with(:destroy,
"/my/key").returns destroy
+ @indirection.expects(:instantiate_request).with(:find,
"/my/key").returns find
cached = mock 'cache'
diff --git a/spec/unit/resource/catalog.rb b/spec/unit/resource/catalog.rb
index 39ddccd..0835bf9 100755
--- a/spec/unit/resource/catalog.rb
+++ b/spec/unit/resource/catalog.rb
@@ -35,6 +35,13 @@ describe Puppet::Resource::Catalog, "when compiling" do
@catalog.write_class_file
end
+ it "should not be expired if use_cached_catalog option is true" do
+ Puppet.settings[:use_cached_catalog] = true
+ @catalog = Puppet::Resource::Catalog.new("host")
+ @catalog.expiration = Time.now
+ @catalog.should_not be_expired
+ end
+
it "should have a client_version attribute" do
@catalog = Puppet::Resource::Catalog.new("host")
@catalog.client_version = 5
diff --git a/spec/unit/util/cacher.rb b/spec/unit/util/cacher.rb
index eb8515e..d02852c 100755
--- a/spec/unit/util/cacher.rb
+++ b/spec/unit/util/cacher.rb
@@ -151,11 +151,11 @@ describe Puppet::Util::Cacher do
it "should not check for a ttl expiration if the class does not
support that method" do
klass = Class.new do
- extend Puppet::Util::Cacher
+ include Puppet::Util::Cacher
end
- klass.singleton_class.cached_attr(:myattr) { "eh" }
- klass.myattr
+ klass.cached_attr(:myattr) { "eh" }
+ lambda { klass.new.myattr }.should_not raise_error
end
it "should automatically expire cached attributes whose ttl has
expired, even if no expirer is present" do
--
1.7.1
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
