Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kube_cronjob_next_schedule_time in the past when cronjob timezone specified #2206

Open
paulclark222 opened this issue Sep 26, 2023 · 12 comments · May be fixed by #2376
Open

kube_cronjob_next_schedule_time in the past when cronjob timezone specified #2206

paulclark222 opened this issue Sep 26, 2023 · 12 comments · May be fixed by #2376
Assignees
@mrueg @dgrisonnet
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@paulclark222
Copy link

What happened:
Defined a timezone for a cronjob:
timeZone: "Europe/London"
Now kube_cronjob_next_schedule_time is always returning the last schedule time rather than the next one.

What you expected to happen:
kube_cronjob_next_schedule_time should be the next scheduled time.

How to reproduce it (as minimally and precisely as possible):
Define a cronjob with non-UTC timezone (may only happen on zones east of UTC). Wait for the job to execute and then check kube_cronjob_next_schedule_time - you will see it's in the past.

  • kube-state-metrics version: 2.10
  • Kubernetes version (use kubectl version): 1.27
  • Cloud provider or hardware configuration: Azure AKS
@paulclark222 paulclark222 added the kind/bug Categorizes issue or PR as related to a bug. label Sep 26, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Sep 26, 2023
@dashpole
Copy link

dashpole commented Oct 5, 2023

/assign @dgrisonnet
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 5, 2023
@CatherineF-dev
Copy link
Contributor

The source code is here:

kube-state-metrics/internal/store/cronjob.go

Lines 238 to 263 in b91dd31

*generator.NewFamilyGeneratorWithStability(
"kube_cronjob_next_schedule_time",
"Next time the cronjob should be scheduled. The time after lastScheduleTime, or after the cron job's creation time if it's never been scheduled. Use this to determine if the job is delayed.",
metric.Gauge,
basemetrics.STABLE,
"",
wrapCronJobFunc(func(j *batchv1.CronJob) *metric.Family {
ms := []*metric.Metric{}
// If the cron job is suspended, don't track the next scheduled time
nextScheduledTime, err := getNextScheduledTime(j.Spec.Schedule, j.Status.LastScheduleTime, j.CreationTimestamp)
if err != nil {
panic(err)
} else if !*j.Spec.Suspend {
ms = append(ms, &metric.Metric{
LabelKeys: []string{},
LabelValues: []string{},
Value: float64(nextScheduledTime.Unix()),
})
}
return &metric.Family{
Metrics: ms,
}
}),
),

Could you dive into it and help debug if you are available?

@paulclark222
Copy link
Author

paulclark222 commented Oct 12, 2023 •
edited
Loading

I'm not a go developer, but I suspect that what's happening is that j.Spec.Schedule doesn't include the timezone, so the cron library is interpreting it in local time (UTC). So if this code were to run just after the cronjob had run (in UTC+1), it would say the next scheduled time was still 1 hour away.

The cronjob library expects the timezone to be specified like this:
schedule, err := cron.ParseStandard("TZ=America/Los_Angeles 0 2 * * 0")
or
loc, _ := time.LoadLocation("Europe/Paris") cronProcess := cron.New(cron.WithLocation(loc))

I'm not sure if this is being done.

@CatherineF-dev
Copy link
Contributor

Hi @paulclark222, j.Spec does have a field named timezone, which hasn't been used

https://github.com/kubernetes/api/blob/master/batch/v1/types.go#L637

@CatherineF-dev
Copy link
Contributor

Could you ramp up golang and try to fix this issue, if you are available and have enough time?

@dgrisonnet dgrisonnet added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 6, 2023
@dgrisonnet
Copy link
Member

The code for the metric predates the support for timezone, so it is not taken into account. We would need to had support for it in ksm.

/help

@k8s-ci-robot
Copy link
Contributor

@dgrisonnet:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

The code for the metric predates the support for timezone, so it is not taken into account. We would need to had support for it in ksm.

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Nov 6, 2023
@mhmtsvr
Copy link

mhmtsvr commented Feb 1, 2024

I encountered the same issue after configuring the timezone. Any recent updates or ETA on a resolution? Thanks!

@sekot
Copy link

sekot commented Feb 23, 2024

+1

@dehoej
Copy link

dehoej commented Apr 22, 2024

+1
Really an issue when you are trying to create alerts if cronjobs are running longer than expected.

@mrueg
Copy link
Member

mrueg commented Apr 22, 2024

/assign

@mrueg mrueg linked a pull request Apr 22, 2024 that will close this issue
Open
@mrueg
Copy link
Member

mrueg commented Apr 22, 2024

I have a fix in #2376

Can you test if this resolves your issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mrueg mrueg

@dgrisonnet dgrisonnet

Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
9 participants
@mrueg @sekot @dashpole @paulclark222 @k8s-ci-robot @dgrisonnet @mhmtsvr @CatherineF-dev @dehoej