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(zfspv): try volume creation on all the nodes #270

Merged
merged 1 commit into from
Apr 2, 2021

Conversation

pawanpraka1
Copy link
Contributor

@pawanpraka1 pawanpraka1 commented Jan 5, 2021

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

pvc gets bound even zfs volume creation keeps on failing on the node.

What this PR does?:

Currently scheduler returns one node and the provisioner uses that node to provision the volume. Once scheduler returns the node list in the priority order instead of returning just one node, the CSI controller can attempt the volume creation on all the nodes. Here, in this case if the volume is not created, the pvc will not be bound as the controller will wait to the volume to be ready or failed. Also, the the node agent will not reconcile for the creation of the volume, it will attempt to create once and fail if not successful. The csi provisioner will keep on requesting to create the volume in case of failure.

Depends on openebs/lib-csi#2

Known Issue?:

if volume creation request in progress and we are deleting the pvc, CSI plugin does not get the delete call and if volume creation succeeds, there will be one stale volume in the system.

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:

pkg/zfs/volume.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Jan 7, 2021

Codecov Report

Merging #270 (7f4fbcd) into master (bd6df9b) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #270   +/-   ##
======================================
  Coverage    1.09%   1.10%           
======================================
  Files          11      11           
  Lines         912     908    -4     
======================================
  Hits           10      10           
+ Misses        902     898    -4     
Impacted Files Coverage Δ
pkg/driver/controller.go 0.65% <0.00%> (+<0.01%) ⬆️

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 bd6df9b...c8d853a. Read the comment docs.

@pawanpraka1 pawanpraka1 changed the title feat(zfspv): wait for zfs volume to be created feat(zfspv): try volume creation on all the nodes Jan 19, 2021
@pawanpraka1 pawanpraka1 added pr/hold-merge hold the merge. and removed pr/hold-review hold the review. labels Jan 19, 2021
@pawanpraka1 pawanpraka1 added this to the 1.4.0 milestone Jan 19, 2021
@pawanpraka1 pawanpraka1 removed the pr/hold-merge hold the merge. label Feb 3, 2021
@pawanpraka1
Copy link
Contributor Author

cc: @iyashu @praveengt @abhranilc

@pawanpraka1 pawanpraka1 modified the milestones: 1.4.0, v1.5.0 Feb 17, 2021
pkg/driver/controller.go Outdated Show resolved Hide resolved
@pawanpraka1 pawanpraka1 modified the milestones: v1.5.0, v1.6.0 Mar 31, 2021
@kmova kmova added the pr/hold-merge hold the merge. label Apr 1, 2021
@pawanpraka1 pawanpraka1 removed the pr/hold-merge hold the merge. label Apr 1, 2021
Currently controller picks one node and the node agent keeps on trying to
create the volume on that node. There might not be enough space available
on that node to create the volume.

The controller can try on all the nodes sequentially and fail
the request if volume creation fails on all the nodes which satisfies the
topology contraints.

Signed-off-by: Pawan <pawan@mayadata.io>
@kmova kmova merged commit 04f7635 into openebs:master Apr 2, 2021
@pawanpraka1 pawanpraka1 deleted the wait branch April 3, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants