From dcebda43a375cfba5da1fb0f60fade5d515b30c1 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Wed, 12 Jun 2024 07:25:15 +0400 Subject: [PATCH] feat: add interface report to coder netcheck --- cli/netcheck.go | 15 +- cli/netcheck_test.go | 6 +- coderd/healthcheck/health/model.go | 2 + codersdk/healthsdk/healthsdk.go | 6 + codersdk/healthsdk/interfaces.go | 73 +++++++ .../healthsdk/interfaces_internal_test.go | 192 ++++++++++++++++++ docs/admin/healthcheck.md | 11 + 7 files changed, 300 insertions(+), 5 deletions(-) create mode 100644 codersdk/healthsdk/interfaces.go create mode 100644 codersdk/healthsdk/interfaces_internal_test.go diff --git a/cli/netcheck.go b/cli/netcheck.go index fb4042b600920..490ed25ce20b2 100644 --- a/cli/netcheck.go +++ b/cli/netcheck.go @@ -10,6 +10,7 @@ import ( "github.com/coder/coder/v2/coderd/healthcheck/derphealth" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/codersdk/healthsdk" "github.com/coder/coder/v2/codersdk/workspacesdk" "github.com/coder/serpent" ) @@ -34,11 +35,21 @@ func (r *RootCmd) netcheck() *serpent.Command { _, _ = fmt.Fprint(inv.Stderr, "Gathering a network report. This may take a few seconds...\n\n") - var report derphealth.Report - report.Run(ctx, &derphealth.ReportOptions{ + var derpReport derphealth.Report + derpReport.Run(ctx, &derphealth.ReportOptions{ DERPMap: connInfo.DERPMap, }) + ifReport, err := healthsdk.RunInterfacesReport() + if err != nil { + return xerrors.Errorf("failed to run interfaces report: %w", err) + } + + report := healthsdk.ClientNetcheckReport{ + DERP: healthsdk.DERPHealthReport(derpReport), + Interfaces: ifReport, + } + raw, err := json.MarshalIndent(report, "", " ") if err != nil { return err diff --git a/cli/netcheck_test.go b/cli/netcheck_test.go index 16b72beb2fd89..bf124fc77896b 100644 --- a/cli/netcheck_test.go +++ b/cli/netcheck_test.go @@ -26,13 +26,13 @@ func TestNetcheck(t *testing.T) { b := out.Bytes() t.Log(string(b)) - var report healthsdk.DERPHealthReport + var report healthsdk.ClientNetcheckReport require.NoError(t, json.Unmarshal(b, &report)) // We do not assert that the report is healthy, just that // it has the expected number of reports per region. - require.Len(t, report.Regions, 1+1) // 1 built-in region + 1 test-managed STUN region - for _, v := range report.Regions { + require.Len(t, report.DERP.Regions, 1+1) // 1 built-in region + 1 test-managed STUN region + for _, v := range report.DERP.Regions { require.Len(t, v.NodeReports, len(v.Region.Nodes)) } } diff --git a/coderd/healthcheck/health/model.go b/coderd/healthcheck/health/model.go index ce332a0fe33ad..50f0078db10b2 100644 --- a/coderd/healthcheck/health/model.go +++ b/coderd/healthcheck/health/model.go @@ -43,6 +43,8 @@ const ( CodeProvisionerDaemonsNoProvisionerDaemons Code = `EPD01` CodeProvisionerDaemonVersionMismatch Code = `EPD02` CodeProvisionerDaemonAPIMajorVersionDeprecated Code = `EPD03` + + CodeInterfaceSmallMTU = `EIF01` ) // Default docs URL diff --git a/codersdk/healthsdk/healthsdk.go b/codersdk/healthsdk/healthsdk.go index 8a00a8a3d63a6..ff7f3b08cda4a 100644 --- a/codersdk/healthsdk/healthsdk.go +++ b/codersdk/healthsdk/healthsdk.go @@ -269,3 +269,9 @@ type WorkspaceProxyReport struct { BaseReport WorkspaceProxies codersdk.RegionsResponse[codersdk.WorkspaceProxy] `json:"workspace_proxies"` } + +// @typescript-ignore ClientNetcheckReport +type ClientNetcheckReport struct { + DERP DERPHealthReport `json:"derp"` + Interfaces InterfacesReport `json:"interfaces"` +} diff --git a/codersdk/healthsdk/interfaces.go b/codersdk/healthsdk/interfaces.go new file mode 100644 index 0000000000000..380a6a71ff1ca --- /dev/null +++ b/codersdk/healthsdk/interfaces.go @@ -0,0 +1,73 @@ +package healthsdk + +import ( + "net" + + "tailscale.com/net/interfaces" + + "github.com/coder/coder/v2/coderd/healthcheck/health" +) + +// @typescript-ignore InterfacesReport +type InterfacesReport struct { + BaseReport + Interfaces []Interface `json:"interfaces"` +} + +// @typescript-ignore Interface +type Interface struct { + Name string `json:"name"` + MTU int `json:"mtu"` + Addresses []string `json:"addresses"` +} + +func RunInterfacesReport() (InterfacesReport, error) { + st, err := interfaces.GetState() + if err != nil { + return InterfacesReport{}, err + } + return generateInterfacesReport(st), nil +} + +func generateInterfacesReport(st *interfaces.State) (report InterfacesReport) { + report.Severity = health.SeverityOK + for name, iface := range st.Interface { + // macOS has a ton of random interfaces, so to keep things helpful, let's filter out any + // that: + // + // - are not enabled + // - don't have any addresses + // - have only link-local addresses (e.g. fe80:...) + if (iface.Flags & net.FlagUp) == 0 { + continue + } + addrs := st.InterfaceIPs[name] + if len(addrs) == 0 { + continue + } + var r bool + healthIface := Interface{ + Name: iface.Name, + MTU: iface.MTU, + } + for _, addr := range addrs { + healthIface.Addresses = append(healthIface.Addresses, addr.String()) + if addr.Addr().IsLinkLocalUnicast() || addr.Addr().IsLinkLocalMulticast() { + continue + } + r = true + } + if !r { + continue + } + report.Interfaces = append(report.Interfaces, healthIface) + if iface.MTU < 1378 { + report.Severity = health.SeverityWarning + report.Warnings = append(report.Warnings, + health.Messagef(health.CodeInterfaceSmallMTU, + "network interface %s has MTU %d (less than 1378), which may cause problems with direct connections", iface.Name, iface.MTU), + ) + } + } + return report +} diff --git a/codersdk/healthsdk/interfaces_internal_test.go b/codersdk/healthsdk/interfaces_internal_test.go new file mode 100644 index 0000000000000..2996c6e1f09e3 --- /dev/null +++ b/codersdk/healthsdk/interfaces_internal_test.go @@ -0,0 +1,192 @@ +package healthsdk + +import ( + "net" + "net/netip" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" + "tailscale.com/net/interfaces" + + "github.com/coder/coder/v2/coderd/healthcheck/health" +) + +func Test_generateInterfacesReport(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + state interfaces.State + severity health.Severity + expectedInterfaces []string + expectedWarnings []string + }{ + { + name: "Empty", + state: interfaces.State{}, + severity: health.SeverityOK, + expectedInterfaces: []string{}, + }, + { + name: "Normal", + state: interfaces.State{ + Interface: map[string]interfaces.Interface{ + "en0": {Interface: &net.Interface{ + MTU: 1500, + Name: "en0", + Flags: net.FlagUp, + }}, + "lo0": {Interface: &net.Interface{ + MTU: 65535, + Name: "lo0", + Flags: net.FlagUp, + }}, + }, + InterfaceIPs: map[string][]netip.Prefix{ + "en0": { + netip.MustParsePrefix("192.168.100.1/24"), + netip.MustParsePrefix("fe80::c13:1a92:3fa5:dd7e/64"), + }, + "lo0": { + netip.MustParsePrefix("127.0.0.1/8"), + netip.MustParsePrefix("::1/128"), + netip.MustParsePrefix("fe80::1/64"), + }, + }, + }, + severity: health.SeverityOK, + expectedInterfaces: []string{"en0", "lo0"}, + }, + { + name: "IgnoreDisabled", + state: interfaces.State{ + Interface: map[string]interfaces.Interface{ + "en0": {Interface: &net.Interface{ + MTU: 1300, + Name: "en0", + Flags: 0, + }}, + "lo0": {Interface: &net.Interface{ + MTU: 65535, + Name: "lo0", + Flags: net.FlagUp, + }}, + }, + InterfaceIPs: map[string][]netip.Prefix{ + "en0": {netip.MustParsePrefix("192.168.100.1/24")}, + "lo0": {netip.MustParsePrefix("127.0.0.1/8")}, + }, + }, + severity: health.SeverityOK, + expectedInterfaces: []string{"lo0"}, + }, + { + name: "IgnoreLinkLocalOnly", + state: interfaces.State{ + Interface: map[string]interfaces.Interface{ + "en0": {Interface: &net.Interface{ + MTU: 1300, + Name: "en0", + Flags: net.FlagUp, + }}, + "lo0": {Interface: &net.Interface{ + MTU: 65535, + Name: "lo0", + Flags: net.FlagUp, + }}, + }, + InterfaceIPs: map[string][]netip.Prefix{ + "en0": {netip.MustParsePrefix("fe80::1:1/64")}, + "lo0": {netip.MustParsePrefix("127.0.0.1/8")}, + }, + }, + severity: health.SeverityOK, + expectedInterfaces: []string{"lo0"}, + }, + { + name: "IgnoreNoAddress", + state: interfaces.State{ + Interface: map[string]interfaces.Interface{ + "en0": {Interface: &net.Interface{ + MTU: 1300, + Name: "en0", + Flags: net.FlagUp, + }}, + "lo0": {Interface: &net.Interface{ + MTU: 65535, + Name: "lo0", + Flags: net.FlagUp, + }}, + }, + InterfaceIPs: map[string][]netip.Prefix{ + "en0": {}, + "lo0": {netip.MustParsePrefix("127.0.0.1/8")}, + }, + }, + severity: health.SeverityOK, + expectedInterfaces: []string{"lo0"}, + }, + { + name: "SmallMTUTunnel", + state: interfaces.State{ + Interface: map[string]interfaces.Interface{ + "en0": {Interface: &net.Interface{ + MTU: 1500, + Name: "en0", + Flags: net.FlagUp, + }}, + "lo0": {Interface: &net.Interface{ + MTU: 65535, + Name: "lo0", + Flags: net.FlagUp, + }}, + "tun0": {Interface: &net.Interface{ + MTU: 1280, + Name: "tun0", + Flags: net.FlagUp, + }}, + }, + InterfaceIPs: map[string][]netip.Prefix{ + "en0": {netip.MustParsePrefix("192.168.100.1/24")}, + "tun0": {netip.MustParsePrefix("10.3.55.9/8")}, + "lo0": {netip.MustParsePrefix("127.0.0.1/8")}, + }, + }, + severity: health.SeverityWarning, + expectedInterfaces: []string{"en0", "lo0", "tun0"}, + expectedWarnings: []string{"tun0"}, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + r := generateInterfacesReport(&tc.state) + require.Equal(t, tc.severity, r.Severity) + gotInterfaces := []string{} + for _, i := range r.Interfaces { + gotInterfaces = append(gotInterfaces, i.Name) + } + slices.Sort(gotInterfaces) + slices.Sort(tc.expectedInterfaces) + require.Equal(t, tc.expectedInterfaces, gotInterfaces) + + require.Len(t, r.Warnings, len(tc.expectedWarnings), + "expected %d warnings, got %d", len(tc.expectedWarnings), len(r.Warnings)) + for _, name := range tc.expectedWarnings { + found := false + for _, w := range r.Warnings { + if strings.Contains(w.String(), name) { + found = true + break + } + } + if !found { + t.Errorf("missing warning for %s", name) + } + } + }) + } +} diff --git a/docs/admin/healthcheck.md b/docs/admin/healthcheck.md index 1b3918a3bb253..44d10dadc6862 100644 --- a/docs/admin/healthcheck.md +++ b/docs/admin/healthcheck.md @@ -328,6 +328,17 @@ version of Coder. > Note: This may be a transient issue if you are currently in the process of > updating your deployment. +### EIF01 + +_Interface with Small MTU_ + +**Problem:** One or more local interfaces have MTU smaller than 1378, which is +the minimum MTU for Coder to establish direct connections without fragmentation. + +**Solution:** Since IP fragmentation can be a source of performance problems, we +recommend you disable the interface when using Coder or +[disable direct connections](../../cli#--disable-direct-connections) + ## EUNKNOWN _Unknown Error_