Skip to content

Commit

Permalink
Enable iburst option for NTP servers loaded from minigraph (#19424)
Browse files Browse the repository at this point in the history
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 <sarcot@microsoft.com>
  • Loading branch information
saiarcot895 authored Jul 11, 2024
1 parent b87521d commit 990a725
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/sonic-config-engine/minigraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -2612,7 +2612,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
results['SYSLOG_SERVER'] = dict((item, {}) for item in syslog_servers)
results['DHCP_SERVER'] = dict((item, {}) for item in dhcp_servers)
results['DHCP_RELAY'] = dhcp_relay_table
results['NTP_SERVER'] = dict((item, {}) for item in ntp_servers)
results['NTP_SERVER'] = dict((item, {'iburst': 'on'}) for item in ntp_servers)
# Set default DNS nameserver from dns.j2
results['DNS_NAMESERVER'] = {}
if os.environ.get("CFGGEN_UNIT_TESTING", "0") == "2":
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/test_cfggen.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ def test_metadata_tacacs(self):
def test_metadata_ntp(self):
argument = ['-m', self.sample_graph_metadata, '-p', self.port_config, '-v', "NTP_SERVER"]
output = self.run_script(argument)
self.assertEqual(utils.to_dict(output.strip()), utils.to_dict("{'10.0.10.1': {}, '10.0.10.2': {}}"))
self.assertEqual(utils.to_dict(output.strip()), utils.to_dict("{'10.0.10.1': {'iburst': 'on'}, '10.0.10.2': {'iburst': 'on'}}"))

def test_dns_nameserver(self):
argument = ['-m', self.sample_graph_metadata, '-p', self.port_config, '-v', "DNS_NAMESERVER"]
Expand Down
8 changes: 4 additions & 4 deletions src/sonic-config-engine/tests/test_chassis_cfggen.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_ntp(self):
argument = ['-m', self.sample_graph, '-p',
self.sample_port_config, '--var-json', 'NTP_SERVER']
output = json.loads(self.run_script(argument))
self.assertDictEqual(output, {'17.39.1.130': {}, '17.39.1.129': {}})
self.assertDictEqual(output, {'17.39.1.130': {'iburst': 'on'}, '17.39.1.129': {'iburst': 'on'}})
# NTP data is present only in the host config
argument = ['-m', self.sample_graph, '--var-json', 'NTP_SERVER']

Expand Down Expand Up @@ -416,7 +416,7 @@ def test_ntp(self):
argument = ['-m', self.sample_graph, '-p',
self.sample_port_config, '--var-json', 'NTP_SERVER']
output = json.loads(self.run_script(argument))
self.assertDictEqual(output, {'17.39.1.130': {}, '17.39.1.129': {}})
self.assertDictEqual(output, {'17.39.1.130': {'iburst': 'on'}, '17.39.1.129': {'iburst': 'on'}})
# NTP data is present only in the host config
argument = ['-m', self.sample_graph, '--var-json', 'NTP_SERVER']

Expand Down Expand Up @@ -884,7 +884,7 @@ def test_ntp(self):
'--var-json', 'NTP_SERVER'
]
output = json.loads(self.run_script(argument))
self.assertDictEqual(output, {'17.39.1.130': {}, '17.39.1.129': {}})
self.assertDictEqual(output, {'17.39.1.130': {'iburst': 'on'}, '17.39.1.129': {'iburst': 'on'}})


def test_mgmt_port(self):
Expand Down Expand Up @@ -1019,7 +1019,7 @@ def test_ntp(self):
'--var-json', 'NTP_SERVER'
]
output = json.loads(self.run_script(argument))
self.assertDictEqual(output, {'17.39.1.130': {}, '17.39.1.129': {}})
self.assertDictEqual(output, {'17.39.1.130': {'iburst': 'on'}, '17.39.1.129': {'iburst': 'on'}})
# NTP data is present only in the host config
argument = ['-m', self.sample_graph, '--var-json', 'NTP_SERVER']

Expand Down
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/test_minigraph_case.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ def test_minigraph_mgmt_port(self):
def test_metadata_ntp(self):
argument = ['-m', self.sample_graph, '-p', self.port_config, '-v', "NTP_SERVER"]
output = self.run_script(argument)
self.assertEqual(output.strip(), "{'10.0.10.1': {}, '10.0.10.2': {}}")
self.assertEqual(output.strip(), "{'10.0.10.1': {'iburst': 'on'}, '10.0.10.2': {'iburst': 'on'}}")

def test_minigraph_vnet(self):
argument = ['-m', self.sample_graph, '-p', self.port_config, '-v', "VNET"]
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/test_multinpu_cfggen.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def test_metadata_tacacs(self):
def test_metadata_ntp(self):
argument = ['-m', self.sample_graph, '-p', self.sample_port_config, '--var-json', "NTP_SERVER"]
output = json.loads(self.run_script(argument))
self.assertDictEqual(output, {'17.39.1.130': {}, '17.39.1.129': {}})
self.assertDictEqual(output, {'17.39.1.130': {'iburst': 'on'}, '17.39.1.129': {'iburst': 'on'}})
#NTP data is present only in the host config
argument = ['-m', self.sample_graph, '--var-json', "NTP_SERVER"]
for asic in range(NUM_ASIC):
Expand Down

0 comments on commit 990a725

Please sign in to comment.