Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Source interface support for NTP #6033

Merged
merged 12 commits into from
Dec 21, 2020
Merged

Conversation

PrabhuSreenivasan
Copy link
Contributor

@PrabhuSreenivasan PrabhuSreenivasan commented Nov 25, 2020

Signed-off-by: Prabhu Sreenivasan prabhu.sreenivasan@broadcom

- Why I did it
Added source interface support for NTP.
Also made NTP start on Mgmt-VRF by default when configured.

- How I did it

  1. Updated hostcfg to listen to global config NTP and NTP_SERVER tables and restart ntp when ever the configuration changes. NTP table includes source interface configuration.
  2. The ntp script updated to by default start on Mgmt-VFT when configured.

- How to verify it
Existing ntp pytest.

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

- Description for the changelog
Source interface support and handle config change without unnecessary restart of ntp-config ntp service.
NTP start on Mgmt-VRF by default when configured

@PrabhuSreenivasan PrabhuSreenivasan force-pushed the ntp_fixes branch 3 times, most recently from 6ec7d0c to 06e2c05 Compare November 25, 2020 12:54
@lguohan
Copy link
Collaborator

lguohan commented Nov 26, 2020

@PrabhuSreenivasan, can you check the build failure?

Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom>
@PrabhuSreenivasan
Copy link
Contributor Author

PrabhuSreenivasan commented Dec 2, 2020

@PrabhuSreenivasan, can you check the build failure?

@lguohan, Fixed the build failure.

@PrabhuSreenivasan PrabhuSreenivasan changed the title NTP patches and fixes Source interface support for NTP Dec 3, 2020
PrabhuSreenivasan and others added 2 commits December 3, 2020 11:14
Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
@PrabhuSreenivasan
Copy link
Contributor Author

@lguohan, As per your suggestion, I have moved the ntp service ordering to a new PR #6115

@lguohan
Copy link
Collaborator

lguohan commented Dec 4, 2020

#pr 6115 is merged. can you rebase this pr? I will approve.

@PrabhuSreenivasan
Copy link
Contributor Author

#pr 6115 is merged. can you rebase this pr? I will approve.

I have rebased this PR. Thanks.

lguohan
lguohan previously approved these changes Dec 4, 2020
@PrabhuSreenivasan
Copy link
Contributor Author

retest this please

@PrabhuSreenivasan
Copy link
Contributor Author

One of the vsimage test failed due to 'hosts': 'vlab-01' not reachable.

15:27:03 =========================== short test summary info ============================
15:27:03 FAILED ntp/test_ntp.py::test_ntp - AnsibleConnectionFailure: Host unreachable

@lguohan
Copy link
Collaborator

lguohan commented Dec 6, 2020

retest this please

@lguohan
Copy link
Collaborator

lguohan commented Dec 6, 2020

not familiar with the error, since your change is related to ntp, so it would be regression introduced in this pr. let me retest,

@lguohan
Copy link
Collaborator

lguohan commented Dec 8, 2020

@PrabhuSreenivasan looks like the test is consistently failing, can you check why the test is failing? we are unable to merge this pr until the test is passing.

Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
@lguohan
Copy link
Collaborator

lguohan commented Dec 11, 2020

seems ntp test still failing?

@PrabhuSreenivasan
Copy link
Contributor Author

retest vs please

@PrabhuSreenivasan
Copy link
Contributor Author

retest vsimage please

Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
@PrabhuSreenivasan
Copy link
Contributor Author

seems ntp test still failing?

Failing due to python2 to python3 change. working on a fix.

root@sonic:/home/admin# sonic-cfggen -d -t /usr/share/sonic/templates/ntp.conf.j2 >/etc/ntp.conf
Traceback (most recent call last):
File "/usr/local/bin/sonic-cfggen", line 433, in
main()
File "/usr/local/bin/sonic-cfggen", line 398, in main
template_data = template.render(data)
File "/usr/local/lib/python3.7/dist-packages/jinja2/environment.py", line 1090, in render
self.environment.handle_exception()
File "/usr/local/lib/python3.7/dist-packages/jinja2/environment.py", line 832, in handle_exception
reraise(*rewrite_traceback_stack(source=source))
File "/usr/local/lib/python3.7/dist-packages/jinja2/_compat.py", line 28, in reraise
raise value.with_traceback(tb)
File "/usr/share/sonic/templates/ntp.conf.j2", line 53, in top-level template code
{% set ns = namespace(source_intf) %}
File "/usr/local/lib/python3.7/dist-packages/jinja2/utils.py", line 697, in init
self.__attrs = dict(*args, **kwargs)
jinja2.exceptions.UndefinedError: 'source_intf' is undefined

Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
@PrabhuSreenivasan
Copy link
Contributor Author

retest vsimage please

@PrabhuSreenivasan
Copy link
Contributor Author

PrabhuSreenivasan commented Dec 14, 2020

All NTP tests are passing. New failure "tacacs/test_rw_user.py::test_rw_user FAILED" seems quite unrelated, but consistently failing.

@PrabhuSreenivasan
Copy link
Contributor Author

retest vsimage please

@ben-gale
Copy link
Collaborator

@lguohan - we seem to be stuck on the same failing test, which does not seem to be related to this change. Can you help (e.g. fix or skip)? We need to get this merged ....

@lguohan
Copy link
Collaborator

lguohan commented Dec 16, 2020

retest vsimage please

lguohan
lguohan previously approved these changes Dec 16, 2020
@lguohan
Copy link
Collaborator

lguohan commented Dec 18, 2020

retest vsimage please

Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
Prabhu Sreenivasan and others added 3 commits December 18, 2020 20:06
Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
Signed-off-by: Prabhu Sreenivasan <prabhu.sreenivasan@broadcom.com>
@lguohan
Copy link
Collaborator

lguohan commented Dec 19, 2020

retest vsimage please

@lguohan lguohan merged commit df2a4de into sonic-net:master Dec 21, 2020
@lguohan
Copy link
Collaborator

lguohan commented Dec 21, 2020

thanks for your contribution!

@PrabhuSreenivasan PrabhuSreenivasan deleted the ntp_fixes branch December 23, 2020 15:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants