-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Timezone sync issue between the host and containers #14000
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,6 @@ | ||
#!/usr/bin/env bash | ||
|
||
TZ=$(cat /etc/timezone) | ||
rm -rf /etc/localtime | ||
ln -sf /usr/share/zoneinfo/$TZ /etc/localtime | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is empty. Do you want to add
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah. I missed it. Thanks for pointing it out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the file. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,17 @@ stdout_logfile=syslog | |
stderr_logfile=syslog | ||
dependent_startup=true | ||
|
||
[program:start] | ||
command=/usr/bin/start.sh | ||
priority=2 | ||
autostart=false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I am wrong but I guess it is not required as the dependent_startup flag is true. So it will eventually gets started post dependent-startup. Also, I have referred other supervisord.conf files and found that it is most of the times false only. |
||
autorestart=false | ||
startsecs=0 | ||
stdout_logfile=syslog | ||
stderr_logfile=syslog | ||
dependent_startup=true | ||
dependent_startup_wait_for=rsyslogd:running | ||
|
||
[program:sflowmgrd] | ||
command=/usr/bin/sflowmgrd | ||
priority=2 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script is only run once when container starting. So the motivation "the auto time / zone sync between the container(s) and host" is not fully achieved. If after container staring, user change timezone on the host, it will not sync to the container. #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we consider an actual deployment scenario, timezone is configured only once before going to the Production. It is not the config which keeps getting changed again and again. But still, in case network admin wants to change the timezone post deployment, it can still be synced across the containers by triggering the config-reload command post changing the timezone on the host.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not agree "timezone is configured only once before going to the Production". The feature name 'Time sync' seems not accurate. You only propagate it when container starting, not sync after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the initial timezone sync has been completed between the host and container (during docker-init), later there will not any mismatch until the timezone on the host has been changed (Which is sort of periodic sync only). And to tackle that user needs to do config-reload to propagate updated timezone inside the containers.
Can you please tell me the actual used case where we need to change the timezone frequently and config reload / container restart is not allowed?
In my opinion, user will at least get some sort of clean mechanism to sync the timezone between the host and containers with this approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always use UTC timezone so we do not have such use case. You raised the original issue #13046. In order to fix your use case, change timezone inside FRR container is good enough. For a timezone fix, there is no need to disruptive dataplane traffic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I was focusing only on the FRR but later I found that this feature is required for other containers too. For instance, sairedis logs from swss where the timezone is synced correctly with the swss container and the host.
root@sonic:/var/log/swss# tail -f sairedis.rec
2023-06-14.21:23:20.593741|s|SAI_OBJECT_TYPE_PORT:oid:0x1000000000133|SAI_PORT_ATTR_QOS_DSCP_TO_TC_MAP=oid:0x140000000003d3
2023-06-14.21:23:20.594524|s|SAI_OBJECT_TYPE_PORT:oid:0x1000000000133|SAI_PORT_ATTR_QOS_TC_TO_QUEUE_MAP=oid:0x140000000003d5
2023-06-14.21:23:20.595066|s|SAI_OBJECT_TYPE_PORT:oid:0x1000000000133|SAI_PORT_ATTR_QOS_TC_TO_PRIORITY_GROUP_MAP=oid:0x140000000003d4
2023-06-14.21:23:20.595703|s|SAI_OBJECT_TYPE_PORT:oid:0x1000000000133|SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL=24
2023-06-14.21:24:16.185792|c|SAI_OBJECT_TYPE_HOSTIF_TRAP_GROUP:oid:0x110000000003d7|SAI_HOSTIF_TRAP_GROUP_ATTR_QUEUE=2
2023-06-14.21:24:16.187074|c|SAI_OBJECT_TYPE_POLICER:oid:0x120000000003d8|SAI_POLICER_ATTR_CBS=1000|SAI_POLICER_ATTR_CIR=1000|SAI_POLICER_ATTR_METER_TYPE=SAI_METER_TYPE_PACKETS|SAI_POLICER_ATTR_MODE=SAI_POLICER_MODE_SR_TCM|SAI_POLICER_ATTR_RED_PACKET_ACTION=SAI_PACKET_ACTION_DROP
2023-06-14.21:24:16.187953|s|SAI_OBJECT_TYPE_HOSTIF_TRAP_GROUP:oid:0x110000000003d7|SAI_HOSTIF_TRAP_GROUP_ATTR_POLICER=oid:0x120000000003d8
2023-06-14.21:24:16.191987|c|SAI_OBJECT_TYPE_HOSTIF:oid:0xd0000000003d9|SAI_HOSTIF_ATTR_GENETLINK_MCGRP_NAME=packets|SAI_HOSTIF_ATTR_TYPE=SAI_HOSTIF_TYPE_GENETLINK|SAI_HOSTIF_ATTR_NAME=psample
2023-06-14.21:24:16.193059|c|SAI_OBJECT_TYPE_HOSTIF_TRAP:oid:0x220000000003da|SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE=SAI_HOSTIF_TRAP_TYPE_SAMPLEPACKET|SAI_HOSTIF_TRAP_ATTR_TRAP_GROUP=oid:0x110000000003d7|SAI_HOSTIF_TRAP_ATTR_PACKET_ACTION=SAI_PACKET_ACTION_TRAP|SAI_HOSTIF_TRAP_ATTR_TRAP_PRIORITY=1
2023-06-14.21:24:16.200215|c|SAI_OBJECT_TYPE_HOSTIF_TABLE_ENTRY:oid:0x230000000003db|SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE=SAI_HOSTIF_TABLE_ENTRY_TYPE_TRAP_ID|SAI_HOSTIF_TABLE_ENTRY_ATTR_TRAP_ID=oid:0x220000000003da|SAI_HOSTIF_TABLE_ENTRY_ATTR_CHANNEL_TYPE=SAI_HOSTIF_TABLE_ENTRY_CHANNEL_TYPE_GENETLINK|SAI_HOSTIF_TABLE_ENTRY_ATTR_HOST_IF=oid:0xd0000000003d9
root@sonic:/var/log/swss# date
Wed Jun 14 21:25:40 PDT 2023
Thus, this feature will fix #13046 and other similar issues too.
BTW, this feature will be optional only. It is not going to harm or disrupt any exiting operation. If someone wants to sync the timezone across the containers, he / she can use it else others can live with the UTC timezone only. There is no force (for the ones who doesn't want to change the timezone) to disrupt the dataplane traffic and the ones who wants to change it, can plan it accordingly as per their deployment strategies. There are several customers, who prefers to have the timezone set as per their geographical location. Thus, in my opinion, it is a good to have feature.
To answer to your previous comment "The feature name 'Time sync' seems not accurate. You only propagate it when container starting, not sync after", we can certainly achieve the periodic sync by simply adding a script per container which will periodically check for the change of TZ on the host but it will be an overhead just for syncing the timezone. Thus, I decided to sync it only at the time of docker init as usually that config is not being updated frequently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you explore the solution of
The benefit is centralized code change. And it behaves the same way only running once when container starting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it works if we already knew the TIMEZONE. For instance, we can set ENV in Dockerfile to pre-set the TZ. But how are we going to make the TZ as variable field in SONIC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I realize above 2 commands only work during creating the container, not during container restart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah :-)