From c8898990fb1f9cac4895074e936178385ce8201c Mon Sep 17 00:00:00 2001 From: Alexander Jung Date: Thu, 18 Apr 2024 16:07:34 +0200 Subject: [PATCH 1/2] refactor: Remove return error, consume multiple strings, no `\n` This small refactor performs a number of actions simultaneously: 1. Removes the unnecessary `error` being returned from `Consume` which was ultimately never used by relevant consumers, resulting unnecessary error handling. 2. Updates the `Consume` prototype to accept multiple lines at once; 3. Employs the new-line at the consumer-level, since all other instances were always appending the newline itself. This improves the code quality. 4. Subsequent to 3, appropraitely discard the the newline which has been retrieved via the `ReadBytes` from the `peekAndRed` of the internal log tail package. Signed-off-by: Alexander Jung --- internal/cli/kraft/cloud/instance/logs/logs.go | 5 +---- internal/cli/kraft/logs/colorful_consumer.go | 16 ++++++++-------- internal/cli/kraft/logs/logs.go | 9 ++------- internal/logtail/logtail.go | 2 +- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/internal/cli/kraft/cloud/instance/logs/logs.go b/internal/cli/kraft/cloud/instance/logs/logs.go index 789c25766..093283afd 100644 --- a/internal/cli/kraft/cloud/instance/logs/logs.go +++ b/internal/cli/kraft/cloud/instance/logs/logs.go @@ -158,10 +158,7 @@ func Logs(ctx context.Context, opts *LogOptions, args ...string) error { return case line, ok := <-logChan: if ok { - if err := consumer.Consume(fmt.Sprintf("%s\n", line)); err != nil { - errGroup = append(errGroup, err) - return - } + consumer.Consume(line) } } } diff --git a/internal/cli/kraft/logs/colorful_consumer.go b/internal/cli/kraft/logs/colorful_consumer.go index eedbfc1aa..50695b223 100644 --- a/internal/cli/kraft/logs/colorful_consumer.go +++ b/internal/cli/kraft/logs/colorful_consumer.go @@ -13,7 +13,7 @@ import ( ) type LogConsumer interface { - Consume(line string) error + Consume(line ...string) } type ColorfulConsumer struct { @@ -40,12 +40,12 @@ func NewColorfulConsumer(streams *iostreams.IOStreams, color bool, prefix string } // Consume implements logConsumer -func (c *ColorfulConsumer) Consume(s string) error { - if c.prefix != "" { - s = fmt.Sprintf("%s | %s", c.color(c.prefix), s) - } - - fmt.Fprint(c.streams.Out, s) +func (c *ColorfulConsumer) Consume(strs ...string) { + for _, s := range strs { + if c.prefix != "" { + s = fmt.Sprintf("%s | %s", c.color(c.prefix), s) + } - return nil + fmt.Fprintf(c.streams.Out, "%s\n", s) + } } diff --git a/internal/cli/kraft/logs/logs.go b/internal/cli/kraft/logs/logs.go index 5dce5a322..17bf6d296 100644 --- a/internal/cli/kraft/logs/logs.go +++ b/internal/cli/kraft/logs/logs.go @@ -193,9 +193,7 @@ func (opts *LogOptions) Run(ctx context.Context, args []string) error { } else { scanner := bufio.NewScanner(fd) for scanner.Scan() { - if err := consumer.Consume(scanner.Text() + "\n"); err != nil { - errGroup = append(errGroup, err) - } + consumer.Consume(scanner.Text()) } } } @@ -285,10 +283,7 @@ loop: // Wait on either channel select { case line := <-logs: - if err := consumer.Consume(line); err != nil { - log.G(ctx).Errorf("could not consume log line: %v", err) - return err - } + consumer.Consume(line) case err := <-errs: eof = true diff --git a/internal/logtail/logtail.go b/internal/logtail/logtail.go index f19999115..2a3973e5b 100644 --- a/internal/logtail/logtail.go +++ b/internal/logtail/logtail.go @@ -117,7 +117,7 @@ func peekAndRead(file *os.File, reader *bufio.Reader, logs *chan string, errs *c } if len(s) > discarded { - *logs <- string(s[discarded:]) + *logs <- string(s[discarded : len(s)-1]) } return false From 9a1f49dc00ebe05c0a0665da87f56c79142a1e87 Mon Sep 17 00:00:00 2001 From: Alexander Jung Date: Thu, 18 Apr 2024 16:21:14 +0200 Subject: [PATCH 2/2] feat(logs): Add helpful troubleshoot message for fatal instances Signed-off-by: Alexander Jung --- .../cli/kraft/cloud/instance/logs/logs.go | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/internal/cli/kraft/cloud/instance/logs/logs.go b/internal/cli/kraft/cloud/instance/logs/logs.go index 093283afd..87294ee3c 100644 --- a/internal/cli/kraft/cloud/instance/logs/logs.go +++ b/internal/cli/kraft/cloud/instance/logs/logs.go @@ -15,6 +15,7 @@ import ( "github.com/spf13/cobra" kraftcloud "sdk.kraft.cloud" + kcinstances "sdk.kraft.cloud/instances" "kraftkit.sh/cmdfactory" "kraftkit.sh/config" @@ -154,7 +155,33 @@ func Logs(ctx context.Context, opts *LogOptions, args ...string) error { case <-ctx.Done(): return case err := <-errChan: - errGroup = append(errGroup, err) + if err != nil { + errGroup = append(errGroup, err) + } + + resp, err := opts.Client.Instances().WithMetro(opts.Metro).Get(ctx, instance) + if err != nil { + errGroup = append(errGroup, err) + return + } + + inst, err := resp.FirstOrErr() + if err != nil { + errGroup = append(errGroup, err) + } + + if inst.StopCodeReason() != kcinstances.StopCodeReasonOK { + message := []string{ + "", + "It looks like the instance exited fatally. To see more details about why, run:", + "", + fmt.Sprintf("\tkraft cloud instance get %s", inst.Name), + "", + } + + consumer.Consume(message...) + } + return case line, ok := <-logChan: if ok {