volumebinding: scheduler queueing hints - CSIStorageCapacity

[bells17]

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

resource	field	Referenced in PreFilter+Filter?	Admission Overwrite Prevention Config	Need to Check Changes in QHint?
CSIStorageCapacity	.metadata.labels	x	x	x
CSIStorageCapacity	.metadata.annotations	x	x	x
CSIStorageCapacity	.nodeTopology	o	o	x
CSIStorageCapacity	.storageClassName	o	o	x
CSIStorageCapacity	.capacity	o	x	o
CSIStorageCapacity	.maximumVolumeSize	o	x	o

ref(ja): https://zenn.dev/bells17/scraps/65bd6891012bdc

Does this PR introduce a user-facing change?

kube-scheduler implements scheduling hints for the VolumeBinding plugin.
The scheduling hints allow the scheduler to retry scheduling a Pod that was previously rejected by the VolumeBinding plugin only if a new resource referenced by the plugin was created or an existing resource referenced by the plugin was updated.

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

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bells17
Once this PR has been reviewed and has the lgtm label, please assign thockin for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• pkg/scheduler/framework/plugins/volumebinding/OWNERS

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]

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-sigs/prow repository.

[bells17]

/cc @sanposhiho @utam0k

[sanposhiho]

/kind feature

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: bells17 / name: bells17 (411d7ae, 2e8edf8, 74f80bf, 5187bf1, c05b069, 362a702, 21ad053, e1e7313, 8a942f3)

[sanposhiho]

How often is CSIStorageCapacity updated in an actual world?

If it's infrequent in general, can we just return Queue when CSIStorageCapacity is created/updated and a Pod has specific .spec.volumes?

e.g.,

func (pl *VolumeBinding) isSchedulableAfterCSIStorageCapacityChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
//...
	for _, vol := range pod.Spec.Volumes {
		if vol.PersistentVolumeClaim != nil || vol.Ephemeral != nil:
			// This Pod might have got unschedulable due to CSIStorageCapacity in a past scheduling cycle.			
			return framework.Queue, nil 
		}
	}
	return framework.QueueSkip, nil

[bells17]

Thank you for your comment.

How often is CSIStorageCapacity updated in an actual world?

CSIStorageCapacity is updated by the external-provisioner of each CSI Driver. The relevant code can be found here:
https://github.com/kubernetes-csi/external-provisioner/blob/v5.0.0/pkg/capacity/capacity.go#L592-L669

There are several events that trigger the update, but with the default settings, each CSIStorageCapacity for a single CSI Driver is updated at least every minute:
https://github.com/kubernetes-csi/external-provisioner/blob/v5.0.0/pkg/capacity/capacity.go#L512-L526
https://github.com/kubernetes-csi/external-provisioner/blob/v5.0.0/cmd/csi-provisioner/csi-provisioner.go#L111

The number of CSIStorageCapacity objects depends on the NodeTopology configuration of the target CSI Driver.
For example, when using network volumes like the gcp-compute-persistent-disk-csi-driver, it can be on a per-zone basis:
https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/blob/v999.1.0/pkg/gce-pd-csi-driver/node.go#L377-L392
When using lvm running on each node, like topolvm, CSIStorageCapacity is created on a per-node basis:
https://github.com/topolvm/topolvm/blob/main/internal/driver/node.go#L557-L566

If it's infrequent in general, can we just return Queue when CSIStorageCapacity is created/updated and a Pod has specific .spec.volumes?

Yes, that approach alone can help skip queueing Pods unrelated to CSIStorageCapacity, reducing unnecessary overhead.

[bells17]

@sanposhiho @toVersus Thank you for your review. I have made the necessary changes, so please take another look.

[toVersus]

Only some nits, other than that LGTM.

[sanposhiho]

It makes me wonder: in the first place, could a Pod without any PVC or ephemeral volume get unschedulable by VolumeBinding plugin?
If No, we likely end up having no QHint for CSIStorageCapacity (for now, at least until we build some observability like I said).

[bells17]

I'm sorry, I made a mistake in correcting the log message.
(This has been fixed)

It makes me wonder: in the first place, could a Pod without any PVC or ephemeral volume get unschedulable by VolumeBinding plugin?

I think the answer to this is No.

If No, we likely end up having no QHint for CSIStorageCapacity (for now, at least until we build some observability like I said).

Regarding this point, I think that if we don't return QueueSkip in the QHint, scheduling will be attempted due to CSIStorageCapacity changes even in cases where the Pod doesn't have a Volume. Would this not cause any issues in your opinion?

[sanposhiho]

Regarding this point, I think that if we don't return QueueSkip in the QHint, scheduling will be attempted due to CSIStorageCapacity changes even in cases where the Pod doesn't have a Volume. Would this not cause any issues in your opinion?

Only QHints of plugins that have rejected a Pod would be run. e.g., if, in a previous scheduling cycle, a Pod is failed by resource fit plugin and PodAffinity plugin, then only QHints of those plugins would be run for the Pod.

Then, if the answer to my first question "could a Pod without any PVC or ephemeral volume get unschedulable by VolumeBinding plugin?" is No, filtering Pods with volume in QHint doesn't make any point since only Pods with volume would reach this QHint in the first place.

[bells17]

Thank you for your input. I understand.
I'll proceed to close the unnecessary PRs.

[toVersus]

/lgtm

[k8s-ci-robot]

@toVersus: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

[bells17]

/close
#124961 (comment)

[k8s-ci-robot]

@bells17: Closed this PR.

In response to this:

/close
#124961 (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-sigs/prow repository.