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

Fixed NTP sync while using DHCP. #1035

Merged
merged 1 commit into from
Oct 14, 2017
Merged

Fixed NTP sync while using DHCP. #1035

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions files/image_config/ntp/ntp.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ server {{ ntp_server }} iburst

#only listen on localhost and eth0 ips (default is to listen on all ip addresses)
interface ignore wildcard
{% if MGMT_INTERFACE %}
Copy link
Contributor

Choose a reason for hiding this comment

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

when will MGMT_INTERFACE be not defined? how to assume that eth0 is the one to use.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the first question, if the mgmt interface is not in the minigraph, then it will not be defined. I think we do have such cases for the default minigraph. It does not have management ip configured so dhcp is enabled.

for the second qeustion, it is a good question. but we are implicit assuming eth0 in several other places like the interface.j2. I think that is one of basic assumption here. This assumption can be relaxed later, but it is probably not related to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@stcheng the changes made here is the similar how interfaces are interpreter int system https://github.com/Azure/sonic-buildimage/blob/master/files/image_config/interfaces/interfaces.j2#L23
MGMT_INTERFACE will not be defined when user will not add corresponding section in minigraph or configDB.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the explanations!

{% for (mgmt_intf, mgmt_prefix) in MGMT_INTERFACE %}
interface listen {{ mgmt_prefix | ip }}
{% endfor %}
{% else %}
interface listen eth0
{% endif %}
interface listen 127.0.0.1

# Access control configuration; see /usr/share/doc/ntp-doc/html/accopt.html for
Expand Down