Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use go-units for disk size/memory size #1642

Open
cfergeau opened this issue Nov 4, 2020 · 3 comments · May be fixed by #4554
Open

Use go-units for disk size/memory size #1642

cfergeau opened this issue Nov 4, 2020 · 3 comments · May be fixed by #4554
Assignees
Labels
kind/bug Something isn't working priority/minor status/pinned Prevents the stale bot from closing the issue

Comments

@cfergeau
Copy link
Contributor

cfergeau commented Nov 4, 2020

At the moment disk sizes/memory sizes are passed as unitless numbers, one has to know disk size uses GiB and memory size uses MiB (or look at crc start --help. We should be adding support for https://github.com/docker/go-units to the option parsing code so that the unit can be specified to make this more user friendly.

@cfergeau cfergeau added the kind/bug Something isn't working label Nov 4, 2020
@stale
Copy link

stale bot commented Jan 3, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status/stale Issue went stale; did not receive attention or no reply from the OP label Jan 3, 2021
@guillaumerose guillaumerose added the status/pinned Prevents the stale bot from closing the issue label Jan 4, 2021
@stale stale bot removed the status/stale Issue went stale; did not receive attention or no reply from the OP label Jan 4, 2021
@rohanKanojia
Copy link
Contributor

@cfergeau : I'm trying this on CRC v2.45.0 . I'm observing that we're showing disk size memory size not in unitless numbers:

~ : $ crc status
CRC VM:          Running
OpenShift:       Running (v4.17.7)
RAM Usage:       20.06GB of 32.68GB
Disk Usage:      1.713GB of 10.95GB (Inside the CRC VM)
Cache Usage:     25.71GB
Cache Directory: /home/rohan/.crc/cache

@cfergeau
Copy link
Contributor Author

cfergeau commented Jan 6, 2025

This is about how the units are passed around in the code, for example pkg/crc/machine/driver.go has func setMemory(host *host.Host, memorySize uint), it's not obvious which unit the memory size is in. This is only one example among many.
However I think I reached the conclusion that go-units was not a great fit for us as it tends to confuse units in 1000 multiples and in 1024 multiples.
`github.com/containers/common/pkg/strongunits' might be better to carry around integer values together with a unit.

@rohanKanojia rohanKanojia self-assigned this Jan 6, 2025
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 6, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 6, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 7, 2025
…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]>
@rohanKanojia rohanKanojia moved this to In Progress in Eclipse JKube Jan 7, 2025
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 7, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 7, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 7, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 8, 2025
…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]>
@rohanKanojia rohanKanojia moved this to Work In Progress in Project planning: crc Jan 8, 2025
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 8, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 8, 2025
…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]>
rohanKanojia added a commit to rohankanojia-forks/crc that referenced this issue Jan 8, 2025
…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]>
@rohanKanojia rohanKanojia moved this from In Progress to Review in Eclipse JKube Jan 9, 2025
@rohanKanojia rohanKanojia moved this from Work In Progress to Ready for review in Project planning: crc Jan 9, 2025
@rohanKanojia rohanKanojia moved this from Ready for review to Work In Progress in Project planning: crc Jan 10, 2025
@rohanKanojia rohanKanojia moved this from Review to In Progress in Eclipse JKube Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working priority/minor status/pinned Prevents the stale bot from closing the issue
Projects
Status: Work In Progress
3 participants