Improve chef_client_* resources

- Add new accept_chef_license property
- Wire up config_directory property in chef_client_cron to match the chef_client_scheduled_task behavior
- Add run_on_battery property to chef_client_scheduled_task
- Move logic into helpers in chef_client_schedule_task
- Add more testing to chef_client_scheduled_task

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

4 files changed, 86 insertions, 19 deletions

diff --git a/lib/chef/resource/chef_client_cron.rb b/lib/chef/resource/chef_client_cron.rb
index 9611da65c6..fb30bd0538 100644
--- a/lib/chef/resource/chef_client_cron.rb
+++ b/lib/chef/resource/chef_client_cron.rb
@@ -89,6 +89,10 @@ class Chef
       property :mailto, String,
         description: "The e-mail address to e-mail any cron task failures to."
 
+      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
+
       property :job_name, String,
         default: Chef::Dist::CLIENT,
         description: "The name of the cron job to create."
@@ -184,6 +188,8 @@ class Chef
           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 << "--chef-license accept " if new_resource.accept_chef_license
           cmd << log_command
           cmd << " || echo \"#{Chef::Dist::PRODUCT} execution failed\"" if new_resource.mailto
           cmd
diff --git a/lib/chef/resource/chef_client_scheduled_task.rb b/lib/chef/resource/chef_client_scheduled_task.rb
index d8cdabe76f..c4d2bc97c6 100644
--- a/lib/chef/resource/chef_client_scheduled_task.rb
+++ b/lib/chef/resource/chef_client_scheduled_task.rb
@@ -67,6 +67,10 @@ class Chef
         description: "Numeric value to go with the scheduled task frequency",
         default: 30
 
+      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
+
       property :start_date, String,
         description: "The start date for the task in m:d:Y format (ex: 12/17/2020).",
         regex: [%r{^[0-1][0-9]\/[0-3][0-9]\/\d{4}$}]
@@ -81,6 +85,10 @@ class Chef
         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.",
         default: 300
 
+      property :run_on_battery, [true, false],
+        description: "Run the #{Chef::Dist::PRODUCT} task when the system is on batteries.",
+        default: true
+
       property :config_directory, String,
         description: "The path of the config directory.",
         default: Chef::Dist::CONF_DIR
@@ -115,22 +123,20 @@ class Chef
           end
         end
 
-        # Fetch path of cmd.exe through environment variable comspec
-        cmd_path = ENV["COMSPEC"]
-
         # According to https://docs.microsoft.com/en-us/windows/desktop/taskschd/schtasks,
         # the :once, :onstart, :onlogon, and :onidle schedules don't accept schedule modifiers
         windows_task new_resource.task_name do
-          run_level          :highest
-          command            "#{cmd_path} /c \"#{client_cmd}\""
-          user               new_resource.user
-          password           new_resource.password
-          frequency          new_resource.frequency.to_sym
-          frequency_modifier new_resource.frequency_modifier if frequency_supports_frequency_modifier?
-          start_time         new_resource.start_time
-          start_day          new_resource.start_date unless new_resource.start_date.nil?
-          random_delay       new_resource.splay if frequency_supports_random_delay?
-          action             %i{create enable}
+          run_level                      :highest
+          command                        full_command
+          user                           new_resource.user
+          password                       new_resource.password
+          frequency                      new_resource.frequency.to_sym
+          frequency_modifier             new_resource.frequency_modifier if frequency_supports_frequency_modifier?
+          start_time                     new_resource.start_time
+          start_day                      new_resource.start_date unless new_resource.start_date.nil?
+          random_delay                   new_resource.splay if frequency_supports_random_delay?
+          disallow_start_if_on_batteries new_resource.splay unless new_resource.run_on_battery
+          action                         %i{create enable}
         end
       end
 
@@ -141,6 +147,18 @@ class Chef
       end
 
       action_class do
