From 5d79f0652041dd44ab0ea633c0d19b7eb268de9d Mon Sep 17 00:00:00 2001 From: Stell Hub Date: Tue, 12 May 2026 18:04:43 +0800 Subject: [PATCH] Improve dmesg tail flag error Signed-off-by: Stell Hub --- pkg/commands/talosctl_wrapper.go | 68 ++++++++++++++++ pkg/commands/talosctl_wrapper_test.go | 108 ++++++++++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 pkg/commands/talosctl_wrapper_test.go diff --git a/pkg/commands/talosctl_wrapper.go b/pkg/commands/talosctl_wrapper.go index 6cc4cf3e..abb60b79 100644 --- a/pkg/commands/talosctl_wrapper.go +++ b/pkg/commands/talosctl_wrapper.go @@ -172,6 +172,11 @@ func wrapTalosCommand(cmd *cobra.Command, cmdName string) *cobra.Command { wrapUpgradeCommand(wrappedCmd, originalRunE) } + // Special handling for dmesg command + if baseCmdName == "dmesg" { + wrapDmesgCommand(wrappedCmd) + } + // Special handling for rotate-ca command if baseCmdName == "rotate-ca" { wrapRotateCACommand(wrappedCmd, originalRunE) @@ -185,6 +190,69 @@ func wrapTalosCommand(cmd *cobra.Command, cmdName string) *cobra.Command { return wrappedCmd } +func wrapDmesgCommand(cmd *cobra.Command) { + originalFlagErrorFunc := cmd.FlagErrorFunc() + cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + if isDmesgTailBoolParseError(err) { + //nolint:wrapcheck // return a replacement error so the operator sees the actionable talm hint instead of the pflag internals. + return errors.WithHint( + errors.WithHint( + errors.New("talm dmesg: --tail is a boolean toggling tail-mode for --follow, not a line count"), + dmesgTailLineCountHint(err), + ), + "to stream only new messages on a follow, run: talm dmesg --follow --tail", + ) + } + + return originalFlagErrorFunc(cmd, err) + }) +} + +func isDmesgTailBoolParseError(err error) bool { + if err == nil { + return false + } + + errString := err.Error() + + return strings.Contains(errString, `for "--tail" flag`) && strings.Contains(errString, "strconv.ParseBool") +} + +func dmesgTailLineCountHint(err error) string { + const fallbackLineCount = "N" + + lineCount := fallbackLineCount + if err != nil { + lineCount = dmesgTailLineCountFromError(err.Error()) + } + + if lineCount == fallbackLineCount { + return "for the last N lines, run: talm dmesg --nodes | tail -n N" + } + + return fmt.Sprintf("for the last %s lines, run: talm dmesg --nodes | tail -n %s", lineCount, lineCount) +} + +func dmesgTailLineCountFromError(errString string) string { + value, ok := strings.CutPrefix(errString, `invalid argument "`) + if !ok { + return "N" + } + + value, _, ok = strings.Cut(value, `" for "--tail" flag`) + if !ok || value == "" { + return "N" + } + + for _, r := range value { + if r < '0' || r > '9' { + return "N" + } + } + + return value +} + func init() { // Import all commands from talosctl package, except those in the exclusion list // Commands to exclude (these are talm-specific or should not be exposed) diff --git a/pkg/commands/talosctl_wrapper_test.go b/pkg/commands/talosctl_wrapper_test.go new file mode 100644 index 00000000..af287a48 --- /dev/null +++ b/pkg/commands/talosctl_wrapper_test.go @@ -0,0 +1,108 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "strings" + "testing" + + "github.com/cockroachdb/errors" + "github.com/spf13/cobra" +) + +func TestContract_WrapDmesgCommand_TailCountGetsActionableHint(t *testing.T) { + var tail bool + + sourceCmd := &cobra.Command{ + Use: "dmesg", + Run: func(_ *cobra.Command, _ []string) { + t.Fatal("dmesg command should not run when --tail has a non-bool value") + }, + } + sourceCmd.Flags().BoolVar(&tail, "tail", false, "specify if only new messages should be sent") + + wrappedCmd := wrapTalosCommand(sourceCmd, "dmesg") + wrappedCmd.SetArgs([]string{"--tail=3"}) + + err := wrappedCmd.Execute() + if err == nil { + t.Fatal("expected --tail=3 to fail") + } + if got := err.Error(); !strings.Contains(got, "--tail is a boolean") || strings.Contains(got, "strconv.ParseBool") { + t.Fatalf("expected actionable --tail error without pflag internals, got: %v", err) + } + + hints := strings.Join(errors.GetAllHints(err), "\n") + for _, want := range []string{ + "talm dmesg --nodes | tail -n 3", + "talm dmesg --follow --tail", + } { + if !strings.Contains(hints, want) { + t.Errorf("expected hint %q in:\n%s", want, hints) + } + } +} + +func TestContract_WrapDmesgCommand_OtherFlagErrorsStayUnchanged(t *testing.T) { + sourceCmd := &cobra.Command{ + Use: "dmesg", + Run: func(_ *cobra.Command, _ []string) { + t.Fatal("dmesg command should not run when an unknown flag is present") + }, + } + + wrappedCmd := wrapTalosCommand(sourceCmd, "dmesg") + wrappedCmd.SetArgs([]string{"--unknown"}) + + err := wrappedCmd.Execute() + if err == nil { + t.Fatal("expected unknown flag to fail") + } + if got := err.Error(); !strings.Contains(got, "unknown flag") || strings.Contains(got, "--tail is a boolean") { + t.Fatalf("expected ordinary cobra flag error, got: %v", err) + } +} + +func TestContract_DmesgTailLineCountFromError(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + { + name: "numeric", + in: `invalid argument "12" for "--tail" flag: strconv.ParseBool: parsing "12": invalid syntax`, + want: "12", + }, + { + name: "non_numeric", + in: `invalid argument "recent" for "--tail" flag: strconv.ParseBool: parsing "recent": invalid syntax`, + want: "N", + }, + { + name: "unrelated", + in: `unknown flag: --tail-lines`, + want: "N", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := dmesgTailLineCountFromError(tc.in); got != tc.want { + t.Errorf("dmesgTailLineCountFromError() = %q, want %q", got, tc.want) + } + }) + } +}