DLPX-96942 Replace ntpsec with chrony (appliance-build changes)#857
Merged
manoj-joseph merged 1 commit intoMay 8, 2026
Merged
Conversation
4f8e032 to
578a639
Compare
21985e3 to
0445988
Compare
0445988 to
7bcb7e4
Compare
d2d2796 to
870f6e1
Compare
prakashsurya
approved these changes
May 5, 2026
prakashsurya
left a comment
Contributor
There was a problem hiding this comment.
we hit a few ESCLs due to the migration logic for ntp to ntpsec.. I don't mean to imply there's anything wrong with the logic here, but just want to emphasize that we're a bit careful with it.. since I think that's often the more difficult part to get right.
I see there's logic to handle both ntp, and ntpsec, migrations.. so I think we're good.. but just wanted to mention it, since I got this subtly wrong last time..
nealquigley
approved these changes
May 8, 2026
870f6e1 to
4304305
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
The Delphix appliance previously used
ntpsecas its NTP daemon (and before that, the legacyntppackage). Both packages are being replaced withchrony(DLPX-96940/96941/96942), motivated by two factors:ntpsecandntprespond to NTP mode 6/7 queries, exposing sensitive system information (OS version, kernel, stratum, etc.) that can aid further attacks against the system.chronydoes not support these query modes by default, eliminating the disclosure.chronyis the default NTP daemon. Making this switch now avoids duplicating the migration effort at that time.Companion PRs
This PR is part of a set of four that must merge together:
Problem
This PR handles the upgrade path: when an appliance running
ntpsec(orntp) upgrades to a version withchrony, the existing NTP configuration must be migrated so that the user's configured NTP servers are preserved andchronyis started in the same enabled/disabled state as the outgoing NTP service.Without this migration, an appliance that had NTP enabled would silently end up with
chronymasked and no servers configured after upgrade.Solution
The
upgrade/upgrade-scripts/executescript now includes an NTP migration block that runs after the new packages are installed. It detects which of the two possible predecessor services was active and reads the corresponding config:ntpsec.servicewas enabled → reads from/etc/ntpsec/ntp.confntp.servicewas enabled → reads from/etc/ntp.confand purges thentppackage (whose removal script would otherwise leaventp.servicein a masked state)chrony.servicestays masked (handled byfix_and_migrate_services)When a source config is found, the script writes a new
/etc/chrony/chrony.confby extracting the relevant entries and translating them to chrony's format:pool <server> iburstlines are preserved as-is (chrony supports the samepooldirective)multicastclient <addr>entries are converted toserver <addr> iburst(the chrony equivalent)ntpsec-specific directives (restrict,driftfilepath) are dropped and replaced with chrony equivalents (driftfile /var/lib/chrony/chrony.drift,makestep 1.0 3,rtcsync)Finally,
chrony.serviceis unmasked and enabled.upgrade/upgrade-scripts/common.sh(fix_and_migrate_services) is also updated to maskchrony.service(instead of the removedntpsec.serviceandntpsec-rotate-stats.timer) when those services are not-found or disabled, ensuring the default disabled state is preserved across not-in-place upgrades.Note on config format: The Delphix appliance only ever writes two kinds of entries to the NTP config via
bos_mgmt.shindlpx-app-gate:pool <server> iburstandmulticastclient <addr>. This has been true for both thentpandntpseceras, so the migration covers all configurations that can exist on a Delphix-managed appliance.The remaining directives present in a typical Ubuntu ntpsec installation are package defaults, not appliance or user configuration. The table below documents each one and how chrony handles it, verified against
man chrony.conf(chrony 4.5) and the Ubuntu defaultchrony.conffrom the package:driftfile /var/lib/ntpsec/ntp.driftdriftfile /var/lib/chrony/chrony.drift(written explicitly by the migration script, matching the Ubuntu default chrony path)leapfile /usr/share/zoneinfo/leap-seconds.listleapsectz right/UTCinstead (reads leap seconds from the system timezone database). The migration script does not add this directive; for a pure NTP client, leap seconds are handled automatically via NTP server announcements.tos maxclock 11tos minclock 4 minsane 3server ntp.ubuntu.comchrony.confusespool ntp.ubuntu.com iburst maxsources 4instead of a bareserverline. Since the migration writes a newchrony.conffrom the pool entries in the original ntpsec config (which include the fourN.ubuntu.pool.ntp.orgentries), coverage is equivalent.restrict default kod nomodify nopeer noquery limitedman chrony.conf: "The default is that no clients are allowed access, i.e. chronyd operates purely as an NTP client."restrict 127.0.0.1/restrict ::1Testing Done
Migration logic (script-level test):
The NTP migration block was extracted and tested in isolation against simulated input configs, covering all meaningful scenarios:
chrony.confchanges (chrony stays masked viafix_and_migrate_services)poollines preserved verbatim,restrictand ntpsec-specific directives dropped, chrony directives (driftfile,makestep,rtcsync) added correctlymulticastclient <addr>correctly converted toserver <addr> iburstAll 14 test assertions passed.
CI builds
NtpServerMonitoringTestflakiness (fixed; see second set of runs below)Upgrade testing:
End-to-end upgrade tests were run against a VM running 2026.4.0.0-snapshot with ntpsec configured, upgrading to the build from #13897. All three migration scenarios passed on blackbox-self-service:
Long-path upgrade test (DLPX-95255):
Full upgrade path 21.0.0.0 → 2025.2.0.1 → 2026.4.0.0, exercising migration from the legacy
ntppackage:dlpx-2025.2.0.1); fixed in #9262The NTP-never-enabled path (ntp.service masked from 21.0,
chronydshould stay masked post-upgrade) was tested manually with AI assist (DLPX-95255). Upgrade performed onmj-ntp-off-21000.dlpxdc.co(21.0.0.0 → 2025.2.0.1 → 2026.4.0.0 pre-push #6498). Post-upgrade state confirmed:chronyinstalled,chrony.servicemasked,GET /service/time→ntpConfig.enabled: false.