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

[Improvement idea] Lack of post-stop chrony control in sfptpd service #18

Open
sasharozenson opened this issue Oct 25, 2024 · 6 comments

Comments

@sasharozenson
Copy link

When running sfptpd alongside chrony, there’s a risk that chrony's clock control state may remain altered if sfptpd fails or crashes. Without specific commands to reset chrony's state after a failure, chrony may continue to operate in an unintended state, potentially affecting the clock.

Suggested Improvement

To address this, I propose adding an ExecStopPost command to the sfptpd.service file. This will ensure that if sfptpd stops unexpectedly, chrony’s state will be reset as needed.

ExecStopPost=/usr/libexec/sfptpd/chrony_clockcontrol.py enable
@abower-amd
Copy link
Collaborator

Thanks for the helpful suggestion!

The idea was that the save and restore verbs would be used in this sort of situation rather than enable or disable so that the initial state is restored, whatever that was. However, this is still not ideal.

With sfptpd-3.8.0 the clock control has been brought into the daemon and now works differently: it adds a section to the chrony environment file which is identified as an sfptpd addition that can then be wiped out when sfptpd restarts. This means that there is no ambiguity about what the sysadmin had in mind as being the normal state.

#define START_BLOCK "### BEGIN sfptpd ###"
#define END_BLOCK "### END sfptpd ###"

I can still see this is a problem if sfptpd crashes out and never gets restarted but perhaps this reduces the urgency of mitigation?

@abower-amd
Copy link
Collaborator

P.S. we really wish chronyd offered runtime control of whether it is disciplining the system clock, like ntpd does. That would be much more robust!

@sasharozenson
Copy link
Author

Aha, I see, thanks!

I'll look into using save/restore as you suggested. Currently, we don't auto-restart sfptpd on failure, so adding the systemd command seems helpful [in our case].

@abower-amd
Copy link
Collaborator

I wouldn't necessarily say the issue is closed - it is a good point! Though we'd have to make sure it was also clean for non-chrony users.

Another option we have up our sleeves is to supervise the chronyd process directly (not via systemd) using the new sfptpd privileged helper. I think this might be how chronyd is controlled by the 'timemaster' tool, too, so there would be precedent for doing that. Then there would be no messy environment editing. In that situation you would want sfptpd always running though (via restart) because that would become the way you ran chronyd full stop, and yes, you'd have an interruption of chrony service if sfptpd restarted.

@sasharozenson
Copy link
Author

Reopening this issue - it was an oopsie closing it earlier. 😊 I spent some time exploring ways to manage chrony and sfptpd with systemd alone, but nothing quite matches the flexibility of chrony_clockcontrol.py for handling environment-specific changes. I'm not sure about adding another helper process, though, wouldn't that also require something to monitor it as well?

@abower-amd
Copy link
Collaborator

abower-amd commented Oct 25, 2024

Hi @sasharozenson yes, but now we have a new process already - the privileged helper is new in v3.8 to allow sfptpd to drop privileges while still supporting hotplug and chrony control, which wouldn't otherwise be possible.

So it could directly start chronyd rather than editing the sysadmin's config and telling systemd what to do. I think the net result might be more robust because it is explicit about who is responsible for the chrony service - I think really only the sysadmin should call systemctl, not other daemons.

Still, the script option is still available, and you can customize it!

@abower-amd abower-amd reopened this Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants