Skip to content

ntp: add rollback migration for server-object time sources - #4969

Draft
xiaotianlyu wants to merge 1 commit into
bottlerocket-os:developfrom
xiaotianlyu:feat/chrony-ntp-object-list
Draft

xiaotianlyu wants to merge 1 commit into
bottlerocket-os:developfrom
xiaotianlyu:feat/chrony-ntp-object-list

Conversation

@xiaotianlyu

@xiaotianlyu xiaotianlyu commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description of changes:

  • Add a rollback migration for object lists in settings.ntp.time-servers.
  • Retain existing URL-list defaults on fresh and upgraded nodes; customers opt in to objects.
  • Leave existing settings unchanged during forward migration.
  • On rollback, preserve addresses and complete options common to every source; match option names and argument text.
  • Write options = [] when no options are common or conflicting options cannot be converted safely.
  • Stop migration with an error for invalid entries instead of dropping servers.
  • Preserve legacy lists, including []; remove settings.ntp.logging on rollback.
  • Add user documentation and datastore regression coverage.

The old template renders all addresses as pool, so rollback loses per-server directives and unique options. Example rollback result:

[settings.ntp]
time-servers = ["169.254.169.123", "time.aws.com"]
options = ["iburst"]

Testing done:

Local:

  • cargo test -p ntp-time-servers: 19 passed, including common/conflicting options, invalid entries, metadata, and pending data.
  • Datastore: 1 passed; migration CLI with an os-release fixture: 4 passed.
  • Migration clippy, formatting, and diff checks: passed.

On-box with all three PRs, aws-k8s-1.32 x86_64:

  • Built the local core-kit, test 1.67.0 AMI, and signed update repository.
  • Fresh installation and upgrades from 1.66.0 preserving default, custom-pool, and empty lists.
  • API switching, invalid-input rejection, empty-list persistence, logging, and reboot.
  • Legacy/object rollback, including no-common-option cases.
  • Single-source rollback to pool 169.254.169.123 iburst, synchronized with that sole source.
  • Five signed migrator cases on temporary datastores: invalid entries, [], and minpoll 4/04 matching.
  • 54 integration assertions passed; chronyd active with NRestarts=0 at state checks.
  • ARM64/non-AWS runtime, the full Bottlerocket workspace suite, and pending-transaction survival through reboot not tested locally.

Before merge:

  • Confirm the target release. The prototype currently registers 1.66.0 -> 1.67.0, matching the completed tests.
  • If shipping in 1.68.0, move registration to 1.67.0 -> 1.68.0 and repeat upgrade/rollback tests with the candidate and final 1.67.0 baseline.
  • Replace the SDK Draft revision with the approved release.
  • Update Core-kit to a release containing the renderer. The checked-in 17.2.0 lacks this feature; the test image used the rebuilt local kit.

Related PRs:

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@xiaotianlyu
xiaotianlyu force-pushed the feat/chrony-ntp-object-list branch from 6c173ea to 35a2d98 Compare October 9, 2026 23:42
Signed-off-by: Melody Lyu <tianlyu@amazon.com>
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.

1 participant