Conversation
…to available values.
…for IP or hostname.
adamshiervani
left a comment
There was a problem hiding this comment.
Thanks for implementing the time sync configuration and also all your other PRs!!
The functionality is solid, but I think the UI needs a remodel. The current design reflects our internal data model rather than what users actually need.
Usually there are two groups with a 95/5 ratio:
- "Just work" - Automatic time sync, don't care how
- "Use my servers" - Enterprise/air-gapped environments with specific NTP servers
Proposed Simplification
Two modes: Automatic | Custom
Time Synchronization
└─ [Automatic ▼]
When "Custom":
┌────────────────────────────────────────────────────────┐
│ Your NTP Servers │
│ [ntp.internal.corp.com ] [×] │
│ [+ Add Server] │
│ │
│ Your HTTP URLs (optional) │
│ [http://time.internal.corp.com ] [×] │
│ [+ Add URL] │
│ │
│ ☑ Fall back to built-in servers if yours fail │
│ ☐ Also try NTP servers from DHCP (for isolated networks)│
└────────────────────────────────────────────────────────┘
Backend Mapping
| UI State | Backend Values |
|---|---|
| Automatic | time_sync_mode: "ntp_and_http", default ordering |
| Custom + fallback | mode: "custom", ordering: [user_ntp, user_http, ntp, http], disable_fallback: false |
| Custom + fallback + DHCP | Same but ordering includes ntp_dhcp after user sources |
| Custom, no fallback | ordering: [user_ntp, user_http], disable_fallback: true |
|
Hi there! I agree with your comments, and that's what I thought of doing. I implemented this as a first-pass.... just to basically understand what the model was behind the scenes. At least this gives power-users access to the advanced features. From here, yeah, I agree. Things like "Disable Fallback" shouldn't be possible with no specified NTP/HTTP URLs.... A lot of the things should be "automatic" unless specified. Then even the "specified" things should only be the servers / URLs. As long as things behind the scenes only hit the servers specified, I think offline users / air-gaped users would be satisfied. I might get around to simplifying this over the next couple of weeks....but not being a React / TS dev...this makes it a bit of a struggle :P |
As a network engineer, I would expect DHCP to be honored first |
I disagree, and it depends on wording, for the SNTP client. (I don't think this i a full stack NTP client, since it doesn't need it.)
Ref: timesyncd.conf(5) |
Closes jetkvm#516, jetkvm#645, jetkvm#59 The backend supports custom NTP servers, HTTP URLs, source ordering, parallel queries, and fallback control for time synchronization, but the frontend only exposes three presets (NTP only, NTP and HTTP, HTTP only). Users who need to specify their own NTP server — the core ask in all three linked issues — have no way to do so through the UI. Add a "Custom" option to the time sync dropdown. When selected, a card appears with input fields for NTP servers and HTTP URLs, following the same list-with-add/remove pattern used by the static IPv4 DNS fields. This is a simplified alternative to jetkvm#1102 which exposed every backend field (source ordering, parallel queries, disable fallback) as direct UI controls. That PR stalled for 3 months due to complexity concerns and UX debate. This PR ships the functionality users actually requested — custom NTP servers — with a minimal UI surface: jetkvm#1102: 753 additions, 15 files, new Combobox modifications This: ~120 additions, 18 files (13 are localization) The advanced fields (TimeSyncOrdering, TimeSyncParallel, TimeSyncDisableFallback) retain their backend defaults and can be surfaced in a follow-up if there is demand. Backend changes: confparser.go — add hostname_or_ipv4_or_ipv6 validation type so NTP server fields accept hostnames like pool.ntp.org, not just raw IPs. config.go — change TimeSyncNTPServers validation from ipv4_or_ipv6 to hostname_or_ipv4_or_ipv6. Frontend changes: CustomTimeSyncCard.tsx — new component with NTP server list and HTTP URL list, field validation, add/remove controls. stores.ts — add optional time_sync_ordering, time_sync_ntp_servers, time_sync_http_urls, time_sync_disable_fallback, time_sync_parallel to NetworkSettings interface. network settings page — uncomment Custom option, render card when time_sync_mode is custom. Translations added for all 13 supported languages.
Closes jetkvm#516, jetkvm#645, jetkvm#59 The backend supports custom NTP servers, HTTP URLs, source ordering, parallel queries, and fallback control for time synchronization, but the frontend only exposes three presets (NTP only, NTP and HTTP, HTTP only). Users who need to specify their own NTP server — the core ask in all three linked issues — have no way to do so through the UI. Add a "Custom" option to the time sync dropdown. When selected, a card appears with input fields for NTP servers and HTTP URLs, following the same list-with-add/remove pattern used by the static IPv4 DNS fields. This is a simplified alternative to jetkvm#1102 which exposed every backend field (source ordering, parallel queries, disable fallback) as direct UI controls. That PR stalled for 3 months due to complexity concerns and UX debate. This PR ships the functionality users actually requested — custom NTP servers — with a minimal UI surface: jetkvm#1102: 753 additions, 15 files, new Combobox modifications This: ~120 additions, 18 files (13 are localization) The advanced fields (TimeSyncOrdering, TimeSyncParallel, TimeSyncDisableFallback) retain their backend defaults and can be surfaced in a follow-up if there is demand. Backend changes: confparser.go — add hostname_or_ipv4_or_ipv6 validation type so NTP server fields accept hostnames like pool.ntp.org, not just raw IPs. config.go — change TimeSyncNTPServers validation from ipv4_or_ipv6 to hostname_or_ipv4_or_ipv6. Frontend changes: CustomTimeSyncCard.tsx — new component with NTP server list and HTTP URL list, field validation, add/remove controls. stores.ts — add optional time_sync_ordering, time_sync_ntp_servers, time_sync_http_urls, time_sync_disable_fallback, time_sync_parallel to NetworkSettings interface. network settings page — uncomment Custom option, render card when time_sync_mode is custom. Translations added for all 13 supported languages.
Closes #516, #645, #59
Summary
Checklist
make test_e2elocally and passedCloses #<issue-number>)