diff --git a/awx/main/models/schedules.py b/awx/main/models/schedules.py index 3993fdf183..046a08a2e8 100644 --- a/awx/main/models/schedules.py +++ b/awx/main/models/schedules.py @@ -35,6 +35,64 @@ __all__ = ['Schedule'] UTC_TIMEZONES = {x: tzutc() for x in dateutil.parser.parserinfo().UTCZONE} +def _assert_timezone_id_is_valid(rrules) -> None: + broken_rrules = [str(rrule) for rrule in rrules if rrule._dtstart and rrule._dtstart.tzinfo is None] + if not broken_rrules: + return + raise ValueError( + f'A valid TZID must be provided (e.g., America/New_York). Invalid: {broken_rrules}', + ) from None + + +def _fast_forward_rrules(rrules, ref_dt=None): + for i, rule in enumerate(rrules): + rrules[i] = _fast_forward_rrule(rule, ref_dt=ref_dt) + return rrules + + +def _fast_forward_rrule(rrule, ref_dt=None): + ''' + Utility to fast forward an rrule, maintaining consistency in the resulting + occurrences. + + Uses the .replace() method to update the rrule with a newer dtstart + The operation ensures that the original occurrences (based on the original dtstart) + will match the occurrences after changing the dtstart. + + Returns a new rrule with a new dtstart + ''' + if rrule._freq not in {dateutil.rrule.HOURLY, dateutil.rrule.MINUTELY}: + return rrule + + if ref_dt is None: + ref_dt = now() + + if rrule._dtstart > ref_dt: + return rrule + + interval = rrule._interval if rrule._interval else 1 + if rrule._freq == dateutil.rrule.HOURLY: + interval *= 60 * 60 + elif rrule._freq == dateutil.rrule.MINUTELY: + interval *= 60 + + # if after converting to seconds the interval is still a fraction, + # just return original rrule + if isinstance(interval, float) and not interval.is_integer(): + return rrule + + seconds_since_dtstart = (ref_dt - rrule._dtstart).total_seconds() + + # it is important to fast forward by a number that is divisible by + # interval. For example, if interval is 7 hours, we fast forward by 7, 14, 21, etc. hours. + # Otherwise, the occurrences after the fast forward might not match the ones before. + # x // y is integer division, lopping off any remainder, so that we get the outcome we want. + interval_aligned_offset = datetime.timedelta(seconds=(seconds_since_dtstart // interval) * interval) + new_start = rrule._dtstart + interval_aligned_offset + new_rrule = rrule.replace(dtstart=new_start) + return new_rrule + + class ScheduleFilterMethods(object): def enabled(self, enabled=True): return self.filter(enabled=enabled) @@ -194,41 +252,26 @@ class Schedule(PrimordialModel, LaunchTimeConfig): return " ".join(rules) @classmethod - def rrulestr(cls, rrule, fast_forward=True, **kwargs): + def rrulestr(cls, rrule, ref_dt=None, **kwargs): """ Apply our own custom rrule parsing requirements """ rrule = Schedule.coerce_naive_until(rrule) kwargs['forceset'] = True - x = dateutil.rrule.rrulestr(rrule, tzinfos=UTC_TIMEZONES, **kwargs) + rruleset = dateutil.rrule.rrulestr(rrule, tzinfos=UTC_TIMEZONES, **kwargs) - for r in x._rrule: - if r._dtstart and r._dtstart.tzinfo is None: - raise ValueError('A valid TZID must be provided (e.g., America/New_York)') + _assert_timezone_id_is_valid(rruleset._rrule) + _assert_timezone_id_is_valid(rruleset._exrule) # Fast forward is a way for us to limit the number of events in the rruleset - # If we are fastforwading and we don't have a count limited rule that is minutely or hourley - # We will modify the start date of the rule to last week to prevent a large number of entries - if fast_forward: - try: - # All rules in a ruleset will have the same dtstart value - # so lets compare the first event to now to see if its > 7 days old - first_event = x[0] - if (now() - first_event).days > 7: - for rule in x._rrule: - # If any rule has a minutely or hourly rule without a count... - if rule._freq in [dateutil.rrule.MINUTELY, dateutil.rrule.HOURLY] and not rule._count: - # hourly/minutely rrules with far-past DTSTART values - # are *really* slow to precompute - # start *from* one week ago to speed things up drastically - new_start = (now() - datetime.timedelta(days=7)).strftime('%Y%m%d') - # Now we want to repalce the DTSTART:T with the new date (which includes the T) - new_rrule = re.sub('(DTSTART[^:]*):[^T]+T', r'\1:{0}T'.format(new_start), rrule) - return Schedule.rrulestr(new_rrule, fast_forward=False) - except IndexError: - pass + # If we are fast forwarding and we don't have a count limited rule that is minutely or hourly + # We will modify the start date of the rule to bring as close to the current date as possible + # Even though the API restricts each rrule to have the same dtstart, each rrule in the rruleset + # can fast forward to a difference dtstart. This is required in order to get stable occurrences. + rruleset._rrule = _fast_forward_rrules(rruleset._rrule, ref_dt=ref_dt) + rruleset._exrule = _fast_forward_rrules(rruleset._exrule, ref_dt=ref_dt) - return x + return rruleset def __str__(self): return u'%s_t%s_%s_%s' % (self.name, self.unified_job_template.id, self.id, self.next_run) @@ -279,10 +322,11 @@ class Schedule(PrimordialModel, LaunchTimeConfig): next_run_actual = None self.next_run = next_run_actual - try: - self.dtstart = future_rs[0].astimezone(pytz.utc) - except IndexError: - self.dtstart = None + if not self.dtstart: + try: + self.dtstart = future_rs[0].astimezone(pytz.utc) + except IndexError: + self.dtstart = None self.dtend = Schedule.get_end_date(future_rs) changed = any(getattr(self, field_name) != starting_values[field_name] for field_name in affects_fields) diff --git a/awx/main/tests/functional/models/test_schedule.py b/awx/main/tests/functional/models/test_schedule.py index 6ad7115373..ba81f424bc 100644 --- a/awx/main/tests/functional/models/test_schedule.py +++ b/awx/main/tests/functional/models/test_schedule.py @@ -121,30 +121,16 @@ class TestComputedFields: assert job_template.next_schedule == expected_schedule -@pytest.mark.django_db -@pytest.mark.parametrize('freq, delta', (('MINUTELY', 1), ('HOURLY', 1))) -def test_past_week_rrule(job_template, freq, delta): - # see: https://github.com/ansible/awx/issues/8071 - recent = datetime.utcnow() - timedelta(days=3) - recent = recent.replace(hour=0, minute=0, second=0, microsecond=0) - recent_dt = recent.strftime('%Y%m%d') - rrule = f'DTSTART;TZID=America/New_York:{recent_dt}T000000 RRULE:FREQ={freq};INTERVAL={delta};COUNT=5' # noqa - sched = Schedule.objects.create(name='example schedule', rrule=rrule, unified_job_template=job_template) - first_event = sched.rrulestr(sched.rrule)[0] - assert first_event.replace(tzinfo=None) == recent - - @pytest.mark.django_db @pytest.mark.parametrize('freq, delta', (('MINUTELY', 1), ('HOURLY', 1))) def test_really_old_dtstart(job_template, freq, delta): # see: https://github.com/ansible/awx/issues/8071 # If an event is per-minute/per-hour and was created a *really long* - # time ago, we should just bump forward to start counting "in the last week" + # time ago, we should just bump forward the dtstart rrule = f'DTSTART;TZID=America/New_York:20150101T000000 RRULE:FREQ={freq};INTERVAL={delta}' # noqa sched = Schedule.objects.create(name='example schedule', rrule=rrule, unified_job_template=job_template) - last_week = (datetime.utcnow() - timedelta(days=7)).date() first_event = sched.rrulestr(sched.rrule)[0] - assert last_week == first_event.date() + assert now() - first_event < timedelta(days=1) # the next few scheduled events should be the next minute/hour incremented next_five_events = list(sched.rrulestr(sched.rrule).xafter(now(), count=5)) diff --git a/awx/main/tests/unit/utils/test_schedule_fast_forward.py b/awx/main/tests/unit/utils/test_schedule_fast_forward.py new file mode 100644 index 0000000000..48ce4b6612 --- /dev/null +++ b/awx/main/tests/unit/utils/test_schedule_fast_forward.py @@ -0,0 +1,132 @@ +import pytest +import datetime +import dateutil + +from django.utils.timezone import now + +from awx.main.models.schedules import _fast_forward_rrule, Schedule +from dateutil.rrule import HOURLY, MINUTELY, MONTHLY + +REF_DT = datetime.datetime(2024, 1, 1, tzinfo=datetime.timezone.utc) + + +@pytest.mark.parametrize( + 'rrulestr', + [ + pytest.param('DTSTART;TZID=America/New_York:20201118T200000 RRULE:FREQ=MINUTELY;INTERVAL=5', id='every-5-min'), + pytest.param('DTSTART;TZID=America/New_York:20201118T200000 RRULE:FREQ=HOURLY;INTERVAL=5', id='every-5-hours'), + pytest.param('DTSTART;TZID=America/New_York:20201118T200000 RRULE:FREQ=YEARLY;INTERVAL=5', id='every-5-years'), + pytest.param( + 'DTSTART;TZID=America/New_York:20201118T200000 RRULE:FREQ=MINUTELY;INTERVAL=5;WKST=SU;BYMONTH=2,3;BYMONTHDAY=18;BYHOUR=5;BYMINUTE=35;BYSECOND=0', + id='every-5-minutes-at-5:35:00-am-on-the-18th-day-of-feb-or-march-with-week-starting-on-sundays', + ), + pytest.param( + 'DTSTART;TZID=America/New_York:20201118T200000 RRULE:FREQ=HOURLY;INTERVAL=5;WKST=SU;BYMONTH=2,3;BYHOUR=5', + id='every-5-hours-at-5-am-in-feb-or-march-with-week-starting-on-sundays', + ), + ], +) +def test_fast_forwarded_rrule_matches_original_occurrence(rrulestr): + ''' + Assert that the resulting fast forwarded date is included in the original rrule + occurrence list + ''' + rruleset = Schedule.rrulestr(rrulestr, ref_dt=REF_DT) + + gen = rruleset.xafter(REF_DT, count=200) + occurrences = [i for i in gen] + + orig_rruleset = dateutil.rrule.rrulestr(rrulestr, forceset=True) + gen = orig_rruleset.xafter(REF_DT, count=200) + orig_occurrences = [i for i in gen] + + assert occurrences == orig_occurrences + + +def test_fast_forward_rrule_hours(): + ''' + Generate an rrule for each hour of the day + + Assert that the resulting fast forwarded date is included in the original rrule + occurrence list + ''' + rrulestr_prefix = 'DTSTART;TZID=America/New_York:20201118T200000 RRULE:FREQ=HOURLY;' + for interval in range(1, 24): + rrulestr = f"{rrulestr_prefix}INTERVAL={interval}" + rruleset = Schedule.rrulestr(rrulestr, ref_dt=REF_DT) + + gen = rruleset.xafter(REF_DT, count=200) + occurrences = [i for i in gen] + + orig_rruleset = dateutil.rrule.rrulestr(rrulestr, forceset=True) + gen = orig_rruleset.xafter(REF_DT, count=200) + orig_occurrences = [i for i in gen] + + assert occurrences == orig_occurrences + + +def test_multiple_rrules(): + ''' + Create an rruleset that contains multiple rrules and an exrule + rruleA: freq HOURLY interval 5, dtstart should be fast forwarded + rruleB: freq HOURLY interval 7, dtstart should be fast forwarded + rruleC: freq MONTHLY interval 1, dtstart should not be fast forwarded + exruleA: freq HOURLY interval 5, dtstart should be fast forwarded + ''' + rrulestr = '''DTSTART;TZID=America/New_York:20201118T200000 + RRULE:FREQ=HOURLY;INTERVAL=5 + RRULE:FREQ=HOURLY;INTERVAL=7 + RRULE:FREQ=MONTHLY + EXRULE:FREQ=HOURLY;INTERVAL=5;BYDAY=MO,TU,WE''' + rruleset = Schedule.rrulestr(rrulestr, ref_dt=REF_DT) + + rruleA, rruleB, rruleC = rruleset._rrule + exruleA = rruleset._exrule[0] + + # assert that each rrule has its own dtstart + assert rruleA._dtstart != rruleB._dtstart + assert rruleA._dtstart != rruleC._dtstart + + assert exruleA._dtstart == rruleA._dtstart + + # the new dtstart should be within INTERVAL amount of hours from REF_DT + assert (REF_DT - rruleA._dtstart) < datetime.timedelta(hours=6) + assert (REF_DT - rruleB._dtstart) < datetime.timedelta(hours=8) + assert (REF_DT - exruleA._dtstart) < datetime.timedelta(hours=6) + + # the freq=monthly rrule's dtstart should not have changed + dateutil_rruleset = dateutil.rrule.rrulestr(rrulestr, forceset=True) + assert rruleC._dtstart == dateutil_rruleset._rrule[2]._dtstart + + gen = rruleset.xafter(REF_DT, count=200) + occurrences = [i for i in gen] + + orig_rruleset = dateutil.rrule.rrulestr(rrulestr, forceset=True) + gen = orig_rruleset.xafter(REF_DT, count=200) + orig_occurrences = [i for i in gen] + + assert occurrences == orig_occurrences + + +def test_future_date_does_not_fast_forward(): + dtstart = now() + datetime.timedelta(days=30) + rrule = dateutil.rrule.rrule(freq=HOURLY, interval=7, dtstart=dtstart) + new_rrule = _fast_forward_rrule(rrule, ref_dt=REF_DT) + assert new_rrule == rrule + + +@pytest.mark.parametrize( + ('freq', 'interval'), + [ + pytest.param(MINUTELY, 15.5555, id="freq-MINUTELY-interval-15.5555"), + pytest.param(MONTHLY, 1, id="freq-MONTHLY-interval-1"), + ], +) +def test_does_not_fast_forward(freq, interval): + ''' + Assert a couple of rrules that should not be fast forwarded + ''' + dtstart = REF_DT - datetime.timedelta(days=30) + rrule = dateutil.rrule.rrule(freq=freq, interval=interval, dtstart=dtstart) + + assert rrule == _fast_forward_rrule(rrule, ref_dt=REF_DT)