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

Change the default unit of memory HW requirement #3112

Open
skycastlelily opened this issue Jul 26, 2024 · 8 comments
Open

Change the default unit of memory HW requirement #3112

skycastlelily opened this issue Jul 26, 2024 · 8 comments
Labels
area | hardware Implementation of hardware requirements

Comments

@skycastlelily
Copy link
Collaborator

If users specify memory: > 4096 ,when they want to get an instance with memory > 4096 MiB, tmt will provide memory > 0 instance.

@happz
Copy link
Collaborator

happz commented Jul 26, 2024

If users specify memory: > 4096 ,when they want to get an instance with memory > 4096 MiB

Maybe, who knows what user wanted :) The should have been more specific :)

tmt will provide memory > 0 instance.

Referenced PR changes the default unit - might be good, might be bad, who knows, but I don't understand how it addresses the issue with > 0. IIUIC, > 4096 should provision an instance with more than 4096 bytes of RAM, which should translate into an instance with let's say 2 GB of RAM. Something very small, but definitely not zero. Can you share more details about this? It seems like two distinct problems in this issue - default unit change, plus maybe a bug in how > 4096 is evaluated today.

@happz happz added the area | hardware Implementation of hardware requirements label Jul 26, 2024
@happz
Copy link
Collaborator

happz commented Jul 26, 2024

If users specify memory: > 4096 ,when they want to get an instance with memory > 4096 MiB

Maybe, who knows what user wanted :) The should have been more specific :)

tmt will provide memory > 0 instance.

Referenced PR changes the default unit - might be good, might be bad, who knows, but I don't understand how it addresses the issue with > 0. IIUIC, > 4096 should provision an instance with more than 4096 bytes of RAM, which should translate into an instance with let's say 2 GB of RAM. Something very small, but definitely not zero. Can you share more details about this? It seems like two distinct problems in this issue - default unit change, plus maybe a bug in how > 4096 is evaluated today.

Edit: when a user says disk.size: "> 40", do they mean bytes or do they mean GB? As you can see, the change of default unit in the name of what the user might mean should probably cover disk.size as well, but then again, it seems to be not connected to how > 4096 turns into a "memory > 0 instance", which is unclear to me & feels really weird, might be a bug.

@happz
Copy link
Collaborator

happz commented Jul 26, 2024

Added as a topic for next week's hacking session agenda - fixing evaluation of > 4096 vs > 0 seems like a bug to fix, the change of default unit needs more discussion as it's a user-facing specification.

@skycastlelily
Copy link
Collaborator Author

skycastlelily commented Jul 26, 2024 via email

@happz
Copy link
Collaborator

happz commented Jul 26, 2024

I see. So it is just a topic for discussion whether the default unit of memory HW requirement should change and what it should be.

@skycastlelily
Copy link
Collaborator Author

skycastlelily commented Jul 26, 2024 via email

@happz
Copy link
Collaborator

happz commented Jul 26, 2024

@skycastlelily would you mind changing the subject of the issue then, to something like "Change the default unit of memory HW requirement"? IIUIC, the translation is perfectly fine, and memory: 4096 is correctly converted into zero, because, as the specification says, the default is bytes. Maybe we could add a check for unexpected sizes, like Beaker translating it to 0 MB would emit a warning, for example.

@skycastlelily skycastlelily changed the title memory constraint is not translated properly if no unit is specified Change the default unit of memory HW requirement Jul 29, 2024
@skycastlelily
Copy link
Collaborator Author

Sure:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area | hardware Implementation of hardware requirements
Projects
None yet
Development

No branches or pull requests

2 participants