Merge pull request #5065 from chef/adamleff/include-node-in-converge-messages

Adding node object to Data collector run_converge message

5 files changed, 32 insertions, 124 deletions

diff --git a/acceptance/data-collector/test/integration/default/serverspec/default_spec.rb b/acceptance/data-collector/test/integration/default/serverspec/default_spec.rb
index be15b96429..f9d365ac58 100644
--- a/acceptance/data-collector/test/integration/default/serverspec/default_spec.rb
+++ b/acceptance/data-collector/test/integration/default/serverspec/default_spec.rb
@@ -110,6 +110,7 @@ shared_examples_for "run_converge.success payload check" do
         expanded_run_list
         message_type
         message_version
+        node
         node_name
         organization_name
         resources
@@ -150,6 +151,7 @@ shared_examples_for "run_converge.failure payload check" do
         expanded_run_list
         message_type
         message_version
+        node
         node_name
         organization_name
         resources
@@ -178,44 +180,6 @@ shared_examples_for "run_converge.failure payload check" do
   end
 end
 
-shared_examples_for "node-update payload check" do
-  describe "node update message" do
-    let(:required_fields) do
-      %w{
-        entity_name
-        entity_type
-        entity_uuid
-        id
-        message_type
-        message_version
-        organization_name
-        recorded_at
-        remote_hostname
-        requestor_name
-        requestor_type
-        run_id
-        service_hostname
-        source
-        task
-        user_agent
-      }
-    end
-    let(:optional_fields) { %{data} }
-
-    it "is not missing any required fields" do
-      payload = JSON.load(command("curl http://localhost:9292/cache/action").stdout)
-      missing_fields = required_fields.select { |key| !payload.key?(key) }
-      expect(missing_fields).to eq([])
-    end
-
-    it "does not have any extra fields" do
-      payload = JSON.load(command("curl http://localhost:9292/cache/action").stdout)
-      extra_fields = payload.keys.select { |key| !required_fields.include?(key) && !optional_fields.include?(key) }
-      expect(extra_fields).to eq([])
-    end
-  end
-end
-
 describe "CCR with no data collector URL configured" do
   include_examples "successful chef run", "chef-client -z -c /etc/chef/no-endpoint.rb"
   include_examples "counter checks", { "run_start" => nil, "run_converge.success" => nil, "run_converge.failure" => nil }
@@ -226,7 +190,6 @@ describe "CCR, local mode, config in solo mode" do
   include_examples "counter checks", { "run_start" => 1, "run_converge.success" => 1, "run_converge.failure" => nil }
   include_examples "run_start payload check"
   include_examples "run_converge.success payload check"
-  include_examples "node-update payload check"
 end
 
 describe "CCR, local mode, config in client mode" do
@@ -239,7 +202,6 @@ describe "CCR, local mode, config in both mode" do
   include_examples "counter checks", { "run_start" => 1, "run_converge.success" => 1, "run_converge.failure" => nil }
   include_examples "run_start payload check"
   include_examples "run_converge.success payload check"
-  include_examples "node-update payload check"
 end
 
 describe "CCR, local mode, config in solo mode, failed run" do
@@ -247,5 +209,4 @@ describe "CCR, local mode, config in solo mode, failed run" do
   include_examples "counter checks", { "run_start" => 1, "run_converge.success" => nil, "run_converge.failure" => 1 }
   include_examples "run_start payload check"
   include_examples "run_converge.failure payload check"
-  include_examples "node-update payload check"
 end
diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb
index e7ef8d39ec..d307fff2ba 100644
--- a/lib/chef/data_collector.rb
+++ b/lib/chef/data_collector.rb
@@ -279,7 +279,6 @@ class Chef
         # we have nothing to report.
         return unless run_status
 
-        send_to_data_collector(Chef::DataCollector::Messages.node_update_message(run_status).to_json)
         send_to_data_collector(
           Chef::DataCollector::Messages.run_end_message(
             run_status: run_status,
diff --git a/lib/chef/data_collector/messages.rb b/lib/chef/data_collector/messages.rb
index 89bad9f9f9..8c2a84b580 100644
--- a/lib/chef/data_collector/messages.rb
+++ b/lib/chef/data_collector/messages.rb
@@ -68,6 +68,7 @@ class Chef
           "id"                     => run_status.run_id,
           "message_version"        => "1.0.0",
           "message_type"           => "run_converge",
+          "node"                   => run_status.node,
           "node_name"              => run_status.node.name,
           "organization_name"      => organization,
           "resources"              => reporter_data[:resources].map(&:report_data),
@@ -90,36 +91,6 @@ class Chef
 
         message
       end
-
-      #
-      # Message payload that is sent to the DataCollector server at the
-      # end of a Chef run.
-      #
-      # @param run_status [Chef::RunStatus] The RunStatus instance for this node/run.
-      #
-      # @return [Hash] A hash containing the node object and related metadata.
-      #
-      def self.node_update_message(run_status)
-        {
-          "entity_name"       => run_status.node.name,
-          "entity_type"       => "node",
-          "entity_uuid"       => node_uuid,
-          "id"                => SecureRandom.uuid,
-          "message_version"   => "1.1.0",
-          "message_type"      => "action",
-          "organization_name" => organization,
-          "recorded_at"       => Time.now.utc.iso8601,
-          "remote_hostname"   => run_status.node["fqdn"],
-          "requestor_name"    => run_status.node.name,
-          "requestor_type"    => "client",
-          "run_id"            => run_status.run_id,
-          "service_hostname"  => chef_server_fqdn(run_status),
-          "source"            => collector_source,
-          "task"              => "update",
-          "user_agent"        => Chef::HTTP::HTTPRequest::DEFAULT_UA,
-          "data"              => run_status.node,
-        }
-      end
     end
   end
 end
diff --git a/lib/chef/data_collector/resource_report.rb b/lib/chef/data_collector/resource_report.rb
index 89b6d26706..e031d4bcb6 100644
--- a/lib/chef/data_collector/resource_report.rb
+++ b/lib/chef/data_collector/resource_report.rb
@@ -65,15 +65,16 @@ class Chef
 
       def to_hash
         hash = {
-          "type"     => new_resource.resource_name.to_sym,
-          "name"     => new_resource.name.to_s,
-          "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_in_milliseconds.to_s,
-          "delta"    => new_resource.respond_to?(:diff) && potentially_changed? ? new_resource.diff : "",
-          "result"   => action.to_s,
-          "status"   => status,
+          "type"           => new_resource.resource_name.to_sym,
+          "name"           => new_resource.name.to_s,
+          "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_in_milliseconds.to_s,
+          "delta"          => new_resource.respond_to?(:diff) && potentially_changed? ? new_resource.diff : "",
+          "ignore_failure" => new_resource.ignore_failure,
+          "result"         => action.to_s,
+          "status"         => status,
         }
 
         if new_resource.cookbook_name
diff --git a/spec/unit/data_collector/messages_spec.rb b/spec/unit/data_collector/messages_spec.rb
index 686e500507..b0c7e692d2 100644
--- a/spec/unit/data_collector/messages_spec.rb
+++ b/spec/unit/data_collector/messages_spec.rb
@@ -18,6 +18,7 @@
 #
 
 require "spec_helper"
+require "ffi_yajl"
 require "chef/data_collector/messages/helpers"
 
 describe Chef::DataCollector::Messages do
@@ -61,7 +62,8 @@ describe Chef::DataCollector::Messages do
   end
 
   describe '#run_end_message' do
-    let(:run_status) { Chef::RunStatus.new(Chef::Node.new, Chef::EventDispatch::Dispatcher.new) }
+    let(:node)       { Chef::Node.new }
+    let(:run_status) { Chef::RunStatus.new(node, Chef::EventDispatch::Dispatcher.new) }
     let(:report1)  { double("report1", report_data: { "status" => "updated" }) }
     let(:report2)  { double("report2", report_data: { "status" => "skipped" }) }
     let(:reporter_data) do
@@ -76,6 +78,20 @@ describe Chef::DataCollector::Messages do
       allow(run_status).to receive(:end_time).and_return(Time.now)
     end
 
+    it "includes a valid node object in the payload" do
+      message = Chef::DataCollector::Messages.run_end_message(reporter_data)
+      expect(message["node"]).to be_an_instance_of(Chef::Node)
+    end
+
+    it "returns a sane JSON representation of the node object" do
+      node.chef_environment = "my_test_environment"
+      node.run_list.add("recipe[my_test_cookbook::default]")
+      message = FFI_Yajl::Parser.parse(Chef::DataCollector::Messages.run_end_message(reporter_data).to_json)
+
+      expect(message["node"]["chef_environment"]).to eq("my_test_environment")
+      expect(message["node"]["run_list"]).to eq(["recipe[my_test_cookbook::default]"])
+    end
+
     context "when the run was successful" do
       let(:required_fields) do
         %w{
@@ -86,6 +102,7 @@ describe Chef::DataCollector::Messages do
           expanded_run_list
           message_type
           message_version
+          node
           node_name
           organization_name
           resources
@@ -136,6 +153,7 @@ describe Chef::DataCollector::Messages do
           expanded_run_list
           message_type
           message_version
+          node
           node_name
           organization_name
           resources
@@ -169,46 +187,4 @@ describe Chef::DataCollector::Messages do
       end
     end
   end
-
-  describe '#node_update_message' do
-    let(:run_status) { Chef::RunStatus.new(Chef::Node.new, Chef::EventDispatch::Dispatcher.new) }
-
-    let(:required_fields) do
-      %w{
-        entity_name
-        entity_type
-        entity_uuid
-        id
-        message_type
-        message_version
-        organization_name
-        recorded_at
-        remote_hostname
-        requestor_name
-        requestor_type
-        run_id
-        service_hostname
-        source
-        task
-        user_agent
-      }
-    end
-    let(:optional_fields) { %w{data} }
-
-    it "is not missing any required fields" do
-      missing_fields = required_fields.select do |key|
-        !Chef::DataCollector::Messages.node_update_message(run_status).key?(key)
-      end
-
-      expect(missing_fields).to eq([])
-    end
-
-    it "does not have any extra fields" do
-      extra_fields = Chef::DataCollector::Messages.node_update_message(run_status).keys.select do |key|
-        !required_fields.include?(key) && !optional_fields.include?(key)
-      end
-
-      expect(extra_fields).to eq([])
-    end
-  end
 end