Change the error message. #2773
Conversation
|
Welcome @gj199575! It looks like this is your first PR to volcano-sh/volcano 🎉 |
volcano-sh-bot
added
the
size/M
|
Hello, you are very welcome to participate in community contributions. From your description, you can see the events and error log information that will continue to be output by Volcano. I think it would be better to create an issue and classify similar PRs in a unified way. |
| @@ -98,8 +98,8 @@ func (alloc *Action) Execute(ssn *framework.Session) { | |||
| allNodes := ssn.NodeList | |||
| predicateFn := func(task *api.TaskInfo, node *api.NodeInfo) error { | |||
| // Check for Resource Predicate | |||
| if !task.InitResreq.LessEqual(node.FutureIdle(), api.Zero) { | |||
| return api.NewFitError(task, node, api.NodeResourceFitFailed) | |||
| if reason := task.InitResreq.LessEqualResource(node.FutureIdle(), api.Zero); len(reason) != 0 { | |||
There was a problem hiding this comment.
Why add a new function here? If a return value is added on the basis of the original function, it indicates whether the resource information of the error is acceptable.
In addition, there are many places where lessEqual is called. Do other places need to be modified or modified iteratively?
There was a problem hiding this comment.
Why add a new function here?
Because the purpose of this PR is to make volcano event belong to a unscheduled pod , is more similar to kube-scheduler 。
So I do not need to change many codes。
And the LessEqual function is used by many places , if I change LessEqual function , it is a huge project。
So I choose a better way to solve my problem ，and do not change volcano a lot。
There was a problem hiding this comment.
If you are worried that the impact of the modification is too large, you can add a new function, but the return value of the new function is recommended to return two (bool, string), the first parameter indicates the result, and the second parameter indicates msg, in the return value character Whether the string is empty indicates the result of the judgment value, which feels inappropriate
ok， I will create a issue |
|
Please refer to the prompt information DCO to repair CI |
gj199575
force-pushed
the
fix_errorMsg
branch
2 times, most recently
from
aee5781
to
3c35cfc
Compare
volcano-sh-bot
added
size/L
ok |
gj199575
force-pushed
the
fix_errorMsg
branch
5 times, most recently
from
dd17d67
to
be8cff3
Compare
|
/assign @jiangkaihua |
|
/priority important-soon |
volcano-sh-bot
added
the
priority/important-soon
| @@ -98,8 +98,8 @@ func (alloc *Action) Execute(ssn *framework.Session) { | |||
| allNodes := ssn.NodeList | |||
| predicateFn := func(task *api.TaskInfo, node *api.NodeInfo) error { | |||
| // Check for Resource Predicate | |||
| if !task.InitResreq.LessEqual(node.FutureIdle(), api.Zero) { | |||
| return api.NewFitError(task, node, api.NodeResourceFitFailed) | |||
| if ok, reason := task.InitResreq.CheckResource(node.FutureIdle(), api.Zero); !ok { | |||
There was a problem hiding this comment.
@gj199575 LessEqual is an imporment foundamental function. What's the difference between the CheckResource and LessEqual, and why we have to add this new function, Would you provide detail reason?
There was a problem hiding this comment.
@gj199575 LessEqual is an imporment foundamental function. What's the difference between the CheckResource and LessEqual, and why we have to add this new function, Would you provide detail reason?
And it is a huge project if I change LessEqual function .So I supply a new function LessEqualWithReason 。 And I will merge LEssEqual and LessEqualWithReason to a funtion 。
volcano-sh-bot
added
lgtm
when pod is unscheduled, make the error message similar to kube-scheduler。 Signed-off-by: gj199575 <409237405@qq.com>
|
[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 |
this MR is the relization of this issue : #2774
when pod is unscheduled, make the error message similar to kube-scheduler。
test message
the default scheduler error message is "0/2 nodes are available: 2 Insufficient cpu."
before this MR
volcano error message is "all nodes are unavailable: 2/2 nodes node(s) resource fit failed."
after this MR
volcano error message is "all nodes are unavailable: 2/2 nodes Insufficient cpu."
And in the future。 I will continue to make volcano error event or something which is showed to user is more similar to k8s default scheduler