Extract Action Collection from Data Collector

See the PR for details on this change.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 44 insertions, 6 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index 1c921eac72..29dadf54e0 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -378,6 +378,11 @@ module ChefConfig
     default :diff_disabled,           false
     default :diff_filesize_threshold, 10000000
     default :diff_output_threshold,   1000000
+
+    # This is true for "local mode" which uses a chef-zero server listening on
+    # localhost one way or another.  This is true for both `chef-solo` (without
+    # the --legacy-mode flag) or `chef-client -z` methods of starting a client run.
+    #
     default :local_mode, false
 
     # Configures the mode of operation for ChefFS, which is applied to the
@@ -445,9 +450,29 @@ module ChefConfig
     end
 
     default :rest_timeout, 300
+
+    # This solo setting is now almost entirely useless.  It is set to true if chef-solo was
+    # invoked that way from the command-line (i.e. from Application::Solo as opposed to
+    # Application::Client).  The more useful information is contained in the :solo_legacy_mode
+    # vs the :local_mode flags which will be set to true or false depending on how solo was
+    # invoked and actually change more of the behavior.  There might be slight differences in
+    # the behavior of :local_mode due to the behavioral differences in Application::Solo vs.
+    # Application::Client and `chef-solo` vs `chef-client -z`, but checking this value and
+    # switching based on it is almost certainly doing the wrong thing and papering over
+    # bugs that should be fixed in one or the other class, and will be brittle and destined
+    # to break in the future (and not necessarily on a major version bump). Checking this value
+    # is also not sufficent to determine if we are not running against a server since this can
+    # be unset but :local_mode may be set.  It would be accurate to check both :solo and :local_mode
+    # to determine if we're not running against a server, but the more semantically accurate test
+    # is going to be combining :solo_legacy_mode and :local_mode.
+    #
+    # TL;DR: `if Chef::Config[:solo]` is almost certainly buggy code, you should use:
+    #        `if Chef::Config[:local_mode] || Chef::Config[:solo_legacy_mode]`
+    #
+    # @api private
     default :solo, false
 
-    # Are we running in old Chef Solo legacy mode?
+    # This is true for old chef-solo legacy mode without any chef-zero server (chef-solo --legacy-mode)
     default :solo_legacy_mode, false
 
     default :splay, nil
@@ -919,7 +944,7 @@ module ChefConfig
       # data collector will not run.
       # Ex: http://my-data-collector.mycompany.com/ingest
       default(:server_url) do
-        if config_parent.solo || config_parent.local_mode
+        if config_parent.solo_legacy_mode || config_parent.local_mode
           nil
         else
           File.join(config_parent.chef_server_url, "/data-collector")
@@ -950,7 +975,7 @@ module ChefConfig
       # generated by the DataCollector when Chef is run in Solo mode. This
       # allows users to associate their Solo nodes with faux organizations
       # without the nodes being connected to an actual Chef Server.
-      default :organization, nil
+      default :organization, "chef_solo"
     end
 
     configurable(:http_proxy)
diff --git a/chef-config/spec/unit/config_spec.rb b/chef-config/spec/unit/config_spec.rb
index b6e88f0bc8..0e17753185 100644
--- a/chef-config/spec/unit/config_spec.rb
+++ b/chef-config/spec/unit/config_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Kyle Goodwin (<kgoodwin@primerevenue.com>)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -1197,10 +1197,23 @@ RSpec.describe ChefConfig::Config do
 
       end
 
-      context "for Chef Solo" do
+      context "for Chef Solo legacy mode" do
 
         before do
-          ChefConfig::Config[:solo] = true
+          ChefConfig::Config[:solo_legacy_mode] = true
+        end
+
+        it "sets the data collector server URL to nil" do
+          ChefConfig::Config[:chef_server_url] = "https://chef.example/organizations/myorg"
+          expect(ChefConfig::Config[:data_collector][:server_url]).to be_nil
+        end
+
+      end
+
+      context "for local mode" do
+
+        before do
+          ChefConfig::Config[:local_mode] = true
         end
 
         it "sets the data collector server URL to nil" do