-
Notifications
You must be signed in to change notification settings - Fork 1.2k
runtime-rs: make vcpu allocation more accurate #10580
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
Merged
lifupan
merged 5 commits into
kata-containers:main
from
pmores:make-vcpu-allocation-more-accurate
Aug 8, 2025
Merged
runtime-rs: make vcpu allocation more accurate #10580
lifupan
merged 5 commits into
kata-containers:main
from
pmores:make-vcpu-allocation-more-accurate
Aug 8, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@fidencio can you please take a look? |
Sure thing, added to my TODO list. |
One thing that I'd love to see, is
|
Done! |
0c4bdb6
to
f004fa0
Compare
f004fa0
to
49b5e0c
Compare
Rebased onto latest |
49b5e0c
to
0cf8108
Compare
Rebased onto |
This commit addresses a part of the same problem as PR kata-containers#7623 did for the golang runtime. So far we've been rounding up individual containers' vCPU requests and then summing them up which can lead to allocation of excess vCPUs as described in the mentioned PR's cover letter. We address this by reversing the order of operations, we sum the (possibly fractional) container requests and only then round up the total. We also align runtime-rs's behaviour with runtime-go in that we now include the default vcpu request from the config file ('default_vcpu') in the total. We diverge from PR kata-containers#7623 in that `default_vcpu` is still treated as an integer (this will be a topic of a separate commit), and that this implementation avoids relying on 32-bit floating point arithmetic as there are some potential problems with using f32. For instance, some numbers commonly used in decimal, notably all of single-decimal-digit numbers 0.1, 0.2 .. 0.9 except 0.5, are periodic in binary and thus fundamentally not representable exactly. Arithmetics performed on such numbers can lead to surprising results, e.g. adding 0.1 ten times gives 1.0000001, not 1, and taking a ceil() results in 2, clearly a wrong answer in vcpu allocation. So instead, we take advantage of the fact that container requests happen to be expressed as a quota/period fraction so we can sum up quotas, fundamentally integral numbers (possibly fractional only due to the need to rewrite them with a common denominator) with much less danger of precision loss. Signed-off-by: Pavel Mores <pmores@redhat.com>
This commit focuses purely on the formal change of type. If any subsequent changes in semantics are needed they are purposely avoided here so that the commit can be reviewed as a 100% formal and 0% semantic change. Signed-off-by: Pavel Mores <pmores@redhat.com>
Also included (as commented out) is a test that does not pass although it should. See source code comment for explanation why fixing this seems beyond the scope of this PR. Signed-off-by: Pavel Mores <pmores@redhat.com>
Configuration information is adjusted after loading from file but so far, there has been no similar check for configuration coming from annotations. This commit introduces re-adjusting config after annotations have been processed. A small refactor was necessary as a prerequisite which introduces function TomlConfig::adjust_config() to make it easier to invoke the adjustment for a whole TomlConfig instance. This function is analogous to the existing validate() function. The immediate motivation for this change is to make sure that 0 in "default_vcpus" annotation will be properly adjusted to 1 as is the case if 0 is loaded from a config file. This is required to match the golang runtime behaviour. Signed-off-by: Pavel Mores <pmores@redhat.com>
This series should make runtime-rs's vcpu allocation behaviour match the behaviour of runtime-go so we can now enable pertinent tests which were skipped so far due the difference between both shims. Signed-off-by: Pavel Mores <pmores@redhat.com>
0cf8108
to
69f2169
Compare
lifupan
approved these changes
Aug 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to bring runtime-rs's behaviour with respect to vcpu allocation in line with runtime-go's behaviour implemented by PR #7623.
In short, the two main goals are
configuration.toml
'sdefault_vcpus
value a float so that fractional vcpus can be expressed.