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

[bugfix]fix creating a hierarchical sub-queue will be rejected #4033

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zhutong196
Copy link

What type of PR is this?

Add one of the following kinds:
/kind bug

Optionally add one of the following areas, help us further classify and filter PRs:
/area webhooks

What this PR does / why we need it:

check whether a queue is a leaf queue. Only leaf queue without pods inside can create sub-queues, there is no check of non-leaf queues.

Which issue(s) this PR fixes:

Fixes # #4031

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@volcano-sh-bot
Copy link
Contributor

Welcome @zhutong196!

It looks like this is your first PR to volcano-sh/volcano.

Thank you, and welcome to Volcano. 😃

@volcano-sh-bot volcano-sh-bot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 24, 2025
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign william-wang
You can assign the PR to them by writing /assign @william-wang in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@volcano-sh-bot volcano-sh-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 24, 2025
@zhutong196
Copy link
Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign william-wang You can assign the PR to them by writing /assign @william-wang in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

@zhutong196 zhutong196 closed this Feb 24, 2025
@zhutong196 zhutong196 reopened this Feb 24, 2025
@zhutong196
Copy link
Author

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: To complete the pull request process, please assign william-wang You can assign the PR to them by writing /assign @william-wang in a comment when ready.
The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

/assign @k82cn @william-wang cc

@Monokaix
Copy link
Member

/milestone v1.12

@Monokaix
Copy link
Member

Should also backport to release-1.11 after meged.

if allocated, ok := parentQueue.Status.Allocated[v1.ResourcePods]; ok && !allocated.IsZero() {
return fmt.Errorf("queue %s cannot be the parent queue of queue %s because it has allocated Pods: %d",
parentQueue.Name, queue.Name, allocated.Value())
queueList, err := config.QueueLister.List(labels.Everything())
Copy link
Member

Choose a reason for hiding this comment

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

These are duplicated logic with validateQueueDeleting, we can abstract a func.

Copy link
Author

Choose a reason for hiding this comment

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

Good suggestion, I will abstract a func of whether there is a child in the repeated judgments of validateQueueDeleting and validateHierarchicalQueue into a method

@Monokaix
Copy link
Member

Please also add UT and sign off your commit.

@volcano-sh-bot volcano-sh-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 24, 2025
@zhutong196 zhutong196 requested a review from Monokaix February 24, 2025 10:40
@JesseStutler
Copy link
Member

please use git commit -s to sign your commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/webhooks kind/bug Categorizes issue or PR as related to a bug. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants