Skip to content

Remove references to archived pupmod-simp-ntpd - #380

Merged
silug merged 4 commits into
masterfrom
issue_374-remove-ntpd-ref
Jul 29, 2026
Merged

Remove references to archived pupmod-simp-ntpd#380
silug merged 4 commits into
masterfrom
issue_374-remove-ntpd-ref

Conversation

@hcaballero2

Copy link
Copy Markdown
Contributor

Removes the dead ntpd references now that pupmod-simp-ntpd is archived and EL7 is dropped: the simp/ntpd metadata dependency and fixture, the EL7 ntpd expectation in server_spec.rb (the class list is now unconditionally chrony), the --ntpd doc example, and the kernel-2/3 ntpdate branch in bootstrap_simp_client (EL8+ always takes the existing chronyd -q path; unit specs updated and passing, 61 examples / 0 failures). Per the issue, simp_client_bootstrap's ntp_servers parameter is kept but its default lookup consolidates onto simp_options::ntp::servers, and the acceptance hieradata key is renamed to match. Pairs with the companion simp_options::ntpd cleanup (simp_options#126).

Fixes #374

🤖 Generated with Claude Code

hcaballero2 and others added 3 commits July 28, 2026 14:57
…op EL7 ntpdate path

- server_spec.rb: collapse the OS-conditional class list to plain ['chrony']
  (the bare 'case' left by the EL7 branch removal was a syntax error)
- simp_client_bootstrap: keep the ntp_servers parameter, consolidating its
  default lookup onto simp_options::ntp::servers per #374 (the kickstart
  templates still consume @ntp_servers)
- default_hiera.yaml: rename the acceptance key to simp_options::ntp::servers
  instead of dropping time-sync coverage
- bootstrap_simp_client: remove the dead kernel-2/3 ntpdate branch; EL8+
  always uses chronyd -q, and the unit specs now assert that path
- drop stray CLAUDE.md symlink from the branch

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The RELENG check requires a version bump beyond the 8.1.0 tag for the
changed source files. Also regenerate the affected REFERENCE.md sections
and update AGENTS.md to match the removed ntpd references.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hcaballero2
hcaballero2 force-pushed the issue_374-remove-ntpd-ref branch from 2d118ae to eda6349 Compare July 28, 2026 20:00
@silug

silug commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review

Overview

Removes all traces of the archived pupmod-simp-ntpd module: the simp/ntpd metadata dependency and test fixture, the EL7-only ntpd expectation in server_spec.rb, the --ntpd knockout doc example, and the kernel-2/3 ntpdate branch in bootstrap_simp_client. The kickstart client's ntp_servers default lookup moves from simp_options::ntpd::servers to simp_options::ntp::servers. Version bumped to 8.2.0 with a CHANGELOG entry. Fixes #374.

Verification performed

  • No leftovers: git grep -i ntpd on the PR branch returns zero hits outside the CHANGELOG. The cleanup is complete — manifests, data, templates, specs, fixtures, and docs are all covered.
  • Unit tests pass: ran spec/unit/bootstrap_simp_client_spec.rb against the PR's script and spec locally — 62 examples, 0 failures.
  • Kernel-branch removal is safe: the ntpdate path only fired on kernel 2/3 (EL≤7). With EL7 dropped from metadata.json, every supported platform (EL8+ = kernel 4.18+) already took the chronyd -q path, so this is dead-code removal with no behavior change.
  • New lookup key is valid: simp_options::ntp::servers exists in pupmod-simp-simp_options (simp_options::ntp class), and simp_options::ntpd is already marked DEPRECATED there. This is also the key the chrony wiring uses, so the consolidation is the right direction.
  • Conventions followed: REFERENCE.md regenerated alongside the manifest doc changes, AGENTS.md kept in sync, CHANGELOG entry format and date are correct.

Issues / suggestions

  1. Silent behavior change for legacy hieradata (main concern). Sites that still set only simp_options::ntpd::servers will now get ntp_servers = [], and the kickstart bootstrap silently skips the time-sync step. That step exists specifically because clock skew can break PKI cert validation during client bootstrap (the class docs say as much). The failure mode is non-fatal but quiet. Since the whole point of the PR is removing ntpd references, a fallback lookup would be counterproductive — instead, add an explicit upgrade note to the CHANGELOG entry: "users who set simp_options::ntpd::servers in hieradata must rename it to simp_options::ntp::servers."
  2. Semver consideration (minor). A lookup-key change that can silently alter deployed behavior is arguably breaking. Recent precedent in this repo (Fix simp::sysctl::ipv6: false ignored when IPv6 sysctl doesn't exist #354, (#310, #325, #366) Sysctl opt-out, ptrace_scope, mail_server opt-in #369) has shipped behavior changes as minor bumps, so 8.2.0 is defensible — but that's another reason the CHANGELOG migration note matters.
  3. Merge coordination with the companion PR. The default is default_value => [], so nothing here hard-depends on Deprecate/remove simp_options::ntpd (ntpd module archived) pupmod-simp-simp_options#126 landing first — merge order is free.

Overlap with PR #362

No semantic overlap. #362 removes chkrootkit, simp_openldap, sudosh, tcpwrappers, xinetd (and others) — it never touches ntpd, and this PR never touches any of #362's modules.

Two files are modified by both PRs (.fixtures.yml, metadata.json), but in well-separated hunks that git will merge cleanly. One coordination note: #362 currently has no version bump or CHANGELOG entry; when it adds one, it will conflict with this PR's — plan the ordering (e.g., merge this PR first, then rebase #362 onto it).

Verdict

Approve with one requested tweak: add the hieradata migration note (simp_options::ntpd::serverssimp_options::ntp::servers) to the CHANGELOG entry. Everything else is clean, complete, and verified.

🤖 Generated with Claude Code

The simp_options::ntpd::servers -> simp_options::ntp::servers lookup-key
move can silently alter deployed behavior, so treat it as a breaking
change: major version bump plus a prominent CHANGELOG migration note.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@silug
silug merged commit 581a96f into master Jul 29, 2026
32 checks passed
@github-project-automation github-project-automation Bot moved this from New to Done in Org Triage Jul 29, 2026
@silug
silug deleted the issue_374-remove-ntpd-ref branch July 29, 2026 19:56
@silug silug removed this from Org Triage Jul 29, 2026
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.

Remove references to archived pupmod-simp-ntpd

3 participants