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

runtime: Allocate default workload vcpus #282

Draft
wants to merge 1 commit into
base: msft-main
Choose a base branch
from

Conversation

ms-mahuber
Copy link

@ms-mahuber ms-mahuber commented Jan 2, 2025

Merge Checklist
  • Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
    • Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
  • Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
  • The upstream/missing label (or upstream/not-needed) has been set on the PR.
Summary
• Let's align assignment with the new mem assignment. Extend our 'memory allocation patch' with 'vcpu'. This way we can set default_vcpus to 0 and add 1 vCPU if no limits are given, similar to pull/281.
• Let's allow for non-integer vCPU limits and round up - no change required. the calculation is already as of today for the example of a limit of 3500millis: ceil(0, 3.5) = 4
• We will not hit the bug where we can ask CLH to allocate 5 vCPUs, then hit a crash. We did only previously hit this through ceil(1+3.5)=5. To prevent this we would have had to define a vCPU limit of 1 or higher (cause then kubelet will check 1+3.5 vs 3.86 and tell use that we can't run the pod. Without this, kubelet thinks we only want 3.5 and let's use go ahead and let's CLH allocate a VM with 5 vCPUs, and then 'boom'). 
	○ With this, we do not need to define a vCPU overhead.
Test Methodology

@ms-mahuber ms-mahuber force-pushed the mahuber/pod-vcpus branch 5 times, most recently from 6b83d36 to 3d7f094 Compare January 3, 2025 01:05
@ms-mahuber ms-mahuber force-pushed the mahuber/pod-vcpus branch 3 times, most recently from 0839ef8 to 43e7c3d Compare January 4, 2025 00:03
- similar to the static_sandbox_default_workload_mem option,
  assign a default number of vcpus to the VM when no limits
  are given, 1 vcpu in this case
- similar to commit c7b8ee9, do not allocate additional vcpus
  when limits are provided

Signed-off-by: Manuel Huber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants