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-1338] [admission] support different kind of workloads other than pods #467

Closed
wants to merge 3 commits into from

Conversation

pbacsko
Copy link
Contributor

@pbacsko pbacsko commented Sep 29, 2022

What is this PR for?

Handle non-pod workloads in the admission controller.

To have a better design & readability, usergroup.go and usergroup_test.go has been moved to a new package. Also, related functions are now bound to a type - they are not global anymore in the "main" package.

What type of PR is it?

  • - Bug Fix
  • - Improvement
  • - Feature
  • - Documentation
  • - Hot Fix
  • - Refactoring

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-1338

How should this be tested?

Screenshots (if appropriate)

Questions:

  • - The licenses files need update.
  • - There is breaking changes for older versions.
  • - It needs documentation.

@pbacsko pbacsko self-assigned this Sep 29, 2022
@pbacsko pbacsko marked this pull request as draft September 29, 2022 14:09
@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #467 (327877c) into master (8ff45c3) will increase coverage by 0.26%.
The diff coverage is 95.91%.

❗ Current head 327877c differs from pull request most recent head 3859fa3. Consider uploading reports for the commit 3859fa3 to get more accurate results

@@            Coverage Diff             @@
##           master     #467      +/-   ##
==========================================
+ Coverage   68.03%   68.30%   +0.26%     
==========================================
  Files          42       42              
  Lines        6921     6989      +68     
==========================================
+ Hits         4709     4774      +65     
- Misses       2042     2044       +2     
- Partials      170      171       +1     
Impacted Files Coverage Δ
pkg/plugin/admissioncontrollers/webhook/webhook.go 40.00% <88.88%> (ø)
...missioncontrollers/webhook/admission_controller.go 70.78% <91.07%> (+1.01%) ⬆️
...missioncontrollers/webhook/annotation/usergroup.go 100.00% <100.00%> (ø)
pkg/cache/amprotocol_mock.go 54.02% <0.00%> (-0.53%) ⬇️
pkg/cache/application.go 71.78% <0.00%> (-0.18%) ⬇️
pkg/cache/context.go 44.96% <0.00%> (-0.08%) ⬇️

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

@pbacsko pbacsko force-pushed the YUNIKORN-1338 branch 9 times, most recently from ad4618d to 7524696 Compare October 6, 2022 09:10
@pbacsko pbacsko marked this pull request as ready for review October 6, 2022 10:27
Copy link
Contributor

@manirajv06 manirajv06 left a comment

Choose a reason for hiding this comment

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

Have given comments..

Copy link
Contributor

@wilfred-s wilfred-s left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants