feat(commands): retire talm dmesg ahead of upstream + merge test plans#207
Conversation
…rs to logs kernel Upstream Talos signaled retirement of the `dmesg` command in siderolabs/talos#13333 ("Kernel logs are available with `talosctl logs kernel` which supports `--tail=N`, we will probably retire `dmesg` command"). The current wrapper-side cushion for the `--tail=N` ParseBool footgun points operators at a pipe workaround that becomes obsolete the moment upstream removes the surface — and `logs kernel --tail=N` is the better target right now anyway, because it matches the line-count semantics operators reach for. Removes `talm dmesg` proactively: - Adds `dmesg` to `excludedCommands` in the talosctl-wrapper init so the upstream `dmesg` command is no longer imported. - Replaces the cushion in `pkg/commands/dmesg_handler.go` with a hidden migration stub. Every `talm dmesg ...` invocation errors with "talm dmesg has been removed" plus a hint pointing at `talm logs kernel --tail=N --nodes <node>` and a reference to siderolabs/talos#13333. - `DisableFlagParsing` on the stub lets operator-supplied flags pass through to RunE so the hint surfaces regardless of what arguments accompanied the call. - Adds `dmesg` to `skipConfigCommands` in main.go so the migration hint fires even when the operator runs the command outside a talm project (mirrors the init / completion treatment). The stub is `Hidden: true` so operators reading `talm --help` don't see a retired command as available. Existing scripts that call `talm dmesg <flags>` get an immediate, actionable migration nudge instead of a silent break on the next Talos bump. Contract pins (TestDmesgRemoved_StubSurfacesMigrationHint, TestDmesgExcludedFromTalosctlWrap): - exactly one `dmesg` entry registered (the stub, not the upstream wrap), - stub is hidden + always-erroring, - error names the migration target (logs kernel + --tail). Test plan (docs/manual-test-plan.md): - I0-1 read-only command sweep: `dmesg --tail` replaced with `logs kernel --tail 3` so the bake hits the new supported path. - I0-1a new entry pinning the migration-stub UX (both bare-invocation and `--tail` invocations error with the hint; stub is hidden from --help; cwd-independent). - H2b regression anchor for reset: `dmesg | grep ephemeral` swapped for `logs kernel | grep ephemeral`. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two test plans (manual-test-plan.md + apply-safety-gates-test-plan.md) were drifting independently and the operator entry-point was unclear: the main plan smoke-tested apply at section C and forwarded "go read the other file" for the per-gate matrix. Fold the safety-gate matrix into manual-test-plan.md as a new Section C-safety placed right after the C. Apply smoke-test section. Each Phase (1 / 2A / 2B / 2C) becomes a subsection (### Phase X), with the Build-under-test boilerplate dropped (already covered by the main plan's "How to use this plan" block) and the Real-Talos-validation / Implementation-health / Known-limitations tail sections preserved. Also sweeps `/tmp/talm-safety` references everywhere in the docs to bare `talm` — the binary convention assumed a personal build path that made no sense in a public test plan. Build instructions now point at `go install ./` or a local build with PATH export. The separate `docs/apply-safety-gates-test-plan.md` file is removed via `git rm`. The cross-reference in section C and the top-level intro line are rewritten to point at the new in-document anchor. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR retires the ChangesDocumentation and Test Plan Updates
Dmesg Command Retirement and Migration Stub
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request retires the dmesg command in favor of logs kernel, aligning with upstream Talos changes. The dmesg command is replaced by a hidden stub that provides an actionable migration hint and is configured to run without requiring a project configuration. Additionally, the manual test plan is updated to reflect these changes and now includes the detailed apply-time safety gate matrix. I have no feedback to provide.
Closes the dmesg footgun for good and consolidates the test plan into one file. Two independent commits in this PR; the second is pure docs refactor.
Commits
feat(commands): retire talm dmesg ahead of upstream, redirect operators to logs kernelUpstream Talos signaled retirement of the
dmesgcommand in siderolabs/talos#13333 ("Kernel logs are available withtalosctl logs kernelwhich supports--tail=N, we will probably retiredmesgcommand"). The wrapper-side cushion for the--tail=NParseBool footgun (#197) pointed operators at a pipe workaround that becomes obsolete the moment upstream removes the command — andlogs kernel --tail=Nis the better target right now anyway, because it matches the line-count semantics operators reach for.Removes
talm dmesgproactively:dmesgtoexcludedCommandsin the talosctl-wrapper init so the upstreamdmesgis no longer imported.pkg/commands/dmesg_handler.gowith a hidden cobra migration stub. Everytalm dmesg ...invocation errors with "talm dmesg has been removed" + a hint pointing attalm logs kernel --tail=N --nodes <node>for the last N lines, ortalm logs kernel --follow --nodes <node>to stream new messages.dmesgtoskipConfigCommandsso the migration hint fires even outside a talm project.Hidden: true(drops fromtalm --help) and usesDisableFlagParsing: true(any operator-supplied flags pass through to RunE).Contract pins:
TestDmesgRemoved_StubSurfacesMigrationHint,TestDmesgExcludedFromTalosctlWrap,TestSkipConfigCommands/dmesg.docs: merge apply-safety-gates checklist into the manual test plandocs/manual-test-plan.mdanddocs/apply-safety-gates-test-plan.mdwere drifting independently. Fold the safety-gate matrix into the main plan as a new Section C-safety. Drop the separate file. Also sweep/tmp/talm-safetybinary references → baretalm(assumes PATH); build instructions now usego install ./or local build + PATH export.Test plan
docs/manual-test-plan.md:dmesg --tail→logs kernel --tail 3in the read-only command sweep.--tailinvocations error with the hint; hidden from --help; cwd-independent).dmesg | grep ephemeral→logs kernel | grep ephemeral.Validation
go build ./...cleango test ./... -count=1all greengolangci-lint run ./...0 issuestalm dmesg --tail=3,talm dmesg --nodes 1.2.3.4,talm dmesg --help,talm dmesgoutside a Chart.yaml dir — all four exit 1 with the migration hint;talm --helpdoes not list dmesg.Summary by CodeRabbit
Documentation
Chores
talm dmesgcommand; users should migrate totalm logs kernelfor kernel log access.