From 1f08ce6848f5fd48490f4abadfb8b5c4e5e84da2 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. The persistent partition size check remains where it is, i.e. it's only done when you try to move to the next page. I couldn't figure out a neat way to incorporate this into the validateAllDiskSizes() function, which gets called before the default persistent size is set in persistentSizeV.PreShow(). This commit also changes the wording of the "disk size is too small" error messages to explicitly state *which* disk is too small, and makes a small tweak to the "partition size" error message. Signed-off-by: Tim Serong --- pkg/config/cos_test.go | 8 +++--- pkg/console/install_panels.go | 49 +++++++++++++++++++++++------------ pkg/console/util.go | 4 +-- pkg/util/disk.go | 4 +-- pkg/util/disk_test.go | 12 ++++----- 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/pkg/config/cos_test.go b/pkg/config/cos_test.go index 3b3bfd84a..6a3124079 100644 --- a/pkg/config/cos_test.go +++ b/pkg/config/cos_test.go @@ -35,22 +35,22 @@ 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, partitionSize: "153600Ki", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, { diskSize: 2000, partitionSize: "1.5Ti", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, { diskSize: 500, partitionSize: "abcd", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, } diff --git a/pkg/console/install_panels.go b/pkg/console/install_panels.go index 4547270bf..1fc061d8f 100644 --- a/pkg/console/install_panels.go +++ b/pkg/console/install_panels.go @@ -407,6 +407,24 @@ func addDiskPanel(c *Console) error { c.AddElement(diskValidatorPanel, diskValidatorV) // Helper functions + validateAllDiskSizes := func() (bool, error) { + installDisk := c.config.Install.Device + dataDisk := c.config.Install.DataDisk + + if dataDisk == "" || installDisk == dataDisk { + if err := validateDiskSize(installDisk, true); err != nil { + return false, updateValidatorMessage(err.Error()) + } + } else { + if err := validateDiskSize(installDisk, false); err != nil { + return false, updateValidatorMessage(err.Error()) + } + if err := validateDataDiskSize(dataDisk); err != nil { + return false, updateValidatorMessage(err.Error()) + } + } + return true, nil; + } closeThisPage := func() { c.CloseElements( diskPanel, @@ -424,15 +442,15 @@ func addDiskPanel(c *Console) error { 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; + } + 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()) - } - diskSize, err := util.GetDiskSizeBytes(c.config.Install.Device) if err != nil { return err @@ -440,13 +458,6 @@ func addDiskPanel(c *Console) error { if _, err := util.ParsePartitionSize(diskSize, persistentSize); err != nil { return updateValidatorMessage(err.Error()) } - } else { - if err := validateDiskSize(installDisk, false); err != nil { - return updateValidatorMessage(err.Error()) - } - if err := validateDataDiskSize(dataDisk); err != nil { - return updateValidatorMessage(err.Error()) - } } if !diskConfirmed { @@ -479,8 +490,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 +503,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 +538,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..d5e9f417e 100644 --- a/pkg/util/disk.go +++ b/pkg/util/disk.go @@ -17,12 +17,12 @@ 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 if !sizeRegexp.MatchString(partitionSize) { - return 0, fmt.Errorf("Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed") + return 0, fmt.Errorf("Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.") } size, err := strconv.ParseUint(partitionSize[:len(partitionSize)-2], 10, 64) diff --git a/pkg/util/disk_test.go b/pkg/util/disk_test.go index 77018531a..0099cd905 100644 --- a/pkg/util/disk_test.go +++ b/pkg/util/disk_test.go @@ -41,32 +41,32 @@ 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, partitionSize: "abcd", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, { diskSize: 2000 * GiByteMultiplier, partitionSize: "1Ti", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, { diskSize: 2000 * GiByteMultiplier, partitionSize: "50Ki", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, { diskSize: 2000 * GiByteMultiplier, partitionSize: "5.5", - err: "Partition size should be ended with 'Mi', 'Gi', and no dot and negative is allowed", + err: "Partition size must end with 'Mi' or 'Gi'. Decimals and negatives are not allowed.", }, { diskSize: 400 * GiByteMultiplier,