pass status to handleSchedulingFailure

[AxeZhan]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

• remove reason from ScheduleResult
• Pass status to scheduler.handleSchedulingFailure instead of an error and a string.
• add WithError for Status

Which issue(s) this PR fixes:

Part of #103853

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

Please note that we're already in Test Freeze for the release-1.26 branch. This means every merged PR will be automatically fast-forwarded via the periodic ci-fast-forward job to the release branch of the upcoming v1.26.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Wed Nov 23 09:30:47 UTC 2022.

[k8s-ci-robot]

@kidddddddddddddddddddddd: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

Hi @kidddddddddddddddddddddd. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[AxeZhan]

/cc @kerthcet
And please do let me know if we have some other refactor works about this.

[kerthcet]

/ok-to-test

[AxeZhan]

/retest

[kerthcet]

Sorry @kidddddddddddddddddddddd , really busy these days, will review this tomorrow or the day after tomorrow?

[alculquicondor]

last comment :)

[kerthcet]

Then revert the change for reasons may be empty now. E.g. we no longer append reasons in NewStatus().WithError(). It should look like:

err := status.AsError()
msg := truncateMessage(err.Error())

[alculquicondor]

ah, we should be calling s.err.Error() in Status.Message. Can you do as a follow up?

[kerthcet]

We can not call s.err here. status.AsError will also return the error, and we're handling failures, so we'll always have an error in status.

[kerthcet]

Sorry, you mean the status.Message()? But somethings status may have no error but only reasons.

[AxeZhan]

ah, we should be calling s.err.Error() in Status.Message. Can you do as a follow up?

We construct s.err with reasons in AsStatus and NewStatus, so if the code of a status is Error, s.err.Error() will be contained by s.Message.
However, if status has a code not Error and has a error in the same time, s.Message() will not return the error msg, and in that case, we need to call s.err.Error() to get the error msg.(like fiterr in scheduling cycle).
I guess you're suggesting in status.Message(), we check the status's code and concate err.Error() into the result string, like:

func (s *Status) Message() string {
	if s == nil {
		return ""
	}
	if s.err != nil && s.code != Error {
		return s.err.Error() + "," + strings.Join(s.reasons, ", ") 
	}
	return strings.Join(s.reasons, ", ")
}

This looks a bit back to the start.

[alculquicondor]

It's not back to the start. The main difference is that we are only serializing when actually needed.

[AxeZhan]

created #114456 , we can have a disscussion/review there.

[AxeZhan]

may have a flaky test: Test_Run_OneVolumeDetachFailNodeWithReadWriteOnce
/retest

[AxeZhan]

/retest

[kerthcet]

/lgtm