Skip to content

force release of quota when AW is deleted in target namespace #365

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

force release of quota when AW is deleted in target namespace #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ac1297d
force release quota when AW does not exists
asm582 May 16, 2023
9f2950c
resolve review
asm582 May 16, 2023
3d9b39a
address review-2
asm582 May 16, 2023
cd2eb7e
address review 3
asm582 May 17, 2023
60f8f00
move release code up in call stack
asm582 May 19, 2023
ebcba6e
qm test config change
asm582 May 25, 2023
a27ae90
remove custompodresources
asm582 May 25, 2023
630a041
fix resorurce unit
asm582 May 25, 2023
bdacd6b
quota config changes
asm582 May 25, 2023
8375e7b
increase priority
asm582 May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/controller/queuejob/queuejob_controller_ex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,8 @@ func (qjm *XController) chooseAgent(qj *arbv1.AppWrapper) string {
//Now evaluate quota
if qjm.serverOption.QuotaEnabled {
if qjm.quotaManager != nil {
//quota trees faile to update with AW deletes, release quota before assigning
qjm.quotaManager.Release(qj)
if fits, preemptAWs, _ := qjm.quotaManager.Fits(qj, qjAggrResources, proposedPreemptions); fits {
klog.V(2).Infof("[chooseAgent] AppWrapper %s has enough quota.\n", qj.Name)
qjm.preemptAWJobs(preemptAWs)
Expand Down Expand Up @@ -1267,7 +1269,9 @@ func (qjm *XController) ScheduleNext() {
if qjm.quotaManager != nil {
var msg string
var preemptAWs []*arbv1.AppWrapper
quotaFits, preemptAWs, msg = qjm.quotaManager.Fits(qj, aggqj, proposedPreemptions)
//quota trees fail to update with AW deletes, release quota before assigning
qjm.quotaManager.Release(qj)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if this makes any sense here now, it seems superfluous. Have you observed a scenario where having this call here would be beneficial?

quotaFits, preemptAWs, msg := qjm.quotaManager.Fits(qj, aggqj, proposedPreemptions)
if quotaFits {
klog.Infof("[ScheduleNext] HOL quota evaluation successful %s for %s activeQ=%t Unsched=%t &qj=%p Version=%s Status=%+v due to quota limits", qj.Name, time.Now().Sub(HOLStartTime), qjm.qjqueue.IfExistActiveQ(qj), qjm.qjqueue.IfExistUnschedulableQ(qj), qj, qj.ResourceVersion, qj.Status)
// Set any jobs that are marked for preemption
Expand Down
2 changes: 2 additions & 0 deletions pkg/quotaplugins/quota-forest/quota-manager/quota/quotamanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,11 @@ func (m *Manager) AllocateForest(forestName string, consumerID string) (response
if consumerInfo == nil {
return nil, fmt.Errorf("consumer %s does not exist, create and add first", consumerID)
}

if forestController.IsConsumerAllocated(consumerID) {
return nil, fmt.Errorf("consumer %s already allocated on forest %s", consumerID, forestName)
}

resourceNames := forestController.GetResourceNames()
forestConsumer, err := consumerInfo.CreateForestConsumer(forestName, resourceNames)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions test/e2e-kuttl/install-quota-subtree.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ spec:
children:
- name: gold
quotas:
hardLimit: false
requests:
cpu: 1075m
memory: 450Mi
Expand Down
2 changes: 1 addition & 1 deletion test/e2e-kuttl/quota-forest/06-install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ metadata:
spec:
service:
spec: {}
priority: 1000
priority: 10000
resources:
metadata: {}
GenericItems:
Expand Down