Correct documentation regarding scheduling

GitarPlayer commented 7 months ago

Please confirm the following

[X] I agree to follow this project's code of conduct.
[X] I have checked the current issues for duplicates.
[X] I understand that AWX is open source software provided for free and that I might not receive a timely response.
[X] I am NOT reporting a (potential) security vulnerability. (These should be emailed to security@ansible.com instead.)

Bug Summary

The documentation states the following:

Jobs are scheduled in UTC. Repeating jobs that run at a specific time of day may move relative to a local timezone when Daylight Savings Time shifts occur. The system resolves the local time zone based time to UTC when the schedule is saved. To ensure your schedules are correctly set, you should set your schedules in UTC time.

But I think this is misleading because the scheduler actually follows DST transitions. What happens according to my understanding:

schedules are represented as dateutil.rrule objects https://github.com/ansible/awx/blob/devel/awx/main/models/schedules.py#L203C13-L203C27
in update_computed_fields_no_save the next occurence of the rrule string is calculated after UTC now(). https://github.com/ansible/awx/blob/devel/awx/main/models/schedules.py#L276
jobs are scheduled according to next_run UTC time and the scheduler's UTC time:https://github.com/ansible/awx/blob/devel/awx/main/dispatch/periodic.py

Since dateutil.rrule is DST aware, jobs scheduled in local time zones should not move relative to a local timezone but relative to UTC time.

Example:

#rrule.py
from dateutil.rrule import rrule, MONTHLY
from dateutil.tz import gettz
from datetime import datetime

# Define the timezone
tz = gettz('America/Chicago')

# Define the start date
start_date = datetime(2023, 1, 1, 9, 0, tzinfo=tz)

# Define the rrule for monthly occurrences at 9 AM Central US time
rule = rrule(freq=MONTHLY, dtstart=start_date, count=12)

# Calculate all occurrences and convert them to UTC
occurrences_utc = [(dt, dt.astimezone(datetime.now().astimezone().tzinfo)) for dt in rule]

for occurence in occurrences_utc:
    print(occurence)

python3 rrule.py:

(datetime.datetime(2023, 1, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 1, 1, 17, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 2, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 2, 1, 17, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 3, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 3, 1, 17, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 4, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 4, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 5, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 5, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 6, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 6, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 7, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 7, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 8, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 8, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 9, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 9, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 10, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 10, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 11, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 11, 1, 16, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))
(datetime.datetime(2023, 12, 1, 9, 0, tzinfo=tzfile('/usr/share/zoneinfo/America/Chicago')), datetime.datetime(2023, 12, 1, 17, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=7200), 'CEST')))

I also scheduled such a job with Ansible Automation Hub 4.5.2 to confirm that it honors DST transitions and it did. We wanted to use AAP for business logic scheduling and in this use case it was important for us that the job run precisely at the same time in local time throughout the year.

AWX version

2.4 Ansible Automation Platform

Select the relevant components

[ ] UI
[ ] UI (tech preview)
[ ] API
[X] Docs
[ ] Collection
[ ] CLI
[ ] Other

Installation method

openshift

Modifications

no

Ansible version

2.15

Operating system

No response

Web browser

No response

Steps to reproduce

Enter a monthly schedule that runs in a local timezone
Wait for DST transition in that timezone

Expected results

According to the documentation the job should shift relative to the local time zone.

Actual results

The job shifts relative to UTC time zone after the DST transition. But it runs correctly in the local time zone throughout the year.

Additional information

No response

fosterseth commented 7 months ago

great observations.

@jbradberry you may curious at taking a look at the rationale provided above

@tvo318 maybe we just drop that statement from the docs, as it doesn't seem applicable anymore

i see we have some coverage around DST https://github.com/fosterseth/awx/blob/0f150aa3b3ebddf3b4898177bbf96ba5bde84261/awx/main/tests/functional/api/test_schedules.py#L433

jbradberry commented 6 months ago

I think the issue is more complex than the docs make it out to be. I do know that the rrule string stored in the database can include timezone information, so it seems to me that @GitarPlayer is probably more or less correct.

ansible / awx