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.

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

kube_cronjob_next_schedule_time has time in the past #500

Closed
balous opened this issue Jul 24, 2018 · 19 comments
Closed

kube_cronjob_next_schedule_time has time in the past #500

balous opened this issue Jul 24, 2018 · 19 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@balous
Copy link

balous commented Jul 24, 2018

Is this a BUG REPORT or FEATURE REQUEST?:

Uncomment only one, leave it on its own line:

/kind bug

/kind feature

What happened:
I've got a cron job with:

  • lastScheduleTime: 2018-07-24T00:00:00Z
  • schedule: 0 2 * * *
  • creationTimestamp: 2018-06-06T13:55:15Z

For this cronjob, current value of kube_cronjob_next_schedule_time metric is 1532397600 (now it is 1532433916, so it is in the past). When formated for my timezone (+0200), the time is 2018-07-24 02:00:00 which is equal to the time the job was run tonight.

What you expected to happen:
kube_cronjob_next_schedule_time should contain time in the future (i.e. 2018-07-25 02:00:00).

How to reproduce it (as minimally and precisely as possible):
It happens to all my cron jobs so I suppose it is enough to create a cronjob and check kube_cronjob_next_schedule_time's value.

Anything else we need to know?:
I've also played with another job, which was created 47 days ago and was suspended since the beginning. I unsuspended it, checked the metric and got 47.75 days in the past.

Also note that the cluster's hosts timezone is set to CEST.

Environment:

  • Kubernetes version (use kubectl version): N/A
  • Kube-state-metrics image version 1.3.1, 1.2.0 and 1.1.0 (not tested the earlier versions)
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 24, 2018
@brancz
Copy link
Member

brancz commented Jul 24, 2018

kube_cronjob_next_schedule_time is calculated based on the last schedule time and the crontab:

func getNextScheduledTime(schedule string, lastScheduleTime *metav1.Time, createdTime metav1.Time) (time.Time, error) {
sched, err := cron.ParseStandard(schedule)
if err != nil {
return time.Time{}, fmt.Errorf("Failed to parse cron job schedule '%s': %s", schedule, err)
}
if !lastScheduleTime.IsZero() {
return sched.Next((*lastScheduleTime).Time), nil
}
if !createdTime.IsZero() {
return sched.Next(createdTime.Time), nil
}
return time.Time{}, fmt.Errorf("Created time and lastScheduleTime are both zero")
}

This looks to me like a timezone normalization problem. This is tricky as Prometheus only handles unix timestamps and next schedule times need to be calculated based on the actual timezone. To me the Next function of the Schedule seems fine though, but it leaves the question whether kube-state-metrics maybe thinks it's in a different timezone?

Also note that the cluster's hosts timezone is set to CEST.

Any idea, how would a container might know about this?

@balous
Copy link
Author

balous commented Jul 25, 2018

I poked around a bit and it seems that K8S API is reporting timestamps with UTC timezone designator (the Z at the end).

@brancz
Copy link
Member

brancz commented Jul 25, 2018

Interesting, so you are saying, that the Kubernetes API is not respecting your timezone settings, and as a result kube-state-metrics is using the same "wrong" timezone. That seems plausible, we should open a new issue on Kubernetes in that case.

@balous
Copy link
Author

balous commented Jul 25, 2018

I think that kubernetes API works correctly - I don't need the API to keep the timezone, I just need that the status contains correct timestamp.

I.e. the job is specified to run at 2AM and the server is in CEST. The job have been started at 2AM CEST. Just the API shows 0AM UTC, which represents the same point in time so there is no problem in it.

@brancz
Copy link
Member

brancz commented Jul 25, 2018

That would mean we need to teach kube-state-metrics the timezone that the cronjob is working off of, which would either mean we need to read it from somewhere (if that information would actually be correct, I don't know) or allow it to be passed as a flag. What do you think?

@balous
Copy link
Author

balous commented Jul 25, 2018

Yes, I was thinking about it and got to the same conclusion.

@balous
Copy link
Author

balous commented Jul 25, 2018

But this is not all - I am not getting the next schedule time shifted by my timezone offset. The original problem was that I am getting something that looks like last run time.

@brancz
Copy link
Member

brancz commented Jul 25, 2018

I think that's because next schedule time is based on the UTC time, and kube-state-metrics thinks it's in UTC. So if we make sure kube-state-metrics parses the timestamp as UTC but calculates the next time with the configured timezone then we should get the right unix timestamp (I think .. timezone stuff is confusing, we should write proper unit tests for this).

@balous
Copy link
Author

balous commented Jul 26, 2018

So I've tried to set timezone in the container:

  • setting TZ environment variable to 'Europe/Prague' has no effect

  • however, kube-state-metrics reacts to /etc/timezone file. Having this file with the proper content in the container fixed my problem

So now I think this is definitely timezone problem. If we can set the kube-state-metrics's timezone to the same zone that is used by the cluster, the problem is fixed.

@brancz
Copy link
Member

brancz commented Jul 26, 2018

I'm not sure setting the timezone to the cluster's timezone is possible unless you do something rather hacky, like mount the hosts /etc/timezone file into the container with a hostpath. I still feel like the flag to set the timezone is the best option.

@balous
Copy link
Author

balous commented Jul 26, 2018

True, /etc/timezone is not the way. But consider using TZ variable instead of adding a new CLI flag for this - it is pretty standard, many apps (i.e. date) obey it.

@brancz
Copy link
Member

brancz commented Jul 26, 2018

sure I think we can do both and have the flag default to the value of the TZ environment variable

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 24, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 23, 2018
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

@zerda
Copy link

zerda commented Oct 14, 2019

Mapping /etc/localtime in host to container worked, but environment TZ=Asia/Shanghai not.

We still need a better way (maybe TZ env support ?) to specify timezone with kube-state-metrics.

@fzyzcjy
Copy link

fzyzcjy commented Apr 8, 2022

Facing the same problem. Is there any updates?

@fzyzcjy
Copy link

fzyzcjy commented Apr 8, 2022

@zerda Hi how did you map the /etc/localtime? I tried the following but with errors:

...
      containers:
      - name: {{ .Chart.Name }}
        volumeMounts:
        - name: host-etc
          mountPath: /etc/localtime
          subPath: localtime
          readOnly: true
      volumes:
        - name: host-etc
          hostPath:
            path: /etc
Error: failed to prepare subPath for volumeMount "host-etc" of container "kube-state-metrics"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

6 participants