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: harvester/harvester#4734 Signed-off-by: Tim Serong <[email protected]>
harvester · Nov 21, 2023 · 5e4963c · 5e4963c
1 parent 3abd2f4
commit 5e4963c
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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,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
@@ -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,