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

fix(pvc): fixing stale ZFSVolume CR issue when deleting pending PVC #145

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

pawanpraka1
Copy link
Contributor

@pawanpraka1 pawanpraka1 commented Jun 4, 2020

fixes: #130

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

Why is this PR required? What issue does it fix?:

PVC will not bound if there are wrong parameters/poolname in the storageclass,
the ZFSVolume CR will be still created and will remain in Pending State,
deletion of the PVC will delete PVC and since PVC is not bound, ZFS-LocalPV
driver will not get the delete call and will leave the ZFSVolume CR hanging there.

What this PR does?:

Reverting the behavior introduced in #121,
Now PVC will be bound but still ZFSVolume will be in Pending state until the volume is created.
Now, with the wrong parameter, the POD will be in ContainerCreating and describe of it will show that mount is failing

  Warning  FailedMount       1s (x2 over 1s)  kubelet, pawan-node-1  MountVolume.SetUp failed for volume "pvc-ce4c30ca-0eda-48ef-9cec-ca2f53356065" : kubernetes.io/csi: mounter.SetupAt failed: rpc error: code = Internal desc = rpc error: code = Internal desc = zfs: mount failed err : could not set the mountpoint, cannot open 'zfspv-pool1/pvc-ce4c30ca-0eda-48ef-9cec-ca2f53356065': dataset does not exist

Does this PR require any upgrade changes?:

No

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

@pawanpraka1 pawanpraka1 requested a review from kmova June 4, 2020 16:44
@pawanpraka1 pawanpraka1 added the bug Something isn't working. label Jun 4, 2020
@pawanpraka1 pawanpraka1 added this to the v0.8.0 milestone Jun 4, 2020
@pawanpraka1 pawanpraka1 requested a review from w3aman June 4, 2020 16:47
@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #145   +/-   ##
=======================================
  Coverage   22.90%   22.90%           
=======================================
  Files          14       14           
  Lines         489      489           
=======================================
  Hits          112      112           
  Misses        376      376           
  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 377b881...71214ce. Read the comment docs.

@kmova
Copy link
Member

kmova commented Jun 5, 2020

@pawanpraka1 can you update/comment this PR with the error that is displayed on the Pod or PVC in case the parameters are incorrectly specified? In other words, how can the user know the reason for pod not getting started?

@pawanpraka1
Copy link
Contributor Author

@kmova updated the PR description.

@kmova
Copy link
Member

kmova commented Jun 5, 2020

not able to mount the dataset - can this be updated in a way that will indicate the action that user has to take to rectify the error. May be like: Unable to create the ZFS Volume. Please check the parameters provided in StorageClass.

PVC will not bound if there are wrong parameters/poolname in the storageclass,
the ZFSVolume CR will be still created and will remain in Pending State,
deletion of the PVC will delete PVC and since PVC is not bound, ZFS-LocalPV
driver will not get the delete call and will leave the ZFSVolume CR hanging there.
Reverting the behavior introduced in openebs#121,
Now PVC will be bound but still ZFSVolume will be in Pending state until the volume is created.

Signed-off-by: Pawan <pawan@mayadata.io>
@pawanpraka1
Copy link
Contributor Author

pawanpraka1 commented Jun 5, 2020

@kmova good point. Improved the logs to say dataset does not exist. This error is coming from POD at the mount time, it can report mount time errors only. The creation failure will be coming from the watcher which will keep on trying to create the volume and keep on failing. Have already 2 tasks in the tracker:

  1. generate event for success/failure
  2. revisit the logs
  Warning  FailedMount       1s (x2 over 1s)  kubelet, pawan-node-1  MountVolume.SetUp failed for volume "pvc-ce4c30ca-0eda-48ef-9cec-ca2f53356065" : kubernetes.io/csi: mounter.SetupAt failed: rpc error: code = Internal desc = rpc error: code = Internal desc = zfs: mount failed err : could not set the mountpoint, cannot open 'zfspv-pool1/pvc-ce4c30ca-0eda-48ef-9cec-ca2f53356065': dataset does not exist

@kmova kmova merged commit 45015bf into openebs:master Jun 8, 2020
@pawanpraka1 pawanpraka1 deleted the pvc branch June 8, 2020 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZFSVolume resource ramains stale in system after deleting pvc
3 participants