diff options
| -rw-r--r-- | lib/chef/resource/chef_client_cron.rb | 4 | ||||
| -rw-r--r-- | lib/chef/resource/chef_client_launchd.rb | 40 | ||||
| -rw-r--r-- | lib/chef/resource/chef_client_scheduled_task.rb | 7 | ||||
| -rw-r--r-- | spec/unit/resource/chef_client_cron_spec.rb | 40 | ||||
| -rw-r--r-- | spec/unit/resource/chef_client_launchd_spec.rb | 62 |
5 files changed, 103 insertions, 50 deletions
diff --git a/lib/chef/resource/chef_client_cron.rb b/lib/chef/resource/chef_client_cron.rb index 7e216a75a3..ab435c39f8 100644 --- a/lib/chef/resource/chef_client_cron.rb +++ b/lib/chef/resource/chef_client_cron.rb @@ -164,7 +164,7 @@ class Chef mailto new_resource.mailto if new_resource.mailto user new_resource.user comment new_resource.comment if new_resource.comment - command cron_command + command client_command end end @@ -193,7 +193,7 @@ class Chef # # @return [String] # - def cron_command + def client_command cmd = "" cmd << "/bin/sleep #{splay_sleep_time(new_resource.splay)}; " cmd << "#{which("nice")} -n #{new_resource.nice} " if new_resource.nice diff --git a/lib/chef/resource/chef_client_launchd.rb b/lib/chef/resource/chef_client_launchd.rb index ea576690fa..a2b024c030 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 ["/bin/bash", "-c", client_command] environment_variables new_resource.environment unless new_resource.environment.empty? nice new_resource.nice low_priority_io true @@ -126,15 +131,32 @@ 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] + # @return [String] # - 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 = "" + cmd << "/bin/sleep #{splay_sleep_time(new_resource.splay)};" + cmd << " #{new_resource.chef_binary_path}" + cmd << " #{new_resource.daemon_options.join(" ")}" unless new_resource.daemon_options.empty? + cmd << " -c #{::File.join(new_resource.config_directory, "client.rb")}" + cmd << " -L #{::File.join(new_resource.log_directory, new_resource.log_file_name)}" + cmd << " --chef-license accept" if new_resource.accept_chef_license + cmd end end end diff --git a/lib/chef/resource/chef_client_scheduled_task.rb b/lib/chef/resource/chef_client_scheduled_task.rb index 319acd3d41..7fde581763 100644 --- a/lib/chef/resource/chef_client_scheduled_task.rb +++ b/lib/chef/resource/chef_client_scheduled_task.rb @@ -70,8 +70,9 @@ class Chef description: "The name of the user that #{Chef::Dist::PRODUCT} runs as.", default: "System", sensitive: true - property :password, String, sensitive: true, - description: "The password for the user that #{Chef::Dist::PRODUCT} runs as." + property :password, String, + description: "The password for the user that #{Chef::Dist::PRODUCT} runs as.", + sensitive: true property :frequency, String, description: "Frequency with which to run the task.", @@ -175,9 +176,11 @@ class Chef "#{cmd_path} /c \"#{client_cmd}\"" end + # # Build command line to pass to cmd.exe # # @return [String] + # def client_cmd cmd = new_resource.chef_binary_path.dup cmd << " -L #{::File.join(new_resource.log_directory, new_resource.log_file_name)}" diff --git a/spec/unit/resource/chef_client_cron_spec.rb b/spec/unit/resource/chef_client_cron_spec.rb index 00d12b82a8..121758ac73 100644 --- a/spec/unit/resource/chef_client_cron_spec.rb +++ b/spec/unit/resource/chef_client_cron_spec.rb @@ -29,6 +29,11 @@ describe Chef::Resource::ChefClientCron do expect(resource.action).to eql([:add]) end + it "supports :add and :remove actions" do + expect { resource.action :add }.not_to raise_error + expect { resource.action :remove }.not_to raise_error + end + it "coerces splay to an Integer" do resource.splay "10" expect(resource.splay).to eql(10) @@ -38,6 +43,10 @@ describe Chef::Resource::ChefClientCron 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 nice is less than -20" do expect { resource.nice(-21) }.to raise_error(Chef::Exceptions::ValidationFailed) end @@ -51,10 +60,6 @@ describe Chef::Resource::ChefClientCron do expect(resource.nice).to eql(10) 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 "log_directory is /Library/Logs/Chef on macOS systems" do node.automatic_attrs[:platform_family] = "mac_os_x" node.automatic_attrs[:platform] = "mac_os_x" @@ -66,11 +71,6 @@ describe Chef::Resource::ChefClientCron do expect(resource.log_directory).to eql("/var/log/chef") end - it "supports :add and :remove actions" do - expect { resource.action :add }.not_to raise_error - expect { resource.action :remove }.not_to raise_error - end - describe "#splay_sleep_time" do it "uses shard_seed attribute if present" do node.automatic_attrs[:shard_seed] = "73399073" @@ -84,7 +84,7 @@ describe Chef::Resource::ChefClientCron do end end - describe "#cron_command" do + describe "#client_command" do before do allow(provider).to receive(:splay_sleep_time).and_return("123") end @@ -92,55 +92,55 @@ describe Chef::Resource::ChefClientCron do 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.cron_command).to eql( + expect(provider.client_command).to eql( "/bin/sleep 123; /opt/chef/bin/chef-client -c #{root_path} -L /var/log/chef/client.log" ) end it "uses daemon_options if set" do resource.daemon_options ["--foo 1", "--bar 2"] - expect(provider.cron_command).to eql( + expect(provider.client_command).to eql( "/bin/sleep 123; /opt/chef/bin/chef-client --foo 1 --bar 2 -c #{root_path} -L /var/log/chef/client.log" ) end it "uses custom config dir if set" do resource.config_directory "/etc/some_other_dir" - expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/some_other_dir/client.rb -L /var/log/chef/client.log") + expect(provider.client_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/some_other_dir/client.rb -L /var/log/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.cron_command).to eql( + 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 it "uses mailto if set" do resource.mailto "bob@example.com" - expect(provider.cron_command).to eql( + expect(provider.client_command).to eql( "/bin/sleep 123; /opt/chef/bin/chef-client -c #{root_path} -L /var/log/chef/client.log || echo \"Chef Infra Client execution failed\"" ) end it "uses custom chef-client binary if set" do resource.chef_binary_path "/usr/local/bin/chef-client" - expect(provider.cron_command).to eql( + expect(provider.client_command).to eql( "/bin/sleep 123; /usr/local/bin/chef-client -c #{root_path} -L /var/log/chef/client.log" ) end it "appends to the log file appending if set to false" do resource.append_log_file false - expect(provider.cron_command).to eql( + expect(provider.client_command).to eql( "/bin/sleep 123; /opt/chef/bin/chef-client -c #{root_path} > /var/log/chef/client.log 2>&1" ) end it "sets the license acceptance flag if set" do resource.accept_chef_license true - expect(provider.cron_command).to eql( + expect(provider.client_command).to eql( "/bin/sleep 123; /opt/chef/bin/chef-client -c #{root_path} --chef-license accept -L /var/log/chef/client.log" ) end @@ -148,8 +148,8 @@ describe Chef::Resource::ChefClientCron do it "uses nice if set" do allow(provider).to receive(:which).with("nice").and_return("/usr/bin/nice") resource.nice(-15) - expect(provider.cron_command).to eql( - "/bin/sleep 123; /usr/bin/nice -n -15 /opt/chef/bin/chef-client -c /etc/chef/client.rb -L /var/log/chef/client.log" + expect(provider.client_command).to eql( + "/bin/sleep 123; /usr/bin/nice -n -15 /opt/chef/bin/chef-client -c #{root_path} -L /var/log/chef/client.log" ) end end diff --git a/spec/unit/resource/chef_client_launchd_spec.rb b/spec/unit/resource/chef_client_launchd_spec.rb index 4d07471ed5..1d0015cb0d 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 |