Skip to content

Enable iburst option for NTP servers loaded from minigraph #19424

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

Merged
merged 3 commits into from
Jul 11, 2024

Conversation

saiarcot895
Copy link
Contributor

@saiarcot895 saiarcot895 commented Jun 28, 2024

Why I did it

In #15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes #19425.

Work item tracking
  • Microsoft ADO (number only): 28623148

How I did it

To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it

Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202311
  • 202405

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 (not mandatory but encouraged)

In sonic-net#15058, the NTP server configuration was modified to add additional
options, such as conditionally enabling iburst, specifying the
association type, and specifying the NTP version. One side effect of
this was that the iburst option which was previously always enabled now
requires it to be explicitly enabled in the config_db.

To restore the old behavior, when loading from minigraph, add
`"iburst": "true"` for each NTP server loaded from minigraph.

Signed-off-by: Saikrishna Arcot <[email protected]>
yxieca
yxieca previously approved these changes Jun 28, 2024
@saiarcot895
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <[email protected]>
@abdosi
Copy link
Contributor

abdosi commented Jul 4, 2024

@saiarcot895 this should fix : #18916.

@abdosi abdosi linked an issue Jul 4, 2024 that may be closed by this pull request
@abdosi
Copy link
Contributor

abdosi commented Jul 4, 2024

@anamehra for viz.

@saiarcot895
Copy link
Contributor Author

On the contrary, this won't truly fix #18916. This PR just hardcodes the 'iburst': 'on' option for NTP servers loaded from minigraph, restoring the old behavior. For #18916 to be actually fixed, there needs to be new CLI options to enable/disable iburst.

Signed-off-by: Saikrishna Arcot <[email protected]>
@saiarcot895
Copy link
Contributor Author

@fastiuk FYI

@saiarcot895 saiarcot895 requested a review from yxieca July 9, 2024 22:34
@arlakshm arlakshm requested a review from mlok-nokia July 10, 2024 17:18
@yxieca yxieca merged commit 990a725 into sonic-net:master Jul 11, 2024
21 checks passed
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
…#19424)

Why I did it
In sonic-net#15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes sonic-net#19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
saiarcot895 added a commit to saiarcot895/sonic-buildimage that referenced this pull request Jul 31, 2024
…#19424)

Why I did it
In sonic-net#15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes sonic-net#19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
@saiarcot895 saiarcot895 deleted the enable-iburst-ntp branch July 31, 2024 02:34
yxieca pushed a commit that referenced this pull request Jul 31, 2024
…19741)

Why I did it
In #15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes #19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Jul 31, 2024
…#19424)

Why I did it
In sonic-net#15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes sonic-net#19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #19750

mssonicbld pushed a commit that referenced this pull request Jul 31, 2024
Why I did it
In #15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes #19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Aug 1, 2024
…#19424)

Why I did it
In sonic-net#15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes sonic-net#19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
yanjundeng pushed a commit to yanjundeng/sonic-buildimage that referenced this pull request Apr 23, 2025
…#19424)

Why I did it
In sonic-net#15058, the NTP server configuration was modified to add additional options, such as conditionally enabling iburst, specifying the association type, and specifying the NTP version. One side effect of this was that the iburst option which was previously always enabled now requires it to be explicitly enabled in the config_db.

Fixes sonic-net#19425.

How I did it
To restore the old behavior, when loading from minigraph, add "iburst": "true" for each NTP server loaded from minigraph.

How to verify it
Tested on a KVM setup, verified that the generated ntp.conf file had the iburst option.

Signed-off-by: Saikrishna Arcot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Archived in project
6 participants