hydrophone
hydrophone copied to clipboard
Fix different timezones in logs
What's the issue?
The logs of hydrophone show time in different time zones. One seems to be local time and other in UTC.
How to reproduce?
bin/hydrophone --conformance
We should be able to fix this and I don't think it will be a difficult resolution. By default the conformance image is going to use the tzinfo from the host it's running on. Typically that's going to be UTC for most cloud providers (including KIND).
We can override that by setting the TZ environment variable on the conformance pod when it's created. The slightly more tricky part will be setting the value of that variable. I don't really want timezone to be a flag folks can set. Instead we can use the time package to get the local tzinfo like:
tz, offset := time.Now().Zone()
We'll need to plumb the return value of tz into the environment variable TZ on the conformance image. But that should fix it. @letmerecall are you interested in trying to code this up?
/assign
Hey @letmerecall how are you doing on this? Are you still interested in trying to impliment a fix?
Hey @rjsadow sorry, and yes I'll find some time and try it this weekend.
We should be able to fix this and I don't think it will be a difficult resolution. By default the conformance image is going to use the tzinfo from the host it's running on. Typically that's going to be UTC for most cloud providers (including KIND).
We can override that by setting the
TZenvironment variable on the conformance pod when it's created. The slightly more tricky part will be setting the value of that variable. I don't really want timezone to be a flag folks can set. Instead we can use thetimepackage to get the local tzinfo like:tz, offset := time.Now().Zone()We'll need to plumb the return value of tz into the environment variable TZ on the conformance image. But that should fix it. @letmerecall are you interested in trying to code this up?
@rjsadow Well, it's a bit more complicated than just tz, offset := time.Now().Zone(). time.Now().Zone() returns an abbreviation (like IST), meanwhile TZ environment variable in conformance image expect a full string (like Asia/Kolkata).
Also, we might need different logic for difference OSes 🤦🏻♂️ (something like this https://stackoverflow.com/questions/68938751/how-to-get-the-full-name-of-the-local-timezone)
Hey @rjsadow If no one is working on it then I can work on this one just I some more details
@Bharadwajshivam28 feel free to give it a shot. Please assign it to yourself when you start working on it.
@Bharadwajshivam28 feel free to give it a shot. Please assign it to yourself when you start working on it.
Thanks....
/assign
meanwhile TZ environment variable in conformance image expect a full string (like Asia/Kolkata).
Are you sure? The image is based on Debian and https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html mentions that format like "utc+HH" would be valid.
However I think there's no tzdata in the image at all:
$ docker run -e "TZ=UTC" --rm --entrypoint=sh registry.k8s.io/conformance:v1.29.1 -c date
Sun Apr 7 18:31:35 UTC 2024
$ docker run -e "TZ=Asia/Kolkata" --rm --entrypoint=sh registry.k8s.io/conformance:v1.29.1 -c date
Sun Apr 7 18:31:29 Asia 2024
The name changes, but the actual times do not. Which would be super confusing.
Setting the offset does work however, but it's equally confusing (my timezone is Europe/Berlin, which is UTC+2, but TZ would require the opposite sign (the correct output would be 20:31 right now):
$ docker run -e "TZ=UTC+2" --rm --entrypoint=sh registry.k8s.io/conformance:v1.29.1 -c date
Sun Apr 7 16:31:35 UTC 2024
Setting this does not affect the go-runner (times should be 20:34), neither does setting TZ=Europe/Berlin:
$ docker run -e "TZ=UTC-02" --rm registry.k8s.io/conformance:v1.29.1
2024/04/07 18:34:37 The resultsDir /tmp/results does not exist, will create it
2024/04/07 18:34:37 Running command:
Command env: []
The Kubernetes project currently lacks enough contributors to adequately respond to all issues.
This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue as fresh with
/remove-lifecycle stale - Close this issue with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.
This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue as fresh with
/remove-lifecycle rotten - Close this issue with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
/remove-lifecycle rotten
The Kubernetes project currently lacks enough contributors to adequately respond to all issues.
This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue as fresh with
/remove-lifecycle stale - Close this issue with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
Are you sure? The image is based on Debian and https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html mentions that format like "utc+HH" would be valid.
However I think there's no tzdata in the image at all:
Thanks for looking into it, and you are correct!
Conformance image seems to use registry.k8s.io/build-image/debian-base-arm64:bookworm-v1.0.4 base which does not include tzdata.
However, even with tzdata some abbreviations are not supported, for example IST is not supported but CET is. This is what I get from latest debian image:
$ docker run -e TZ="UTC" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 11:59:22 UTC 2024
$ docker run -e TZ="Asia/Kolkata" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 12:01:09 IST 2024
$ docker run -e TZ="IST" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 06:31:18 IST 2024
$ docker run -e TZ="CET" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 07:31:29 CET 2024
$ docker run -e TZ="Europe/Berlin" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 07:31:50 CET 2024
Also, should we think about adding tzdata to conformance image here? 🤔 https://github.com/kubernetes/kubernetes/blob/master/test/conformance/image/Dockerfile
How about we just make the hydrophone CLI log in UTC?
the conformance results should always be in UTC currently
/remove-lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all issues.
This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue as fresh with
/remove-lifecycle stale - Close this issue with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale
Given conformance is the original use case for SIG testing to sponsor this project, I really think we should just use UTC locally
It's also less fragile than depending on the conformance image having updated tzdata (it currently has no tzdata)
By default the conformance image is going to use the tzinfo from the host it's running on. Typically that's going to be UTC for most cloud providers (including KIND).
Actually tzdata isn't something mounted into a container typically, it's from the container image files, and we don't install tzdata into the e2e image (I also think we shouldn't, it will be out of date anyhow and conformance results are in UTC)
@rjsadow thoughts on above?
I agree, if we can have all the logs be UTC then that answers the ask.
@BenTheElder can be closed.
/close Thanks! 🙌
@BenTheElder: Closing this issue.
In response to this:
/close Thanks! 🙌
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-sigs/prow repository.