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

But I don't see the start of my tests in the logs, Also I have not set any -F option

@mhkarimi1383 , Let me have a look at it.

@mhkarimi1383 I did try locally also. I got stuck myself because of PVC not getting Bound. FInally figured out that it was due to GO int limitation which was causing parsing issue. All test in develop passes after this fix.

#387

For failure in current PR. It seems to be failing during the mount. Pod is not going into Running state. This PR involves changes to mount logic also. Can you please test it manually also.

E0420 10:03:54.534370 1 grpc.go:79] GRPC error: rpc error: code = Internal desc = failed to format and mount the volume error: format of disk "/dev/lvmvg/pvc-bb191605-f11e-4e6d-94b1-a6ee8a2f80cf" failed: type:("ext4") target:("/var/lib/kubelet/pods/7cbb193c-b511-4ffb-b38f-1018d4f1c7da/volumes/kubernetes.io~csi/pvc-bb191605-f11e-4e6d-94b1-a6ee8a2f80cf/mount") options:("defaults") errcode:(exit status 1) output:(mke2fs 1.47.0 (5-Feb-2023)
mkfs.ext4: invalid blocks '-F' on device ''

@abhilashshetty04

Will apply https://patch-diff.githubusercontent.com/raw/openebs/lvm-localpv/pull/387.patch locally and will do build and test

@abhilashshetty04

Will apply https://patch-diff.githubusercontent.com/raw/openebs/lvm-localpv/pull/387.patch locally and will do build and test

This patch is unrelated to the errors that we have on this PR. You might have to look into mount code changes that was done.

Found the problem with this,

package main

import (
	"fmt"
	"strings"
)

func main() {
	splitted := strings.Split("", " ")
	fmt.Println(len(splitted))
	fmt.Println(splitted[0])
}

will print

1

Notice empty line

This will cause the format command to be something like

mkfs.ext4 "" -F -m0 ./a.img

Now every thing is correct, I just have to work on the test case

I0424 20:51:44.835644       1 grpc.go:72] GRPC call: /csi.v1.Node/NodePublishVolume requests {"target_path":"/var/lib/kubelet/pods/e081a5a3-346c-45fd-bebd-770fdb3f78de/volumes/kubernetes.io~csi/pvc-eb7f5060-9c8e-46ff-88bd-a7fd04aaf571/mount","volume_capability":{"AccessType":{"Mount":{"fs_type":"ext4"}},"access_mode":{"mode":1}},"volume_context":{"csi.storage.k8s.io/ephemeral":"false","csi.storage.k8s.io/pod.name":"format-options-verifier-7d5d49cf75-nw4lp","csi.storage.k8s.io/pod.namespace":"openebs","csi.storage.k8s.io/pod.uid":"e081a5a3-346c-45fd-bebd-770fdb3f78de","csi.storage.k8s.io/serviceAccount.name":"default","openebs.io/cas-type":"localpv-lvm","openebs.io/format-options":"-b 4096 -N 5000000","openebs.io/volgroup":"lvmvg","storage.kubernetes.io/csiProvisionerIdentity":"1745527750375-7479-local.csi.openebs.io"},"volume_id":"pvc-eb7f5060-9c8e-46ff-88bd-a7fd04aaf571"}
I0424 20:51:44.844322       1 mount_linux.go:516] Disk "/dev/lvmvg/pvc-eb7f5060-9c8e-46ff-88bd-a7fd04aaf571" appears to be unformatted, attempting to format as type: "ext4" with options: [-b 4096 -N 5000000 -F -m0 /dev/lvmvg/pvc-eb7f5060-9c8e-46ff-88bd-a7fd04aaf571]

Also I found test problem

Containers are not able to do tune2fs since they only get a mapped device I think

Found the issue, We are using deployments to test things but here I only need the container to success fully exit

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