Allow pools and servers by SuperQ · Pull Request #215 · hassio-addons/addon-chrony

SuperQ · 2025-11-09T14:56:39Z

Proposed Changes

Chrony allows both pools and servers to be used simultaneously. For example using a local time server in addition to a public NTP service.

Support both ntp_pool and ntp_server to be used at the same time.
Add maxsources option to pool configuration.

Related Issues

None

Summary by CodeRabbit

New Features
- Added a configuration option to limit servers selected from NTP pool DNS records (1–16, default 4).
Documentation
- Clarified pool vs. server behavior, added explicit empty-value examples for optional settings.
- Marked legacy mode option as deprecated/ignored.
Configuration Changes
- Default server list is now empty; either a pool or explicit servers must be configured.
- Mode is now optional; validation requires at least one of pool or servers.

coderabbitai · 2025-11-09T14:56:48Z

Walkthrough

Refactors chrony setup to remove the mode discriminant, add pool_maxsources, and allow configuration via either an NTP pool or explicit servers; updates docs, schema defaults, and the init script to validate and apply pool and server settings accordingly.

Changes

Cohort / File(s)	Summary
Documentation `chrony/DOCS.md`	Adds `pool_maxsources` (1–16, default 4); marks `mode` as deprecated/ignored; rewrites `ntp_pool` and `ntp_server` sections and adds explicit empty-value examples (`ntp_pool: ""`, `ntp_server: []`).
Configuration Schema `chrony/config.yaml`	Adds `pool_maxsources` field (int 1–16, default 4); removes required `mode` (made optional/omitted); changes `ntp_server` default to empty list (`[]`) while keeping schema as list of strings.
Init script `chrony/rootfs/etc/s6-overlay/s6-rc.d/init-chrony/run`	Removes `mode` branching; validates that at least one of `ntp_pool` or `ntp_server` is present; when `ntp_pool` set, appends `pool <ntp_pool> iburst maxsources <pool_maxsources>` and records it; iterates `ntp_server` to append `server <server> iburst`; emits `initstepslew` with aggregated servers.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Config as config.yaml
  participant Init as init-chrony/run
  participant Chrony as /etc/chrony.conf

  Note over Config,Init: Load configuration values
  Config->>Init: ntp_pool, ntp_server[], pool_maxsources

  Init->>Init: validate (ntp_pool or ntp_server must be non-empty)
  alt ntp_pool present
    Init->>Chrony: append "pool <ntp_pool> iburst maxsources <pool_maxsources>"
    Note right of Init: record pool in serverlist
  end

  alt ntp_server contains entries
    loop for each server
      Init->>Chrony: append "server <server> iburst"
      Note right of Init: add server to serverlist
    end
  end

  Init->>Chrony: run "initstepslew <serverlist>"
  Note over Chrony: Chrony configured and slewed

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pay extra attention to:
- chrony/rootfs/etc/s6-overlay/s6-rc.d/init-chrony/run — validation logic and string assembly for pool/server lines and initstepslew.
- chrony/config.yaml — schema constraints for pool_maxsources and changed defaults for ntp_server.
- chrony/DOCS.md — ensure examples and deprecation note match actual behavior.

Poem

🐰 I nibbled mode away and planted maxsources four,
Now pools and servers hop together to the door,
I stitch the config lines with iburst bright and keen,
Chrony wakes, the clocks align — a hopping time machine! 🕰️🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Allow pools and servers' directly reflects the main change: enabling simultaneous use of both ntp_pool and ntp_server configurations, which was the PR's primary objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18ba043 and 63f612b.

📒 Files selected for processing (3)

chrony/DOCS.md (2 hunks)
chrony/config.yaml (1 hunks)
chrony/rootfs/etc/s6-overlay/s6-rc.d/init-chrony/run (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Chrony allows both pools and servers to be used simultaneously. For example using a local time server in addition to a public NTP service. * Support both `ntp_pool` and `ntp_server` to be used at the same time. * Add `maxsources` option to pool configuration. Signed-off-by: SuperQ <superq@gmail.com>

SuperQ · 2025-11-09T14:58:48Z

Technically I think ntp_pool should be an array, since multiple pools is supported by Chrony. But that would be a breaking change.

nethershaw · 2025-11-12T07:03:21Z

It's also simply wrong to talk about Chrony working in "modes" because it can be configured with pools, servers, and peers simultaneously. If it needs a breaking change to be correct, I say: break it.

SuperQ · 2025-11-12T07:19:08Z

Yup, also was planning on adding refclock to the list. There are some nice Raspbery Pi GPS hats that can be used.

github-actions · 2025-12-12T08:23:03Z