+        #
+        # The full command to run in the scheduled task
+        #
+        # @return [String]
+        #
+        def full_command
+          # Fetch path of cmd.exe through environment variable comspec
+          cmd_path = ENV["COMSPEC"]
+
+          "#{cmd_path} /c \'#{client_cmd}\'"
+        end
+
         # Build command line to pass to cmd.exe
         #
         # @return [String]
@@ -151,6 +169,7 @@ class Chef
 
           # Add custom options
           cmd << " #{new_resource.daemon_options.join(" ")}" if new_resource.daemon_options.any?
+          cmd << " --chef-license accept" if new_resource.accept_chef_license
           cmd
         end
 
diff --git a/spec/unit/resource/chef_client_cron_spec.rb b/spec/unit/resource/chef_client_cron_spec.rb
index 85472d6f09..00f4e9346c 100644
--- a/spec/unit/resource/chef_client_cron_spec.rb
+++ b/spec/unit/resource/chef_client_cron_spec.rb
@@ -77,33 +77,43 @@ describe Chef::Resource::ChefClientCron do
     end
 
     it "creates a valid command if using all default properties" do
-      expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -L /var/log/chef/client.log")
+      expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/chef/client.rb -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("/bin/sleep 123; /opt/chef/bin/chef-client --foo 1 --bar 2 -L /var/log/chef/client.log")
+      expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client --foo 1 --bar 2 -c /etc/chef/client.rb -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")
     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("/bin/sleep 123; /opt/chef/bin/chef-client -L /var/log/my-chef/my-client.log")
+      expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/chef/client.rb -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("/bin/sleep 123; /opt/chef/bin/chef-client -L /var/log/chef/client.log || echo \"Chef Infra Client execution failed\"")
+      expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/chef/client.rb -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("/bin/sleep 123; /usr/local/bin/chef-client -L /var/log/chef/client.log")
+      expect(provider.cron_command).to eql("/bin/sleep 123; /usr/local/bin/chef-client -c /etc/chef/client.rb -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("/bin/sleep 123; /opt/chef/bin/chef-client > /var/log/chef/client.log 2>&1")
+      expect(provider.cron_command).to eql("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/chef/client.rb > /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("/bin/sleep 123; /opt/chef/bin/chef-client -c /etc/chef/client.rb --chef-license accept -L /var/log/chef/client.log")
     end
   end
 end
diff --git a/spec/unit/resource/chef_client_scheduled_task_spec.rb b/spec/unit/resource/chef_client_scheduled_task_spec.rb
index 6ddda0c8ce..10a6e6b219 100644
--- a/spec/unit/resource/chef_client_scheduled_task_spec.rb
+++ b/spec/unit/resource/chef_client_scheduled_task_spec.rb
@@ -67,4 +67,36 @@ describe Chef::Resource::ChefClientScheduledTask do
     expect { resource.action :add }.not_to raise_error
     expect { resource.action :remove }.not_to raise_error
   end
+
+  describe "#client_cmd" do
+    it "creates a valid command if using all default properties" do
+      expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb")
+    end
+
+    it "uses daemon_options if set" do
+      resource.daemon_options ["--foo 1", "--bar 2"]
+      expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb --foo 1 --bar 2")
+    end
+
+    it "uses custom config dir if set" do
+      resource.config_directory "C:/foo/bar"
+      expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L C:/foo/bar/log/client.log -c C:/foo/bar/client.rb")
+    end
+
+    it "uses custom log files / paths if set" do
+      resource.log_file_name "my-client.log"
+      resource.log_directory "C:/foo/bar"
+      expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L C:/foo/bar/my-client.log -c /etc/chef/client.rb")
+    end
+
+    it "uses custom chef-client binary if set" do
+      resource.chef_binary_path "C:/foo/bar/chef-client"
+      expect(provider.client_cmd).to eql("C:/foo/bar/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb")
+    end
+
+    it "sets the license acceptance flag if set" do
+      resource.accept_chef_license true
+      expect(provider.client_cmd).to eql("C:/opscode/chef/bin/chef-client -L /etc/chef/log/client.log -c /etc/chef/client.rb --chef-license accept")
+    end
+  end
 end