From 6e7f65bc599ecbe34bfc3bb3cb4f8f035b4f6ef6 Mon Sep 17 00:00:00 2001
From: Ethan <39577870+ethanndickson@users.noreply.github.com>
Date: Wed, 20 Nov 2024 15:50:12 +1100
Subject: [PATCH] fix(cli): properly handle build log streaming during `coder
 ping` (#15600)

Closes #15584.

- The `Collecting Diagnostics` spinner now starts after the workspace
build logs (if any) have finished streaming.
- Removes network interfaces with negative MTUs from `healthsdk`
diagnostics.
- Improves the wording on diagnostics for MTUs below the 'safe' value to
indicate that direct connections may be degraded, or rendered unusable
(i.e. if every packet is dropped).
---
 cli/ping.go                      | 12 ++++++------
 codersdk/healthsdk/interfaces.go |  7 +++++--
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/cli/ping.go b/cli/ping.go
index cd3f6e3a8b..a54687cf2c 100644
--- a/cli/ping.go
+++ b/cli/ping.go
@@ -103,11 +103,6 @@ func (r *RootCmd) ping() *serpent.Command {
 			ctx, cancel := context.WithCancel(inv.Context())
 			defer cancel()
 
-			spin := spinner.New(spinner.CharSets[5], 100*time.Millisecond)
-			spin.Writer = inv.Stderr
-			spin.Suffix = pretty.Sprint(cliui.DefaultStyles.Keyword, " Collecting diagnostics...")
-			spin.Start()
-
 			notifyCtx, notifyCancel := inv.SignalNotifyContext(ctx, StopSignals...)
 			defer notifyCancel()
 
@@ -118,10 +113,15 @@ func (r *RootCmd) ping() *serpent.Command {
 				workspaceName,
 			)
 			if err != nil {
-				spin.Stop()
 				return err
 			}
 
+			// Start spinner after any build logs have finished streaming
+			spin := spinner.New(spinner.CharSets[5], 100*time.Millisecond)
+			spin.Writer = inv.Stderr
+			spin.Suffix = pretty.Sprint(cliui.DefaultStyles.Keyword, " Collecting diagnostics...")
+			spin.Start()
+
 			opts := &workspacesdk.DialAgentOptions{}
 
 			if r.verbose {
diff --git a/codersdk/healthsdk/interfaces.go b/codersdk/healthsdk/interfaces.go
index 714e1ecbdb..fe3bc032a7 100644
--- a/codersdk/healthsdk/interfaces.go
+++ b/codersdk/healthsdk/interfaces.go
@@ -68,11 +68,14 @@ func generateInterfacesReport(st *interfaces.State) (report InterfacesReport) {
 			continue
 		}
 		report.Interfaces = append(report.Interfaces, healthIface)
-		if iface.MTU < safeMTU {
+		// Some loopback interfaces on Windows have a negative MTU, which we can
+		// safely ignore in diagnostics.
+		if iface.MTU > 0 && iface.MTU < safeMTU {
 			report.Severity = health.SeverityWarning
 			report.Warnings = append(report.Warnings,
 				health.Messagef(health.CodeInterfaceSmallMTU,
-					"Network interface %s has MTU %d (less than %d), which may degrade the quality of direct connections", iface.Name, iface.MTU, safeMTU),
+					"Network interface %s has MTU %d (less than %d), which may degrade the quality of direct "+
+						"connections or render them unusable.", iface.Name, iface.MTU, safeMTU),
 			)
 		}
 	}