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

[YUNIKORN-1908] Support GPU in placeholders #651

Closed
wants to merge 2 commits into from

Conversation

wilfred-s
Copy link
Contributor

What is this PR for?

Extended resources and huge pages can not over commit. The pod validator checks for a Limit setting in the pod spec. The pod fails validation without it. Placeholder creation must make sure that the pods pass the validator.
Set limits for all extended resources and hugepages-* as they cannot be over committed.

Clean up of duplicated task group definition in the test.

What type of PR is it?

  • - Bug Fix

What is the Jira issue?

How should this be tested?

Unit test added
e2e test for placeholders should be extended but we do not have GPU nodes in kind, follow up jira logged to

Extended resources and huge pages can not over commit. The pod validator
checks for a Limit setting in the pod spec. The pod fails validation
without it. Placeholder creation must make sure that the pods pass the
validator.
Set limits for all extended resources and hugepages-* as they cannot be
over committed.

Clean up of duplicated task group definition in the test.
@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #651 (81a46ea) into master (06f4e58) will increase coverage by 0.06%.
Report is 2 commits behind head on master.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
+ Coverage   71.33%   71.40%   +0.06%     
==========================================
  Files          51       51              
  Lines        8104     8116      +12     
==========================================
+ Hits         5781     5795      +14     
+ Misses       2127     2124       -3     
- Partials      196      197       +1     
Files Changed Coverage Δ
pkg/common/utils/gang_utils.go 96.87% <85.71%> (+7.78%) ⬆️
pkg/cache/placeholder.go 93.15% <100.00%> (+0.29%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@wilfred-s wilfred-s requested a review from pbacsko August 11, 2023 03:38
Copy link
Contributor

@craigcondit craigcondit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM.

@wilfred-s wilfred-s deleted the YUNIKORN-1908 branch November 21, 2023 03:19
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.

3 participants