search

vmware-tanzu / velero

Backup and migrate Kubernetes applications and their persistent volumes
https://velero.io
Apache License 2.0
8.4k stars 1.37k forks source link

Upgrade to robfig/cron/v3 to support time zone specification #7793

Open kaovilai opened 1 month ago

kaovilai commented 1 month ago

Breaking change (can be mitigated if needed in the future): v1 branch accepted an optional seconds field at the beginning of the cron spec. This is non-standard and has led to a lot of confusion. The new default parser conforms to the standard as described by the Cron wikipedia page.. It is unlikely that this affects us per https://github.com/vmware-tanzu/velero/pull/31

Other notes:

CRON_TZ is now the recommended way to specify the timezone of a single schedule, which is sanctioned by the specification. The legacy "TZ=" prefix will continue to be supported since it is unambiguous and easy to do so.

References: https://pkg.go.dev/github.com/robfig/cron/v3#readme-upgrading-to-v3-june-2019 Signed-off-by: Tiger Kaovilai tkaovila@redhat.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #7792

Please indicate you've done the following:

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 58.67%. Comparing base (27392d3) to head (160a2e1). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #7793 +/- ## ======================================= Coverage 58.66% 58.67% ======================================= Files 345 345 Lines 28733 28739 +6 ======================================= + Hits 16856 16862 +6 Misses 10448 10448 Partials 1429 1429 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

blackpiglet commented 1 month ago

Please check the failed cases in the CI action.

kaovilai commented 1 month ago

It's not failing locally...

kaovilai commented 1 month ago

@blackpiglet looks like it was a flaky test

blackpiglet commented 1 month ago

Thanks. Is this PR target for the v1.14.0 RC? If not, how about revisiting this PR after the release-1.14 branch cut?

kaovilai commented 1 month ago

Is this PR target for the v1.14.0 RC? If not, how about revisiting this PR after the release-1.14 branch cut?

Sounds good.

kaovilai commented 1 month ago

Not 1.14 target

reasonerjt commented 1 month ago

Thanks @kaovilai It would be great to make update to the doc to add an example for the more advanced format: https://velero.io/docs/v1.14/backup-reference/#schedule-a-backup

But this is optional since it's briefly covered in the Wikipedia link