kubelet: perform the admission checks that preemption will not help first to avoid meaningless pod eviction

[SataQiu]

What type of PR is this?

/kind bug
/kind cleanup

What this PR does / why we need it:

When generalFilter fails, HandleAdmissionFailure will attempt to release resources for the critical Pod by triggering Pod eviction. This is a high cost operation and maybe unhelpful. We should perform the resource unrelated checks first and quick return to avoid meaningless pod eviction.

kubernetes/pkg/kubelet/preemption/preemption.go

Lines 62 to 87 in d2be69a

	func (c *CriticalPodAdmissionHandler) HandleAdmissionFailure(admitPod *v1.Pod, failureReasons []lifecycle.PredicateFailureReason) ([]lifecycle.PredicateFailureReason, error) {
	if !kubetypes.IsCriticalPod(admitPod) {
	return failureReasons, nil
	}
	// InsufficientResourceError is not a reason to reject a critical pod.
	// Instead of rejecting, we free up resources to admit it, if no other reasons for rejection exist.
	nonResourceReasons := []lifecycle.PredicateFailureReason{}
	resourceReasons := []*admissionRequirement{}
	for _, reason := range failureReasons {
	if r, ok := reason.(*lifecycle.InsufficientResourceError); ok {
	resourceReasons = append(resourceReasons, &admissionRequirement{
	resourceName: r.ResourceName,
	quantity: r.GetInsufficientAmount(),
	})
	} else {
	nonResourceReasons = append(nonResourceReasons, reason)
	}
	}
	if len(nonResourceReasons) > 0 {
	// Return only reasons that are not resource related, since critical pods cannot fail admission for resource reasons.
	return nonResourceReasons, nil
	}
	err := c.evictPodsToFreeRequests(admitPod, admissionRequirementList(resourceReasons))
	// if no error is returned, preemption succeeded and the pod is safe to admit.
	return nil, err
	}

Which issue(s) this PR fixes:

Fixes #

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.:

[pacoxu]

• I even did not know about the rejection before reviewing this PR. So I opened update kubernetes.io/os description as it can also be used in pod labels website#40332.

My concern is that this would rarely happen the pod would be rejected for OSSelector or OSField.

• I am not sure if this is widely used or used in some special customers.

From a logical standpoint, this PR looks good to me. However, it would be helpful if you could provide a specific use case for it. For instance, imagine that a Windows Pod with high resource requirements is requested and labeled as such but without any corresponding Windows node selector or affinity. In this scenario, the pod may end up being scheduled on a Linux node and potentially cause an eviction of other pods on that node - particularly if the evicted pods have lower priority than the new Windows Pod.

If a large resource is requested, the pod should not be scheduled here. Therefore, I am unable to create a complete test case to reproduce this bug.

[SataQiu]

@pacoxu To be honest, It is not a usual problem. But how about these scenarios where the scheduler is not involved?
For example, a new critical static Pod with high memory requests is deployed over HTTP endpoint, kubelet mistakenly think that evicting some Pods can get it up and running, but finally failed by OS label mismatch. It is unhelpful to perform eviction prematurely.

[pacoxu]

/lgtm
but I still think this would be caused by old-style use and some user mistake configuration(with os label but no node selector/node affinity).

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 3e03565994eadcdfb121a980239e2c1b68b0ac54

[SataQiu]

/assign @klueska

[SataQiu]

/triage accepted

[marquiz]

lgtm

ping @klueska could you take a look

[SataQiu]

Hi @klueska
Would you mind taking a look?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrunalp, SataQiu

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/kubelet/OWNERS [mrunalp]

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