From 171a9e33d55a103f165a989d28e86b3ac863c987 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 13 Feb 2023 19:11:45 -0800 Subject: [PATCH] When removing hidden times and returning the data, make sure we're not updating the running schedule. --- salt/utils/schedule.py | 13 ++-- .../unit/utils/scheduler/test_helpers.py | 63 +++++++++++++++++-- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/salt/utils/schedule.py b/salt/utils/schedule.py index 7fb539d2ff2..b31293c5f80 100644 --- a/salt/utils/schedule.py +++ b/salt/utils/schedule.py @@ -208,20 +208,21 @@ class Schedule: pillar_schedule = self.opts.get("pillar", {}).get("schedule", {}) if not isinstance(pillar_schedule, dict): raise ValueError("Schedule must be of type dict.") - schedule.update(pillar_schedule) + schedule.update(copy.deepcopy(pillar_schedule)) if include_opts: opts_schedule = self.opts.get("schedule", {}) if not isinstance(opts_schedule, dict): raise ValueError("Schedule must be of type dict.") - schedule.update(opts_schedule) + schedule.update(copy.deepcopy(opts_schedule)) if remove_hidden: _schedule = copy.deepcopy(schedule) - for job in _schedule: - if isinstance(_schedule[job], dict): - for item in _schedule[job]: + for job in schedule: + if isinstance(schedule[job], dict): + for item in schedule[job]: if item.startswith("_"): - del schedule[job][item] + del _schedule[job][item] + return _schedule return schedule def _check_max_running(self, func, data, opts, now): diff --git a/tests/pytests/unit/utils/scheduler/test_helpers.py b/tests/pytests/unit/utils/scheduler/test_helpers.py index 7936e6c48c8..9c05e10aede 100644 --- a/tests/pytests/unit/utils/scheduler/test_helpers.py +++ b/tests/pytests/unit/utils/scheduler/test_helpers.py @@ -1,24 +1,77 @@ +import copy +import datetime import logging log = logging.getLogger(__name__) -def test_get_schedule(schedule): +def test_get_schedule_remove_hidden_true(schedule): """ verify that the _get_schedule function works when remove_hidden is True and schedule data - contains enabled key + contains enabled key and that opts["schedule"] + is not changed. """ job_name = "test_get_schedule" job = { "schedule": { "enabled": True, - job_name: {"function": "test.ping", "seconds": 60}, + job_name: { + "function": "test.ping", + "seconds": 60, + "_next_fire_time": datetime.datetime(2023, 2, 13, 18, 25, 16, 271796), + "_splay": None, + "_seconds": 3600, + "_next_scheduled_fire_time": datetime.datetime( + 2023, 2, 13, 18, 25, 16, 271796 + ), + "_skip_reason": "disabled", + "_skipped_time": datetime.datetime(2023, 2, 13, 17, 26, 16, 271381), + "_skipped": True, + }, + } + } + + expected_ret = { + "enabled": True, + job_name: {"function": "test.ping", "seconds": 60}, + } + # Add the job to the scheduler + schedule.opts.update(copy.deepcopy(job)) + + ret = schedule._get_schedule(remove_hidden=True) + assert expected_ret == ret + assert schedule.opts["schedule"][job_name] == job["schedule"][job_name] + + +def test_get_schedule_remove_hidden_false(schedule): + """ + verify that the _get_schedule function works + when remove_hidden is False and schedule data + contains enabled key and that opts["schedule"] + is not changed. + """ + + job_name = "test_get_schedule" + job = { + "schedule": { + "enabled": True, + job_name: { + "function": "test.ping", + "seconds": 60, + "_next_fire_time": datetime.datetime(2023, 2, 13, 18, 25, 16), + "_splay": None, + "_seconds": 3600, + "_next_scheduled_fire_time": datetime.datetime(2023, 2, 13, 18, 25, 16), + "_skip_reason": "disabled", + "_skipped_time": datetime.datetime(2023, 2, 13, 17, 26, 16), + "_skipped": True, + }, } } # Add the job to the scheduler - schedule.opts.update(job) + schedule.opts.update(copy.deepcopy(job)) - ret = schedule._get_schedule(remove_hidden=True) + ret = schedule._get_schedule(remove_hidden=False) assert job["schedule"] == ret