refactor : Use strongunits.MiB / strongunits.GiB to store memory …

…size (#1642) Currently we're using memory values in `uint64` variable. This works but sometimes it can be confusing to figure out what unit we're using if variable is not named correctly. Instead, we can rely on strongunits types for byte,MegaByte and GigaByte for these values. Signed-off-by: Rohan Kumar <[email protected]>
crc-org · Jan 8, 2025 · 090f992 · 090f992
1 parent cebafa4
commit 090f992
Show file tree

Hide file tree

Showing 19 changed files with 210 additions and 172 deletions.
diff --git a/cmd/crc/cmd/start.go b/cmd/crc/cmd/start.go
@@ -10,6 +10,8 @@ import (
 	"strings"
 	"text/template"
 
+	"github.com/containers/common/pkg/strongunits"
+
 	"github.com/Masterminds/semver/v3"
 	"github.com/crc-org/crc/v2/pkg/crc/cluster"
 	crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
@@ -38,7 +40,7 @@ func init() {
 	flagSet.StringP(crcConfig.Bundle, "b", constants.GetDefaultBundlePath(crcConfig.GetPreset(config)), crcConfig.BundleHelpMsg(config))
 	flagSet.StringP(crcConfig.PullSecretFile, "p", "", fmt.Sprintf("File path of image pull secret (download from %s)", constants.CrcLandingPageURL))
 	flagSet.UintP(crcConfig.CPUs, "c", constants.GetDefaultCPUs(crcConfig.GetPreset(config)), "Number of CPU cores to allocate to the instance")
-	flagSet.UintP(crcConfig.Memory, "m", constants.GetDefaultMemory(crcConfig.GetPreset(config)), "MiB of memory to allocate to the instance")
+	flagSet.UintP(crcConfig.Memory, "m", uint(constants.GetDefaultMemory(crcConfig.GetPreset(config))), "MiB of memory to allocate to the instance")
 	flagSet.UintP(crcConfig.DiskSize, "d", constants.DefaultDiskSize, "Total size in GiB of the disk used by the instance")
 	flagSet.StringP(crcConfig.NameServer, "n", "", "IPv4 address of nameserver to use for the instance")
 	flagSet.Bool(crcConfig.DisableUpdateCheck, false, "Don't check for update")
@@ -69,8 +71,8 @@ func runStart(ctx context.Context) (*types.StartResult, error) {
 
 	startConfig := types.StartConfig{
 		BundlePath:        config.Get(crcConfig.Bundle).AsString(),
-		Memory:            config.Get(crcConfig.Memory).AsUInt(),
-		DiskSize:          config.Get(crcConfig.DiskSize).AsUInt(),
+		Memory:            strongunits.MiB(config.Get(crcConfig.Memory).AsUInt()),
+		DiskSize:          strongunits.GiB(config.Get(crcConfig.DiskSize).AsUInt()),
 		CPUs:              config.Get(crcConfig.CPUs).AsUInt(),
 		NameServer:        config.Get(crcConfig.NameServer).AsString(),
 		PullSecret:        cluster.NewInteractivePullSecretLoader(config),
@@ -181,13 +183,13 @@ func (s *startResult) prettyPrintTo(writer io.Writer) error {
 }
 
 func validateStartFlags() error {
-	if err := validation.ValidateMemory(config.Get(crcConfig.Memory).AsUInt(), crcConfig.GetPreset(config)); err != nil {
+	if err := validation.ValidateMemory(strongunits.MiB(config.Get(crcConfig.Memory).AsUInt()), crcConfig.GetPreset(config)); err != nil {
 		return err
 	}
 	if err := validation.ValidateCPUs(config.Get(crcConfig.CPUs).AsUInt(), crcConfig.GetPreset(config)); err != nil {
 		return err
 	}
-	if err := validation.ValidateDiskSize(config.Get(crcConfig.DiskSize).AsUInt()); err != nil {
+	if err := validation.ValidateDiskSize(strongunits.GiB(config.Get(crcConfig.DiskSize).AsUInt())); err != nil {
 		return err
 	}
 	if err := validation.ValidateBundle(config.Get(crcConfig.Bundle).AsString(), crcConfig.GetPreset(config)); err != nil {

diff --git a/cmd/crc/cmd/status.go b/cmd/crc/cmd/status.go
@@ -9,6 +9,10 @@ import (
 	"path/filepath"
 	"text/tabwriter"
 
+	"github.com/spf13/cast"
+
+	"github.com/containers/common/pkg/strongunits"
+
 	"github.com/cheggaaa/pb/v3"
 	"github.com/crc-org/crc/v2/pkg/crc/constants"
 	"github.com/crc-org/crc/v2/pkg/crc/daemonclient"
@@ -44,14 +48,14 @@ type status struct {
 	CrcStatus            string                       `json:"crcStatus,omitempty"`
 	OpenShiftStatus      types.OpenshiftStatus        `json:"openshiftStatus,omitempty"`
 	OpenShiftVersion     string                       `json:"openshiftVersion,omitempty"`
-	DiskUsage            int64                        `json:"diskUsage,omitempty"`
-	DiskSize             int64                        `json:"diskSize,omitempty"`
-	CacheUsage           int64                        `json:"cacheUsage,omitempty"`
+	DiskUsage            strongunits.B                `json:"diskUsage,omitempty"`
+	DiskSize             strongunits.B                `json:"diskSize,omitempty"`
+	CacheUsage           strongunits.B                `json:"cacheUsage,omitempty"`
 	CacheDir             string                       `json:"cacheDir,omitempty"`
-	RAMSize              int64                        `json:"ramSize,omitempty"`
-	RAMUsage             int64                        `json:"ramUsage,omitempty"`
-	PersistentVolumeUse  int                          `json:"persistentVolumeUsage,omitempty"`
-	PersistentVolumeSize int                          `json:"persistentVolumeSize,omitempty"`
+	RAMSize              strongunits.B                `json:"ramSize,omitempty"`
+	RAMUsage             strongunits.B                `json:"ramUsage,omitempty"`
+	PersistentVolumeUse  strongunits.B                `json:"persistentVolumeUsage,omitempty"`
+	PersistentVolumeSize strongunits.B                `json:"persistentVolumeSize,omitempty"`
 	Preset               preset.Preset                `json:"preset"`
 }
 
@@ -67,8 +71,8 @@ func runWatchStatus(writer io.Writer, client *daemonclient.Client, cacheDir stri
 
 	status := getStatus(client, cacheDir)
 	// do not render RAM size/use
-	status.RAMSize = -1
-	status.RAMUsage = -1
+	status.RAMSize = 0
+	status.RAMUsage = 0
 	renderError := render(status, writer, outputFormat)
 	if renderError != nil {
 		return renderError
@@ -107,8 +111,8 @@ func runWatchStatus(writer io.Writer, client *daemonclient.Client, cacheDir stri
 			}
 		}
 
-		ramBar.SetTotal(loadResult.RAMSize)
-		ramBar.SetCurrent(loadResult.RAMUse)
+		ramBar.SetTotal(cast.ToInt64(loadResult.RAMSize))
+		ramBar.SetCurrent(cast.ToInt64(loadResult.RAMUse))
 		for i, cpuLoad := range loadResult.CPUUse {
 			cpuBars[i].SetCurrent(cpuLoad)
 		}
@@ -173,7 +177,7 @@ func getStatus(client *daemonclient.Client, cacheDir string) *status {
 		DiskSize:             clusterStatus.DiskSize,
 		RAMSize:              clusterStatus.RAMSize,
 		RAMUsage:             clusterStatus.RAMUse,
-		CacheUsage:           size,
+		CacheUsage:           strongunits.B(cast.ToUint64(size)),
 		PersistentVolumeUse:  clusterStatus.PersistentVolumeUse,
 		PersistentVolumeSize: clusterStatus.PersistentVolumeSize,
 		CacheDir:             cacheDir,
@@ -197,7 +201,7 @@ func (s *status) prettyPrintTo(writer io.Writer) error {
 
 	lines = append(lines, line{s.Preset.ForDisplay(), openshiftStatus(s)})
 
-	if s.RAMSize != -1 && s.RAMUsage != -1 {
+	if s.RAMSize != 0 && s.RAMUsage != 0 {
 		lines = append(lines, line{"RAM Usage", fmt.Sprintf(
 			"%s of %s",
 			units.HumanSize(float64(s.RAMUsage)),

diff --git a/cmd/crc/cmd/status_test.go b/cmd/crc/cmd/status_test.go
@@ -29,6 +29,8 @@ func setUpClient(t *testing.T) *mocks.Client {
 		CrcStatus:        string(state.Running),
 		OpenshiftStatus:  string(types.OpenshiftRunning),
 		OpenshiftVersion: "4.5.1",
+		RAMUse:           8_000_000_000,
+		RAMSize:          10_000_000_000,
 		DiskUse:          10_000_000_000,
 		DiskSize:         20_000_000_000,
 		Preset:           preset.OpenShift,
@@ -59,7 +61,7 @@ func TestPlainStatus(t *testing.T) {
 
 	expected := `CRC VM:          Running
 OpenShift:       Running (v4.5.1)
-RAM Usage:       0B of 0B
+RAM Usage:       8GB of 10GB
 Disk Usage:      10GB of 20GB (Inside the CRC VM)
 Cache Usage:     10kB
 Cache Directory: %s
@@ -77,6 +79,8 @@ func TestStatusWithoutPodman(t *testing.T) {
 		CrcStatus:        string(state.Running),
 		OpenshiftStatus:  string(types.OpenshiftRunning),
 		OpenshiftVersion: "4.5.1",
+		RAMUse:           15_000_000_000,
+		RAMSize:          20_000_000_000,
 		DiskUse:          10_000_000_000,
 		DiskSize:         20_000_000_000,
 		Preset:           preset.OpenShift,
@@ -89,7 +93,7 @@ func TestStatusWithoutPodman(t *testing.T) {
 
 	expected := `CRC VM:          Running
 OpenShift:       Running (v4.5.1)
-RAM Usage:       0B of 0B
+RAM Usage:       15GB of 20GB
 Disk Usage:      10GB of 20GB (Inside the CRC VM)
 Cache Usage:     10kB
 Cache Directory: %s
@@ -118,6 +122,8 @@ func TestJsonStatus(t *testing.T) {
   "diskSize": 20000000000,
   "cacheUsage": 10000,
   "cacheDir": "%s",
+  "ramSize": 10000000000,
+  "ramUsage": 8000000000,
   "preset": "openshift"
 }
 `

diff --git a/pkg/crc/api/api_client_test.go b/pkg/crc/api/api_client_test.go
@@ -7,6 +7,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/containers/common/pkg/strongunits"
+
 	apiClient "github.com/crc-org/crc/v2/pkg/crc/api/client"
 	crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
 	"github.com/crc-org/crc/v2/pkg/crc/constants"
@@ -68,10 +70,10 @@ func TestStatus(t *testing.T) {
 			CrcStatus:        "Running",
 			OpenshiftStatus:  "Running",
 			OpenshiftVersion: "4.5.1",
-			DiskUse:          int64(10000000000),
-			DiskSize:         int64(20000000000),
-			RAMUse:           int64(1000),
-			RAMSize:          int64(2000),
+			DiskUse:          strongunits.B(10000000000),
+			DiskSize:         strongunits.B(20000000000),
+			RAMUse:           strongunits.B(1000),
+			RAMSize:          strongunits.B(2000),
 			Preset:           preset.OpenShift,
 		},
 		statusResult,

diff --git a/pkg/crc/api/client/types.go b/pkg/crc/api/client/types.go
@@ -1,6 +1,7 @@
 package client
 
 import (
+	"github.com/containers/common/pkg/strongunits"
 	"github.com/crc-org/crc/v2/pkg/crc/machine/state"
 	"github.com/crc-org/crc/v2/pkg/crc/machine/types"
 	"github.com/crc-org/crc/v2/pkg/crc/preset"
@@ -23,12 +24,12 @@ type ClusterStatusResult struct {
 	CrcStatus            string
 	OpenshiftStatus      string
 	OpenshiftVersion     string `json:"OpenshiftVersion,omitempty"`
-	DiskUse              int64
-	DiskSize             int64
-	RAMUse               int64
-	RAMSize              int64
-	PersistentVolumeUse  int `json:"PersistentVolumeUse,omitempty"`
-	PersistentVolumeSize int `json:"PersistentVolumeSize,omitempty"`
+	DiskUse              strongunits.B
+	DiskSize             strongunits.B
+	RAMUse               strongunits.B
+	RAMSize              strongunits.B
+	PersistentVolumeUse  strongunits.B `json:"PersistentVolumeUse,omitempty"`
+	PersistentVolumeSize strongunits.B `json:"PersistentVolumeSize,omitempty"`
 	Preset               preset.Preset
 }
 

diff --git a/pkg/crc/api/handlers.go b/pkg/crc/api/handlers.go
@@ -4,6 +4,8 @@ import (
 	gocontext "context"
 	"net/http"
 
+	"github.com/containers/common/pkg/strongunits"
+
 	"github.com/crc-org/crc/v2/pkg/crc/api/client"
 	"github.com/crc-org/crc/v2/pkg/crc/cluster"
 	crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
@@ -121,8 +123,8 @@ func (h *Handler) Start(c *context) error {
 func getStartConfig(cfg crcConfig.Storage, args client.StartConfig) types.StartConfig {
 	return types.StartConfig{
 		BundlePath:               cfg.Get(crcConfig.Bundle).AsString(),
-		Memory:                   cfg.Get(crcConfig.Memory).AsUInt(),
-		DiskSize:                 cfg.Get(crcConfig.DiskSize).AsUInt(),
+		Memory:                   strongunits.MiB(cfg.Get(crcConfig.Memory).AsUInt()),
+		DiskSize:                 strongunits.GiB(cfg.Get(crcConfig.DiskSize).AsUInt()),
 		CPUs:                     cfg.Get(crcConfig.CPUs).AsUInt(),
 		NameServer:               cfg.Get(crcConfig.NameServer).AsString(),
 		PullSecret:               cluster.NewNonInteractivePullSecretLoader(cfg, args.PullSecretFile),

diff --git a/pkg/crc/cluster/cluster.go b/pkg/crc/cluster/cluster.go
@@ -13,6 +13,10 @@ import (
 	"strings"
 	"time"
 
+	"github.com/spf13/cast"
+
+	"github.com/containers/common/pkg/strongunits"
+
 	"github.com/crc-org/crc/v2/pkg/crc/constants"
 	"github.com/crc-org/crc/v2/pkg/crc/errors"
 	"github.com/crc-org/crc/v2/pkg/crc/logging"
@@ -64,46 +68,46 @@ func checkCertValidity(sshRunner *ssh.Runner, cert string) (bool, error) {
 }
 
 // Return size of disk, used space in bytes and the mountpoint
-func GetRootPartitionUsage(sshRunner *ssh.Runner) (int64, int64, error) {
+func GetRootPartitionUsage(sshRunner *ssh.Runner) (strongunits.B, strongunits.B, error) {
 	cmd := "df -B1 --output=size,used,target /sysroot | tail -1"
 
 	out, _, err := sshRunner.Run(cmd)
 
 	if err != nil {
-		return 0, 0, err
+		return strongunits.B(0), strongunits.B(0), err
 	}
 	diskDetails := strings.Split(strings.TrimSpace(out), " ")
 	diskSize, err := strconv.ParseInt(diskDetails[0], 10, 64)
 	if err != nil {
-		return 0, 0, err
+		return strongunits.B(0), strongunits.B(0), err
 	}
 	diskUsage, err := strconv.ParseInt(diskDetails[1], 10, 64)
 	if err != nil {
-		return 0, 0, err
+		return strongunits.B(0), strongunits.B(0), err
 	}
-	return diskSize, diskUsage, nil
+	return strongunits.B(cast.ToUint64(diskSize)), strongunits.B(cast.ToUint64(diskUsage)), nil
 }
 
 // GetRAMUsage return RAM size and RAM usage in bytes
-func GetRAMUsage(sshRunner *ssh.Runner) (int64, int64, error) {
+func GetRAMUsage(sshRunner *ssh.Runner) (strongunits.B, strongunits.B, error) {
 	cmd := "awk '/^Mem/ {print $2,$3}' <(free -b)"
 	out, _, err := sshRunner.Run(cmd)
 
 	if err != nil {
-		return 0, 0, err
+		return strongunits.B(0), strongunits.B(0), err
 	}
 
 	ramDetails := strings.Split(strings.TrimSpace(out), " ")
 	ramSize, err := strconv.ParseInt(ramDetails[0], 10, 64)
 	if err != nil {
-		return 0, 0, err
+		return strongunits.B(0), strongunits.B(0), err
 	}
 	ramUsage, err := strconv.ParseInt(ramDetails[1], 10, 64)
 	if err != nil {
-		return 0, 0, err
+		return strongunits.B(0), strongunits.B(0), err
 	}
 
-	return ramSize, ramUsage, nil
+	return strongunits.B(cast.ToUint64(ramSize)), strongunits.B(cast.ToUint64(ramUsage)), nil
 }
 
 // GetCPUUsage return CPU usage array, index correspond to CPU number, value is load % (values between 0 nad 100)
@@ -130,7 +134,7 @@ func GetCPUUsage(sshRunner *ssh.Runner) ([]int64, error) {
 
 }
 
-func GetPVCUsage(sshRunner *ssh.Runner) (int, error) {
+func GetPVCUsage(sshRunner *ssh.Runner) (strongunits.B, error) {
 	cmd := `#!/bin/bash
 mountpoints=$(lsblk --output=mountpoints | grep pvc | uniq | tr '\n' ' ')
 if [ -z "$mountpoints" ]; then
@@ -144,13 +148,13 @@ sudo df -B1 --output=size $mountpoints | awk ' { sum += $1 } END { printf "%d",
 	}
 	out = strings.TrimSpace(out)
 	if len(out) == 0 {
-		return 0, nil
+		return strongunits.B(0), nil
 	}
 	ans, err := strconv.Atoi(out)
 	if err != nil {
-		return 0, err
+		return strongunits.B(0), err
 	}
-	return ans, nil
+	return strongunits.B(cast.ToUint64(ans)), nil
 }
 
 func EnsureSSHKeyPresentInTheCluster(ctx context.Context, ocConfig oc.Config, sshPublicKeyPath string) error {

diff --git a/pkg/crc/config/settings.go b/pkg/crc/config/settings.go
@@ -156,7 +156,7 @@ func defaultCPUs(cfg Storage) uint {
 }
 
 func defaultMemory(cfg Storage) uint {
-	return constants.GetDefaultMemory(GetPreset(cfg))
+	return uint(constants.GetDefaultMemory(GetPreset(cfg)))
 }
 
 func defaultBundlePath(cfg Storage) string {

diff --git a/pkg/crc/config/settings_test.go b/pkg/crc/config/settings_test.go
@@ -5,6 +5,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/containers/common/pkg/strongunits"
+
 	"github.com/crc-org/crc/v2/pkg/crc/constants"
 	crcpreset "github.com/crc-org/crc/v2/pkg/crc/preset"
 	"github.com/crc-org/crc/v2/pkg/crc/version"
@@ -16,11 +18,12 @@ import (
 
 // override for ValidateMemory in validations.go to disable the physical memory check
 func validateMemoryNoPhysicalCheck(value interface{}, preset crcpreset.Preset) (bool, string) {
-	v, err := cast.ToUintE(value)
+	valueAsInt, err := cast.ToUintE(value)
 	if err != nil {
 		return false, fmt.Sprintf("requires integer value in MiB >= %d", constants.GetDefaultMemory(preset))
 	}
-	if v < constants.GetDefaultMemory(preset) {
+	memory := strongunits.MiB(valueAsInt)
+	if memory < constants.GetDefaultMemory(preset) {
 		return false, fmt.Sprintf("requires memory in MiB >= %d", constants.GetDefaultMemory(preset))
 	}
 	return true, ""

diff --git a/pkg/crc/config/validations.go b/pkg/crc/config/validations.go
@@ -5,6 +5,8 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/containers/common/pkg/strongunits"
+
 	"github.com/crc-org/crc/v2/pkg/crc/constants"
 	"github.com/crc-org/crc/v2/pkg/crc/network/httpproxy"
 	crcpreset "github.com/crc-org/crc/v2/pkg/crc/preset"
@@ -31,10 +33,11 @@ func validateString(value interface{}) (bool, string) {
 
 // validateDiskSize checks if provided disk size is valid in the config
 func validateDiskSize(value interface{}) (bool, string) {
-	diskSize, err := cast.ToUintE(value)
+	valueAsInt, err := cast.ToUintE(value)
 	if err != nil {
 		return false, fmt.Sprintf("could not convert '%s' to integer", value)
 	}
+	diskSize := strongunits.GiB(valueAsInt)
 	if err := validation.ValidateDiskSize(diskSize); err != nil {
 		return false, err.Error()
 	}
@@ -70,11 +73,12 @@ func validateCPUs(value interface{}, preset crcpreset.Preset) (bool, string) {
 // validateMemory checks if provided memory is valid in the config
 // It's defined as a variable so that it can be overridden in tests to disable the physical memory check
 var validateMemory = func(value interface{}, preset crcpreset.Preset) (bool, string) {
-	v, err := cast.ToUintE(value)
+	valueAsInt, err := cast.ToUintE(value)
 	if err != nil {
 		return false, fmt.Sprintf("requires integer value in MiB >= %d", constants.GetDefaultMemory(preset))
 	}
-	if err := validation.ValidateMemory(v, preset); err != nil {
+	memory := strongunits.MiB(valueAsInt)
+	if err := validation.ValidateMemory(memory, preset); err != nil {
 		return false, err.Error()
 	}
 	return true, ""