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

Initial NTP HLD #1296

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Initial NTP HLD #1296

merged 2 commits into from
Jun 21, 2023

Conversation

fastiuk
Copy link
Contributor

@fastiuk fastiuk commented Mar 20, 2023 •
edited
Loading

SONiC Network Time Protocol HLD
It covers the next items:

  • Configuring NTP global parameters
  • Adding/removing new NTP servers
  • Change the configuration for NTP servers
  • Show NTP status
  • Show NTP configuration
Repo PR title State
sonic-buildimage [NTP] Add NTP extended configuration GitHub issue/pull request detail
sonic-host-services [NTP] Update NTP configuration via ConfigDB GitHub issue/pull request detail
sonic-swss-common [NTP] Add NTP configuration tables GitHub issue/pull request detail
sonic-utilities [NTP] Configure NTP according to extended configuration GitHub issue/pull request detail

@fastiuk fastiuk mentioned this pull request Mar 20, 2023
Closed
saravanan-i
saravanan-i reviewed Mar 21, 2023
View reviewed changes
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
@zhangyanzhao
Copy link
Collaborator

community review recording https://zoom.us/rec/share/8Dp-uE8wWl3pxp6Oku9jGIv711fZ0nX2A6Xf3V1nrYbNxS47cCne7kpsmWfAfGR4.G5wIanqyNmPj5xkD

@zhangyanzhao
Copy link
Collaborator

@fastiuk Please remember to update the Command Reference doc with your new command lines and also please complete the YANG model update before the PR is merged. Thanks.

@zhangyanzhao
Copy link
Collaborator

add @venkatmahalingam as reviewer per ask. Thanks.

@venkatmahalingam
Copy link
Collaborator

@bsun-sudo Please review this HLD.

bsun-sudo
bsun-sudo reviewed Mar 22, 2023
View reviewed changes
doc/ntp/ntp-design.md Show resolved Hide resolved
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
doc/ntp/ntp-design.md Show resolved Hide resolved
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
doc/ntp/ntp-design.md Show resolved Hide resolved
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
doc/ntp/ntp-design.md Show resolved Hide resolved
doc/ntp/ntp-design.md Show resolved Hide resolved
bsun-sudo
bsun-sudo reviewed Mar 23, 2023
View reviewed changes
doc/ntp/ntp-design.md Show resolved Hide resolved
doc/ntp/ntp-design.md Show resolved Hide resolved
doc/ntp/ntp-design.md Show resolved Hide resolved
@fastiuk fastiuk force-pushed the ntp_extended_configuration branch from 9a3e19c to ca8917d Compare May 7, 2023 00:29
@fastiuk
Copy link
Contributor Author

fastiuk commented May 7, 2023

Updated HLD based on comments that were left

@fastiuk fastiuk force-pushed the ntp_extended_configuration branch from ca8917d to a976175 Compare May 7, 2023 00:42
zhangyanzhao
zhangyanzhao requested changes May 8, 2023
View reviewed changes
Copy link
Collaborator

@zhangyanzhao zhangyanzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the code PRs

@zhangyanzhao
Copy link
Collaborator

@fastiuk can you please add the code PRs? Thanks.

This was referenced May 13, 2023
Merged
Merged
Merged
Merged
@fastiuk
Copy link
Contributor Author

fastiuk commented May 15, 2023

Added code PRs.
@venkatmahalingam / @bsun-sudo / @saravanan-i please review

@fastiuk fastiuk force-pushed the ntp_extended_configuration branch from a976175 to 9312102 Compare June 2, 2023 18:15
@qiluo-msft qiluo-msft requested a review from yxieca June 5, 2023 18:28
@yxieca yxieca requested a review from saiarcot895 June 12, 2023 23:31
@fastiuk fastiuk force-pushed the ntp_extended_configuration branch from 9312102 to 75a8e9c Compare June 13, 2023 13:37
@fastiuk
Initial NTP HLD
129102d 
Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>
@fastiuk fastiuk force-pushed the ntp_extended_configuration branch from 75a8e9c to 129102d Compare June 17, 2023 11:49
yxieca
yxieca reviewed Jun 19, 2023
View reviewed changes
doc/ntp/ntp-design.md Outdated Show resolved Hide resolved
@yxieca yxieca merged commit fde55e4 into sonic-net:master Jun 21, 2023
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Dec 11, 2023
@fastiuk
[NTP] Add NTP extended configuration (#15058)
5efb123 
hld [#1296](sonic-net/SONiC#1296)
closes [#1254](sonic-net/SONiC#1254)
depends-on [#60](sonic-net/sonic-host-services#60), [#781](sonic-net/sonic-swss-common#781), [#2835](sonic-net/sonic-utilities#2835), [#10749](sonic-net/sonic-mgmt#10749)

#### Why I did it
To cover the next AIs:
* Configure NTP global parameters
* Add/remove new NTP servers
* Change the configuration for NTP servers
* Show NTP status
* Show NTP configuration

### How I did it
* Add YANG model for a new configuration
* Extend configuration templates to support new knobs

### Description for the changelog
* Add ability to configure NTP global parameters such as authentication, dhcp, admin state
* Change the configuration for NTP servers
* Add an ability to show NTP configuration

#### Link to config_db schema for YANG module changes
[NTP configuration](https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md#ntp-and-syslog-servers)
This was referenced Dec 13, 2023
Merged
Closed
@skg-net
Copy link
Member

skg-net commented Feb 6, 2024

@fastiuk Can you please update the Quality Metric (Alpha/Beta/GA) for the feature either in this PR comments or in HLD itself based on https://github.com/sonic-net/SONiC/blob/master/doc/SONiC%20feature%20quality%20definition.md
Thanks

@fastiuk
Copy link
Contributor Author

fastiuk commented Feb 7, 2024

@fastiuk Can you please update the Quality Metric (Alpha/Beta/GA) for the feature either in this PR comments or in HLD itself based on https://github.com/sonic-net/SONiC/blob/master/doc/SONiC%20feature%20quality%20definition.md Thanks

It old available feature, we didn't have HLD for it.
It is GA level

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

@venkatmahalingam venkatmahalingam venkatmahalingam left review comments

@bsun-sudo bsun-sudo bsun-sudo left review comments

@saravanan-i saravanan-i saravanan-i left review comments

@zhangyanzhao zhangyanzhao zhangyanzhao approved these changes

@yxieca yxieca yxieca approved these changes

@saiarcot895 saiarcot895 Awaiting requested review from saiarcot895

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@fastiuk @zhangyanzhao @venkatmahalingam @skg-net @yxieca @bsun-sudo @saravanan-i