-
Notifications
You must be signed in to change notification settings - Fork 38.8k
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
Pass whole PVC to provisioner plugins #34611
Conversation
Gluster provisioner is interested in pvc.Namespace and I don't want to add at as a new field in VolumeOptions - it would contain almost whole PVC. Let's pass direct reference to PVC instead and let the provisioner to pick information it is interested in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// Reclamation policy for a persistent volume | ||
PersistentVolumeReclaimPolicy api.PersistentVolumeReclaimPolicy | ||
// PV.Name of the appropriate PersistentVolume. Used to generate cloud | ||
// volume name. | ||
PVName string | ||
// PVC.Name of the PersistentVolumeClaim; only set during dynamic provisioning. | ||
PVCName string | ||
// PVC is reference to the claim that lead to provisioning of a new PV. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leads
I see this, it is in my queue, but I can't get to it quite yet. |
Thanks @jsafrane . Patch lgtm. |
LGTM! |
LGTM |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Automatic merge from submit-queue Makeuse of PVC namespace when provisioning gluster volumes. Depends on #34611
Automatic merge from submit-queue Pass whole PVC to provisioner plugins Gluster provisioner is interested in namespace of PVCs that are being provisioned and I don't want to add at as a new field in `volume.VolumeOptions` - it would contain almost whole PVC. Let's rework `VolumeOptions` and pass direct reference to PVC there instead of some "interesting" fields and let the provisioner to pick information it is interested in. There was lot of refactoring in volume plugins to apply this change (too many plugins), however the logic is simple and it's all the same in all plugins. @rootfs @humblec
Gluster provisioner is interested in namespace of PVCs that are being provisioned and I don't want to add at as a new field in
volume.VolumeOptions
- it would contain almost whole PVC.Let's rework
VolumeOptions
and pass direct reference to PVC there instead of some "interesting" fields and let the provisioner to pick information it is interested in.There was lot of refactoring in volume plugins to apply this change (too many plugins), however the logic is simple and it's all the same in all plugins.
@rootfs @humblec
This change is![Reviewable](https://camo.githubusercontent.com/2d899f4291d07d3cd2fa4aaae1e3b243f164c23fce87d30a589ace0d496a444c/68747470733a2f2f72657669657761626c652e6b756265726e657465732e696f2f7265766965775f627574746f6e2e737667)