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 <[email protected]>
harvester · Nov 27, 2023 · 9b6dd1e · 9b6dd1e
1 parent d29437e
commit 9b6dd1e
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 30 deletions.
diff --git 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
@@ -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,29 +442,22 @@ 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
 			}
 			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
@@ -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
@@ -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
@@ -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,