Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CreateZFSVolume does not respect fsType=zfs -> dataset #143

Closed
cruwe opened this issue Jun 1, 2020 · 1 comment · Fixed by #144
Closed

CreateZFSVolume does not respect fsType=zfs -> dataset #143

cruwe opened this issue Jun 1, 2020 · 1 comment · Fixed by #144
Labels
enhancement Add new functionality to existing feature Need community involvement Needs community involvement on some action item.
Milestone

Comments

@cruwe
Copy link
Contributor

cruwe commented Jun 1, 2020

What steps did you take and what happened:
With storageclass

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: zfs-dataset
allowVolumeExpansion: true
parameters:
  fsType: "zfs"
provisioner: zfs.csi.openebs.io

the created datasets end up with type volume.

What did you expect to happen:

With fsType set, I'd like to have a dataset.

Anything else you would like to add:
The parameter req.GetParameters()["fstype"] in func CreateZFSVolume(req *csi.CreateVolumeRequest) (string, error) is inconsistent with the CRD. Either it is fstype in the CRD or it is req.GetParameters()["fsType"].

cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 1, 2020
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 1, 2020
Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
@pawanpraka1
Copy link
Contributor

@cruwe you can use fstype as "zfs" in the storageclass. And the Controller will fetch fstype from the storageclass and updated fsType of the ZFSVolume CRDs while creating it.

cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 2, 2020
More specifically,
- introduce helper functions to lookup values based on a sequence of
  possible keys and ignore case all together and
- lookup the fstype based on a sequence of possible spellings.
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 3, 2020
Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 3, 2020
Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 3, 2020
More specifically,
- introduce helper functions to lookup values based on a sequence of
  possible keys and ignore case all together and
- lookup the fstype based on a sequence of possible spellings.

Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 3, 2020
Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 3, 2020
…nebs#143

More specifically,
- introduce helper function to get maps with all keys set to lowercase,
- introduce lookup helper based on such maps and
- change lookups for CreateVolumeRequest()s and CreateVolume()s so that
  parameter keys are processed as lowercase irrespective of actual
  spelling.

Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 4, 2020
…nebs#143

More specifically,
- introduce helper function to get maps with all keys set to lowercase,
- introduce lookup helper based on such maps and
- change lookups for CreateVolumeRequest()s and CreateVolume()s so that
  parameter keys are processed as lowercase irrespective of actual
  spelling.

Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 4, 2020
…nebs#143

More specifically,
- introduce helper function to get maps with all keys set to lowercase,
- introduce lookup helper based on such maps and
- change lookups for CreateVolumeRequest()s and CreateVolume()s so that
  parameter keys are processed as lowercase irrespective of actual
  spelling.

Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
pawanpraka1 pushed a commit that referenced this issue Jun 4, 2020
More specifically,
- introduce helper function to get maps with all keys set to lowercase,
- introduce lookup helper based on such maps and
- change lookups for CreateVolumeRequest()s and CreateVolume()s so that
  parameter keys are processed as lowercase irrespective of actual
  spelling.

Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
@pawanpraka1 pawanpraka1 added Need community involvement Needs community involvement on some action item. enhancement Add new functionality to existing feature labels Jun 5, 2020
@pawanpraka1 pawanpraka1 added this to the v0.8.0 milestone Jun 5, 2020
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 9, 2020
…nebs#143

More specifically,
- provide an example storageclass, pvc and deployment which superfically
  seems to work,
- make the driver mount exportfs binary and nfs state dir / files (/etc/
   exports probably unnecessary)
- make the provider ignore the lately introduced legacy mount as a stop-
  gap measure (zfs, sahrenfs and mountpoint=legacy do not match) and
- move the sharenfs prop to the v1 api
cruwe added a commit to cruwe/zfs-localpv that referenced this issue Jun 9, 2020
…nebs#143

More specifically,
- provide an example storageclass, pvc and deployment which superfically
  seems to work,
- make the driver mount exportfs binary and nfs state dir / files (/etc/
   exports probably unnecessary)
- make the provider ignore the lately introduced legacy mount as a stop-
  gap measure (zfs, sahrenfs and mountpoint=legacy do not match) and
- move the sharenfs prop to the v1 api

Signed-off-by: Christopher J. Ruwe <cjr@cruwe.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Add new functionality to existing feature Need community involvement Needs community involvement on some action item.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants