Add FSType for CSI volume source

[NickrenREN]

What this PR does / why we need it:
Add FSType for CSI volume source to specify filesystems (alpha defaults to ext4)

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #58183

Special notes for your reviewer:

Release note:

Add FSType for CSI volume source to specify filesystems

/assign @saad-ali
cc @vladimirvivien

[liggitt]

cc @kubernetes/sig-storage-api-reviews

[jsafrane]

/approve
from storage point of view

[jsafrane]

Comment above says that Implicitly inferred to be "ext4" if unspecified. We need some code for that if csiSource.FSType is empty.

[NickrenREN]

I saw that in drivers repo, we would defaults FSType to ext4 in function formatAndMount if it was not set. Should we deal with this here ?
btw, when this gets merged, i will update the external provisioner to add FSType for newly created PV if it is set in StorageClass parameters.

[jsafrane]

We don't have control about all CSI drivers in the wold to fall back to ext4 when nothing is specified. Other drivers may choose a different default, so it's Kubernetes who should enforce ext4 here.

[NickrenREN]

done, thanks, PTAL

[NickrenREN]

/assign @jsafrane

[jsafrane]

/lgtm

[NickrenREN]

/assign @thockin

[vladimirvivien]

@NickrenREN last time I had to manually make type changes in staging/src/k8s.io/api/core/v1/types.go . You will need to make FSType change there as well.

[NickrenREN]

@vladimirvivien i have already changed that in my first commit

[saad-ali]

/lgtm
/approve

One more thing to consider for API review:

Does this aligns with how we plan to expose block storage long term? Having the fstype inside the volume source, if I remember correctly was an API choice that we somewhat regretted when trying to introduce block volume support. That said, all the in-tree volumes currently have fstype as part of the volume source, so this aligns with that.

@mtanino, @erinboyd any comments??

[msau42]

During the block design, the idea came up that it could be cleaner if fstype was in the PVC instead of per volume type, so that we could standardize fs handling across all volume types instead of having each plugin handle it.

[mtanino]

@saad-ali @msau42

IIUC,

During the block design, the idea came up that it could be cleaner if fstype was in the PVC instead of per volume type, so that we could standardize fs handling across all volume types instead of having each plugin handle it.

Do you mean volume source rather than volume type?
I believe each volume source or persistent volume source should have fstype since this parameter is characteristic of volume.
But adding fstype into PVC then just pass through the value to PV would be interesting.
because;

• volumeMode and fstype are strongly related parameter
• we can easily validate volumeMode and fstype if both of them are in the same PVC object

However, I believe changing this affects big(?) impact to all plugins, also we need keep backward compatibility, therefore I think we don't have a plan to change this.

@erinboyd @screeley44
Any comments?

[msau42]

Yeah I meant putting the fsType in the higher level PV/PVC, instead of as a parameter in each volume-specific source. Then fs formatting and supporting scenarios like fs-on-block could be done by Kubernetes instead of every single plugin.

But yes, this is a major change and would be messy when also considering backwards compatibility, so we just stayed with the current way of specifying fsType in every volume source spec.

[mtanino]

@saad-ali
Thus I agree this API change.

[NickrenREN]

thanks @mtanino , ping @saad-ali @msau42 @thockin for approval

[saad-ali]

Sounds good to me. Will need to clean this up for Kubernetes 2.0!

@thockin for API approval

[thockin]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, NickrenREN, saad-ali, thockin

Associated issue: #58183

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

Needs approval from an approver in each of these OWNERS Files:

• api/OWNERS [thockin]
• docs/api-reference/OWNERS [thockin]
• pkg/apis/core/OWNERS [thockin]
• pkg/volume/csi/OWNERS [jsafrane,saad-ali,thockin]
• staging/src/k8s.io/api/OWNERS [thockin]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.