diff --git a/changelog/60997.fixed b/changelog/60997.fixed new file mode 100644 index 00000000000..779005f5014 --- /dev/null +++ b/changelog/60997.fixed @@ -0,0 +1 @@ +Fix cron.present duplicating entries when changing timespec to special. diff --git a/salt/states/cron.py b/salt/states/cron.py index c8576727190..bfbfbea0eb3 100644 --- a/salt/states/cron.py +++ b/salt/states/cron.py @@ -355,6 +355,9 @@ def present( return ret if special is None: + antidata = __salt__["cron.rm_special"]( + user, name, special=special, identifier=identifier + ) data = __salt__["cron.set_job"]( user=user, minute=minute, @@ -368,6 +371,7 @@ def present( identifier=identifier, ) else: + antidata = __salt__["cron.rm_job"](user, name, identifier=identifier) data = __salt__["cron.set_special"]( user=user, special=special, @@ -376,15 +380,23 @@ def present( commented=commented, identifier=identifier, ) - if data == "present": + + if data == "present" and antidata == "removed": + ret["comment"] = "Duplicate Cron {} removed".format(name) + return ret + + if data == "present" and antidata == "absent": ret["comment"] = "Cron {} already present".format(name) return ret - if data == "new": + if data == "new" and antidata == "absent": ret["comment"] = "Cron {} added to {}'s crontab".format(name, user) ret["changes"] = {user: name} return ret + if data == "new" and antidata == "removed": + ret["comment"] = "Cron {} time specification changed".format(name) + if data == "updated": ret["comment"] = "Cron {} updated".format(name) ret["changes"] = {user: name} diff --git a/tests/unit/states/test_cron.py b/tests/unit/states/test_cron.py index 0732b2d889f..989c5dcc74f 100644 --- a/tests/unit/states/test_cron.py +++ b/tests/unit/states/test_cron.py @@ -24,6 +24,8 @@ class CronTestCase(TestCase, LoaderModuleMockMixin): "cron.list_tab": cronmod.list_tab, "cron.rm_job": cronmod.rm_job, "cron.set_job": cronmod.set_job, + "cron.rm_special": cronmod.rm_special, + "cron.set_special": cronmod.set_special, }, } return {cron: loader_gloabals, cronmod: loader_gloabals} @@ -133,6 +135,37 @@ class CronTestCase(TestCase, LoaderModuleMockMixin): "* 2 * * * foo", ) + def test_present_special(self): + cron.present(name="foo", special="@hourly", identifier="1", user="root") + self.assertMultiLineEqual( + self.get_crontab(), + "# Lines below here are managed by Salt, do not edit\n" + "# SALT_CRON_IDENTIFIER:1\n" + "@hourly foo", + ) + + def test_present_special_after_unspecial(self): + """cron.present should remove an unspecial entry with the same identifier""" + cron.present(name="foo", hour="1", identifier="1", user="root") + cron.present(name="foo", special="@hourly", identifier="1", user="root") + self.assertMultiLineEqual( + self.get_crontab(), + "# Lines below here are managed by Salt, do not edit\n" + "# SALT_CRON_IDENTIFIER:1\n" + "@hourly foo", + ) + + def test_present_unspecial_after_special(self): + """cron.present should remove an special entry with the same identifier""" + cron.present(name="foo", special="@hourly", identifier="1", user="root") + cron.present(name="foo", hour="1", identifier="1", user="root") + self.assertMultiLineEqual( + self.get_crontab(), + "# Lines below here are managed by Salt, do not edit\n" + "# SALT_CRON_IDENTIFIER:1\n" + "* 1 * * * foo", + ) + def test_remove(self): with patch.dict(cron.__opts__, {"test": True}): self.set_crontab(