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

[202311][cherry-pick][NTP] Add NTP extended configuration #17487

Merged
merged 15 commits into from
Dec 21, 2023

Conversation

fastiuk
Copy link
Contributor

@fastiuk fastiuk commented Dec 13, 2023

hld #1296
closes #1254
cherry-picks #15058

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

A picture of a cute animal (it is my cat Finn)

PXL_20230413_140941610 PORTRAIT

@fastiuk fastiuk changed the title [NTP] Add NTP extended configuration [202311][cherry-pick][NTP] Add NTP extended configuration Dec 13, 2023
@fastiuk fastiuk requested a review from saiarcot895 December 13, 2023 15:44
@fastiuk fastiuk self-assigned this Dec 13, 2023
@yxieca
Copy link
Contributor

yxieca commented Dec 13, 2023

@fastiuk this seems to be a feature PR. I don't think we should cherry-pick feature PR(s) into a release branch.

@fastiuk
Copy link
Contributor Author

fastiuk commented Dec 14, 2023

@fastiuk this seems to be a feature PR. I don't think we should cherry-pick feature PR(s) into a release branch.

It is. But 4 out of 5 PRs for this feature were merged, and are part of 202311. So I think it is mandatory to cherry-pick the last one

@yxieca
Copy link
Contributor

yxieca commented Dec 14, 2023

@fastiuk test_ntp is consistently failure. Can you make the test compatible with before and after this change?

@fastiuk fastiuk force-pushed the dev-ntp-configuration branch 2 times, most recently from 7104c73 to 9a16bf3 Compare December 15, 2023 13:11
Signed-off-by: Yevhen Fastiuk <[email protected]>
@fastiuk fastiuk force-pushed the dev-ntp-configuration branch from 9a16bf3 to 2ea6582 Compare December 15, 2023 13:14
Signed-off-by: Yevhen Fastiuk <[email protected]>
Signed-off-by: Yevhen Fastiuk <[email protected]>
Signed-off-by: Yevhen Fastiuk <[email protected]>
Signed-off-by: Yevhen Fastiuk <[email protected]>
Signed-off-by: Yevhen Fastiuk <[email protected]>
* Align the description for setting interface
* Fix the usage of scoped variable

Signed-off-by: Yevhen Fastiuk <[email protected]>
@fastiuk fastiuk force-pushed the dev-ntp-configuration branch from 2ea6582 to ec1d0d4 Compare December 15, 2023 13:26
Copy link

Commenter does not have sufficient privileges for PR 17487 in repo sonic-net/sonic-buildimage

1 similar comment
Copy link

Commenter does not have sufficient privileges for PR 17487 in repo sonic-net/sonic-buildimage

@fastiuk
Copy link
Contributor Author

fastiuk commented Dec 15, 2023

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fastiuk
Copy link
Contributor Author

fastiuk commented Dec 16, 2023

@fastiuk test_ntp is consistently failure. Can you make the test compatible with before and after this change?

Tests passed. Please merge it

@fastiuk
Copy link
Contributor Author

fastiuk commented Dec 17, 2023

@yxieca Any updates?

@yxieca yxieca merged commit f78cb9c into sonic-net:202311 Dec 21, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants