No clear default value for thin provisioning of filesystems

[abuisine]

What steps did you take and what happened:
I created a simple storageClass as follows :

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
 name: openebs-zfspv
allowVolumeExpansion: true
parameters:
 recordsize: "128k"
 fstype: "zfs"
 poolname: "zfspv-pool"
provisioner: zfs.csi.openebs.io

and then created both a PVC and a Pod in order to trigger the CSI provisioner which then creates a zfs FS

What did you expect to happen:
I would have expected a thick provisioning aka reservation or refreservation property on the created zfs FS but I got none.

As there is no default value in the documentation : https://github.com/openebs/zfs-localpv/blob/develop/docs/storageclasses.md#thinprovision-optional-parameter,
I had a look at the code and there is, it seems, a no default value :

zfs-localpv/pkg/apis/openebs.io/zfs/v1alpha1/zfsvolume.go

Line 151 in 1c4a810

// Default Value: no.

The output of the following commands will help us better understand what's going on:

zfs list -o name,quota,refreserv,reserv
NAME                                                 QUOTA  REFRESERV  RESERV
zfspv-pool                                            none       none    none
zfspv-pool/pvc-b16e1228-ff11-4101-a117-01f65ce3257e    21G       none    none

Anything else you would like to add:
I would suggest :

• to remove the comment in the code as there is actually no default behavior within this CSI provisioner
• add in the documentation an information about the default behavior of ZFS (which is thinprovision: yes, at least in my setup)

Also it is worth to be noted that setting thinprovision: "no" works as expected, my problem is limited to the default behavior.

Environment:

• LocalPV-ZFS version: zfs-localpv-2.4.0
• Kubernetes version (use kubectl version): v1.27.12
• Kubernetes installer & version: v1.27.12+rke2r1
• Cloud provider or hardware configuration: VM on ProxmoxVE
• OS (e.g. from /etc/os-release): Flatcar 3815.2.0

[Abhinandan-Purkait]

@abuisine Does #542 Solve this issue?

[abuisine]

Hi there, unfortunately no there is still a misleading comment that let users think that default is "thick" provisioning

zfs-localpv/pkg/apis/openebs.io/zfs/v1/zfsvolume.go

Line 152 in 4eaa7c1

// Default Value: no.

As far as I can tell, the PR, although being very interesting, is not related to this issue.

[abuisine]

I am not familiar with the code of zfs-localpv but quickly looking at it I did not see any thin provisioning default value management. Could you please confirm ?
I true, I can propose a PR to solve comments + doc

[Abhinandan-Purkait]

I am not familiar with the code of zfs-localpv but quickly looking at it I did not see any thin provisioning default value management. Could you please confirm ? I true, I can propose a PR to solve comments + doc

When you say thin provisioning default value management what are the things you are referring to? As of today the current code just adds the below property for a thick volume:

	if vol.Spec.ThinProvision == "no" {
		reservationProperty := "reservation=" + vol.Spec.Capacity
		ZFSVolArg = append(ZFSVolArg, "-o", reservationProperty)
	}

[abuisine]

I am referring to the fact that not setting anything on the storageClass will result in reservation not being set aka thin provisioning.
So the default behavior seems to me as being thinprovision: "yes" whereas the comments in the code stipulate that the default value is no

[abuisine]

The documentation should precise the default behavior, but this is a bonus.

[Abhinandan-Purkait]

The documentation should precise the default behavior, but this is a bonus.

Definitely, we could do better here @abuisine would you mind making the change? Otherwise this can be taken up by someone.

[abuisine]

Ok will do.

[Abhinandan-Purkait]

I am referring to the fact that not setting anything on the storageClass will result in reservation not being set aka thin provisioning. So the default behavior seems to me as being thinprovision: "yes" whereas the comments in the code stipulate that the default value is no

Okay, so I looked into the code again. Seems like there are two branches of call happening, based on the filesystem.
So if the filesystem is zfs, we create a dataset with property defined in the below snippet.

if vol.Spec.ThinProvision == "no" {
    reservationProperty := "reservation=" + vol.Spec.Capacity
    ZFSVolArg = append(ZFSVolArg, "-o", reservationProperty)
}

But if the filesystem is anything else, we create a zvol with with property defined in the below snippet.

if vol.Spec.ThinProvision == "yes" {
  ZFSVolArg = append(ZFSVolArg, "-s")
}

In the second case there is no handling for thinprovision: "no", it seems. Seems wrong, wdyt?

[abuisine]

In the second case there is no handling for thinprovision: "no", it seems. Seems wrong, wdyt?
Let me have a deeper look :)

[abuisine]

I ran some quick tests, it seems that:

• for filesystems, default is no reservation and no refreservation which means thin provisioning
• for volumes, default is no reservation but a refreservation is set which means thick provisioning

I will adjust my doc PR to clarify.
I assume this is something to manage in #542 as well.

[avishnu]

I ran some quick tests, it seems that:

* for filesystems, default is no `reservation` and no `refreservation` which means **thin** provisioning

* for volumes, default is no `reservation` but a `refreservation` is set which means **thick** provisioning

I will adjust my doc PR to clarify. I assume this is something to manage in #542 as well.

Can you please confirm if you tested ZFS or LocalPV ZFS to establish the above behavior?

[abuisine]

Can you please confirm if you tested ZFS or LocalPV ZFS to establish the above behavior?

Sorry, I used the ZFS command line

[Abhinandan-Purkait]

@abuisine @avishnu I ran some tests to check the behaviour for each value of the param thinProvision for the LocalPV ZFS. Here are the observations:

1. Default, i.e the thinProvision not specified in the storageclass at all

➜  ~ zfs list -o name,quota,refquota,reservation,refreservation,type
NAME                                                 QUOTA  REFQUOTA  RESERV  REFRESERV  TYPE
zfspv-pool                                            none      none    none       none  filesystem --> pool
zfspv-pool/pvc-51c96448-27ee-4aeb-8d42-ade05b2c3ef0      -         -    none      4.06G  volume
zfspv-pool/pvc-cce47d93-97f3-49c3-9981-d4d93a7cca9d     4G      none    none       none  filesystem

2. thinProvision: no specified in the storageclass

➜  ~ zfs list -o name,quota,refquota,reservation,refreservation,type
NAME                                                 QUOTA  REFQUOTA  RESERV  REFRESERV  TYPE
zfspv-pool                                            none      none    none       none  filesystem --> pool
zfspv-pool/pvc-c965a64c-d870-40b7-a4a4-c8b2f2b1d562      -         -    none      4.06G  volume
zfspv-pool/pvc-b01d5742-7ac4-42a0-9fd1-5435436ffa6c     4G      none      4G       none  filesystem

3.thinProvision: yes specified in the storageclass

➜  ~ zfs list -o name,quota,refquota,reservation,refreservation,type
NAME                                                 QUOTA  REFQUOTA  RESERV  REFRESERV  TYPE
zfspv-pool                                            none      none    none       none  filesystem --> pool
zfspv-pool/pvc-571a7947-b45f-4718-887a-82e8513432ec      -         -    none       none  volume
zfspv-pool/pvc-e48c25e4-92e9-4189-b56a-5a7080ff99aa     4G      none    none       none  filesystem

[abuisine]

Ok same results than what I infered with my CLI tests.
The documentation is therefore good to go.