Skip to content

Commit

Permalink
fix: hierarchical queue webhook validation use listing podgroups instead
Browse files Browse the repository at this point in the history
Signed-off-by: JesseStutler <chenzicong4@huawei.com>
  • Loading branch information
JesseStutler committed Dec 23, 2024
1 parent b0c1a56 commit 10ccc99
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 7 deletions.
21 changes: 17 additions & 4 deletions pkg/webhooks/admission/queues/validate/validate_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package validate

import (
"context"
"fmt"
"strconv"
"strings"
Expand All @@ -29,6 +30,7 @@ import (
"k8s.io/klog/v2"

schedulingv1beta1 "volcano.sh/apis/pkg/apis/scheduling/v1beta1"
"volcano.sh/volcano/pkg/cli/podgroup"
"volcano.sh/volcano/pkg/webhooks/router"
"volcano.sh/volcano/pkg/webhooks/schema"
"volcano.sh/volcano/pkg/webhooks/util"
Expand Down Expand Up @@ -255,10 +257,21 @@ func validateHierarchicalQueue(queue *schedulingv1beta1.Queue) error {
return fmt.Errorf("failed to get parent queue of queue %s: %v", queue.Name, err)
}

if parentQueue.Status.Pending+parentQueue.Status.Running+parentQueue.Status.Unknown+parentQueue.Status.Inqueue > 0 {
return fmt.Errorf("queue %s cannot be the parent queue of queue %s because it has PodGroups (pending: %d, running: %d, unknown: %d, inqueue: %d)",
parentQueue.Name, queue.Name, parentQueue.Status.Pending,
parentQueue.Status.Running, parentQueue.Status.Unknown, parentQueue.Status.Inqueue)
pgList, err := config.VolcanoClient.SchedulingV1beta1().PodGroups("").List(context.Background(), metav1.ListOptions{})
if err != nil {
return fmt.Errorf("failed to get all podgroups from api-server: %v", err)
}

pgStats := &podgroup.PodGroupStatistics{}
for _, pg := range pgList.Items {
if pg.Spec.Queue == parentQueue.Name {
pgStats.StatPodGroupCountsForQueue(&pg)
}
}

if pgStats.Pending+pgStats.Running+pgStats.Unknown+pgStats.Inqueue+pgStats.Completed > 0 {
return fmt.Errorf("queue %s cannot be the parent queue of queue %s because it has PodGroups (pending: %d, running: %d, unknown: %d, inqueue: %d, completed: %d)",
parentQueue.Name, queue.Name, pgStats.Pending, pgStats.Running, pgStats.Unknown, pgStats.Inqueue, pgStats.Completed)
}

klog.V(3).Infof("Validation passed for hierarchical queue %s with parent queue %s",
Expand Down
21 changes: 18 additions & 3 deletions pkg/webhooks/admission/queues/validate/validate_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1049,8 +1049,18 @@ func TestAdmitHierarchicalQueues(t *testing.T) {
Spec: schedulingv1beta1.QueueSpec{
Parent: "root",
},
Status: schedulingv1beta1.QueueStatus{
Running: 2,
}

jobInQueue := schedulingv1beta1.PodGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "job-in-queue",
Namespace: "test",
},
Spec: schedulingv1beta1.PodGroupSpec{
Queue: "queue-with-jobs",
},
Status: schedulingv1beta1.PodGroupStatus{
Phase: schedulingv1beta1.PodGroupRunning,
},
}

Expand All @@ -1077,6 +1087,11 @@ func TestAdmitHierarchicalQueues(t *testing.T) {
t.Errorf("Create queue with jobs failed for %v.", err)
}

_, err = config.VolcanoClient.SchedulingV1beta1().PodGroups("test").Create(context.TODO(), &jobInQueue, metav1.CreateOptions{})
if err != nil {
t.Errorf("Create job %s/%s failed for %v", jobInQueue.Namespace, jobInQueue.Name, err)
}

_, err = config.VolcanoClient.SchedulingV1beta1().Queues().Create(context.TODO(), &queueWithoutJobs, metav1.CreateOptions{})
if err != nil {
t.Errorf("Create queue without jobs failed for %v.", err)
Expand Down Expand Up @@ -1130,7 +1145,7 @@ func TestAdmitHierarchicalQueues(t *testing.T) {
reviewResponse: &admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Message: "queue queue-with-jobs cannot be the parent queue of queue parent-queue-with-jobs because it has PodGroups (pending: 0, running: 2, unknown: 0, inqueue: 0)",
Message: "queue queue-with-jobs cannot be the parent queue of queue parent-queue-with-jobs because it has PodGroups (pending: 0, running: 1, unknown: 0, inqueue: 0, completed: 0)",
},
},
},
Expand Down

0 comments on commit 10ccc99

Please sign in to comment.