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

add e2e cases for preempt #892

Merged
merged 13 commits into from
Jul 1, 2020
Merged

Conversation

Thor-wl
Copy link
Contributor

@Thor-wl Thor-wl commented Jun 30, 2020

No description provided.

Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
Signed-off-by: Thor <1187526662@qq.com>
@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 30, 2020
@TravisBuddy
Copy link

Travis tests have failed

Hey @Thor-wl,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: a83cfd20-bace-11ea-bd85-f15c2d376ac7

Signed-off-by: Thor <1187526662@qq.com>
@TravisBuddy
Copy link

Travis tests have failed

Hey @Thor-wl,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 2bce03e0-bada-11ea-bd85-f15c2d376ac7

Signed-off-by: Thor <1187526662@qq.com>
@@ -604,7 +513,7 @@ var _ = Describe("Job E2E Test", func() {
if expectPod%1 == 1 {
expectPod--
}
err = wait.Poll(100*time.Millisecond, oneMinute, func() (bool, error) {
err = wait.Poll(100*time.Millisecond, twoMinute, func() (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why change this, how come the scheduling is so slow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to be releated the cluster environment at that time.It will be timeout in rare cases. I've tried serval times and it woks well after set the timeout to be 2 minutes.

@@ -700,7 +609,7 @@ var _ = Describe("Job E2E Test", func() {
if expectPod%1 == 1 {
expectPod--
}
err = wait.Poll(100*time.Millisecond, oneMinute, func() (bool, error) {
err = wait.Poll(100*time.Millisecond, twoMinute, func() (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

ctx := initTestContext(options{
priorityClasses: map[string]int32{
masterPriority: masterPriorityValue,
workerPriority: workerPriorityValue,
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggest fix them in a separate pr, maybe call high/lowPriority

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, i will fix the name in next pr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it has been fixed at #893

MinMember: 1,
MinResources: &thirtyCPU,
},
Status: schedulingv1beta1.PodGroupStatus{
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove, create will ignore 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.

it has been fixed at #893

},
Spec: schedulingv1beta1.PodGroupSpec{
MinMember: 1,
MinResources: &thirtyCPU,
Copy link
Collaborator

Choose a reason for hiding this comment

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

donot hard coded , should calculate according to the cluster resource

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good advice. There are lots of hard coded in e2e cases and we need a adequate review later

@hzxuzhonghu hzxuzhonghu changed the title 0630: add e2e cases add e2e cases for preempt Jul 1, 2020
@hzxuzhonghu hzxuzhonghu added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 1, 2020
@hzxuzhonghu
Copy link
Collaborator

/lgtm

@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: Thor-wl

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 lgtm Indicates that a PR is ready to be merged. label Jul 1, 2020
@volcano-sh-bot volcano-sh-bot merged commit 2e24614 into volcano-sh:master Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants