Skip to content

Commit

Permalink
Use UTC as default timezone when schedule Actions cron tasks (#31742)
Browse files Browse the repository at this point in the history
Fix #31657.

According to the
[doc](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onschedule)
of GitHub Actions, The timezone for cron should be UTC, not the local
timezone. And Gitea Actions doesn't have any reasons to change this, so
I think it's a bug.

However, Gitea Actions has extended the syntax, as it supports
descriptors like `@weekly` and `@every 5m`, and supports specifying the
timezone like `TZ=UTC 0 10 * * *`. So we can make it use UTC only when
the timezone is not specified, to be compatible with GitHub Actions, and
also respect the user's specified.

It does break the feature because the times to run tasks would be
changed, and it may confuse users. So I don't think we should backport
this.

## ⚠️ BREAKING ⚠️

If the server's local time zone is not UTC, a scheduled task would run
at a different time after upgrading Gitea to this version.
  • Loading branch information
wolfogre authored Aug 1, 2024
1 parent 333c9ed commit 21a73ae
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 12 deletions.
20 changes: 9 additions & 11 deletions models/actions/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/timeutil"
webhook_module "code.gitea.io/gitea/modules/webhook"

"github.com/robfig/cron/v3"
)

// ActionSchedule represents a schedule of a workflow file
Expand Down Expand Up @@ -53,8 +51,6 @@ func GetReposMapByIDs(ctx context.Context, ids []int64) (map[int64]*repo_model.R
return repos, db.GetEngine(ctx).In("id", ids).Find(&repos)
}

var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)

// CreateScheduleTask creates new schedule task.
func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error {
// Return early if there are no rows to insert
Expand All @@ -80,19 +76,21 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error {
now := time.Now()

for _, spec := range row.Specs {
specRow := &ActionScheduleSpec{
RepoID: row.RepoID,
ScheduleID: row.ID,
Spec: spec,
}
// Parse the spec and check for errors
schedule, err := cronParser.Parse(spec)
schedule, err := specRow.Parse()
if err != nil {
continue // skip to the next spec if there's an error
}

specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix())

// Insert the new schedule spec row
if err = db.Insert(ctx, &ActionScheduleSpec{
RepoID: row.RepoID,
ScheduleID: row.ID,
Spec: spec,
Next: timeutil.TimeStamp(schedule.Next(now).Unix()),
}); err != nil {
if err = db.Insert(ctx, specRow); err != nil {
return err
}
}
Expand Down
25 changes: 24 additions & 1 deletion models/actions/schedule_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package actions

import (
"context"
"strings"
"time"

"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
Expand Down Expand Up @@ -32,8 +34,29 @@ type ActionScheduleSpec struct {
Updated timeutil.TimeStamp `xorm:"updated"`
}

// Parse parses the spec and returns a cron.Schedule
// Unlike the default cron parser, Parse uses UTC timezone as the default if none is specified.
func (s *ActionScheduleSpec) Parse() (cron.Schedule, error) {
return cronParser.Parse(s.Spec)
parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
schedule, err := parser.Parse(s.Spec)
if err != nil {
return nil, err
}

// If the spec has specified a timezone, use it
if strings.HasPrefix(s.Spec, "TZ=") || strings.HasPrefix(s.Spec, "CRON_TZ=") {
return schedule, nil
}

specSchedule, ok := schedule.(*cron.SpecSchedule)
// If it's not a spec schedule, like "@every 5m", timezone is not relevant
if !ok {
return schedule, nil
}

// Set the timezone to UTC
specSchedule.Location = time.UTC
return specSchedule, nil
}

func init() {
Expand Down
71 changes: 71 additions & 0 deletions models/actions/schedule_spec_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package actions

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestActionScheduleSpec_Parse(t *testing.T) {
// Mock the local timezone is not UTC
local := time.Local
tz, err := time.LoadLocation("Asia/Shanghai")
require.NoError(t, err)
defer func() {
time.Local = local
}()
time.Local = tz

now, err := time.Parse(time.RFC3339, "2024-07-31T15:47:55+08:00")
require.NoError(t, err)

tests := []struct {
name string
spec string
want string
wantErr assert.ErrorAssertionFunc
}{
{
name: "regular",
spec: "0 10 * * *",
want: "2024-07-31T10:00:00Z",
wantErr: assert.NoError,
},
{
name: "invalid",
spec: "0 10 * *",
want: "",
wantErr: assert.Error,
},
{
name: "with timezone",
spec: "TZ=America/New_York 0 10 * * *",
want: "2024-07-31T14:00:00Z",
wantErr: assert.NoError,
},
{
name: "timezone irrelevant",
spec: "@every 5m",
want: "2024-07-31T07:52:55Z",
wantErr: assert.NoError,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := &ActionScheduleSpec{
Spec: tt.spec,
}
got, err := s.Parse()
tt.wantErr(t, err)

if err == nil {
assert.Equal(t, tt.want, got.Next(now).UTC().Format(time.RFC3339))
}
})
}
}

0 comments on commit 21a73ae

Please sign in to comment.