enhancements icon indicating copy to clipboard operation
enhancements copied to clipboard

KEP 3140: Promote CronJob's TimeZone support to GA

Open soltysh opened this issue 3 years ago • 7 comments

  • One-line PR description: Promote CronJob's TimeZone support to GA

  • Issue link: https://github.com/kubernetes/enhancements/issues/3140

  • Other comments: none

/assign @smarterclayton

soltysh avatar Aug 01 '22 15:08 soltysh

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Dec 28 '22 15:12 k8s-triage-robot

@soltysh - are you planning to get back to it?

wojtek-t avatar Jan 02 '23 08:01 wojtek-t

@soltysh - are you planning to get back to it?

Yes, I'm hoping to get back to it while updating the KEP for GA.

soltysh avatar Jan 03 '23 14:01 soltysh

This is ready for final reviews /assign @janetkuo

soltysh avatar Jan 18 '23 10:01 soltysh

/assign @atiratree

soltysh avatar Jan 18 '23 10:01 soltysh

Oh, and (this is an aside): I want to check that right now there's not an admission-time check that a timezone is valid / a Warning: if it's not?

Just a question, because I don't think we'd need a KEP to add that mechanism - if we ever wanted to. It's also tricky because it'd be annoying to duplicate the logic into the API server.

sftim avatar Jan 18 '23 10:01 sftim

Oh, and (this is an aside): I want to check that right now there's not an admission-time check that a timezone is valid / a Warning: if it's not?

Just a question, because I don't think we'd need a KEP to add that mechanism - if we ever wanted to. It's also tricky because it'd be annoying to duplicate the logic into the API server.

No, we don't have any admission, the only warning we currently have is about using the unsupported TZ or CRON_TZ env variable in .spec.schedule, see here and here. I don't think we want to add such admission because as you said it might be considered annoying by some, but anyone is free to make one, also we have reasonably strict validation in place, but that only applies on create and update to CronJob resource.

soltysh avatar Jan 20 '23 12:01 soltysh

/assign @deads2k for PPR

soltysh avatar Jan 20 '23 12:01 soltysh

this comment is not a blocker for graduation!

I don't think we want to add such admission because as you said it might be considered annoying by some, but anyone is free to make one, also we have reasonably strict validation in place, but that only applies on create and update to CronJob resource.

How about, optionally, we document a ValidatingAdmissionPolicy that rejects setting a timezone the wrong way? For example, mention it in the post-release blog article. We can do that even if ValidatingAdmissionPolicy stays alpha…

I'd like to be able to explain to end uses how to make sure that your cluster only uses the newly-GA timezone support.

sftim avatar Jan 21 '23 22:01 sftim

I'd like to be able to explain to end uses how to make sure that your cluster only uses the newly-GA timezone support.

That's a very good idea, I'll try to make sure we have something in the blog post that I'll try to put together for this feature. I'd prefer not to put in the official docs, though.

soltysh avatar Feb 02 '23 11:02 soltysh

/cc @deads2k

logicalhan avatar Feb 02 '23 16:02 logicalhan

How stable are the metrics you are using to verify that this feature works?

cronjob_controller_rate_limiter_use and cron_job_creation_skew are they stable enough to be promoted in stability level?

This is a good question and deserves an answer. We haven't been consistent (yet) about pressing on metric stability, but I think its a good idea for us to tighten in the future.

/hold to get an answer for metrics

PRR looks good

/approve

deads2k avatar Feb 02 '23 21:02 deads2k

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, janetkuo, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Feb 02 '23 21:02 k8s-ci-robot

cronjob_controller_rate_limiter_use and cron_job_creation_skew are they stable enough to be promoted in stability level?

cron_job_creation_skew I've noticed it wasn't promoted to stable and did that in https://github.com/kubernetes/kubernetes/pull/113008 the other one is coming from component-base and I see majority of them are still alpha, so that'll be either sig-instrumentation or sig-arch call to push them to stable, which I'm happy to drive forward.

/hold cancel

soltysh avatar Feb 03 '23 14:02 soltysh