From 091df73931ecff79927ab9b55c0f284024c6a58e Mon Sep 17 00:00:00 2001 From: Adam Leff Date: Mon, 21 Nov 2016 22:01:50 -0500 Subject: Ensure Data Collector resource report exists before updating In certain cases when using custom resources, we have seen evidence of resource_updated and resource_failed events getting fired before the DataCollector instance has a valid resource report created to track such updates/failures. This change ensures a resource report exists before attempting to modify it, regardless of what order these events may fire. Signed-off-by: Adam Leff --- lib/chef/data_collector.rb | 25 ++++++++++++++++--------- spec/unit/data_collector_spec.rb | 22 ++++++---------------- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb index 82c41edfdc..dbf1150dc9 100644 --- a/lib/chef/data_collector.rb +++ b/lib/chef/data_collector.rb @@ -174,12 +174,13 @@ class Chef # resource, and we only care about tracking top-level resources. def resource_current_state_loaded(new_resource, action, current_resource) return if nested_resource?(new_resource) - update_current_resource_report(create_resource_report(new_resource, action, current_resource)) + initialize_resource_report_if_needed(new_resource, action, current_resource) end # see EventDispatch::Base#resource_up_to_date # Mark our ResourceReport status accordingly def resource_up_to_date(new_resource, action) + initialize_resource_report_if_needed(new_resource, action) current_resource_report.up_to_date unless nested_resource?(new_resource) end @@ -190,15 +191,15 @@ class Chef def resource_skipped(new_resource, action, conditional) return if nested_resource?(new_resource) - resource_report = create_resource_report(new_resource, action) - resource_report.skipped(conditional) - update_current_resource_report(resource_report) + initialize_resource_report_if_needed(new_resource, action) + current_resource_report.skipped(conditional) end # see EventDispatch::Base#resource_updated # Flag the current ResourceReport instance as updated (as long as it's # a top-level resource). def resource_updated(new_resource, action) + initialize_resource_report_if_needed(new_resource, action) current_resource_report.updated unless nested_resource?(new_resource) end @@ -207,6 +208,7 @@ class Chef # long as it's a top-level resource, and update the run error text # with the proper Formatter. def resource_failed(new_resource, action, exception) + initialize_resource_report_if_needed(new_resource, action) current_resource_report.failed(exception) unless nested_resource?(new_resource) update_error_description( Formatters::ErrorMapper.resource_failed( @@ -224,7 +226,7 @@ class Chef if current_resource_report && !nested_resource?(new_resource) current_resource_report.finish add_resource_report(current_resource_report) - update_current_resource_report(nil) + clear_current_resource_report end end @@ -402,10 +404,6 @@ class Chef @run_status = run_status end - def update_current_resource_report(resource_report) - @current_resource_report = resource_report - end - def update_error_description(discription_hash) @error_descriptions = discription_hash end @@ -414,6 +412,11 @@ class Chef @deprecations << { message: message, url: url, location: location } end + def initialize_resource_report_if_needed(new_resource, action, current_resource = nil) + return unless current_resource_report.nil? + @current_resource_report = create_resource_report(new_resource, action, current_resource) + end + def create_resource_report(new_resource, action, current_resource = nil) Chef::DataCollector::ResourceReport.new( new_resource, @@ -422,6 +425,10 @@ class Chef ) end + def clear_current_resource_report + @current_resource_report = nil + end + def detect_unprocessed_resources # create a Set containing all resource+action combinations from # the Resource Collection diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb index b3e2d931a7..4ae227ba3c 100644 --- a/spec/unit/data_collector_spec.rb +++ b/spec/unit/data_collector_spec.rb @@ -371,12 +371,10 @@ describe Chef::DataCollector::Reporter do end context "when resource is not a nested resource" do - it "creates the resource report and stores it as the current one" do + it "initializes the resource report" do allow(reporter).to receive(:nested_resource?).and_return(false) - expect(reporter).to receive(:create_resource_report) + expect(reporter).to receive(:initialize_resource_report_if_needed) .with(new_resource, action, current_resource) - .and_return(resource_report) - expect(reporter).to receive(:update_current_resource_report).with(resource_report) reporter.resource_current_state_loaded(new_resource, action, current_resource) end end @@ -418,7 +416,6 @@ describe Chef::DataCollector::Reporter do before do allow(reporter).to receive(:nested_resource?) - allow(reporter).to receive(:create_resource_report).and_return(resource_report) allow(resource_report).to receive(:skipped) end @@ -431,17 +428,10 @@ describe Chef::DataCollector::Reporter do end context "when the resource is not a nested resource" do - it "creates the resource report and stores it as the current one" do + it "initializes the resource report and marks it as skipped" do allow(reporter).to receive(:nested_resource?).and_return(false) - expect(reporter).to receive(:create_resource_report) - .with(new_resource, action) - .and_return(resource_report) - expect(reporter).to receive(:update_current_resource_report).with(resource_report) - reporter.resource_skipped(new_resource, action, conditional) - end - - it "marks the resource report as skipped" do - allow(reporter).to receive(:nested_resource?).with(new_resource).and_return(false) + allow(reporter).to receive(:current_resource_report).and_return(resource_report) + expect(reporter).to receive(:initialize_resource_report_if_needed).with(new_resource, action) expect(resource_report).to receive(:skipped).with(conditional) reporter.resource_skipped(new_resource, action, conditional) end @@ -548,7 +538,7 @@ describe Chef::DataCollector::Reporter do end it "nils out the current resource report" do - expect(reporter).to receive(:update_current_resource_report).with(nil) + expect(reporter).to receive(:clear_current_resource_report) reporter.resource_completed(new_resource) end end -- cgit v1.2.1