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-1687] add e2e for user & group quota enforcement #634

Closed
wants to merge 2 commits into from

Conversation

FrankYang0529
Copy link
Member

@FrankYang0529 FrankYang0529 commented Jul 14, 2023

What is this PR for?

  • Single specific user limit
    • A user can't create more than maxapplications.
    • A user can't create more than maxresoruces.
    • A user can create apps in other queues.
    • Users not in the limit can create more than maxapplications.
    • Users not in the limit can create more than maxresources.
  • Single specific group limit
    • A group can't create more than maxapplications.
    • A group can't create more than maxresoruces.
    • A group can create apps in other queues.
    • Groups not in the limit can create more than maxapplications.
    • Groups not in the limit can create more than maxresources.
  • User limit is lower than group limit.
  • Group limit is lower than user limit.

What type of PR is it?

  • - Improvement

What is the Jira issue?

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

How should this be tested?

All e2e test cases pass.

@wilfred-s wilfred-s self-requested a review July 18, 2023 02:44
@FrankYang0529 FrankYang0529 force-pushed the YUNIKORN-1687 branch 2 times, most recently from 63b0109 to 7ff5d37 Compare July 18, 2023 15:04
@manirajv06
Copy link
Contributor

@FrankYang0529 How should we take this up? Should we cover all cases described in https://issues.apache.org/jira/browse/YUNIKORN-1871 and plan for review? I think better to cover all the cases and review together.

@FrankYang0529
Copy link
Member Author

@FrankYang0529 How should we take this up? Should we cover all cases described in https://issues.apache.org/jira/browse/YUNIKORN-1871 and plan for review? I think better to cover all the cases and review together.

Hi @manirajv06, yes, I agree it's better to cover all the cases in this PR. I will update the PR and remove WIP once it's ready for review. I will do my best to finish it this week. Thank you!

@FrankYang0529
Copy link
Member Author

@FrankYang0529 How should we take this up? Should we cover all cases described in https://issues.apache.org/jira/browse/YUNIKORN-1871 and plan for review? I think better to cover all the cases and review together.

Hi @manirajv06, yes, I agree it's better to cover all the cases in this PR. I will update the PR and remove WIP once it's ready for review. I will do my best to finish it this week. Thank you!

@manirajv06 , I have to create another PR (apache/yunikorn-core#595) for YUNIKORN-1871, because it's unit test in yunikorn-core. I will finish the unit test first and then back to this PR for e2e test. Thank you.

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #634 (0d9232d) into master (8b26c37) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #634   +/-   ##
=======================================
  Coverage   71.87%   71.87%           
=======================================
  Files          51       51           
  Lines        8079     8079           
=======================================
  Hits         5807     5807           
  Misses       2074     2074           
  Partials      198      198           

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

@manirajv06
Copy link
Contributor

@FrankYang0529 Now that apache/yunikorn-core#595 has gone in, can you clean up this pr? and also, can you ensure all cases has been covered as e2e tests?

@FrankYang0529
Copy link
Member Author

@FrankYang0529 Now that apache/yunikorn-core#595 has gone in, can you clean up this pr? and also, can you ensure all cases has been covered as e2e tests?

Hi @manirajv06, sorry for the late reply. I will update test cases in this weekend. Thank you.

@FrankYang0529 FrankYang0529 marked this pull request as ready for review September 3, 2023 15:27
@FrankYang0529
Copy link
Member Author

FrankYang0529 commented Sep 3, 2023

Hi @manirajv06, I would like to split wildcard and config change cases to another two Jira, because I don't want to make this PR too big. Thank you.

@FrankYang0529
Copy link
Member Author

Created another two Jira. Thank you.

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