[Feat.]Raw Block Volume Support for LocalPV #1536
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
prateekpandey14
suggested changes
Nov 29, 2019
kmova
reviewed
Nov 29, 2019
kmova
reviewed
Nov 29, 2019
| path := bd.Spec.FileSystem.Mountpoint | ||
|
|
There was a problem hiding this comment.
this change is not required - as the blkpath is already returned by the function. Also, block path should be based on dev links - because after restart the path can change.
There was a problem hiding this comment.
path is over-ridden blkPath, below this function is called. blkPath is now based on dev links.
kmova
reviewed
Nov 29, 2019
kmova
reviewed
Nov 29, 2019
kmova
reviewed
Nov 29, 2019
akhilerm
reviewed
Nov 29, 2019
| @@ -90,7 +93,7 @@ func (p *Provisioner) ProvisionBlockDevice(opts pvController.VolumeOptions, volu | |||
| WithAnnotations(volAnnotations). | |||
| WithReclaimPolicy(opts.PersistentVolumeReclaimPolicy). | |||
| WithAccessModes(pvc.Spec.AccessModes). | |||
| WithVolumeMode(fs). | |||
| WithVolumeMode(volumeMode). | |||
| WithCapacityQty(pvc.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]). | |||
| WithLocalHostPathFormat(path, fsType). | |||
There was a problem hiding this comment.
Why is the fsType required here?
In Raw Block mode, only the path is required in LocalVolumeSource.
There was a problem hiding this comment.
In case of block mode will FSType be ignored? Does path contain block path?
There was a problem hiding this comment.
Yes, path is over-ridden by the blkPath.
rahulchheda
force-pushed
the
rawVolume
branch
from
f35ed8e to
6f82061
Compare
ghost
reviewed
Dec 3, 2019
There was a problem hiding this comment.
Found some fixes!
P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).
golangcibot
reviewed
Dec 3, 2019
| When("remove finalizer", func() { | ||
| It("finalizer should come back after provisioner restart", func() { | ||
| bdcName := "bdc-pvc-" + string(pvcObj.GetUID()) | ||
| bdcObj, err := ops.BDCClient.WithNamespace(string(artifacts.OpenebsNamespace)).Get(bdcName, |
There was a problem hiding this comment.
shadow: declaration of "err" shadows declaration at line 38 (from govet)
kmova
reviewed
Dec 6, 2019
kmova
reviewed
Dec 6, 2019
kmova
approved these changes
Dec 6, 2019
|
BDD logs for better visibility : |
golangcibot
reviewed
Dec 6, 2019
akhilerm
approved these changes
Dec 6, 2019
added 8 commits
December 6, 2019 18:33
Signed-off-by: Rahul M Chheda <[email protected]>
Signed-off-by: Rahul M Chheda <[email protected]>
- Added Builder Function for support & checking Block Device - Returned true, for support of the Block Device - Provided the path for the raw Block device, if provided Signed-off-by: Rahul M Chheda <[email protected]>
…upport, and some other minor changes highlighter below
- Added containerBuilder function (funcName: WithVolumeDevices Builder)
- Added pvcBuilder function (funcname: WithVolumeMode Builder)
- Added 2 BDD TestCases :
- [+ve] Basic Raw Block Volume Support
- [-ve] If PVC volumeMode is "Block", but mountPath is provided in Deployment PodSpecTemplate
Signed-off-by: Rahul M Chheda <[email protected]>
…ion/method Signed-off-by: Rahul M Chheda <[email protected]>
- localPv-provisioner removed if-else case for providing blockdevice path or mountPath
- localPV-provisioner change PV volumeMode type to "Block", if PVC has it.
- Added appropraite comments in the BDD
Signed-off-by: Rahul M Chheda <[email protected]>
Signed-off-by: Rahul M Chheda <[email protected]>
Signed-off-by: Rahul M Chheda <[email protected]>
rahulchheda
force-pushed
the
rawVolume
branch
from
fc4629f to
3cdd154
Compare
golangcibot
reviewed
Dec 6, 2019
Signed-off-by: Rahul M Chheda <[email protected]>
rahulchheda
force-pushed
the
rawVolume
branch
from
3cdd154 to
69cb662
Compare
shubham14bajpai
pushed a commit
to shubham14bajpai/maya
that referenced
this pull request
Dec 27, 2019
- Added Builder Function for support & checking Block Device
- Returned true, for support of the Block Device
- Provided the path for the raw Block device, if provided
- Added containerBuilder function (funcName: WithVolumeDevices Builder)
- Added pvcBuilder function (funcname: WithVolumeMode Builder)
- Added 2 BDD TestCases :
- [+ve] Basic Raw Block Volume Support
- [-ve] If PVC volumeMode is "Block", but mountPath is provided in Deployment
PodSpecTemplate
Signed-off-by: Rahul M Chheda <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Checklist:
documentationtagbreaking-changestagrequires-upgradetag