diff options
| -rw-r--r-- | lib/chef/client.rb | 1 | ||||
| -rw-r--r-- | lib/chef/data_collector.rb | 73 | ||||
| -rw-r--r-- | lib/chef/data_collector/messages.rb | 6 | ||||
| -rw-r--r-- | lib/chef/data_collector/resource_report.rb | 11 | ||||
| -rw-r--r-- | lib/chef/event_dispatch/base.rb | 6 | ||||
| -rw-r--r-- | spec/unit/data_collector/messages_spec.rb | 2 | ||||
| -rw-r--r-- | spec/unit/data_collector_spec.rb | 107 |
7 files changed, 153 insertions, 53 deletions
diff --git a/lib/chef/client.rb b/lib/chef/client.rb index c857da1b93..3d78cae2ca 100644 --- a/lib/chef/client.rb +++ b/lib/chef/client.rb @@ -278,6 +278,7 @@ class Chef do_windows_admin_check run_context = setup_run_context + events.run_context_setup(run_context) if Chef::Config[:audit_mode] != :audit_only converge_error = converge_and_save(run_context) diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb index c8dd354f60..60846b488d 100644 --- a/lib/chef/data_collector.rb +++ b/lib/chef/data_collector.rb @@ -51,12 +51,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, + attr_reader :all_resource_reports, :status, :exception, :error_descriptions, :expanded_run_list, :run_status, :http, :current_resource_report, :enabled def initialize - @completed_resources = [] + @all_resource_reports = [] @current_resource_loaded = nil @error_descriptions = {} @expanded_run_list = {} @@ -64,6 +64,19 @@ class Chef @enabled = true end + # see EventDispatch::Base#run_context_setup + # Upon receipt, we will create a resource report for every + # resource in the resource collection. This is needed in + # order to send ALL resources to the Data Collector server + # regardless of whether or not they were processed. + def run_context_setup(run_context) + run_context.resource_collection.all_resources.each do |resource| + Array(resource.action).each do |action| + create_resource_report(resource, action) + end + end + end + # see EventDispatch::Base#run_started # Upon receipt, we will send our run start message to the # configured DataCollector endpoint. Depending on whether @@ -100,13 +113,9 @@ 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 - ) - ) + resource_report = find_or_create_resource_report(new_resource, action) + resource_report.current_resource = current_resource + update_current_resource_report(resource_report) end # see EventDispatch::Base#resource_up_to_date @@ -116,19 +125,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 find the correct 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 = find_or_create_resource_report(new_resource, action) + resource_report.skipped(conditional) + update_current_resource_report(resource_report) end # see EventDispatch::Base#resource_updated @@ -154,13 +159,11 @@ 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) update_current_resource_report(nil) end end @@ -271,7 +274,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 @@ -321,6 +324,32 @@ class Chef @error_descriptions = discription_hash end + def create_resource_report(resource, action) + report = Chef::DataCollector::ResourceReport.new( + resource, + action + ) + + @all_resource_reports << report + + report + end + + def find_or_create_resource_report(resource, action) + report = all_resource_reports.find do |r| + r.new_resource.class == resource.class && + r.new_resource.name == resource.name && + r.new_resource.source_line == resource.source_line && + r.new_resource.action.include?(action) + end + + if report.nil? + report = create_resource_report(resource, action) + end + + report + 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..15b451a6b0 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(&:for_json), "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.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..f3e272b0fb 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,10 @@ 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 to_hash hash = { "type" => new_resource.resource_name.to_sym, @@ -61,7 +66,7 @@ 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, + "duration" => elapsed_time_in_milliseconds.to_s, "delta" => new_resource.respond_to?(:diff) ? new_resource.diff : "", "result" => action.to_s, "status" => status, diff --git a/lib/chef/event_dispatch/base.rb b/lib/chef/event_dispatch/base.rb index 6c6b2fa3bb..0b386ab851 100644 --- a/lib/chef/event_dispatch/base.rb +++ b/lib/chef/event_dispatch/base.rb @@ -43,6 +43,12 @@ class Chef def run_failed(exception) end + # Called when the run_context has been set up. + # Most helpful for event handlers that wish to inspect + # the resource collection. + def run_context_setup(run_context) + end + # Called right after ohai runs. def ohai_completed(node) end diff --git a/spec/unit/data_collector/messages_spec.rb b/spec/unit/data_collector/messages_spec.rb index dee86a3907..bb3b158611 100644 --- a/spec/unit/data_collector/messages_spec.rb +++ b/spec/unit/data_collector/messages_spec.rb @@ -67,7 +67,7 @@ describe Chef::DataCollector::Messages do let(:reporter_data) do { run_status: run_status, - completed_resources: [resource1, resource2], + resources: [resource1, resource2], } end diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb index 78b0aaf97d..fb47fe3012 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 @@ -155,6 +156,21 @@ describe Chef::DataCollector::Reporter do let(:reporter) { described_class.new } let(:run_status) { Chef::RunStatus.new(Chef::Node.new, Chef::EventDispatch::Dispatcher.new) } + describe '#run_context_setup' do + it "creates resource reports for each resource in the collection" do + resource1 = double("resource1", action: [:create]) + resource2 = double("resource2", action: [:enable, :start]) + resource_collection = double("resource_collection", all_resources: [resource1, resource2]) + run_context = double("run_context", resource_collection: resource_collection) + + expect(reporter).to receive(:create_resource_report).with(resource1, :create) + expect(reporter).to receive(:create_resource_report).with(resource2, :enable) + expect(reporter).to receive(:create_resource_report).with(resource2, :start) + + reporter.run_context_setup(run_context) + end + end + describe '#run_started' do before do allow(reporter).to receive(:update_run_status) @@ -197,6 +213,7 @@ describe Chef::DataCollector::Reporter 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 +224,13 @@ describe Chef::DataCollector::Reporter do end context "when resource is not a nested resource" do - it "updates the resource report" do + it "finds 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(:find_or_create_resource_report) + .with(new_resource, action) + .and_return(resource_report) + expect(resource_report).to receive(:current_resource=).with(current_resource) + 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 +272,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(:find_or_create_resource_report).and_return(resource_report) allow(resource_report).to receive(:skipped) end @@ -269,13 +285,12 @@ describe Chef::DataCollector::Reporter do end context "when the resource is not a nested resource" do - it "updates the resource report" do + it "finds the correct 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(:find_or_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 +364,15 @@ 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(: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 +383,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 +400,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) @@ -499,4 +509,53 @@ describe Chef::DataCollector::Reporter do end end end + + describe '#find_or_create_resource_report' do + let(:node) { Chef::Node.new } + let(:run_context) { Chef::RunContext.new(node, nil, nil) } + + let(:resource1) do + Chef::ResourceBuilder.new( + type: :file, + name: "test1", + run_context: run_context) + .build + end + + let(:resource2) do + Chef::ResourceBuilder.new( + type: :file, + name: "test2", + run_context: run_context) + .build { action :delete } + end + + let(:resource3) do + Chef::ResourceBuilder.new( + type: :service, + name: "test3", + run_context: run_context) + .build + end + + let(:report1) { Chef::DataCollector::ResourceReport.new(resource1, resource1.action.first) } + let(:report2) { Chef::DataCollector::ResourceReport.new(resource2, resource2.action.first) } + let(:report3) { Chef::DataCollector::ResourceReport.new(resource3, resource3.action.first) } + + context "when a matching resource report exists" do + it "returns the resource report" do + allow(reporter).to receive(:all_resource_reports).and_return([report1, report2, report3]) + expect(reporter).not_to receive(:create_resource_report) + expect(reporter.send(:find_or_create_resource_report, resource1, resource1.action.first)).to eq(report1) + end + end + + context "when a matching resource report does not exist" do + it "creates a new resource report and returns it" do + allow(reporter).to receive(:all_resource_reports).and_return([report1, report2]) + expect(reporter).to receive(:create_resource_report).with(resource3, resource3.action.first).and_return("report_for_resource3") + expect(reporter.send(:find_or_create_resource_report, resource3, resource3.action.first)).to eq("report_for_resource3") + end + end + end end |