Revert "scheduler/NodeUnschedulable: reduce pod scheduling latency"

[sanposhiho]

This reverts commit 28dbe8a.

What type of PR is this?

/kind bug
/priority critical-urgent
/triage accepted

What this PR does / why we need it:

NodeUnschedulable QueueingHint may miss Node related events that make Pod schedulable because of preCheck.
It's similar to: #119177 (comment)

So:

1. Node is added. But, it's filtered out by preCheck (due to node's unready or whatever) and the scheduling queue doesn't receive NodeAdded. (noderesourcefit cannot receive NodeAdded.)
2. Node is updated and ready now. But, this event is NodeUpdated.

In such scenarios, NodeUnschedulable returns QueueSkip to the event due to (2), because (2) is just a taint update of unready → read, which looks unrelated to NodeUnschedulable.

Which issue(s) this PR fixes:

Temporal fix for #122284

Special notes for your reviewer:

@kubernetes/sig-scheduling-leads
Same as #122285.
If possible, we have to get this PR in v1.29, but the release date is very close.
The user impact is that sometimes Pods, which was rejected by NodeUnschedulable, could be stuck in unschedQ during max 5min even though a new Node is created.
Can we somehow include it in v1.29? or do we have to wait for the patch release.

Does this PR introduce a user-facing change?

QueueingHint implementation for NodeUnschedulable is reverted because we found potential scenarios where events that make Pods schedulable could be missed.

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.29 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.29.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Tue Dec 12 22:21:49 UTC 2023.

[ameukam]

/milestone v1.29

[pacoxu]

/hold
as #122289 is merged. discussion thread in https://kubernetes.slack.com/archives/C2C40FMNF/p1702438650646789

[sanposhiho]

Given #122289 is merged, it's OK to revert the change in the patch release.

[sanposhiho]

/unhold

It's good to go now. I'll also cherry-pick to v1.29.

[kerthcet]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kerthcet, sanposhiho

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:

• pkg/scheduler/OWNERS [kerthcet,sanposhiho]

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

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 595a81812600c943b6bd9a52c62cc663ce1e3db7

[alculquicondor]

Since the feature as a whole was already disabled, why not just do a fix forward instead of a revert first?

[sanposhiho]

@alculquicondor

The revert is for a cherry-pick. v1.29 still encounter a problem when the feature gate is explicitly enabled.
Or, do we usually consider a bug-fix for features, which are disabled by default, not to deserve a cherry-pick?

[kerthcet]

This is for cherry-pick as introduced a scheduling latency bug. The quick fix is only for not blocking the 1.29 release.

[alculquicondor]

Or, do we usually consider a bug-fix for features, which are disabled by default, not to deserve a cherry-pick?

If it's beta, it should be fixed.

I missed that the fix is the revert itself. I thought there would be a follow up.