Skip to content

[ntp-admin] SMF service should explicitly depend on chrony setup#10727

Open
karencfv wants to merge 1 commit into
oxidecomputer:mainfrom
karencfv:ntp-svc-should-depend-on-chrony-setup
Open

[ntp-admin] SMF service should explicitly depend on chrony setup#10727
karencfv wants to merge 1 commit into
oxidecomputer:mainfrom
karencfv:ntp-svc-should-depend-on-chrony-setup

Conversation

@karencfv

@karencfv karencfv commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Just a small change for correctness. We want to make sure ntp-admin depends on chrony having been configured, not just the NTP service being online. Also, the refresh behaviour on configuration being changed should be preserved.

I tagged a bunch of people who've worked on this service for a review. Wasn't exactly sure who to request a review from here.

@davepacheco

Copy link
Copy Markdown
Collaborator

Out of curiosity, why is this needed? I'm just thinking that if chrony is taking a while to come up, then with this change, we can't reach the ntp-admin service to even check the status. In an ideal world it would be nice if ntp-admin came up and operated as best it could (even if it's mostly returning errors) while chrony wasn't up yet.

@karencfv

karencfv commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@davepacheco ntp-admin already indirectly depends on chrony-setup via the ntp service. Also, the chrony-setup service only generates and writes the chrony config file, it doesn't start chrony (ntp service does that). This means the existing behaviour is that if chrony takes a while to come up, ntp-admin would not come up.

This PR only makes the dependency explicit.

Going forward, I'm working on collecting more debug data from the NTP zone (#10407). To be able to answer questions like "Can I reach the upstream NTP server" I need to read chrony-setup's property group to get the necessary values. Preserving the refresh behaviour on configuration being changed means we avoid getting any false positives during a chrony-setup refresh.

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

Successfully merging this pull request may close these issues.

3 participants