How is the new formatOptions going to be set by user? via storageclass? There is no mkfsOptions unlike mountOptions as provided by storageclass.
Please write a test as well to show how this is used.

I think formatOptions need to be parsed from storage class at GetVolAndMountInfo ?

Hi

formatOptions is added to MountInfo just like mountOptions

So, It will work if you add it to storageclass

Also there is no test for the mounter itself

I think formatOptions need to be parsed from storage class at GetVolAndMountInfo ?

GetVolAndMountInfo is parsing and building mountinfo from csi node publish request, but I don't see anything for format options in csi request. So I wonder how that's going to work. The storageclass parsing is being done in NewVolumeParams that's part of controller plugin.
cc: @abhilashshetty04

Also there is no test for the mounter itself

You can run this manually and document here the steps to set formatOptions.

I think formatOptions need to be parsed from storage class at GetVolAndMountInfo ?

GetVolAndMountInfo is parsing and building mountinfo from csi node publish request, but I don't see anything for format options in csi request. So I wonder how that's going to work. The storageclass parsing is being done in NewVolumeParams that's part of controller plugin.
cc: @abhilashshetty04

It's my first time that I'm contributing to this project so I just looked for where mountOptions is handled to handle formatOptions too

Hello again

I'm using NixOS, and I can't run make, waiting on #351

I think formatOptions need to be parsed from storage class at GetVolAndMountInfo ?

I checked out more and I saw that we have to put this on where we are mounting volume for the first time, So there is no need to add this into GetVolAndMountInfo, we have to handle that just like mountOptions

Not able to test due to #353 and #352

@mhkarimi1383
Also, please refer to the change https://github.com/openebs/zfs-localpv/pull/542/files . Something similar is likely required here also for these formatOptions to be usable.
cc: @abhilashshetty04 @Abhinandan-Purkait

@dsharma-dc

golint-ci returns error

pkg/driver/controller.go:338:6: declared and not used: vol (typecheck)
	var vol *lvmapi.LVMVolume
	   ^
pkg/lvm/mount.go:31:2: could not import k8s.io/mount-utils (-: cannot compile Go 1.22 code) (typecheck)
	"k8s.io/mount-utils"
	^
pkg/lvm/mount.go:73:14: undefined: mount (typecheck)
	mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: utilexec.New()}
	           ^
pkg/lvm/mount.go:97:14: undefined: mount (typecheck)
	mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: utilexec.New()}
	           ^
pkg/lvm/mount.go:99:19: undefined: mount (typecheck)
	dev, ref, err := mount.GetDeviceNameFromMount(mounter, targetPath)
	                ^
make: *** [Makefile:263: golint] Error 1

I think to update to k8s.io/mount-utils we have to update go version

@dsharma-dc

golint-ci returns error

pkg/driver/controller.go:338:6: declared and not used: vol (typecheck)
	var vol *lvmapi.LVMVolume
	   ^
pkg/lvm/mount.go:31:2: could not import k8s.io/mount-utils (-: cannot compile Go 1.22 code) (typecheck)
	"k8s.io/mount-utils"
	^
pkg/lvm/mount.go:73:14: undefined: mount (typecheck)
	mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: utilexec.New()}
	           ^
pkg/lvm/mount.go:97:14: undefined: mount (typecheck)
	mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: utilexec.New()}
	           ^
pkg/lvm/mount.go:99:19: undefined: mount (typecheck)
	dev, ref, err := mount.GetDeviceNameFromMount(mounter, targetPath)
	                ^
make: *** [Makefile:263: golint] Error 1

I think to update to k8s.io/mount-utils we have to update go version

Would you like to do that as part of this PR? or instead try using a mount-utils that might work with go 1.19.

I can do both, whatever is Ok :)

I can do both, whatever is Ok :)

Thanks, I'd say then try using a compatible mount-utils version(I hope no other issues pop up), and Go upgrade can be taken up as a new PR.

@dsharma-dc

Ok, I will revert my changes to go.mod, etc. And will change the package again

I have fixed package versions and switched to a correct version of mount utils

And I was able to build project successfully and tests passed

@abhilashshetty04, @tiagolobocastro

I have done with tests ;)

Also it's good to make tests to run in parallel in CI I think

Also it's good to make tests to run in parallel in CI I think

Lets do a separate PR for that. We dont have bors or similar utility inplace for that right now in localpv repo.

Also it's good to make tests to run in parallel in CI I think

Lets do a separate PR for that. We dont have bors or similar utility inplace for that right now in localpv repo.

Yeah we should check if we don't have any naming conflicts, etc in resources that are creating in K8s but ginkgo has support for parallel tests

Also it's good to make tests to run in parallel in CI I think

Lets do a separate PR for that. We dont have bors or similar utility inplace for that right now in localpv repo.

Yeah we should check if we don't have any naming conflicts, etc in resources that are creating in K8s but ginkgo has support for parallel tests

Got it. We definately check it out.

Thanks, Please squash commits before merging @mhkarimi1383

I will

@abhilashshetty04
Squshed.

I had to recreate branch and apply diff of the PR, Because of some merge commits, etc.

anything pending before merge here, @mhkarimi1383 @abhilashshetty04 ?

anything pending before merge here, @mhkarimi1383 @abhilashshetty04 ?

I'm Done with this PR, LGTM

I can Also add a Test case for xfs using it's block size option, But IDK if xfs_info is available to the pods or not

Can you please rebase @mhkarimi1383

@abhilashshetty04 Rebased

@mhkarimi1383 Could you please update the documentation here: https://github.com/openebs/lvm-localpv/blob/develop/docs/storageclasses.md , based on your changes?

@dsharma-dc I will

@dsharma-dc

Covered in
#396