Add splay property to chef_client_launchd

Use the same splay method we used in chef_client_cron resource.

Signed-off-by: Tim Smith <tsmith@chef.io>

2 files changed, 78 insertions, 25 deletions

diff --git a/lib/chef/resource/chef_client_launchd.rb b/lib/chef/resource/chef_client_launchd.rb
index ea576690fa..43d7e9bb8f 100644
--- a/lib/chef/resource/chef_client_launchd.rb
+++ b/lib/chef/resource/chef_client_launchd.rb
@@ -58,6 +58,12 @@ class Chef
         callbacks: { "should be a positive number" => proc { |v| v > 0 } },
         default: 30
 
+      property :splay, [Integer, String],
+        default: 300,
+        coerce: proc { |x| Integer(x) },
+        callbacks: { "should be a positive number" => proc { |v| v > 0 } },
+        description: "A random number of seconds between 0 and X to add to interval so that all #{Chef::Dist::CLIENT} commands don't execute at the same time."
+
       property :accept_chef_license, [true, false],
         description: "Accept the Chef Online Master License and Services Agreement. See <https://www.chef.io/online-master-agreement/>",
         default: false
@@ -108,8 +114,7 @@ class Chef
           username new_resource.user
           working_directory new_resource.working_directory
           start_interval new_resource.interval * 60
-          program new_resource.chef_binary_path
-          program_arguments all_daemon_options
+          program_arguments client_command
           environment_variables new_resource.environment unless new_resource.environment.empty?
           nice new_resource.nice
           low_priority_io true
@@ -126,15 +131,35 @@ class Chef
 
       action_class do
         #
-        # Take daemon_options property and append extra daemon options from other properties
-        # to build the complete set of options we pass to the client
+        # Generate a uniformly distributed unique number to sleep from 0 to the splay time
+        #
+        # @param [Integer] splay The number of seconds to splay
+        #
+        # @return [Integer]
+        #
+        def splay_sleep_time(splay)
+          seed = node["shard_seed"] || Digest::MD5.hexdigest(node.name).to_s.hex
+          random = Random.new(seed.to_i)
+          random.rand(splay)
+        end
+
+        #
+        # random sleep time + chef-client + daemon option properties + license acceptance
         #
         # @return [Array]
         #
-        def all_daemon_options
-          options = new_resource.daemon_options + ["-L", ::File.join(new_resource.log_directory, new_resource.log_file_name), "-c", ::File.join(new_resource.config_directory, "client.rb")]
-          options.append("--chef-license", "accept") if new_resource.accept_chef_license
-          options
+        def client_command
+          cmd = ["/bin/sleep",
+                 "#{splay_sleep_time(new_resource.splay)};",
+                 new_resource.chef_binary_path] +
+            new_resource.daemon_options +
+            ["-c",
+            ::File.join(new_resource.config_directory, "client.rb"),
+            "-L",
+            ::File.join(new_resource.log_directory, new_resource.log_file_name),
+            ]
+          cmd.append("--chef-license", "accept") if new_resource.accept_chef_license
+          cmd
         end
       end
     end
diff --git a/spec/unit/resource/chef_client_launchd_spec.rb b/spec/unit/resource/chef_client_launchd_spec.rb
index 4d07471ed5..1d66da3e39 100644
--- a/spec/unit/resource/chef_client_launchd_spec.rb
+++ b/spec/unit/resource/chef_client_launchd_spec.rb
@@ -29,15 +29,24 @@ describe Chef::Resource::ChefClientLaunchd do
     expect(resource.action).to eql([:enable])
   end
 
-  it "builds a default value for chef_binary_path dist values" do
-    expect(resource.chef_binary_path).to eql("/opt/chef/bin/chef-client")
-  end
-
   it "supports :enable and :disable actions" do
     expect { resource.action :enable }.not_to raise_error
     expect { resource.action :disable }.not_to raise_error
   end
 
