Skip to content

Commit 9b64c02

Browse files
rohanKanojiapraveenkumar
rohanKanojia
authored and
praveenkumar
committed
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]>
1 parent fdbf400 commit 9b64c02

19 files changed

+211
-172
lines changed

‎cmd/crc/cmd/start.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"strings"
1111
"text/template"
1212

13+
"github.com/containers/common/pkg/strongunits"
14+
1315
"github.com/Masterminds/semver/v3"
1416
"github.com/crc-org/crc/v2/pkg/crc/cluster"
1517
crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
@@ -38,7 +40,7 @@ func init() {
3840
flagSet.StringP(crcConfig.Bundle, "b", constants.GetDefaultBundlePath(crcConfig.GetPreset(config)), crcConfig.BundleHelpMsg(config))
3941
flagSet.StringP(crcConfig.PullSecretFile, "p", "", fmt.Sprintf("File path of image pull secret (download from %s)", constants.CrcLandingPageURL))
4042
flagSet.UintP(crcConfig.CPUs, "c", constants.GetDefaultCPUs(crcConfig.GetPreset(config)), "Number of CPU cores to allocate to the instance")
41-
flagSet.UintP(crcConfig.Memory, "m", constants.GetDefaultMemory(crcConfig.GetPreset(config)), "MiB of memory to allocate to the instance")
43+
flagSet.UintP(crcConfig.Memory, "m", uint(constants.GetDefaultMemory(crcConfig.GetPreset(config))), "MiB of memory to allocate to the instance")
4244
flagSet.UintP(crcConfig.DiskSize, "d", constants.DefaultDiskSize, "Total size in GiB of the disk used by the instance")
4345
flagSet.StringP(crcConfig.NameServer, "n", "", "IPv4 address of nameserver to use for the instance")
4446
flagSet.Bool(crcConfig.DisableUpdateCheck, false, "Don't check for update")
@@ -69,8 +71,8 @@ func runStart(ctx context.Context) (*types.StartResult, error) {
6971

7072
startConfig := types.StartConfig{
7173
BundlePath: config.Get(crcConfig.Bundle).AsString(),
72-
Memory: config.Get(crcConfig.Memory).AsUInt(),
73-
DiskSize: config.Get(crcConfig.DiskSize).AsUInt(),
74+
Memory: strongunits.MiB(config.Get(crcConfig.Memory).AsUInt()),
75+
DiskSize: strongunits.GiB(config.Get(crcConfig.DiskSize).AsUInt()),
7476
CPUs: config.Get(crcConfig.CPUs).AsUInt(),
7577
NameServer: config.Get(crcConfig.NameServer).AsString(),
7678
PullSecret: cluster.NewInteractivePullSecretLoader(config),
@@ -181,13 +183,13 @@ func (s *startResult) prettyPrintTo(writer io.Writer) error {
181183
}
182184

183185
func validateStartFlags() error {
184-
if err := validation.ValidateMemory(config.Get(crcConfig.Memory).AsUInt(), crcConfig.GetPreset(config)); err != nil {
186+
if err := validation.ValidateMemory(strongunits.MiB(config.Get(crcConfig.Memory).AsUInt()), crcConfig.GetPreset(config)); err != nil {
185187
return err
186188
}
187189
if err := validation.ValidateCPUs(config.Get(crcConfig.CPUs).AsUInt(), crcConfig.GetPreset(config)); err != nil {
188190
return err
189191
}
190-
if err := validation.ValidateDiskSize(config.Get(crcConfig.DiskSize).AsUInt()); err != nil {
192+
if err := validation.ValidateDiskSize(strongunits.GiB(config.Get(crcConfig.DiskSize).AsUInt())); err != nil {
191193
return err
192194
}
193195
if err := validation.ValidateBundle(config.Get(crcConfig.Bundle).AsString(), crcConfig.GetPreset(config)); err != nil {

‎cmd/crc/cmd/status.go

+17-13
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import (
99
"path/filepath"
1010
"text/tabwriter"
1111

12+
"github.com/spf13/cast"
13+
14+
"github.com/containers/common/pkg/strongunits"
15+
1216
"github.com/cheggaaa/pb/v3"
1317
"github.com/crc-org/crc/v2/pkg/crc/constants"
1418
"github.com/crc-org/crc/v2/pkg/crc/daemonclient"
@@ -44,14 +48,14 @@ type status struct {
4448
CrcStatus string `json:"crcStatus,omitempty"`
4549
OpenShiftStatus types.OpenshiftStatus `json:"openshiftStatus,omitempty"`
4650
OpenShiftVersion string `json:"openshiftVersion,omitempty"`
47-
DiskUsage int64 `json:"diskUsage,omitempty"`
48-
DiskSize int64 `json:"diskSize,omitempty"`
49-
CacheUsage int64 `json:"cacheUsage,omitempty"`
51+
DiskUsage strongunits.B `json:"diskUsage,omitempty"`
52+
DiskSize strongunits.B `json:"diskSize,omitempty"`
53+
CacheUsage strongunits.B `json:"cacheUsage,omitempty"`
5054
CacheDir string `json:"cacheDir,omitempty"`
51-
RAMSize int64 `json:"ramSize,omitempty"`
52-
RAMUsage int64 `json:"ramUsage,omitempty"`
53-
PersistentVolumeUse int `json:"persistentVolumeUsage,omitempty"`
54-
PersistentVolumeSize int `json:"persistentVolumeSize,omitempty"`
55+
RAMSize strongunits.B `json:"ramSize,omitempty"`
56+
RAMUsage strongunits.B `json:"ramUsage,omitempty"`
57+
PersistentVolumeUse strongunits.B `json:"persistentVolumeUsage,omitempty"`
58+
PersistentVolumeSize strongunits.B `json:"persistentVolumeSize,omitempty"`
5559
Preset preset.Preset `json:"preset"`
5660
}
5761

@@ -67,8 +71,8 @@ func runWatchStatus(writer io.Writer, client *daemonclient.Client, cacheDir stri
6771

6872
status := getStatus(client, cacheDir)
6973
// do not render RAM size/use
70-
status.RAMSize = -1
71-
status.RAMUsage = -1
74+
status.RAMSize = 0
75+
status.RAMUsage = 0
7276
renderError := render(status, writer, outputFormat)
7377
if renderError != nil {
7478
return renderError
@@ -107,8 +111,8 @@ func runWatchStatus(writer io.Writer, client *daemonclient.Client, cacheDir stri
107111
}
108112
}
109113

110-
ramBar.SetTotal(loadResult.RAMSize)
111-
ramBar.SetCurrent(loadResult.RAMUse)
114+
ramBar.SetTotal(cast.ToInt64(loadResult.RAMSize))
115+
ramBar.SetCurrent(cast.ToInt64(loadResult.RAMUse))
112116
for i, cpuLoad := range loadResult.CPUUse {
113117
cpuBars[i].SetCurrent(cpuLoad)
114118
}
@@ -173,7 +177,7 @@ func getStatus(client *daemonclient.Client, cacheDir string) *status {
173177
DiskSize: clusterStatus.DiskSize,
174178
RAMSize: clusterStatus.RAMSize,
175179
RAMUsage: clusterStatus.RAMUse,
176-
CacheUsage: size,
180+
CacheUsage: strongunits.B(cast.ToUint64(size)),
177181
PersistentVolumeUse: clusterStatus.PersistentVolumeUse,
178182
PersistentVolumeSize: clusterStatus.PersistentVolumeSize,
179183
CacheDir: cacheDir,
@@ -197,7 +201,7 @@ func (s *status) prettyPrintTo(writer io.Writer) error {
197201

198202
lines = append(lines, line{s.Preset.ForDisplay(), openshiftStatus(s)})
199203

200-
if s.RAMSize != -1 && s.RAMUsage != -1 {
204+
if s.RAMSize != 0 && s.RAMUsage != 0 {
201205
lines = append(lines, line{"RAM Usage", fmt.Sprintf(
202206
"%s of %s",
203207
units.HumanSize(float64(s.RAMUsage)),

‎cmd/crc/cmd/status_test.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ func setUpClient(t *testing.T) *mocks.Client {
2929
CrcStatus: string(state.Running),
3030
OpenshiftStatus: string(types.OpenshiftRunning),
3131
OpenshiftVersion: "4.5.1",
32+
RAMUse: 8_000_000_000,
33+
RAMSize: 10_000_000_000,
3234
DiskUse: 10_000_000_000,
3335
DiskSize: 20_000_000_000,
3436
Preset: preset.OpenShift,
@@ -59,7 +61,7 @@ func TestPlainStatus(t *testing.T) {
5961

6062
expected := `CRC VM: Running
6163
OpenShift: Running (v4.5.1)
62-
RAM Usage: 0B of 0B
64+
RAM Usage: 8GB of 10GB
6365
Disk Usage: 10GB of 20GB (Inside the CRC VM)
6466
Cache Usage: 10kB
6567
Cache Directory: %s
@@ -77,6 +79,8 @@ func TestStatusWithoutPodman(t *testing.T) {
7779
CrcStatus: string(state.Running),
7880
OpenshiftStatus: string(types.OpenshiftRunning),
7981
OpenshiftVersion: "4.5.1",
82+
RAMUse: 15_000_000_000,
83+
RAMSize: 20_000_000_000,
8084
DiskUse: 10_000_000_000,
8185
DiskSize: 20_000_000_000,
8286
Preset: preset.OpenShift,
@@ -89,7 +93,7 @@ func TestStatusWithoutPodman(t *testing.T) {
8993

9094
expected := `CRC VM: Running
9195
OpenShift: Running (v4.5.1)
92-
RAM Usage: 0B of 0B
96+
RAM Usage: 15GB of 20GB
9397
Disk Usage: 10GB of 20GB (Inside the CRC VM)
9498
Cache Usage: 10kB
9599
Cache Directory: %s
@@ -118,6 +122,8 @@ func TestJsonStatus(t *testing.T) {
118122
"diskSize": 20000000000,
119123
"cacheUsage": 10000,
120124
"cacheDir": "%s",
125+
"ramSize": 10000000000,
126+
"ramUsage": 8000000000,
121127
"preset": "openshift"
122128
}
123129
`

‎pkg/crc/api/api_client_test.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"path/filepath"
88
"testing"
99

10+
"github.com/containers/common/pkg/strongunits"
11+
1012
apiClient "github.com/crc-org/crc/v2/pkg/crc/api/client"
1113
crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
1214
"github.com/crc-org/crc/v2/pkg/crc/constants"
@@ -68,10 +70,10 @@ func TestStatus(t *testing.T) {
6870
CrcStatus: "Running",
6971
OpenshiftStatus: "Running",
7072
OpenshiftVersion: "4.5.1",
71-
DiskUse: int64(10000000000),
72-
DiskSize: int64(20000000000),
73-
RAMUse: int64(1000),
74-
RAMSize: int64(2000),
73+
DiskUse: strongunits.B(10000000000),
74+
DiskSize: strongunits.B(20000000000),
75+
RAMUse: strongunits.B(1000),
76+
RAMSize: strongunits.B(2000),
7577
Preset: preset.OpenShift,
7678
},
7779
statusResult,

‎pkg/crc/api/client/types.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package client
22

33
import (
4+
"github.com/containers/common/pkg/strongunits"
45
"github.com/crc-org/crc/v2/pkg/crc/machine/state"
56
"github.com/crc-org/crc/v2/pkg/crc/machine/types"
67
"github.com/crc-org/crc/v2/pkg/crc/preset"
@@ -23,12 +24,12 @@ type ClusterStatusResult struct {
2324
CrcStatus string
2425
OpenshiftStatus string
2526
OpenshiftVersion string `json:"OpenshiftVersion,omitempty"`
26-
DiskUse int64
27-
DiskSize int64
28-
RAMUse int64
29-
RAMSize int64
30-
PersistentVolumeUse int `json:"PersistentVolumeUse,omitempty"`
31-
PersistentVolumeSize int `json:"PersistentVolumeSize,omitempty"`
27+
DiskUse strongunits.B
28+
DiskSize strongunits.B
29+
RAMUse strongunits.B
30+
RAMSize strongunits.B
31+
PersistentVolumeUse strongunits.B `json:"PersistentVolumeUse,omitempty"`
32+
PersistentVolumeSize strongunits.B `json:"PersistentVolumeSize,omitempty"`
3233
Preset preset.Preset
3334
}
3435

‎pkg/crc/api/handlers.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
gocontext "context"
55
"net/http"
66

7+
"github.com/containers/common/pkg/strongunits"
8+
79
"github.com/crc-org/crc/v2/pkg/crc/api/client"
810
"github.com/crc-org/crc/v2/pkg/crc/cluster"
911
crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
@@ -121,8 +123,8 @@ func (h *Handler) Start(c *context) error {
121123
func getStartConfig(cfg crcConfig.Storage, args client.StartConfig) types.StartConfig {
122124
return types.StartConfig{
123125
BundlePath: cfg.Get(crcConfig.Bundle).AsString(),
124-
Memory: cfg.Get(crcConfig.Memory).AsUInt(),
125-
DiskSize: cfg.Get(crcConfig.DiskSize).AsUInt(),
126+
Memory: strongunits.MiB(cfg.Get(crcConfig.Memory).AsUInt()),
127+
DiskSize: strongunits.GiB(cfg.Get(crcConfig.DiskSize).AsUInt()),
126128
CPUs: cfg.Get(crcConfig.CPUs).AsUInt(),
127129
NameServer: cfg.Get(crcConfig.NameServer).AsString(),
128130
PullSecret: cluster.NewNonInteractivePullSecretLoader(cfg, args.PullSecretFile),

‎pkg/crc/cluster/cluster.go

+18-14
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ import (
1313
"strings"
1414
"time"
1515

16+
"github.com/spf13/cast"
17+
18+
"github.com/containers/common/pkg/strongunits"
19+
1620
"github.com/crc-org/crc/v2/pkg/crc/constants"
1721
"github.com/crc-org/crc/v2/pkg/crc/errors"
1822
"github.com/crc-org/crc/v2/pkg/crc/logging"
@@ -64,46 +68,46 @@ func checkCertValidity(sshRunner *ssh.Runner, cert string) (bool, error) {
6468
}
6569

6670
// Return size of disk, used space in bytes and the mountpoint
67-
func GetRootPartitionUsage(sshRunner *ssh.Runner) (int64, int64, error) {
71+
func GetRootPartitionUsage(sshRunner *ssh.Runner) (strongunits.B, strongunits.B, error) {
6872
cmd := "df -B1 --output=size,used,target /sysroot | tail -1"
6973

7074
out, _, err := sshRunner.Run(cmd)
7175

7276
if err != nil {
73-
return 0, 0, err
77+
return strongunits.B(0), strongunits.B(0), err
7478
}
7579
diskDetails := strings.Split(strings.TrimSpace(out), " ")
7680
diskSize, err := strconv.ParseInt(diskDetails[0], 10, 64)
7781
if err != nil {
78-
return 0, 0, err
82+
return strongunits.B(0), strongunits.B(0), err
7983
}
8084
diskUsage, err := strconv.ParseInt(diskDetails[1], 10, 64)
8185
if err != nil {
82-
return 0, 0, err
86+
return strongunits.B(0), strongunits.B(0), err
8387
}
84-
return diskSize, diskUsage, nil
88+
return strongunits.B(cast.ToUint64(diskSize)), strongunits.B(cast.ToUint64(diskUsage)), nil
8589
}
8690

8791
// GetRAMUsage return RAM size and RAM usage in bytes
88-
func GetRAMUsage(sshRunner *ssh.Runner) (int64, int64, error) {
92+
func GetRAMUsage(sshRunner *ssh.Runner) (strongunits.B, strongunits.B, error) {
8993
cmd := "awk '/^Mem/ {print $2,$3}' <(free -b)"
9094
out, _, err := sshRunner.Run(cmd)
9195

9296
if err != nil {
93-
return 0, 0, err
97+
return strongunits.B(0), strongunits.B(0), err
9498
}
9599

96100
ramDetails := strings.Split(strings.TrimSpace(out), " ")
97101
ramSize, err := strconv.ParseInt(ramDetails[0], 10, 64)
98102
if err != nil {
99-
return 0, 0, err
103+
return strongunits.B(0), strongunits.B(0), err
100104
}
101105
ramUsage, err := strconv.ParseInt(ramDetails[1], 10, 64)
102106
if err != nil {
103-
return 0, 0, err
107+
return strongunits.B(0), strongunits.B(0), err
104108
}
105109

106-
return ramSize, ramUsage, nil
110+
return strongunits.B(cast.ToUint64(ramSize)), strongunits.B(cast.ToUint64(ramUsage)), nil
107111
}
108112

109113
// 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) {
130134

131135
}
132136

133-
func GetPVCUsage(sshRunner *ssh.Runner) (int, error) {
137+
func GetPVCUsage(sshRunner *ssh.Runner) (strongunits.B, error) {
134138
cmd := `#!/bin/bash
135139
mountpoints=$(lsblk --output=mountpoints | grep pvc | uniq | tr '\n' ' ')
136140
if [ -z "$mountpoints" ]; then
@@ -144,13 +148,13 @@ sudo df -B1 --output=size $mountpoints | awk ' { sum += $1 } END { printf "%d",
144148
}
145149
out = strings.TrimSpace(out)
146150
if len(out) == 0 {
147-
return 0, nil
151+
return strongunits.B(0), nil
148152
}
149153
ans, err := strconv.Atoi(out)
150154
if err != nil {
151-
return 0, err
155+
return strongunits.B(0), err
152156
}
153-
return ans, nil
157+
return strongunits.B(cast.ToUint64(ans)), nil
154158
}
155159

156160
func EnsureSSHKeyPresentInTheCluster(ctx context.Context, ocConfig oc.Config, sshPublicKeyPath string) error {

‎pkg/crc/config/settings.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func defaultCPUs(cfg Storage) uint {
156156
}
157157

158158
func defaultMemory(cfg Storage) uint {
159-
return constants.GetDefaultMemory(GetPreset(cfg))
159+
return uint(constants.GetDefaultMemory(GetPreset(cfg)))
160160
}
161161

162162
func defaultBundlePath(cfg Storage) string {

‎pkg/crc/config/settings_test.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"path/filepath"
66
"testing"
77

8+
"github.com/containers/common/pkg/strongunits"
9+
810
"github.com/crc-org/crc/v2/pkg/crc/constants"
911
crcpreset "github.com/crc-org/crc/v2/pkg/crc/preset"
1012
"github.com/crc-org/crc/v2/pkg/crc/version"
@@ -16,11 +18,12 @@ import (
1618

1719
// override for ValidateMemory in validations.go to disable the physical memory check
1820
func validateMemoryNoPhysicalCheck(value interface{}, preset crcpreset.Preset) (bool, string) {
19-
v, err := cast.ToUintE(value)
21+
valueAsInt, err := cast.ToUintE(value)
2022
if err != nil {
2123
return false, fmt.Sprintf("requires integer value in MiB >= %d", constants.GetDefaultMemory(preset))
2224
}
23-
if v < constants.GetDefaultMemory(preset) {
25+
memory := strongunits.MiB(valueAsInt)
26+
if memory < constants.GetDefaultMemory(preset) {
2427
return false, fmt.Sprintf("requires memory in MiB >= %d", constants.GetDefaultMemory(preset))
2528
}
2629
return true, ""

‎pkg/crc/config/validations.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"runtime"
66
"strings"
77

8+
"github.com/containers/common/pkg/strongunits"
9+
810
"github.com/crc-org/crc/v2/pkg/crc/constants"
911
"github.com/crc-org/crc/v2/pkg/crc/network/httpproxy"
1012
crcpreset "github.com/crc-org/crc/v2/pkg/crc/preset"
@@ -31,10 +33,11 @@ func validateString(value interface{}) (bool, string) {
3133

3234
// validateDiskSize checks if provided disk size is valid in the config
3335
func validateDiskSize(value interface{}) (bool, string) {
34-
diskSize, err := cast.ToUintE(value)
36+
valueAsInt, err := cast.ToUintE(value)
3537
if err != nil {
3638
return false, fmt.Sprintf("could not convert '%s' to integer", value)
3739
}
40+
diskSize := strongunits.GiB(valueAsInt)
3841
if err := validation.ValidateDiskSize(diskSize); err != nil {
3942
return false, err.Error()
4043
}
@@ -70,11 +73,12 @@ func validateCPUs(value interface{}, preset crcpreset.Preset) (bool, string) {
7073
// validateMemory checks if provided memory is valid in the config
7174
// It's defined as a variable so that it can be overridden in tests to disable the physical memory check
7275
var validateMemory = func(value interface{}, preset crcpreset.Preset) (bool, string) {
73-
v, err := cast.ToUintE(value)
76+
valueAsInt, err := cast.ToUintE(value)
7477
if err != nil {
7578
return false, fmt.Sprintf("requires integer value in MiB >= %d", constants.GetDefaultMemory(preset))
7679
}
77-
if err := validation.ValidateMemory(v, preset); err != nil {
80+
memory := strongunits.MiB(valueAsInt)
81+
if err := validation.ValidateMemory(memory, preset); err != nil {
7882
return false, err.Error()
7983
}
8084
return true, ""

0 commit comments

Comments
 (0)