(#5882) Added error-handling to puppet inspect when auditing

If auditing a resource fails, the report will contain failure events for every
audited property of the resource. The report itself will also be marked as
"failed".

Paired-With: Paul Berry

2 files changed, 86 insertions, 13 deletions

diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index b4d263545..5254afbd0 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -64,30 +64,45 @@ class Puppet::Application::Inspect < Puppet::Application
       audited_attributes = ral_resource[:audit]
       next unless audited_attributes
 
-      audited_resource = ral_resource.to_resource
-
       status = Puppet::Resource::Status.new(ral_resource)
-      audited_attributes.each do |name|
-        next if audited_resource[name].nil?
-        # Skip :absent properties of :absent resources. Really, it would be nicer if the RAL returned nil for those, but it doesn't. ~JW
-        if name == :ensure or audited_resource[:ensure] != :absent or audited_resource[name] != :absent
+
+      begin
+        audited_resource = ral_resource.to_resource
+      rescue StandardError => detail
+        puts detail.backtrace if Puppet[:trace]
+        ral_resource.err "Could not inspect #{ral_resource}; skipping: #{detail}"
+        audited_attributes.each do |name|
           event = ral_resource.event(
-            :previous_value => audited_resource[name],
-            :property       => name,
-            :status         => "audit",
-            :audited        => true,
-            :message        => "inspected value is #{audited_resource[name].inspect}"
+            :property => name,
+            :status   => "failure",
+            :audited  => true,
+            :message  => "failed to inspect #{name}"
           )
           status.add_event(event)
         end
+      else
+        audited_attributes.each do |name|
+          next if audited_resource[name].nil?
+          # Skip :absent properties of :absent resources. Really, it would be nicer if the RAL returned nil for those, but it doesn't. ~JW
+          if name == :ensure or audited_resource[:ensure] != :absent or audited_resource[name] != :absent
+            event = ral_resource.event(
+              :previous_value => audited_resource[name],
+              :property       => name,
+              :status         => "audit",
+              :audited        => true,
+              :message        => "inspected value is #{audited_resource[name].inspect}"
+            )
+            status.add_event(event)
+          end
+        end
       end
-      @report.add_resource_status(status)
       if Puppet[:archive_files] and ral_resource.type == :file and audited_attributes.include?(:content)
         path = ral_resource[:path]
         if File.readable?(path)
           dipper.backup(path)
         end
       end
+      @report.add_resource_status(status)
     end
 
     finishtime = Time.now
diff --git a/spec/unit/application/inspect_spec.rb b/spec/unit/application/inspect_spec.rb
index 0c7b61f59..15b0c40ad 100644
--- a/spec/unit/application/inspect_spec.rb
+++ b/spec/unit/application/inspect_spec.rb
@@ -144,7 +144,6 @@ describe Puppet::Application::Inspect do
         end
 
         it "should not send unreadable files" do
-          pending "see bug #5882"
           File.open(@file, 'w') { |f| f.write('stuff') }
           File.chmod(0, @file)
           Puppet::FileBucketFile::Rest.any_instance.expects(:head).never
@@ -197,6 +196,65 @@ describe Puppet::Application::Inspect do
         end
       end
     end
+
+    describe "when there are failures" do
+      before :each do
+        Puppet::Type.newtype(:stub_type) do
+          newparam(:name) do
+            desc "The name var"
+            isnamevar
+          end
+
+          newproperty(:content) do
+            desc "content"
+            def retrieve
+              raise "failed"
+            end
+          end
+        end
+
+        @catalog = Puppet::Resource::Catalog.new
+        Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(@catalog)
+
+        Puppet::Transaction::Report::Rest.any_instance.expects(:save).with do |request|
+          @report = request.instance
+        end
+      end
+
+      after :each do
+        Puppet::Type.rmtype(:stub_type)
+      end
+
+      it "should mark the report failed and create failed events for each property" do
+        @resource = Puppet::Resource.new(:stub_type, 'foo', :parameters => {:audit => "all"})
+        @catalog.add_resource(@resource)
+
+        @inspect.run_command
+
+        @report.status.should == "failed"
+        #@report.logs.select{|log| log.message =~ /Could not inspect/}.count.should == 1
+        @report.resource_statuses.count.should == 1
+        @report.resource_statuses['Stub_type[foo]'].events.count.should == 1
+
+        event = @report.resource_statuses['Stub_type[foo]'].events.first
+        event.property.should == "content"
+        event.status.should == "failure"
+        event.audited.should == true
+        event.instance_variables.should_not include("@previous_value")
+      end
+
+      it "should continue to the next resource" do
+        @resource = Puppet::Resource.new(:stub_type, 'foo', :parameters => {:audit => "all"})
+        @other_resource = Puppet::Resource.new(:stub_type, 'bar', :parameters => {:audit => "all"})
+        @catalog.add_resource(@resource)
+        @catalog.add_resource(@other_resource)
+
+        @inspect.run_command
+
+        @report.resource_statuses.count.should == 2
+        @report.resource_statuses.keys.should =~ ['Stub_type[foo]', 'Stub_type[bar]']
+      end
+    end
   end
 
   after :all do