+  it "coerces splay to an Integer" do
+    resource.splay "10"
+    expect(resource.splay).to eql(10)
+  end
+
+  it "raises an error if splay is not a positive number" do
+    expect { resource.splay("-10") }.to raise_error(Chef::Exceptions::ValidationFailed)
+  end
+
+  it "builds a default value for chef_binary_path dist values" do
+    expect(resource.chef_binary_path).to eql("/opt/chef/bin/chef-client")
+  end
+
   it "raises an error if interval is not a positive number" do
     expect { resource.interval("-10") }.to raise_error(Chef::Exceptions::ValidationFailed)
   end
@@ -60,39 +69,58 @@ describe Chef::Resource::ChefClientLaunchd do
     expect(resource.nice).to eql(10)
   end
 
-  describe "#all_daemon_options" do
-    it "returns log and config flags if by default" do
-      expect(provider.all_daemon_options).to eql(
-        ["-L", "/Library/Logs/Chef/client.log", "-c", "/etc/chef/client.rb"]
+  describe "#splay_sleep_time" do
+    it "uses shard_seed attribute if present" do
+      node.automatic_attrs[:shard_seed] = "73399073"
+      expect(provider.splay_sleep_time(300)).to satisfy { |v| v >= 0 && v <= 300 }
+    end
+
+    it "uses a hex conversion of a md5 hash of the splay if present" do
+      node.automatic_attrs[:shard_seed] = nil
+      allow(node).to receive(:name).and_return("test_node")
+      expect(provider.splay_sleep_time(300)).to satisfy { |v| v >= 0 && v <= 300 }
+    end
+  end
+
+  describe "#client_command" do
+    before do
+      allow(provider).to receive(:splay_sleep_time).and_return("123")
+    end
+
+    let(:root_path) { windows? ? "C:\\chef/client.rb" : "/etc/chef/client.rb" }
+
+    it "creates a valid command if using all default properties" do
+      expect(provider.client_command).to eql(
+        ["/bin/sleep", "123;", "/opt/chef/bin/chef-client", "-c", root_path, "-L", "/Library/Logs/Chef/client.log"]
       )
     end
 
-    it "appends to any passed daemon options" do
+    it "adds custom daemon options from daemon_options property" do
       resource.daemon_options %w{foo bar}
-      expect(provider.all_daemon_options).to eql(
-        ["foo", "bar", "-L", "/Library/Logs/Chef/client.log", "-c", "/etc/chef/client.rb"]
+      expect(provider.client_command).to eql(
+        ["/bin/sleep", "123;", "/opt/chef/bin/chef-client", "foo", "bar", "-c", root_path, "-L", "/Library/Logs/Chef/client.log"]
       )
     end
 
     it "adds license acceptance flags if the property is set" do
       resource.accept_chef_license true
-      expect(provider.all_daemon_options).to eql(
-        ["-L", "/Library/Logs/Chef/client.log", "-c", "/etc/chef/client.rb", "--chef-license", "accept"]
+      expect(provider.client_command).to eql(
+        ["/bin/sleep", "123;", "/opt/chef/bin/chef-client", "-c", root_path, "-L", "/Library/Logs/Chef/client.log", "--chef-license", "accept"]
       )
     end
 
     it "uses custom config dir if set" do
       resource.config_directory "/etc/some_other_dir"
-      expect(provider.all_daemon_options).to eql(
-        ["-L", "/Library/Logs/Chef/client.log", "-c", "/etc/some_other_dir/client.rb"]
+      expect(provider.client_command).to eql(
+        ["/bin/sleep", "123;", "/opt/chef/bin/chef-client", "-c", "/etc/some_other_dir/client.rb", "-L", "/Library/Logs/Chef/client.log"]
       )
     end
 
     it "uses custom log files / paths if set" do
       resource.log_file_name "my-client.log"
       resource.log_directory "/var/log/my-chef/"
-      expect(provider.all_daemon_options).to eql(
-        ["-L", "/var/log/my-chef/my-client.log", "-c", "/etc/chef/client.rb"]
+      expect(provider.client_command).to eql(
+        ["/bin/sleep", "123;", "/opt/chef/bin/chef-client", "-c", root_path, "-L", "/var/log/my-chef/my-client.log"]
       )
     end
   end