From 4be0fcf656a6d8c9ee8d013b743c93b98f1dc21d Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Thu, 8 Feb 2024 16:03:15 +1100 Subject: [PATCH] Add preflight check for network card speed This check ensures that the network interfaces configured during installation are all 10Gbps. If they're less than this, during interactive install, a warning message will be displayed on the network screen. For automated installs, as with other preflight checks, failure will abort the install unless `harvester.install.skipchecks=true` is set. A few oddities worth mentioning: * We can't do the network speed check upfront with the other checks because we don't know which interfaces to test until we're on the network screen (or have them from the remote config). * I've moved `c.config.SkipChecks = true` from the KeyEnter handler in addPreflightCheckPanel() to askCreateV.PreShow(). Imagine the case where the initial preflight checks all pass (so that panel isn't shown), but later the network speed check fails. In this case, SkipChecks wouldn't be set, and the installation would abort in the installV.PreShow() function. * virtio NICs when testing under libvirt don't report speed via /sys/class/net/$IFACE/speed (or, rather, the speed is set to -1) so the test is skipped in this case. Other NIC types under libvirt (e.g. e1000) all seem to be 1Gbps only. Related issue: https://github.com/harvester/harvester/issues/1154 Signed-off-by: Tim Serong --- pkg/console/install_panels.go | 74 ++++++++++++++++++------- pkg/preflight/checks.go | 49 +++++++++++++--- pkg/preflight/checks_test.go | 20 +++++++ pkg/preflight/testdata/eth0-speed-100 | 1 + pkg/preflight/testdata/eth0-speed-1000 | 1 + pkg/preflight/testdata/eth0-speed-10000 | 1 + pkg/preflight/testdata/eth0-speed-2500 | 1 + 7 files changed, 119 insertions(+), 28 deletions(-) create mode 100644 pkg/preflight/testdata/eth0-speed-100 create mode 100644 pkg/preflight/testdata/eth0-speed-1000 create mode 100644 pkg/preflight/testdata/eth0-speed-10000 create mode 100644 pkg/preflight/testdata/eth0-speed-2500 diff --git a/pkg/console/install_panels.go b/pkg/console/install_panels.go index 2b3302398..a0f9fa022 100644 --- a/pkg/console/install_panels.go +++ b/pkg/console/install_panels.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation" "github.com/harvester/harvester-installer/pkg/config" + "github.com/harvester/harvester-installer/pkg/preflight" "github.com/harvester/harvester-installer/pkg/util" "github.com/harvester/harvester-installer/pkg/version" "github.com/harvester/harvester-installer/pkg/widgets" @@ -57,6 +58,22 @@ var ( preflightWarnings []string ) +func (c *Console) doNetworkSpeedCheck(interfaces []config.NetworkInterface) (warnings []string) { + for _, iface := range interfaces { + msg, err := preflight.NetworkSpeedCheck{Dev: iface.Name}.Run() + if err != nil { + // Preflight checks that fail to run at all are + // logged, rather than killing the installer + logrus.Error(err) + continue + } + if len(msg) > 0 { + warnings = append(warnings, msg) + } + } + return +} + func (c *Console) layoutInstall(g *gocui.Gui) error { var err error once.Do(func() { @@ -705,7 +722,6 @@ func addPreflightCheckPanel(c *Console) error { go util.SleepAndReboot() return c.setContentByName(notePanel, "Installation halted. Rebooting system in 5 seconds") } - c.config.SkipChecks = true preflightCheckV.Close() return showNext(c, askCreatePanel) }, @@ -753,6 +769,11 @@ func addAskCreatePanel(c *Console) error { } askCreateV.FirstPage = true askCreateV.PreShow = func() error { + // If we're in the interactive installer at this point, it means the + // user wants the installation to succeed, regardless of whether any + // of the initial preflight checks failed, or if the later network + // speed check fails. + c.config.SkipChecks = true askCreateV.Value = c.config.Install.Mode if alreadyInstalled { return c.setContentByName(titlePanel, "Harvester already installed. Choose configuration mode") @@ -1269,7 +1290,12 @@ func addNetworkPanel(c *Console) error { return err } bondNoteV.Focus = false - return c.setContentByName(bondNotePanel, bondNote) + bondNoteMsg := bondNote + // This is just for display purposes on the network screen + for _, warning := range c.doNetworkSpeedCheck(mgmtNetwork.Interfaces) { + bondNoteMsg += "\n" + warning + } + return c.setContentByName(bondNotePanel, bondNoteMsg) } updateValidatorMessage := func(msg string) error { @@ -1965,25 +1991,6 @@ func addInstallPanel(c *Console) error { installV := widgets.NewPanel(c.Gui, installPanel) installV.PreShow = func() error { go func() { - if !alreadyInstalled && len(preflightWarnings) > 0 { - if c.config.SkipChecks { - // User is happy to skip checks so let installation proceed, - // but still log the warning messages (this happens for both - // interactive and automatic/PXE install) - for _, warning := range preflightWarnings { - logrus.Warning(warning) - } - } else { - // Checks were not explicitly skipped, fail the install - // (this will happen when PXE booted if checks fail and - // you don't set harvester.install.skipcheck=true) - for _, warning := range preflightWarnings { - logrus.Error(warning) - printToPanel(c.Gui, warning, installPanel) - } - return - } - } // in alreadyInstalled mode and auto configuration, the network is not available if alreadyInstalled && c.config.Automatic == true && c.config.ManagementInterface.Method == "dhcp" { configureInstallModeDHCP(c) @@ -2050,6 +2057,31 @@ func addInstallPanel(c *Console) error { } c.config.ManagementInterface.Interfaces = tmpInterfaces + if !alreadyInstalled { + // Have to handle preflight warnings here because we can't check + // the NIC speed until we've got the correct set of interfaces. + preflightWarnings = append(preflightWarnings, c.doNetworkSpeedCheck(c.config.ManagementInterface.Interfaces)...) + if len(preflightWarnings) > 0 { + if c.config.SkipChecks { + // User is happy to skip checks so let installation proceed, + // but still log the warning messages (this happens for both + // interactive and automatic/PXE install) + for _, warning := range preflightWarnings { + logrus.Warning(warning) + } + } else { + // Checks were not explicitly skipped, fail the install + // (this will happen when PXE booted if checks fail and + // you don't set harvester.install.skipcheck=true) + for _, warning := range preflightWarnings { + logrus.Error(warning) + printToPanel(c.Gui, warning, installPanel) + } + return + } + } + } + // We need ForceGPT because cOS only supports ForceGPT (--force-gpt) flag, not ForceMBR! c.config.ForceGPT = !c.config.ForceMBR diff --git a/pkg/preflight/checks.go b/pkg/preflight/checks.go index 7104f3f2e..54fd930e6 100644 --- a/pkg/preflight/checks.go +++ b/pkg/preflight/checks.go @@ -14,17 +14,20 @@ import ( const ( // Constants here from Hardware Requirements in the documentaiton // https://docs.harvesterhci.io/v1.3/install/requirements/#hardware-requirements - MinCPUTest = 8 - MinCPUProd = 16 - MinMemoryTest = 32 - MinMemoryProd = 64 + MinCPUTest = 8 + MinCPUProd = 16 + MinMemoryTest = 32 + MinMemoryProd = 64 + MinNetworkGbpsTest = 1 + MinNetworkGbpsProd = 10 ) var ( // So that we can fake this stuff up for unit tests - execCommand = exec.Command - procMemInfo = "/proc/meminfo" - devKvm = "/dev/kvm" + execCommand = exec.Command + procMemInfo = "/proc/meminfo" + devKvm = "/dev/kvm" + sysClassNetDevSpeed = "/sys/class/net/%s/speed" ) // The Run() method of a preflight.Check returns a string. If the string @@ -39,6 +42,9 @@ type CPUCheck struct{} type MemoryCheck struct{} type VirtCheck struct{} type KVMHostCheck struct{} +type NetworkSpeedCheck struct { + Dev string +} func (c CPUCheck) Run() (msg string, err error) { out, err := execCommand("/usr/bin/nproc", "--all").Output() @@ -129,3 +135,32 @@ func (c KVMHostCheck) Run() (msg string, err error) { } return } + +func (c NetworkSpeedCheck) Run() (msg string, err error) { + speedPath := fmt.Sprintf(sysClassNetDevSpeed, c.Dev) + out, err := os.ReadFile(speedPath) + if err != nil { + return + } + speedMbps, _ := strconv.Atoi(strings.TrimSpace(string(out))) + if speedMbps < 1 { + // speedMbps will be 0 if strconv.Atoi fails for some reason, + // or -1 (if you can believe that) when using virtio NICs when + // testing under virtualization. + err = fmt.Errorf("unable to determine NIC speed from %s (got %d)", speedPath, speedMbps) + return + } + // We need floats because 2.5Gbps ethernet is a thing. + var speedGbps float32 = float32(speedMbps) / 1000 + if speedGbps < MinNetworkGbpsTest { + // Does anyone even _have_ < 1Gbps networking kit anymore? + // Still, it's theoretically possible someone could have messed + // up their switch config and be running 100Mbps... + msg = fmt.Sprintf("Link speed of %s is only %dMpbs. Harvester requires at least %dGbps for testing and %dGbps for production use.", + c.Dev, speedMbps, MinNetworkGbpsTest, MinNetworkGbpsProd) + } else if speedGbps < MinNetworkGbpsProd { + msg = fmt.Sprintf("Link speed of %s is %gGbps. Harvester requires at least %dGbps for production use.", + c.Dev, speedGbps, MinNetworkGbpsProd) + } + return +} diff --git a/pkg/preflight/checks_test.go b/pkg/preflight/checks_test.go index 9018ca5f4..d856395b7 100644 --- a/pkg/preflight/checks_test.go +++ b/pkg/preflight/checks_test.go @@ -147,3 +147,23 @@ func TestKVMHostCheck(t *testing.T) { assert.Equal(t, expectedOutput, msg) } } + +func TestNetworkSpeedCheck(t *testing.T) { + defaultSysClassNetDevSpeed := sysClassNetDevSpeed + defer func() { sysClassNetDevSpeed = defaultSysClassNetDevSpeed }() + + expectedOutputs := map[string]string{ + "./testdata/%s-speed-100": "Link speed of eth0 is only 100Mpbs. Harvester requires at least 1Gbps for testing and 10Gbps for production use.", + "./testdata/%s-speed-1000": "Link speed of eth0 is 1Gbps. Harvester requires at least 10Gbps for production use.", + "./testdata/%s-speed-2500": "Link speed of eth0 is 2.5Gbps. Harvester requires at least 10Gbps for production use.", + "./testdata/%s-speed-10000": "", + } + + check := NetworkSpeedCheck{"eth0"} + for file, expectedOutput := range expectedOutputs { + sysClassNetDevSpeed = file + msg, err := check.Run() + assert.Nil(t, err) + assert.Equal(t, expectedOutput, msg) + } +} diff --git a/pkg/preflight/testdata/eth0-speed-100 b/pkg/preflight/testdata/eth0-speed-100 new file mode 100644 index 000000000..29d6383b5 --- /dev/null +++ b/pkg/preflight/testdata/eth0-speed-100 @@ -0,0 +1 @@ +100 diff --git a/pkg/preflight/testdata/eth0-speed-1000 b/pkg/preflight/testdata/eth0-speed-1000 new file mode 100644 index 000000000..83b33d238 --- /dev/null +++ b/pkg/preflight/testdata/eth0-speed-1000 @@ -0,0 +1 @@ +1000 diff --git a/pkg/preflight/testdata/eth0-speed-10000 b/pkg/preflight/testdata/eth0-speed-10000 new file mode 100644 index 000000000..5caff40c4 --- /dev/null +++ b/pkg/preflight/testdata/eth0-speed-10000 @@ -0,0 +1 @@ +10000 diff --git a/pkg/preflight/testdata/eth0-speed-2500 b/pkg/preflight/testdata/eth0-speed-2500 new file mode 100644 index 000000000..1a1e818a7 --- /dev/null +++ b/pkg/preflight/testdata/eth0-speed-2500 @@ -0,0 +1 @@ +2500