Retrieving a catalog and getting the facts to submit with the catalog request
are distinct operations, and should be done separately. This is also to prepare
for adding the ability to determine the node name based on a fact, in which
case the node name needs to be determined before it is used for either the
catalog or the report.

Paired-With: Jacob Helwig <[email protected]>
Signed-off-by: Nick Lewis <[email protected]>
---
 lib/puppet/configurer.rb     |   21 ++++++++++-----------
 spec/unit/configurer_spec.rb |   24 ++++++++++++------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index d0251de..596d2dc 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -84,16 +84,8 @@ class Puppet::Configurer
   end
 
   # Get the remote catalog, yo.  Returns nil if no catalog can be found.
-  def retrieve_catalog
-    if Puppet::Resource::Catalog.indirection.terminus_class == :rest
-      # This is a bit complicated.  We need the serialized and escaped facts,
-      # and we need to know which format they're encoded in.  Thus, we
-      # get a hash with both of these pieces of information.
-      fact_options = facts_for_uploading
-    else
-      fact_options = {}
-    end
-
+  def retrieve_catalog(fact_options)
+    fact_options ||= {}
     # First try it with no cache, then with the cache.
     unless (Puppet[:use_cached_catalog] and result = 
retrieve_catalog_from_cache(fact_options)) or result = 
retrieve_new_catalog(fact_options)
       if ! Puppet[:usecacheonfailure]
@@ -130,13 +122,20 @@ class Puppet::Configurer
       Puppet.err "Failed to prepare catalog: #{detail}"
     end
 
+    if Puppet::Resource::Catalog.indirection.terminus_class == :rest
+      # This is a bit complicated.  We need the serialized and escaped facts,
+      # and we need to know which format they're encoded in.  Thus, we
+      # get a hash with both of these pieces of information.
+      fact_options = facts_for_uploading
+    end
+
     options[:report] ||= Puppet::Transaction::Report.new("apply")
     report = options[:report]
     Puppet::Util::Log.newdestination(report)
 
     if catalog = options[:catalog]
       options.delete(:catalog)
-    elsif ! catalog = retrieve_catalog
+    elsif ! catalog = retrieve_catalog(fact_options)
       Puppet.err "Could not retrieve catalog; skipping run"
       return
     end
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index 6c4f9b9..d454b5f 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -375,21 +375,21 @@ describe Puppet::Configurer do
         Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_terminus] == true }.returns @catalog
         Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_cache] == true }.never
 
-        @agent.retrieve_catalog.should == @catalog
+        @agent.retrieve_catalog({}).should == @catalog
       end
 
       it "should compile a new catalog if none is found in the cache" do
         Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_terminus] == true }.returns nil
         Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_cache] == true }.returns @catalog
 
-        @agent.retrieve_catalog.should == @catalog
+        @agent.retrieve_catalog({}).should == @catalog
       end
     end
 
     it "should use the Catalog class to get its catalog" do
       Puppet::Resource::Catalog.expects(:find).returns @catalog
 
-      @agent.retrieve_catalog
+      @agent.retrieve_catalog({})
     end
 
     it "should use its node_name_value to retrieve the catalog" do
@@ -397,13 +397,13 @@ describe Puppet::Configurer do
       Puppet.settings[:node_name_value] = "myhost.domain.com"
       Puppet::Resource::Catalog.expects(:find).with { |name, options| name == 
"myhost.domain.com" }.returns @catalog
 
-      @agent.retrieve_catalog
+      @agent.retrieve_catalog({})
     end
 
     it "should default to returning a catalog retrieved directly from the 
server, skipping the cache" do
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_cache] == true }.returns @catalog
 
-      @agent.retrieve_catalog.should == @catalog
+      @agent.retrieve_catalog({}).should == @catalog
     end
 
     it "should log and return the cached catalog when no catalog can be 
retrieved from the server" do
@@ -412,21 +412,21 @@ describe Puppet::Configurer do
 
       Puppet.expects(:notice)
 
-      @agent.retrieve_catalog.should == @catalog
+      @agent.retrieve_catalog({}).should == @catalog
     end
 
     it "should not look in the cache for a catalog if one is returned from the 
server" do
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_cache] == true }.returns @catalog
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_terminus] == true }.never
 
-      @agent.retrieve_catalog.should == @catalog
+      @agent.retrieve_catalog({}).should == @catalog
     end
 
     it "should return the cached catalog when retrieving the remote catalog 
throws an exception" do
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_cache] == true }.raises "eh"
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_terminus] == true }.returns @catalog
 
-      @agent.retrieve_catalog.should == @catalog
+      @agent.retrieve_catalog({}).should == @catalog
     end
 
     it "should log and return nil if no catalog can be retrieved from the 
server and :usecacheonfailure is disabled" do
@@ -436,27 +436,27 @@ describe Puppet::Configurer do
 
       Puppet.expects(:warning)
 
-      @agent.retrieve_catalog.should be_nil
+      @agent.retrieve_catalog({}).should be_nil
     end
 
     it "should return nil if no cached catalog is available and no catalog can 
be retrieved from the server" do
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_cache] == true }.returns nil
       Puppet::Resource::Catalog.expects(:find).with { |name, options| 
options[:ignore_terminus] == true }.returns nil
 
-      @agent.retrieve_catalog.should be_nil
+      @agent.retrieve_catalog({}).should be_nil
     end
 
     it "should convert the catalog before returning" do
       Puppet::Resource::Catalog.stubs(:find).returns @catalog
 
       @agent.expects(:convert_catalog).with { |cat, dur| cat == @catalog 
}.returns "converted catalog"
-      @agent.retrieve_catalog.should == "converted catalog"
+      @agent.retrieve_catalog({}).should == "converted catalog"
     end
 
     it "should return nil if there is an error while retrieving the catalog" do
       Puppet::Resource::Catalog.expects(:find).at_least_once.raises "eh"
 
-      @agent.retrieve_catalog.should be_nil
+      @agent.retrieve_catalog({}).should be_nil
     end
   end
 
-- 
1.7.5.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.

Reply via email to