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

Also why in Makefile we are using sudo to build images?
It will break systems with podman

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.22%. Comparing base (8feeaa0) to head (12087cf).
Report is 60 commits behind head on develop.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop     #335      +/-   ##
===========================================
- Coverage    98.66%   97.22%   -1.44%     
===========================================
  Files            2        2              
  Lines          673      902     +229     
===========================================
+ Hits           664      877     +213     
- Misses           5       18      +13     
- Partials         4        7       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Also getting

/usr/local/bin/lvm-driver: No such file or directory

after building and running built image

Both with sudo and without sudo while building...

I think this is due to some shared libs
We could make go build to do everything statistically

As I understood that's because of my Operating System (NixOS), since it has different type of packaging/paths

Also why in Makefile we are using sudo to build images? It will break systems with podman

Perhaps because Podman is pretty new compared to Docker, when this Makefile was written. We may want to change that at some point.

@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

@mhkarimi1383 Did you see this comment yet?

@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

@mhkarimi1383 Did you see this comment yet?

Yes,

But since I'm a bit new to this project I need to run the project locally on my minikube setup
Due to shared libs problem I can't test it locally

It is OK to remove sudo and make builds static?
Or I have to do it in another PR?

It is OK to remove sudo and make builds static? Or I have to do it in another PR?

Why is static builds required? Isn't having all required inputs in the nix-shell enough?

It is OK to remove sudo and make builds static? Or I have to do it in another PR?

Why is static builds required? Isn't having all required inputs in the nix-shell enough?

Paths are different there and I think because of that
I'm getting

/usr/local/bin/lvm-driver: No such file or directory

While running the built image

I think making builds static will make the binary file work everywhere :)

Just guessing :)
I think I have to give static build a try

After trying to do static link some libs are missing again, getting

ldd /usr/local/bin/lvm-driver
	/nix/store/9v5d40jyvmwgnq1nj8f19ji2rcc5dksd-glibc-2.37-45/lib/ld-linux-x86-64.so.2 (0x7f2f6b592000)
	libpthread.so.0 => /nix/store/9v5d40jyvmwgnq1nj8f19ji2rcc5dksd-glibc-2.37-45/lib/ld-linux-x86-64.so.2 (0x7f2f6b592000)
	libc.so.6 => /nix/store/9v5d40jyvmwgnq1nj8f19ji2rcc5dksd-glibc-2.37-45/lib/ld-linux-x86-64.so.2 (0x7f2f6b592000)
Error relocating /usr/local/bin/lvm-driver: __vfprintf_chk: symbol not found
Error relocating /usr/local/bin/lvm-driver: __fprintf_chk: symbol not found

I can't test this project when using NixOS, Using docker for build is a good option too

For me problem solved by these changes

If it's OK, I will create a new PR to apply them (I don't know if you are Ok with using musl)

diff --git a/Makefile b/Makefile
index dda7432..27501c3 100644
--- a/Makefile
+++ b/Makefile
@@ -222,7 +222,7 @@ lvm-driver-image: lvm-driver
    @echo "+ Generating ${CSI_DRIVER} image"
    @echo "--------------------------------"
    @cp bin/${CSI_DRIVER}/${CSI_DRIVER} buildscripts/${CSI_DRIVER}/
-   cd buildscripts/${CSI_DRIVER} && sudo docker build -t ${IMAGE_ORG}/${CSI_DRIVER}:${IMAGE_TAG} ${DBUILD_ARGS} . && sudo docker tag ${IMAGE_ORG}/${CSI_DRIVER}:${IMAGE_TAG} quay.io/${IMAGE_ORG}/${CSI_DRIVER}:${IMAGE_TAG}
+   cd buildscripts/${CSI_DRIVER} && docker build -t ${IMAGE_ORG}/${CSI_DRIVER}:${IMAGE_TAG} ${DBUILD_ARGS} . && docker tag ${IMAGE_ORG}/${CSI_DRIVER}:${IMAGE_TAG} quay.io/${IMAGE_ORG}/${CSI_DRIVER}:${IMAGE_TAG}
    @rm buildscripts/${CSI_DRIVER}/${CSI_DRIVER}
 
 .PHONY: ansible-runner-image
@@ -230,7 +230,7 @@ ansible-runner-image:
    @echo "------------------"
    @echo "--> Build ansible-runner image for lvm-localpv e2e-tests" 
    @echo "------------------"
-   sudo docker build . -f e2e-tests/Dockerfile -t ${IMAGE_ORG}/lvm-localpv-e2e:ci
+   docker build . -f e2e-tests/Dockerfile -t ${IMAGE_ORG}/lvm-localpv-e2e:ci
 
 .PHONY: ci
 ci:
diff --git a/buildscripts/build.sh b/buildscripts/build.sh
index a6e7be5..9b3374e 100755
--- a/buildscripts/build.sh
+++ b/buildscripts/build.sh
@@ -100,8 +100,9 @@ env GOOS=$GOOS GOARCH=$GOARCH go build -ldflags \
     "-X github.com/openebs/lvm-localpv/pkg/version.GitCommit=${GIT_COMMIT} \
     -X main.CtlName='${CTLNAME}' \
     -X github.com/openebs/lvm-localpv/pkg/version.Version=${VERSION} \
-    -X github.com/openebs/lvm-localpv/pkg/version.VersionMeta=${VERSION_META}"\
-    -o $output_name\
+    -X github.com/openebs/lvm-localpv/pkg/version.VersionMeta=${VERSION_META} \
+    -linkmode external -w -extldflags '-static'"\
+    -a -o $output_name\
     ./cmd
 
 echo ""
diff --git a/shell.nix b/shell.nix
index d070f65..58acd8d 100644
--- a/shell.nix
+++ b/shell.nix
@@ -4,7 +4,9 @@ let
 in
 pkgs.mkShell {
   name = "scripts-shell";
+
   buildInputs = with pkgs; [
+    nix-ld                # Enable nix-ld linker
     chart-testing
     ginkgo
     git
@@ -16,7 +18,14 @@ pkgs.mkShell {
     minikube
     semver-tool
     yq-go
+    musl                  # Musl for static linking
+    gcc
+    binutils
+    gdb
+    coreutils
+    stdenv.cc.cc
   ];
+
   shellHook = ''
     export HOME=${builtins.getEnv "HOME"}
     export GOPATH=$(pwd)/nix/.go

I would say if above changes to build let you test things locally, you can go ahead and make only functional changes in this PR and let the build remain as is for now.
cc: @tiagolobocastro , @abhilashshetty04 , @abhilashshetty04