Skip to content

Commit

Permalink
console: validate disk size earlier, update error wording
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@tserong
tserong committed Nov 22, 2023
1 parent 3abd2f4 commit ebc9017
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 30 deletions.
8 changes: 4 additions & 4 deletions pkg/config/cos_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
},
}

Expand Down
49 changes: 33 additions & 16 deletions pkg/console/install_panels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/console/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/disk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions pkg/util/disk_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit ebc9017

Please sign in to comment.