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

Add support for ntp server configuration from dhcp #158

Open
netsandbox opened this issue Jul 3, 2023 · 13 comments
Open

Add support for ntp server configuration from dhcp #158

netsandbox opened this issue Jul 3, 2023 · 13 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@netsandbox
Copy link

BurmillaOS Version: (ros os version)
v1.9.6

If you are using dhcp, it is quite common to also send a ntp_servers option (42) with the dhcp response.
But currently the dhcpcd don't even request the ntp_servers option (it is commented out):

$ sudo system-docker exec network grep ntp /etc/dhcpcd.conf
#option ntp_servers

It would be great if configuring ntp servers from dhcp could be implemented.

@olljanat
Copy link
Member

olljanat commented Jul 3, 2023

We use https://github.com/NetworkConfiguration/dhcpcd inside of network system container this feature would need to be implemented in there.

@netsandbox
Copy link
Author

Actually it is there, but you have to provide a hook, see: https://github.com/NetworkConfiguration/dhcpcd/blob/master/hooks/50-ntp.conf

@olljanat
Copy link
Member

Ah, but that is not enough because ntp client is running on different container. If you want to use custom NTP you need to configure those with cloud-init like this https://burmillaos.org/docs/configuration/advanced/write-files/#writing-files-in-specific-system-services

@olljanat olljanat closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2023
@netsandbox
Copy link
Author

Using cloud-init would make our deployment over complicated, because we have isolated networks, which all have their own ntp server. So I would need to create a cloud-init for each network and let our deployment pick the right cloud-init for the new host.

Maybe for the future it makes sense to put the ntp client inside the network container, as this is also a network service, and would make it possible to distribute the ntp server by dhcp.

@olljanat olljanat added enhancement New feature or request help wanted Extra attention is needed labels Aug 23, 2023
@olljanat olljanat added this to the v2.0.0 milestone Aug 23, 2023
@olljanat
Copy link
Member

Using cloud-init would make our deployment over complicated, because we have isolated networks, which all have their own ntp server. So I would need to create a cloud-init for each network and let our deployment pick the right cloud-init for the new host.

What is purpose to use so overly complicated ntp server setup?

Maybe for the future it makes sense to put the ntp client inside the network container, as this is also a network service, and would make it possible to distribute the ntp server by dhcp.

In general best practice is have just one process per containers so this would need process which can handle both dhcp client and ntp client.

Anyway, if this is critical for you then perhaps you can try what happens when you enable option ntp_servers in /etc/dhcpcd.conf? (which you can also do by utilizing write_files in cloud-init.

Where dhcpcd will write that information about ntp servers and it ntp client able read it then?
Both network and ntp system containers can see same file system so in theory it might work with that one.

If you can proof that it works then perhaps we can fix it to v2.0.0 release version. Now when I'm looking there looks to be bug somewhere on build process anyway because on template config that option is actually already enabled:

# Most distributions have NTP support.
option ntp_servers

@olljanat olljanat reopened this Aug 23, 2023
@netsandbox
Copy link
Author

I have enabled option ntp_servers in /etc/dhcpcd.conf of the network container, restarted the container and run:

$ sudo system-docker exec network dhcpcd -MA4 -U eth0 | grep ntp
ntp_servers=x.x.x.x

So yes there is the dhcp provided ntp server.

@netsandbox
Copy link
Author

Now when I'm looking there looks to be bug somewhere on build process anyway because on template config that option is actually already enabled

The dhcpcd template is only used when there are WifiNetworks configured:

if len(cfg.Rancher.Network.WifiNetworks) > 0 {
generateDhcpcdFiles(cfg)
generateWpaFiles(cfg)
}

So dhcpcd configuration changes must go in the template and must be inline changed in the Dockerfile:

sed -i -e 's/duid/clientid/g' /etc/dhcpcd.conf && \
echo -e '\n# Ignore Docker container interfaces\ndenyinterfaces veth*\n' >> /etc/dhcpcd.conf && \

And the inline change to remove the # from option ntp_servers is currently missing.

Maybe it is better to always use the template, regardless if WifiNetworks are configured or not, to manage the configuration only in one place.

@olljanat
Copy link
Member

olljanat commented Mar 5, 2024

Included to v2.0.0

@netsandbox
Copy link
Author

@olljanat actually this is not working in v2 because you just implemented requesting ntp_servers from dhcp, but not the hook mentioned in #158 (comment) to update the /etc/ntp.conf.

@olljanat
Copy link
Member

I don't fully follow what is actually needed. Please, create pull request with valid logic.

@olljanat olljanat reopened this Apr 18, 2024
@netsandbox
Copy link
Author

dhcpcd don't automatically modifies /etc/ntp.conf by default.

You need to place hook https://github.com/NetworkConfiguration/dhcpcd/blob/master/hooks/50-ntp.conf in the network container in /lib/dhcpcd/dhcpcd-hooks to let dhcpcd update /etc/ntp.conf.

@netsandbox
Copy link
Author

netsandbox commented Apr 18, 2024

Sadyl I'm not familiar with the build process, so I don't know what to do to get the new hook file in the network container.

@olljanat
Copy link
Member

Those are part of base image defined in these files. Can be tested with normal Docker build + run https://github.com/burmilla/os/tree/master/images/01-base

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants