-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
volumebinding: scheduler queueing hints - PersistentVolumeClaim #124959
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bells17 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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-sigs/prow repository. |
/cc @sanposhiho @utam0k |
/kind feature |
ed19dba
to
8af54d6
Compare
/retest |
bdae41a
to
c98c26a
Compare
c98c26a
to
333ed68
Compare
b72b4f6
to
07a6ffd
Compare
/retest |
pkg/scheduler/framework/plugins/volumebinding/volume_binding.go
Outdated
Show resolved
Hide resolved
pkg/scheduler/framework/plugins/volumebinding/volume_binding.go
Outdated
Show resolved
Hide resolved
f4cd48b
to
d4bbe7f
Compare
/retest |
d4bbe7f
to
42fb6e4
Compare
/retest |
@sanposhiho Thank you for your review. I have made the necessary changes, so please take another look. |
/lgtm |
LGTM label has been added. Git tree hash: 73a7afd4eae8c58c5eef61e7c30d92b2126b9d51
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only nits
) | ||
|
||
if pod.Namespace != newPVC.Namespace { | ||
logger.V(5).Info("PersistentVolumeClaim was created or updated, but it doesn't make this pod schedulable") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.V(5).Info("PersistentVolumeClaim was created or updated, but it doesn't make this pod schedulable") | |
logger.V(5).Info("PersistentVolumeClaim was created or updated, but it doesn't make this pod schedulable because the PVC belongs to a different namespace") |
pkg/scheduler/framework/plugins/volumebinding/volume_binding.go
Outdated
Show resolved
Hide resolved
New changes are detected. LGTM label has been removed. |
/retest |
pkg/scheduler/framework/plugins/volumebinding/volume_binding.go
Outdated
Show resolved
Hide resolved
c119ce3
to
217c8a3
Compare
// We bind PVCs with PVs, so any changes may make the pods schedulable. | ||
{Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add | framework.Update}}, | ||
// QHintFn is added only for PersistentVolumeClaim because: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really "only" pvc? I haven't looked at @YamasouA's PRs deeply yet though, they look alrighty; no reference to other resources.
What type of PR is this?
/kind feature
What this PR does / why we need it:
kube-scheduler implements scheduling hints for the VolumeBinding plugin.
The scheduling hints allow the scheduler to determine whether to retry or skip scheduling a Pod based on the changes made to the PersistentVolumeClaim resource referenced by the plugin.
Which issue(s) this PR fixes:
Part of #118893
KEP: https://github.com/kubernetes/enhancements/blob/master/keps/sig-scheduling/4247-queueinghint/README.md
Base PR: #124939
Special notes for your reviewer:
Fields Impacting QueueingHintFn
PersistentVolume (PV) is not included in this table because it can undergo extensive changes when a conversion is performed by csi-translation-lib.
ref(ja): https://zenn.dev/bells17/scraps/65bd6891012bdc
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: