From 5e4963cefb58dec29d7079a59589231bc023ca2a Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Tue, 21 Nov 2023 14:15:32 +1100 Subject: [PATCH] console: validate disk size earlier, update error wording This commit adds disk size validation in the calls to diskConfirm() and dataDiskConfirm(), so that as you walk through the fields you'll see potential error messages as you go, rather than having to wait until leaving the last field on the page. The error message clears if you press the up arrow to go to previous fields. This commit also changes the wording of the error message to explicitly state when the installation disk is too small. Fixes: https://github.com/harvester/harvester/issues/4734 Signed-off-by: Tim Serong --- pkg/config/cos_test.go | 2 +- pkg/console/install_panels.go | 68 +++++++++++++++++++++-------------- pkg/console/util.go | 4 +-- pkg/util/disk.go | 2 +- pkg/util/disk_test.go | 4 +-- 5 files changed, 48 insertions(+), 32 deletions(-) diff --git a/pkg/config/cos_test.go b/pkg/config/cos_test.go index 3b3bfd84a..1113d3adf 100644 --- a/pkg/config/cos_test.go +++ b/pkg/config/cos_test.go @@ -35,7 +35,7 @@ func TestCalcCosPersistentPartSize(t *testing.T) { { diskSize: 150, partitionSize: "100Gi", - err: "Disk size is too small. Minimum 250Gi is required", + err: "Installation disk size is too small. Minimum 250Gi is required", }, { diskSize: 300, diff --git a/pkg/console/install_panels.go b/pkg/console/install_panels.go index 4547270bf..ca85791e9 100644 --- a/pkg/console/install_panels.go +++ b/pkg/console/install_panels.go @@ -361,6 +361,9 @@ func addDiskPanel(c *Console) error { } persistentSizeV.Value = defaultValue } + // Save the value back to the config so it's available when we call + // validateAllDiskSizes(), which can happen before persistentSizeConfirm() + c.config.Install.PersistentPartitionSize = persistentSizeV.Value return nil } setLocation(persistentSizeV, 3) @@ -407,47 +410,54 @@ func addDiskPanel(c *Console) error { c.AddElement(diskValidatorPanel, diskValidatorV) // Helper functions - closeThisPage := func() { - c.CloseElements( - diskPanel, - dataDiskPanel, - askForceMBRPanel, - diskValidatorPanel, - diskNotePanel, - askForceMBRTitlePanel, - persistentSizePanel, - ) - } - gotoPrevPage := func(g *gocui.Gui, v *gocui.View) error { - closeThisPage() - diskConfirmed = false - return showNext(c, askCreatePanel) - } - gotoNextPage := func(g *gocui.Gui, v *gocui.View) error { + validateAllDiskSizes := func() (bool, error) { installDisk := c.config.Install.Device dataDisk := c.config.Install.DataDisk persistentSize := c.config.Install.PersistentPartitionSize if dataDisk == "" || installDisk == dataDisk { if err := validateDiskSize(installDisk, true); err != nil { - return updateValidatorMessage(err.Error()) + return false, updateValidatorMessage(err.Error()) } diskSize, err := util.GetDiskSizeBytes(c.config.Install.Device) if err != nil { - return err + return false, err } if _, err := util.ParsePartitionSize(diskSize, persistentSize); err != nil { - return updateValidatorMessage(err.Error()) + return false, updateValidatorMessage(err.Error()) } } else { if err := validateDiskSize(installDisk, false); err != nil { - return updateValidatorMessage(err.Error()) + return false, updateValidatorMessage(err.Error()) } if err := validateDataDiskSize(dataDisk); err != nil { - return updateValidatorMessage(err.Error()) + return false, updateValidatorMessage(err.Error()) } } + return true, nil; + } + closeThisPage := func() { + c.CloseElements( + diskPanel, + dataDiskPanel, + askForceMBRPanel, + diskValidatorPanel, + diskNotePanel, + askForceMBRTitlePanel, + persistentSizePanel, + ) + } + gotoPrevPage := func(g *gocui.Gui, v *gocui.View) error { + closeThisPage() + diskConfirmed = false + return showNext(c, askCreatePanel) + } + gotoNextPage := func(g *gocui.Gui, v *gocui.View) error { + // Don't proceed to the next page if disk size validation fails + if valid, err := validateAllDiskSizes(); !valid || err != nil { + return err; + } if !diskConfirmed { diskConfirmed = true @@ -479,8 +489,9 @@ func addDiskPanel(c *Console) error { c.config.Install.Device = device if len(diskOpts) > 1 { - if err := updateValidatorMessage(""); err != nil { - return err + // Show error if disk size validation fails, but allow proceeding to next field + if _, err := validateAllDiskSizes(); err != nil { + return err; } if device == dataDisk { return showNext(c, persistentSizePanel, dataDiskPanel) @@ -491,8 +502,9 @@ func addDiskPanel(c *Console) error { if err := c.setContentByName(diskNotePanel, persistentSizeNote); err != nil { return err } - if err := updateValidatorMessage(""); err != nil { - return err + // Show error if disk size validation fails, but allow proceeding to next field + if _, err := validateAllDiskSizes(); err != nil { + return err; } return showNext(c, persistentSizePanel) } @@ -525,6 +537,10 @@ func addDiskPanel(c *Console) error { if err := c.setContentByName(diskNotePanel, persistentSizeNote); err != nil { return err } + // Show error if disk size validation fails, but allow proceeding to next field + if _, err := validateAllDiskSizes(); err != nil { + return err; + } return showNext(c, persistentSizePanel) } diff --git a/pkg/console/util.go b/pkg/console/util.go index 96f1abbf4..567e7bec7 100644 --- a/pkg/console/util.go +++ b/pkg/console/util.go @@ -638,7 +638,7 @@ func validateDiskSize(devPath string, single bool) error { limit = config.MultipleDiskMinSizeGiB } if util.ByteToGi(diskSizeBytes) < uint64(limit) { - return fmt.Errorf("Disk size is too small. Minimum %dGi is required", limit) + return fmt.Errorf("Installation disk size is too small. Minimum %dGi is required", limit) } return nil @@ -650,7 +650,7 @@ func validateDataDiskSize(devPath string) error { return err } if util.ByteToGi(diskSizeBytes) < config.HardMinDataDiskSizeGiB { - return fmt.Errorf("Disk size is too small. Minimum %dGi is required", config.HardMinDataDiskSizeGiB) + return fmt.Errorf("Data disk size is too small. Minimum %dGi is required", config.HardMinDataDiskSizeGiB) } return nil diff --git a/pkg/util/disk.go b/pkg/util/disk.go index 7bb368e65..c2c68bba7 100644 --- a/pkg/util/disk.go +++ b/pkg/util/disk.go @@ -17,7 +17,7 @@ const ( func ParsePartitionSize(diskSizeBytes uint64, partitionSize string) (uint64, error) { if diskSizeBytes < MinDiskSize { - return 0, fmt.Errorf("Disk size is too small. Minimum %dGi is required", ByteToGi(MinDiskSize)) + return 0, fmt.Errorf("Installation disk size is too small. Minimum %dGi is required", ByteToGi(MinDiskSize)) } actualDiskSizeBytes := diskSizeBytes - fixedOccupiedSize diff --git a/pkg/util/disk_test.go b/pkg/util/disk_test.go index 77018531a..cf1c0dcbd 100644 --- a/pkg/util/disk_test.go +++ b/pkg/util/disk_test.go @@ -41,12 +41,12 @@ func TestParsePartitionSize(t *testing.T) { { diskSize: 100 * GiByteMultiplier, partitionSize: "50Gi", - err: "Disk size is too small. Minimum 250Gi is required", + err: "Installation disk size is too small. Minimum 250Gi is required", }, { diskSize: 249 * GiByteMultiplier, partitionSize: "50Gi", - err: "Disk size is too small. Minimum 250Gi is required", + err: "Installation disk size is too small. Minimum 250Gi is required", }, { diskSize: 2000 * GiByteMultiplier,