sonic-buildimage
sonic-buildimage copied to clipboard
sonic-net
[NTP] 🐞 Fix config template to init default parameters
fixes #17906
Why I did it
To fix NTP config generation from the minigraph and save backward compatability
Work item tracking
- Microsoft ADO (number only):
How I did it
Align ntp.conf.j2 template to generate config out of empty NTP_SERVER DB configuration
How to verify it
Out of that NTP_SERVER configuration:
{
"10.210.25.32": {},
"10.75.202.2": {}
}
The next config in ntp.conf file should be produced:
server 10.210.25.32
restrict 10.210.25.32 kod limited nomodify notrap noquery nopeer
server 10.75.202.2
restrict 10.75.202.2 kod limited nomodify notrap noquery nopeer
Which release branch to backport (provide reason below if selected)
- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [ ] 202211
- [ ] 202305
- [x] 202311
Tested branch (Please provide the tested image version)
- [ ]
- [ ]
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (it is my cat Finn)
You mentioned "The NTP_SERVER configuration generated from the minigraph doesn't meet the new schema requirements" in the issue. To solve the problem, it is better to fix the schema (https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-ntp.yang) instead of fixing the config generation behavior. The behavior is old from 2017. Any new schema design should be backward-compatible. #Closed
Please check db_migrator, https://github.com/sonic-net/sonic-utilities/blob/master/scripts/db_migrator.py Do we need to update db_migrator to migrate old ntp config to new schema?
You mentioned "The NTP_SERVER configuration generated from the minigraph doesn't meet the new schema requirements" in the issue. To solve the problem, it is better to fix the schema (https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-ntp.yang) instead of fixing the config generation behavior. The behavior is old from 2017. Any new schema design should be backward-compatible.
Minigraph doesn't use YANG to generate config, because there are default values in YANG. Anyway I already changed ntp.conf.j2 template according to your request, so it will preserve backward compatibility
Please check db_migrator, https://github.com/sonic-net/sonic-utilities/blob/master/scripts/db_migrator.py Do we need to update db_migrator to migrate old ntp config to new schema?
No need according to changes @qiluo-msft requested
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
The PR checker was failed because ntpsec does not support nopeer and notrap flags anymore: https://docs.ntpsec.org/latest/accopt.html
Fixed it by removing nopeer and notrap flags.
How do we configure some of the config params like config.iburst, config.resolve_as, etc. for NTP server used in this j2 file via sonic-mgmt? Is there any document to refer to?
@oleksandrivantsiv @dgsudharsan could you please help to review this fix?
How do we configure some of the config params like config.iburst, config.resolve_as, etc. for NTP server used in this j2 file via sonic-mgmt? Is there any document to refer to?
config.iburst only trough the DB. So far only server can be added via SONiC CLI. config.resolve_as is internal option for a NTP server and shouldn't be configured directly. It used to save resolved address, because when you configure time.google.com for example it changes address every time. So now config.resolve_as is configured via SONiC CLI when new NTP server is added.
@oleksandrivantsiv @dgsudharsan could you please help to review this fix?
@oleksandrivantsiv @dgsudharsan : checkers are passed. All comments handled, please approve if looks good