Merge pull request #12170 from wheatevo/fix-cron-resource-commented-job-handling

Fix cron resource commented job handling

2 files changed, 49 insertions, 1 deletions

diff --git a/lib/chef/provider/cron.rb b/lib/chef/provider/cron.rb
index 77c1973fb4..694f7ba4b8 100644
--- a/lib/chef/provider/cron.rb
+++ b/lib/chef/provider/cron.rb
@@ -100,7 +100,10 @@ class Chef
         newcron = get_crontab_entry
 
         if @cron_exists
-          unless cron_different?
+          # Only compare the crontab if the current resource has a set command.
+          # This may not be set in cases where the Chef comment exists but the
+          # crontab command was commented out.
+          if current_resource.property_is_set?(:command) && !cron_different?
             logger.debug("#{new_resource}: Skipping existing cron entry")
             return
           end
diff --git a/spec/unit/provider/cron_spec.rb b/spec/unit/provider/cron_spec.rb
index 9a276bfffd..3e56dbc8b1 100644
--- a/spec/unit/provider/cron_spec.rb
+++ b/spec/unit/provider/cron_spec.rb
@@ -392,6 +392,27 @@ describe Chef::Provider::Cron do
         expect(cron.command).to eq("/bin/true")
       end
     end
+
+    context "with a matching entry with a commented crontab line" do
+      it "should set cron_exists" do
+        allow(@provider).to receive(:read_crontab).and_return(<<~CRONTAB)
+          0 2 * * * /some/other/command
+
+          # Chef Name: cronhole some stuff
+          #* * * * * /bin/true
+        CRONTAB
+        cron = @provider.load_current_resource
+        expect(@provider.cron_exists).to eq(true)
+        expect(@provider.cron_empty).to eq(false)
+        expect(cron.minute).to eq("*")
+        expect(cron.hour).to eq("*")
+        expect(cron.day).to eq("*")
+        expect(cron.month).to eq("*")
+        expect(cron.weekday).to eq("*")
+        expect(cron.time).to eq(nil)
+        expect(cron.property_is_set?(:command)).to eq(false)
+      end
+    end
   end
 
   describe "cron_different?" do
@@ -691,6 +712,30 @@ describe Chef::Provider::Cron do
       end
     end
 
+    context "when there is a crontab with a matching section with a commented crontab line in it" do
+      before :each do
+        @provider.cron_exists = true
+      end
+
+      it "should add the crontab to the entry" do
+        allow(@provider).to receive(:read_crontab).and_return(<<~CRONTAB)
+          0 2 * * * /some/other/command
+
+          # Chef Name: cronhole some stuff
+          # * * * * * /bin/true
+        CRONTAB
+        expect(@provider).to receive(:write_crontab).with(<<~ENDCRON)
+          0 2 * * * /some/other/command
+
+          # Chef Name: cronhole some stuff
+          * * * * * /bin/true
+          # * * * * * /bin/true
+        ENDCRON
+        @new_resource.minute "*"
+        @provider.run_action(:create)
+      end
+    end
+
     context "when there is a crontab with a matching and identical section" do
       context "when environment variable is not used" do
         before :each do