Skip to content

Commit

Permalink
refactor : Use strongunits.MiB / strongunits.GiB to store memory …
Browse files Browse the repository at this point in the history 
…size (crc-org#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]>
  • Loading branch information
@rohanKanojia
rohanKanojia committed Jan 7, 2025
1 parent a697b9e commit b431f7e
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 129 deletions.
5 changes: 3 additions & 2 deletions cmd/crc/cmd/start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"github.com/containers/common/pkg/strongunits"
"io"
"os"
"runtime"
Expand Down Expand Up @@ -69,8 +70,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),
Expand Down
27 changes: 14 additions & 13 deletions cmd/crc/cmd/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"errors"
"fmt"
"github.com/containers/common/pkg/strongunits"
"io"
"net/url"
"os"
Expand Down Expand Up @@ -44,14 +45,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"`
}

Expand All @@ -67,8 +68,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
Expand Down Expand Up @@ -107,8 +108,8 @@ func runWatchStatus(writer io.Writer, client *daemonclient.Client, cacheDir stri
}
}

ramBar.SetTotal(loadResult.RAMSize)
ramBar.SetCurrent(loadResult.RAMUse)
ramBar.SetTotal(int64(loadResult.RAMSize))
ramBar.SetCurrent(int64(loadResult.RAMUse))
for i, cpuLoad := range loadResult.CPUUse {
cpuBars[i].SetCurrent(cpuLoad)
}
Expand Down Expand Up @@ -173,7 +174,7 @@ func getStatus(client *daemonclient.Client, cacheDir string) *status {
DiskSize: clusterStatus.DiskSize,
RAMSize: clusterStatus.RAMSize,
RAMUsage: clusterStatus.RAMUse,
CacheUsage: size,
CacheUsage: strongunits.B(size),
PersistentVolumeUse: clusterStatus.PersistentVolumeUse,
PersistentVolumeSize: clusterStatus.PersistentVolumeSize,
CacheDir: cacheDir,
Expand All @@ -197,7 +198,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)),
Expand Down
2 changes: 0 additions & 2 deletions cmd/crc/cmd/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func TestPlainStatus(t *testing.T) {

expected := `CRC VM: Running
OpenShift: Running (v4.5.1)
RAM Usage: 0B of 0B
Disk Usage: 10GB of 20GB (Inside the CRC VM)
Cache Usage: 10kB
Cache Directory: %s
Expand Down Expand Up @@ -89,7 +88,6 @@ func TestStatusWithoutPodman(t *testing.T) {

expected := `CRC VM: Running
OpenShift: Running (v4.5.1)
RAM Usage: 0B of 0B
Disk Usage: 10GB of 20GB (Inside the CRC VM)
Cache Usage: 10kB
Cache Directory: %s
Expand Down
9 changes: 5 additions & 4 deletions pkg/crc/api/api_client_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package api

import (
"github.com/containers/common/pkg/strongunits"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -68,10 +69,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,
Expand Down
13 changes: 7 additions & 6 deletions pkg/crc/api/client/types.go
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/crc/api/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

import (
gocontext "context"
"github.com/containers/common/pkg/strongunits"
"net/http"

"github.com/crc-org/crc/v2/pkg/crc/api/client"
Expand Down Expand Up @@ -121,8 +122,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),
Expand Down
9 changes: 6 additions & 3 deletions pkg/crc/machine/config/config.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package config

import "github.com/crc-org/crc/v2/pkg/crc/network"
import (
"github.com/containers/common/pkg/strongunits"
"github.com/crc-org/crc/v2/pkg/crc/network"
)

type MachineConfig struct {
// CRC system bundle
Expand All @@ -9,10 +12,10 @@ type MachineConfig struct {
// Virtual machine configuration
Name string
// Memory holds value in MiB
Memory uint
Memory strongunits.MiB
CPUs uint
// DiskSize holds value in GiB
DiskSize uint
DiskSize strongunits.GiB
ImageSourcePath string
ImageFormat string
SSHKeyPath string
Expand Down
8 changes: 2 additions & 6 deletions pkg/crc/machine/config/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ import (
"github.com/crc-org/machine/libmachine/drivers"
)

func ConvertGiBToBytes(gib uint) uint64 {
return uint64(gib) * 1024 * 1024 * 1024
}

func InitVMDriverFromMachineConfig(machineConfig MachineConfig, driver *drivers.VMDriver) {
driver.CPU = machineConfig.CPUs
driver.Memory = machineConfig.Memory
driver.DiskCapacity = ConvertGiBToBytes(machineConfig.DiskSize)
driver.Memory = uint(machineConfig.Memory)
driver.DiskCapacity = uint64(machineConfig.DiskSize.ToBytes())
driver.BundleName = machineConfig.BundleName
driver.ImageSourcePath = machineConfig.ImageSourcePath
driver.ImageFormat = machineConfig.ImageFormat
Expand Down
16 changes: 8 additions & 8 deletions pkg/crc/machine/driver.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package machine

import (
"github.com/crc-org/crc/v2/pkg/crc/machine/config"
"github.com/containers/common/pkg/strongunits"
"github.com/crc-org/crc/v2/pkg/libmachine/host"
libmachine "github.com/crc-org/machine/libmachine/drivers"
)
Expand All @@ -21,12 +21,12 @@ func updateDriverValue(host *host.Host, setDriverValue valueSetter) error {
return updateDriverConfig(host, driver)
}

func setMemory(host *host.Host, memorySize uint) error {
func setMemory(host *host.Host, memorySize strongunits.MiB) error {
memorySetter := func(driver *libmachine.VMDriver) bool {
if driver.Memory == memorySize {
if driver.Memory == uint(memorySize.ToBytes()) {
return false
}
driver.Memory = memorySize
driver.Memory = uint(memorySize)
return true
}

Expand All @@ -45,13 +45,13 @@ func setVcpus(host *host.Host, vcpus uint) error {
return updateDriverValue(host, vcpuSetter)
}

func setDiskSize(host *host.Host, diskSizeGiB uint) error {
func setDiskSize(host *host.Host, diskSize strongunits.GiB) error {
diskSizeSetter := func(driver *libmachine.VMDriver) bool {
capacity := config.ConvertGiBToBytes(diskSizeGiB)
if driver.DiskCapacity == capacity {
capacity := diskSize.ToBytes()
if driver.DiskCapacity == uint64(capacity) {
return false
}
driver.DiskCapacity = capacity
driver.DiskCapacity = uint64(capacity)
return true
}

Expand Down
26 changes: 13 additions & 13 deletions pkg/crc/machine/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package machine
import (
"context"
"fmt"
"github.com/containers/common/pkg/strongunits"

"github.com/crc-org/crc/v2/pkg/crc/cluster"
"github.com/crc-org/crc/v2/pkg/crc/config"
Expand All @@ -11,7 +12,6 @@ import (
"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"
"github.com/docker/go-units"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -54,7 +54,7 @@ func (client *client) Status() (*types.ClusterStatusResult, error) {
return createClusterStatusResult(vmStatus, vm.bundle.GetBundleType(), vm.bundle.GetVersion(), ip, ramSize, ramUse, diskSize, diskUse, pvSize, pvUse, openShiftStatusSupplier)
}

func createClusterStatusResult(vmStatus state.State, bundleType preset.Preset, vmBundleVersion, vmIP string, diskSize, diskUse, ramSize, ramUse int64, pvUse, pvSize int, openShiftStatusSupplier openShiftStatusSupplierFunc) (*types.ClusterStatusResult, error) {
func createClusterStatusResult(vmStatus state.State, bundleType preset.Preset, vmBundleVersion, vmIP string, diskSize, diskUse, ramSize, ramUse strongunits.B, pvUse, pvSize strongunits.B, openShiftStatusSupplier openShiftStatusSupplierFunc) (*types.ClusterStatusResult, error) {
clusterStatusResult := &types.ClusterStatusResult{
CrcStatus: vmStatus,
OpenshiftVersion: vmBundleVersion,
Expand Down Expand Up @@ -93,8 +93,8 @@ func (client *client) GetClusterLoad() (*types.ClusterLoadResult, error) {
if err != nil {
if errors.Is(err, errMissingHost(client.name)) {
return &types.ClusterLoadResult{
RAMUse: -1,
RAMSize: -1,
RAMUse: 0,
RAMSize: 0,
CPUUse: nil,
}, nil
}
Expand All @@ -108,8 +108,8 @@ func (client *client) GetClusterLoad() (*types.ClusterLoadResult, error) {
}
if vmStatus != state.Running {
return &types.ClusterLoadResult{
RAMUse: -1,
RAMSize: -1,
RAMUse: 0,
RAMSize: 0,
CPUUse: nil,
}, nil
}
Expand All @@ -124,7 +124,7 @@ func (client *client) GetClusterLoad() (*types.ClusterLoadResult, error) {
}, nil
}

func (client *client) getDiskDetails(vm *virtualMachine) (int64, int64) {
func (client *client) getDiskDetails(vm *virtualMachine) (strongunits.B, strongunits.B) {
disk, err, _ := client.diskDetails.Memoize("disks", func() (interface{}, error) {
sshRunner, err := vm.SSHRunner()
if err != nil {
Expand All @@ -141,7 +141,7 @@ func (client *client) getDiskDetails(vm *virtualMachine) (int64, int64) {
logging.Debugf("Cannot get root partition usage: %v", err)
return 0, 0
}
return disk.([]int64)[0], disk.([]int64)[1]
return strongunits.B(disk.([]int64)[0]), strongunits.B(disk.([]int64)[1])
}

func getOpenShiftStatus(ctx context.Context, ip string) types.OpenshiftStatus {
Expand Down Expand Up @@ -174,7 +174,7 @@ func getStatus(status *cluster.Status) types.OpenshiftStatus {
return types.OpenshiftStopped
}

func (client *client) getRAMStatus(vm *virtualMachine) (int64, int64) {
func (client *client) getRAMStatus(vm *virtualMachine) (strongunits.B, strongunits.B) {
ram, err, _ := client.ramDetails.Memoize("ram", func() (interface{}, error) {
sshRunner, err := vm.SSHRunner()
if err != nil {
Expand All @@ -190,10 +190,10 @@ func (client *client) getRAMStatus(vm *virtualMachine) (int64, int64) {

if err != nil {
logging.Debugf("Cannot get RAM usage: %v", err)
return -1, -1
return 0, 0
}

return ram.([]int64)[0], ram.([]int64)[1]
return strongunits.B(ram.([]int64)[0]), strongunits.B(ram.([]int64)[1])
}

func (client *client) getCPUStatus(vm *virtualMachine) []int64 {
Expand All @@ -214,7 +214,7 @@ func (client *client) getCPUStatus(vm *virtualMachine) []int64 {

}

func (client *client) getPVCSize(vm *virtualMachine) (int, int) {
func (client *client) getPVCSize(vm *virtualMachine) (strongunits.B, strongunits.B) {
sshRunner, err := vm.SSHRunner()
if err != nil {
logging.Debugf("Cannot get SSH runner: %v", err)
Expand All @@ -227,5 +227,5 @@ func (client *client) getPVCSize(vm *virtualMachine) (int, int) {
logging.Debugf("Cannot get PVC usage: %v", err)
return 0, 0
}
return used, total.AsInt() * units.GB
return strongunits.B(used), strongunits.GiB(total.AsInt()).ToBytes()
}
Loading

0 comments on commit b431f7e

Please sign in to comment.