Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
soltysh committed Jan 31, 2022
1 parent b64b3bf commit 8ce2edf
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 8 deletions.
19 changes: 12 additions & 7 deletions keps/sig-apps/3140-TimeZone-support-in-CronJob/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fully GA-ed it's time to satisfy the original request.

### Goals

- Add the field `.spec.timeZone` which allows specifying a valid TimeZone
- Add the field `.spec.timeZone` which allows specifying a valid TimeZone name

### Non-Goals

Expand All @@ -95,7 +95,7 @@ and make progress.

Add the field `.spec.timeZone` to the CronJob resource. The cronjob controller
will take the field into account when scheduling the next Job run. In case the
field is not specified or is empty the controller will maitain the current
field is not specified or is empty the controller will maintain the current
behavior which is to rely on the time zone of the kube-controller-manager
process.

Expand Down Expand Up @@ -125,20 +125,22 @@ CronJobs can be created per user.

### CronJob API

The `CronJobSpec` structure is expanded with new `TimeZone` field which allows
The `.spec` for a CronJob is expanded with new `timeZone` field which allows
specifying the name of the time zone to be used. Missing or empty value of the
field indicates the current behavior, which relies on the time zone of the
kube-controller-manager process.

In the API code, that looks like:

```golang

type CronJobSpec struct {

// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
Schedule string
// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
Schedule string

// Time zone for the above schedule
TimeZone *string
// Time zone for the above schedule
TimeZone *string

}
```
Expand Down Expand Up @@ -489,6 +491,9 @@ Why should this KEP _not_ be implemented?

## Alternatives

Another approach was to specify time zone as an offset to UTC, but using the
name instead seems more user friendly.

<!--
What other approaches did you consider, and why did you rule them out? These do
not need to be as detailed as the proposal, but should include enough
Expand Down
3 changes: 2 additions & 1 deletion keps/sig-apps/3140-TimeZone-support-in-CronJob/kep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ status: implementable
creation-date: 2022-01-14
reviewers:
- "@ravig"
- "@atiratree"
- "@iterion"
approvers:
- "@janetkuo"
Expand Down Expand Up @@ -41,4 +42,4 @@ disable-supported: true

# The following PRR answers are required at beta release
metrics:
- my_feature_metric
# - my_feature_metric

0 comments on commit 8ce2edf

Please sign in to comment.