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

feat(validation): adding validation for ZFSPV CR parameters #66

Merged
merged 5 commits into from
Apr 14, 2020

Conversation

pawanpraka1
Copy link
Contributor

@pawanpraka1 pawanpraka1 commented Mar 20, 2020

Validating few parameters for the ZFSVolume custom resource

  • compression can be "on", "off", "lzjb", "gzip", "gzip-[1-9]", "zle" and "lz4"
  • encryption can be "on", "off", "aes-128-ccm", "aes-192-ccm", "aes-256-ccm", "aes-128-gcm", "aes-192-gcm", and "aes-256-gcm"
  • dedup can be "on" and "off"
  • poolname can be string
  • ownernodeid can be present
  • thinprovision can be "yes" and "no"
  • volumetype can be "DATASET" and "ZVOL"

Also added required fields needed to create ZFSVolume CR

  • ownerNodeID
  • poolname
  • volumeType
  • capacity

For the below wrong yaml

apiVersion: openebs.io/v1alpha1
kind: ZFSVolume
metadata:
  name: pvc-1667faf6-d79b-4a5d-9811-2c4886127f67
  namespace: openebs
spec:
  capacity: "8589934592"
  compression: "yes"
  ownerNodeID: "pawan-2"
  poolName: "zfspv-pool"
  volumeType: "ZVOL"

Applying it will throw this error

$ kubectl apply -f zfspvcr.yaml
The ZFSVolume "pvc-test" is invalid: spec.compression: Invalid value: "": spec.compression in body should match '^(on|off|lzjb|gzip|gzip-[1-9]|zle|lz4)$'

Signed-off-by: Pawan pawan@mayadata.io

@pawanpraka1 pawanpraka1 added the enhancement Add new functionality to existing feature label Mar 20, 2020
@pawanpraka1 pawanpraka1 added this to the v0.6.0 milestone Mar 20, 2020
@pawanpraka1 pawanpraka1 requested a review from kmova March 20, 2020 13:14
@codecov-io
Copy link

codecov-io commented Mar 20, 2020

Codecov Report

Merging #66 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #66   +/-   ##
=======================================
  Coverage   23.57%   23.57%           
=======================================
  Files          14       14           
  Lines         475      475           
=======================================
  Hits          112      112           
  Misses        362      362           
  Partials        1        1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6033789...bcd84e0. Read the comment docs.

deploy/zfs-operator.yaml Outdated Show resolved Hide resolved
@kmova
Copy link
Member

kmova commented Mar 28, 2020

@pawanpraka1 -- any validations required for the ZFS Snapshot CRD?

@pawanpraka1
Copy link
Contributor Author

@pawanpraka1 -- any validations required for the ZFS Snapshot CRD?

no @kmova, user will modify ZFSVolume CR only for the property change.

@kmova
Copy link
Member

kmova commented Mar 28, 2020

Makes sense. on second thought, does adding validations for non-editable fields help in making sure the code also doesn't misbehave, can work as an added validation as the contributions to the project increase?

@pawanpraka1
Copy link
Contributor Author

pawanpraka1 commented Mar 29, 2020

Sure, I will add the validation for snapshot also.

Copy link
Member

@akhilerm akhilerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes looks good.

Copy link
Member

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm , thanks

Validating few parameters for the ZFSVolume custom resource

- compression can be "on", "off", "lzjb", "gzip", "gzip-[1-9]", "zle" and "lz4"
- encryption can be "on", "off", "aes-128-ccm", "aes-192-ccm", "aes-256-ccm", "aes-128-gcm", "aes-192-gcm", and "aes-256-gcm"
- dedup can be "on" and "off"
- poolname can be string
- ownernodeid can be string
- thinprovision can be "yes" and "no"
- volumetype can be "DATASET" and "ZVOL"

Also added required fields needed to create ZFSVolume CR
- ownerNodeID
- poolname
- volumeType
- capacity

Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
upgrade/crd.yaml Outdated Show resolved Hide resolved
upgrade/crd.yaml Outdated Show resolved Hide resolved
upgrade/crd.yaml Outdated Show resolved Hide resolved
upgrade/crd.yaml Show resolved Hide resolved
upgrade/crd.yaml Outdated Show resolved Hide resolved
upgrade/crd.yaml Outdated Show resolved Hide resolved
Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
@kmova kmova merged commit ae724ee into openebs:master Apr 14, 2020
pawanpraka1 added a commit to pawanpraka1/zfs-localpv that referenced this pull request Apr 14, 2020
Validating few parameters for the ZFSVolume custom resource

- compression can be "on", "off", "lzjb", "gzip", "gzip-[1-9]", "zle" and "lz4"
- encryption can be "on", "off", "aes-128-ccm", "aes-192-ccm", "aes-256-ccm", "aes-128-gcm", "aes-192-gcm", and "aes-256-gcm"
- dedup can be "on" and "off"
- poolname can be string
- ownernodeid can be string
- thinprovision can be "yes" and "no"
- volumetype can be "DATASET" and "ZVOL"

Also added required fields needed to create ZFSVolume CR
- ownerNodeID
- poolname
- volumeType
- capacity


Signed-off-by: Pawan <pawan@mayadata.io>
@pawanpraka1 pawanpraka1 mentioned this pull request Apr 14, 2020
@pawanpraka1 pawanpraka1 deleted the crd branch April 14, 2020 12:20
kmova pushed a commit that referenced this pull request Apr 14, 2020
Validating few parameters for the ZFSVolume custom resource

- compression can be "on", "off", "lzjb", "gzip", "gzip-[1-9]", "zle" and "lz4"
- encryption can be "on", "off", "aes-128-ccm", "aes-192-ccm", "aes-256-ccm", "aes-128-gcm", "aes-192-gcm", and "aes-256-gcm"
- dedup can be "on" and "off"
- poolname can be string
- ownernodeid can be string
- thinprovision can be "yes" and "no"
- volumetype can be "DATASET" and "ZVOL"

Also added required fields needed to create ZFSVolume CR
- ownerNodeID
- poolname
- volumeType
- capacity


Signed-off-by: Pawan <pawan@mayadata.io>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants