[YUNIKORN-2661] Fix hard-coded boolean in setLimit #883
Conversation
| newLimit := resources.NewResourceFromMap(map[string]resources.Quantity{ | ||
| "mem": 20, | ||
| "vcore": 20}) | ||
| root.setLimit(strings.Split(queuePath1, configs.DOT), newLimit.Clone(), 3, false, user, true) // override | ||
| assert.Assert(t, resources.Equals(newLimit, childQ.maxResources)) | ||
| newLimit2 := resources.NewResourceFromMap(map[string]resources.Quantity{ | ||
| "mem": 30, | ||
| "vcore": 30}) | ||
| root.setLimit(strings.Split(queuePath1, configs.DOT), newLimit2.Clone(), 3, true, user, true) // no override | ||
| assert.Assert(t, resources.Equals(newLimit, childQ.maxResources)) | ||
| root.setLimit(strings.Split(queuePath1, configs.DOT), newLimit2.Clone(), 3, true, user, false) // override -> changes qt.doWildCardCheck | ||
| assert.Assert(t, resources.Equals(newLimit2, childQ.maxResources)) | ||
| root.setLimit(strings.Split(queuePath1, configs.DOT), newLimit2.Clone(), 3, false, user, false) // no override | ||
| assert.Assert(t, resources.Equals(newLimit2, childQ.maxResources)) |
There was a problem hiding this comment.
@manirajv06 please check this test logic, I'm not 100% sure that I understand what's going on inside the QueueTracker.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #883 +/- ##
==========================================
+ Coverage 77.34% 77.64% +0.29%
==========================================
Files 97 97
Lines 12132 12119 -13
==========================================
+ Hits 9384 9410 +26
+ Misses 2417 2387 -30
+ Partials 331 322 -9 ☔ View full report in Codecov by Sentry. |
| assert.Assert(t, resources.Equals(newLimit, childQ.maxResources)) | ||
| root.setLimit(strings.Split(queuePath1, configs.DOT), newLimit2.Clone(), 3, true, user, false) // override -> changes qt.doWildCardCheck | ||
| assert.Assert(t, resources.Equals(newLimit2, childQ.maxResources)) | ||
| root.setLimit(strings.Split(queuePath1, configs.DOT), newLimit2.Clone(), 3, false, user, false) // no override |
There was a problem hiding this comment.
Same limit (newLimit2) has applied here too. So how do we ensure override really happens or not?
useWildCard flag helps to know whether wild card user (*) settings has applied for that specific user or not. True means applied, false means no. In case user specific settings applied for that specific use, useWildcard flag is "no".
doWildCheck is required only when we are trying to clear or reset wild card user settings for all existing users not clearing or resetting user specific settings for that specific user. For example, When config had settings for wild card user (*) earlier but not now. Before clearing/reset settings for specific user object, there is a need to pass this flag to do the actual clearance.
There was a problem hiding this comment.
LGTM, @manirajv06 please check the requested changes were added.
What is this PR for?
Remove the hard coded "false" value for
setLimit()in the recursive call.What type of PR is it?
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2661
How should this be tested?
Screenshots (if appropriate)
Questions: