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.

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

Jump to bottom

Add allocated field in queue status to indicate used resources in queue. #2592

Merged
merged 4 commits into from
Dec 12, 2022
Merged

Add allocated field in queue status to indicate used resources in queue. #2592

merged 4 commits into from
Dec 12, 2022

Conversation

jiangkaihua
Copy link
Contributor

@volcano-sh-bot volcano-sh-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 6, 2022
@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 6, 2022
@jiangkaihua jiangkaihua closed this Dec 6, 2022
@jiangkaihua jiangkaihua reopened this Dec 6, 2022
@jiangkaihua jiangkaihua force-pushed the queue-allocated-status branch 3 times, most recently from f97d8bf to 93afc6b Compare December 6, 2022 10:48
@jiangkaihua jiangkaihua closed this Dec 6, 2022
@jiangkaihua jiangkaihua reopened this Dec 6, 2022
@jiangkaihua jiangkaihua force-pushed the queue-allocated-status branch 9 times, most recently from 3dc607c to f32b7e9 Compare December 6, 2022 18:13
@volcano-sh-bot volcano-sh-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 6, 2022
@jiangkaihua jiangkaihua force-pushed the queue-allocated-status branch 3 times, most recently from 74d6c40 to 4a8bc9e Compare December 7, 2022 07:10
@jiangkaihua jiangkaihua changed the title [WIP] Queue allocated status Add allocated field in queue status to indicate used resources in queue. Dec 7, 2022
@volcano-sh-bot volcano-sh-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 7, 2022
william-wang
william-wang reviewed Dec 7, 2022
View reviewed changes
test/e2e/util/queue.go Outdated
@@ -52,6 +52,9 @@ func CreateQueueWithQueueSpec(ctx *TestContext, queueSpec *QueueSpec) {
_, err := ctx.Vcclient.SchedulingV1beta1().Queues().Create(context.TODO(), queue, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred(), "failed to create queue %s", queueSpec.Name)
}

// wait for queue state turns to be open
time.Sleep(5 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

We ever spent lot's effort to shortage the ci running time, please eveluate the sleep duration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reduced to 3 seconds. Less waiting time would cause queue status not initialized and not Open, thus failed to apply job to the nil state queue.

pkg/scheduler/framework/session.go
@@ -189,6 +191,34 @@ func closeSession(ssn *Session) {
ju := newJobUpdater(ssn)
ju.UpdateAll()

// calculate allocated resources on each queue
Copy link
Member

Choose a reason for hiding this comment

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

please make new function to update queue status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please make new function to update queue status.

A new function updateQueueStatus() was abstracted.

@volcano-sh-bot volcano-sh-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 12, 2022
jiangkaihua added 4 commits December 12, 2022 15:37
Update queue API.
987b99e 
Signed-off-by: jiangkaihua <jiangkaihua1@huawei.com>
Update yamls.
411e4af 
Signed-off-by: jiangkaihua <jiangkaihua1@huawei.com>
Update queue status process.
7617852 
Signed-off-by: jiangkaihua <jiangkaihua1@huawei.com>
Update UT & e2e cases.
a1cecb4 
Signed-off-by: jiangkaihua <jiangkaihua1@huawei.com>
@jiangkaihua jiangkaihua force-pushed the queue-allocated-status branch 3 times, most recently from 44c7fec to a1cecb4 Compare December 12, 2022 08:11
@volcano-sh-bot volcano-sh-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 12, 2022
william-wang
william-wang approved these changes Dec 12, 2022
View reviewed changes
Copy link
Member

@william-wang william-wang left a comment

Choose a reason for hiding this comment

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

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2022
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

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

The pull request process is described 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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 12, 2022
@volcano-sh-bot volcano-sh-bot merged commit aebe79c into volcano-sh:master Dec 12, 2022
volcano-sh-bot added a commit that referenced this pull request Dec 21, 2022
@volcano-sh-bot
Merge pull request #2604 from jiangkaihua/release-1.7
88db488 
[Cherry-pick] Add allocated field in queue status to indicate used resources in queue. #2592
@wangyang0616 wangyang0616 mentioned this pull request Apr 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@william-wang william-wang william-wang approved these changes

@hwdef hwdef Awaiting requested review from hwdef

@shinytang6 shinytang6 Awaiting requested review from shinytang6

Assignees

@william-wang william-wang

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jiangkaihua @volcano-sh-bot @william-wang