From 409dc3bfe320d5c359104ae769f7e0cb0c528f38 Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Wed, 7 Oct 2026 11:56:54 +1000 Subject: [PATCH] fix(logs): show all matched logs when --since filter specified --- cmd/uc/deploy.go | 2 +- cmd/uc/machine/logs.go | 9 ++++----- cmd/uc/service/logs.go | 10 ++++------ internal/cli/logs/logs.go | 19 +++++++++++++++--- internal/cli/logs/logs_test.go | 36 ++++++++++++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 15 deletions(-) diff --git a/cmd/uc/deploy.go b/cmd/uc/deploy.go index 4974c59c..e6914a10 100644 --- a/cmd/uc/deploy.go +++ b/cmd/uc/deploy.go @@ -305,7 +305,7 @@ const defaultFailedContainerLogsTail = 10 // UNCLOUD_FAILED_CONTAINER_LOGS_TAIL environment variable override when set and valid. func failedContainerLogsTail() int { if v := os.Getenv("UNCLOUD_FAILED_CONTAINER_LOGS_TAIL"); v != "" { - if tail, err := logs.Tail(v); err == nil && (tail == -1 || tail > 0) { + if tail, err := logs.ParseTail(v); err == nil && (tail == -1 || tail > 0) { return tail } } diff --git a/cmd/uc/machine/logs.go b/cmd/uc/machine/logs.go index 4223be54..a75f82be 100644 --- a/cmd/uc/machine/logs.go +++ b/cmd/uc/machine/logs.go @@ -69,6 +69,10 @@ func runLogs(ctx context.Context, uncli *cli.CLI, services []string, opts logs.O if err != nil { return err } + tail, err := opts.TailLines() + if err != nil { + return err + } if len(services) == 0 { services = []string{api.SystemServiceUncloud} @@ -80,11 +84,6 @@ func runLogs(ctx context.Context, uncli *cli.CLI, services []string, opts logs.O } } - tail, err := logs.Tail(opts.Tail) - if err != nil { - return err - } - c, err := uncli.ConnectCluster(ctx) if err != nil { return fmt.Errorf("connect to cluster: %w", err) diff --git a/cmd/uc/service/logs.go b/cmd/uc/service/logs.go index c196ddfd..066d68f4 100644 --- a/cmd/uc/service/logs.go +++ b/cmd/uc/service/logs.go @@ -83,6 +83,10 @@ func RunLogs(ctx context.Context, uncli *cli.CLI, args []string, opts logs.Optio if err != nil { return err } + tail, err := opts.TailLines() + if err != nil { + return err + } serviceArgs, err := logs.ParseServiceArgs(args) if err != nil { @@ -112,12 +116,6 @@ func RunLogs(ctx context.Context, uncli *cli.CLI, args []string, opts logs.Optio } } - // Parse tail option. - tail, err := logs.Tail(opts.Tail) - if err != nil { - return err - } - c, err := uncli.ConnectCluster(ctx) if err != nil { return fmt.Errorf("connect to cluster: %w", err) diff --git a/internal/cli/logs/logs.go b/internal/cli/logs/logs.go index 46bf1119..bb0773d6 100644 --- a/internal/cli/logs/logs.go +++ b/internal/cli/logs/logs.go @@ -19,6 +19,18 @@ type Options struct { Machines []string } +// TailLines resolves the default tail limit after parsing flags. An empty opts.Tail means the user +// did not specify a limit, so --since can select all matching logs without overriding an explicit --tail. +func (opts Options) TailLines() (int, error) { + if opts.Tail == "" { + if opts.Since != "" { + return -1, nil + } + return 100, nil + } + return ParseTail(opts.Tail) +} + func Flags(options *Options) *pflag.FlagSet { set := &pflag.FlagSet{} @@ -35,8 +47,9 @@ func Flags(options *Options) *pflag.FlagSet { " --since 2024-05-14T22:50:00 RFC 3339 date/time using client local timezone\n"+ " --since 2024-01-31T10:30:00Z RFC 3339 date/time in UTC\n"+ " --since 1763953966 Unix timestamp (seconds since January 1, 1970)") - set.StringVarP(&options.Tail, "tail", "n", "100", - "Show the most recent logs and limit the number of lines shown per replica. Use 'all' to show all logs.") + set.StringVarP(&options.Tail, "tail", "n", "", + "Show the most recent logs and limit the number of lines shown per replica. Use 'all' to show all logs.\n"+ + "Defaults to 100, or 'all' when --since is set.") set.StringVar(&options.Until, "until", "", "Show logs generated before the given timestamp. Accepts relative duration, RFC 3339 date, or Unix timestamp.\n"+ "See --since for examples.") @@ -46,7 +59,7 @@ func Flags(options *Options) *pflag.FlagSet { return set } -func Tail(tail string) (int, error) { +func ParseTail(tail string) (int, error) { if tail == "all" { return -1, nil } diff --git a/internal/cli/logs/logs_test.go b/internal/cli/logs/logs_test.go index f82845f4..f6963ca3 100644 --- a/internal/cli/logs/logs_test.go +++ b/internal/cli/logs/logs_test.go @@ -7,6 +7,42 @@ import ( "github.com/stretchr/testify/require" ) +func TestOptionsTailLines(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + want int + }{ + {"default", nil, 100}, + {"follow", []string{"-f"}, 100}, + {"since", []string{"--since", "1h"}, -1}, + {"since and follow", []string{"--since", "1h", "-f"}, -1}, + {"until only", []string{"--until", "1h"}, 100}, + {"time range", []string{"--since", "3h", "--until", "1h"}, -1}, + {"explicit default with since", []string{"--since", "1h", "--tail", "100"}, 100}, + {"explicit limit with since", []string{"--since", "1h", "-n", "20"}, 20}, + {"explicit limit before since", []string{"-n", "20", "--since", "1h"}, 20}, + {"explicit all", []string{"--tail", "all"}, -1}, + {"explicit zero with since", []string{"--since", "1h", "-n", "0", "-f"}, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var options Options + require.NoError(t, Flags(&options).Parse(tt.args)) + tail, err := options.TailLines() + require.NoError(t, err) + assert.Equal(t, tt.want, tail) + }) + } + + var options Options + require.NoError(t, Flags(&options).Parse([]string{"--since", "1h", "--tail", "invalid"})) + _, err := options.TailLines() + require.ErrorContains(t, err, "invalid --tail value") +} + func TestParseServiceArgs(t *testing.T) { t.Parallel()