diff options
author | Adam Leff <adam@leff.co> | 2016-06-24 15:43:33 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-06-24 15:43:33 -0400 |
commit | 6ec0b68deadb618c21b4571e5c82438b52d30645 (patch) | |
tree | 9774539f592f1b8b9b0cd333679ec0d99b1d5b8e | |
parent | 8d7a2a121463f654a66c69a76071a2f0e9c155b0 (diff) | |
parent | bb56a100caf7de9b7f0bbcb1377ba7f7cae05f90 (diff) | |
download | chef-6ec0b68deadb618c21b4571e5c82438b52d30645.tar.gz |
Merge pull request #5058 from chef/adamleff/IPO-196/add-unprocessed-resources-to-data-collector
Expand data_collector resource list to include all resources
-rw-r--r-- | lib/chef/data_collector.rb | 99 | ||||
-rw-r--r-- | lib/chef/data_collector/messages.rb | 6 | ||||
-rw-r--r-- | lib/chef/data_collector/resource_report.rb | 17 | ||||
-rw-r--r-- | spec/unit/data_collector/messages_spec.rb | 6 | ||||
-rw-r--r-- | spec/unit/data_collector_spec.rb | 64 |
5 files changed, 133 insertions, 59 deletions
diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb index c8dd354f60..e7ef8d39ec 100644 --- a/lib/chef/data_collector.rb +++ b/lib/chef/data_collector.rb @@ -22,6 +22,7 @@ require "uri" require "chef/event_dispatch/base" require "chef/data_collector/messages" require "chef/data_collector/resource_report" +require "ostruct" class Chef @@ -51,12 +52,12 @@ class Chef # and exports its data through a webhook-like mechanism to a configured # endpoint. class Reporter < EventDispatch::Base - attr_reader :completed_resources, :status, :exception, :error_descriptions, - :expanded_run_list, :run_status, :http, + attr_reader :all_resource_reports, :status, :exception, :error_descriptions, + :expanded_run_list, :run_context, :run_status, :http, :current_resource_report, :enabled def initialize - @completed_resources = [] + @all_resource_reports = [] @current_resource_loaded = nil @error_descriptions = {} @expanded_run_list = {} @@ -93,6 +94,29 @@ class Chef send_run_completion(status: "failure") end + # see EventDispatch::Base#converge_start + # Upon receipt, we stash the run_context for use at the + # end of the run in order to determine what resource+action + # combinations have not yet fired so we can report on + # unprocessed resources. + def converge_start(run_context) + @run_context = run_context + end + + # see EventDispatch::Base#converge_complete + # At the end of the converge, we add any unprocessed resources + # to our report list. + def converge_complete + detect_unprocessed_resources + end + + # see EventDispatch::Base#converge_failed + # At the end of the converge, we add any unprocessed resources + # to our report list + def converge_failed(exception) + detect_unprocessed_resources + end + # see EventDispatch::Base#resource_current_state_loaded # Create a new ResourceReport instance that we'll use to track # the state of this resource during the run. Nested resources are @@ -100,13 +124,7 @@ 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( - Chef::DataCollector::ResourceReport.new( - new_resource, - action, - current_resource - ) - ) + update_current_resource_report(create_resource_report(new_resource, action, current_resource)) end # see EventDispatch::Base#resource_up_to_date @@ -116,19 +134,15 @@ class Chef end # see EventDispatch::Base#resource_skipped - # If this is a top-level resource, we create a ResourceReport instance - # (because a skipped resource does not trigger the + # If this is a top-level resource, we create a ResourceReport + # instance (because a skipped resource does not trigger the # resource_current_state_loaded event), and flag it as skipped. def resource_skipped(new_resource, action, conditional) return if nested_resource?(new_resource) - update_current_resource_report( - Chef::DataCollector::ResourceReport.new( - new_resource, - action - ) - ) - current_resource_report.skipped(conditional) + resource_report = create_resource_report(new_resource, action) + resource_report.skipped(conditional) + update_current_resource_report(resource_report) end # see EventDispatch::Base#resource_updated @@ -154,13 +168,12 @@ class Chef end # see EventDispatch::Base#resource_completed - # Mark the ResourceReport instance as finished (for timing details) - # and add it to the list of resources encountered during this run. + # Mark the ResourceReport instance as finished (for timing details). # This marks the end of this resource during this run. def resource_completed(new_resource) if current_resource_report && !nested_resource?(new_resource) current_resource_report.finish - add_completed_resource(current_resource_report) + add_resource_report(current_resource_report) update_current_resource_report(nil) end end @@ -271,7 +284,7 @@ class Chef Chef::DataCollector::Messages.run_end_message( run_status: run_status, expanded_run_list: expanded_run_list, - completed_resources: completed_resources, + resources: all_resource_reports, status: opts[:status], error_descriptions: error_descriptions ).to_json @@ -297,8 +310,12 @@ class Chef Chef::Config[:data_collector][:token] end - def add_completed_resource(resource_report) - @completed_resources << resource_report + def add_resource_report(resource_report) + @all_resource_reports << OpenStruct.new( + resource: resource_report.new_resource, + action: resource_report.action, + report_data: resource_report.to_hash + ) end def disable_data_collector_reporter @@ -321,6 +338,38 @@ class Chef @error_descriptions = discription_hash end + def create_resource_report(new_resource, action, current_resource = nil) + Chef::DataCollector::ResourceReport.new( + new_resource, + action, + current_resource + ) + end + + def detect_unprocessed_resources + # create a Set containing all resource+action combinations from + # the Resource Collection + collection_resources = Set.new + run_context.resource_collection.all_resources.each do |resource| + Array(resource.action).each do |action| + collection_resources.add([resource, action]) + end + end + + # Delete from the Set any resource+action combination we have + # already processed. + all_resource_reports.each do |report| + collection_resources.delete([report.resource, report.action]) + end + + # The items remaining in the Set are unprocessed resource+actions, + # so we'll create new resource reports for them which default to + # a state of "unprocessed". + collection_resources.each do |resource, action| + add_resource_report(create_resource_report(resource, action)) + end + end + # If we are getting messages about a resource while we are in the middle of # another resource's update, we assume that the nested resource is just the # implementation of a provider, and we want to hide it from the reporting diff --git a/lib/chef/data_collector/messages.rb b/lib/chef/data_collector/messages.rb index e23262c9e4..89bad9f9f9 100644 --- a/lib/chef/data_collector/messages.rb +++ b/lib/chef/data_collector/messages.rb @@ -70,15 +70,15 @@ class Chef "message_type" => "run_converge", "node_name" => run_status.node.name, "organization_name" => organization, - "resources" => reporter_data[:completed_resources].map(&:for_json), + "resources" => reporter_data[:resources].map(&:report_data), "run_id" => run_status.run_id, "run_list" => run_status.node.run_list.for_json, "start_time" => run_status.start_time.utc.iso8601, "end_time" => run_status.end_time.utc.iso8601, "source" => collector_source, "status" => reporter_data[:status], - "total_resource_count" => reporter_data[:completed_resources].count, - "updated_resource_count" => reporter_data[:completed_resources].select { |r| r.status == "updated" }.count, + "total_resource_count" => reporter_data[:resources].count, + "updated_resource_count" => reporter_data[:resources].select { |r| r.report_data["status"] == "updated" }.count, } message["error"] = { diff --git a/lib/chef/data_collector/resource_report.rb b/lib/chef/data_collector/resource_report.rb index 1793fe2c9d..89b6d26706 100644 --- a/lib/chef/data_collector/resource_report.rb +++ b/lib/chef/data_collector/resource_report.rb @@ -22,13 +22,14 @@ class Chef class DataCollector class ResourceReport - attr_reader :action, :current_resource, :elapsed_time, :new_resource, :status - attr_accessor :conditional, :exception + attr_reader :action, :elapsed_time, :new_resource, :status + attr_accessor :conditional, :current_resource, :exception def initialize(new_resource, action, current_resource = nil) @new_resource = new_resource @action = action @current_resource = current_resource + @status = "unprocessed" end def skipped(conditional) @@ -54,6 +55,14 @@ class Chef @elapsed_time = new_resource.elapsed_time end + def elapsed_time_in_milliseconds + elapsed_time.nil? ? nil : (elapsed_time * 1000).to_i + end + + def potentially_changed? + %w{updated failed}.include?(status) + end + def to_hash hash = { "type" => new_resource.resource_name.to_sym, @@ -61,8 +70,8 @@ class Chef "id" => new_resource.identity.to_s, "after" => new_resource.state_for_resource_reporter, "before" => current_resource ? current_resource.state_for_resource_reporter : {}, - "duration" => (elapsed_time * 1000).to_i.to_s, - "delta" => new_resource.respond_to?(:diff) ? new_resource.diff : "", + "duration" => elapsed_time_in_milliseconds.to_s, + "delta" => new_resource.respond_to?(:diff) && potentially_changed? ? new_resource.diff : "", "result" => action.to_s, "status" => status, } diff --git a/spec/unit/data_collector/messages_spec.rb b/spec/unit/data_collector/messages_spec.rb index dee86a3907..686e500507 100644 --- a/spec/unit/data_collector/messages_spec.rb +++ b/spec/unit/data_collector/messages_spec.rb @@ -62,12 +62,12 @@ describe Chef::DataCollector::Messages do describe '#run_end_message' do let(:run_status) { Chef::RunStatus.new(Chef::Node.new, Chef::EventDispatch::Dispatcher.new) } - let(:resource1) { double("resource1", for_json: "resource_data", status: "updated") } - let(:resource2) { double("resource2", for_json: "resource_data", status: "skipped") } + let(:report1) { double("report1", report_data: { "status" => "updated" }) } + let(:report2) { double("report2", report_data: { "status" => "skipped" }) } let(:reporter_data) do { run_status: run_status, - completed_resources: [resource1, resource2], + resources: [report1, report2], } end diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb index 78b0aaf97d..831643347b 100644 --- a/spec/unit/data_collector_spec.rb +++ b/spec/unit/data_collector_spec.rb @@ -20,6 +20,7 @@ require "spec_helper" require "chef/data_collector" +require "chef/resource_builder" describe Chef::DataCollector do describe ".register_reporter?" do @@ -193,10 +194,32 @@ describe Chef::DataCollector::Reporter do end end + describe '#converge_start' do + it "stashes the run_context for later use" do + reporter.converge_start("test_context") + expect(reporter.run_context).to eq("test_context") + end + end + + describe '#converge_complete' do + it "detects and processes any unprocessed resources" do + expect(reporter).to receive(:detect_unprocessed_resources) + reporter.converge_complete + end + end + + describe '#converge_failed' do + it "detects and processes any unprocessed resources" do + expect(reporter).to receive(:detect_unprocessed_resources) + reporter.converge_failed("exception") + end + end + describe '#resource_current_state_loaded' do let(:new_resource) { double("new_resource") } let(:action) { double("action") } let(:current_resource) { double("current_resource") } + let(:resource_report) { double("resource_report") } context "when resource is a nested resource" do it "does not update the resource report" do @@ -207,14 +230,12 @@ describe Chef::DataCollector::Reporter do end context "when resource is not a nested resource" do - it "updates the resource report" do + it "creates the resource report and stores it as the current one" do allow(reporter).to receive(:nested_resource?).and_return(false) - expect(Chef::DataCollector::ResourceReport).to receive(:new).with( - new_resource, - action, - current_resource) - .and_return("resource_report") - expect(reporter).to receive(:update_current_resource_report).with("resource_report") + expect(reporter).to receive(:create_resource_report) + .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 @@ -256,7 +277,7 @@ describe Chef::DataCollector::Reporter do before do allow(reporter).to receive(:nested_resource?) - allow(reporter).to receive(:current_resource_report).and_return(resource_report) + allow(reporter).to receive(:create_resource_report).and_return(resource_report) allow(resource_report).to receive(:skipped) end @@ -269,13 +290,12 @@ describe Chef::DataCollector::Reporter do end context "when the resource is not a nested resource" do - it "updates the resource report" do + it "creates the resource report and stores it as the current one" do allow(reporter).to receive(:nested_resource?).and_return(false) - expect(Chef::DataCollector::ResourceReport).to receive(:new).with( - new_resource, - action) - .and_return("resource_report") - expect(reporter).to receive(:update_current_resource_report).with("resource_report") + 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 @@ -349,15 +369,16 @@ describe Chef::DataCollector::Reporter do let(:resource_report) { double("resource_report") } before do - allow(reporter).to receive(:add_completed_resource) allow(reporter).to receive(:update_current_resource_report) + allow(reporter).to receive(:add_resource_report) + allow(reporter).to receive(:current_resource_report) allow(resource_report).to receive(:finish) end context "when there is no current resource report" do - it "does not add the updated resource" do + it "does not touch the current resource report" do allow(reporter).to receive(:current_resource_report).and_return(nil) - expect(reporter).not_to receive(:add_completed_resource) + expect(reporter).not_to receive(:update_current_resource_report) reporter.resource_completed(new_resource) end end @@ -368,9 +389,9 @@ describe Chef::DataCollector::Reporter do end context "when the resource is a nested resource" do - it "does not add the updated resource" do + it "does not mark the resource as finished" do allow(reporter).to receive(:nested_resource?).with(new_resource).and_return(true) - expect(reporter).not_to receive(:add_completed_resource) + expect(resource_report).not_to receive(:finish) reporter.resource_completed(new_resource) end end @@ -385,11 +406,6 @@ describe Chef::DataCollector::Reporter do reporter.resource_completed(new_resource) end - it "adds the resource to the updated resource list" do - expect(reporter).to receive(:add_completed_resource).with(resource_report) - reporter.resource_completed(new_resource) - end - it "nils out the current resource report" do expect(reporter).to receive(:update_current_resource_report).with(nil) reporter.resource_completed(new_resource) |