reduce the volume name gap with external-provisioner #178
Conversation
k8s-ci-robot
added
kind/bug
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @smileusd. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/L
controller/controller.go
Outdated
| func (ctrl *ProvisionController) makeVolumeName(pvcUID string) (string, error) { | ||
| // create persistent name based on a volumeNamePrefix and volumeNameUUIDLength | ||
| // of PVC's UID | ||
| if len(ctrl.volumeNamePrefix) == 0 { | ||
| return "", fmt.Errorf("Volume name prefix cannot be of length 0") | ||
| } | ||
| if ctrl.volumeNameUUIDLength == 0 { | ||
| return "", fmt.Errorf("Volume length cannot be 0") | ||
| } | ||
| if len(pvcUID) == 0 { | ||
| return "", fmt.Errorf("corrupted PVC object, it is missing UID") | ||
| } | ||
| if ctrl.volumeNameUUIDLength == -1 { | ||
| // Default behavior is to not truncate or remove dashes | ||
| return fmt.Sprintf("%s-%s", ctrl.volumeNamePrefix, pvcUID), nil | ||
| } | ||
| // Else we remove all dashes from UUID and truncate to volumeNameUUIDLength | ||
| return fmt.Sprintf("%s-%s", ctrl.volumeNamePrefix, strings.Replace(string(pvcUID), "-", "", -1)[0:ctrl.volumeNameUUIDLength]), nil | ||
| } |
There was a problem hiding this comment.
I think it would be better to keep this weird function in the external-provisioner and here just provide a configurable callback stored in ProvisionController:
type VolumeNameHook func(claim *v1.PersistentVolumeClaim) string
type ProvisionController struct {
...
volumeNameHook VolumeNameHook
}
...
func VolumeName(hook VolumeNameHook) func(*ProvisionController) error {
return func(c *ProvisionController) error {
if c.HasRun() {
return errRuntime
}
c.volumeNameHook = hook
return nil
}
}With the default value getProvisionedVolumeNameForClaim.
There was a problem hiding this comment.
Thank you for your suggestion. I try to do this.
There was a problem hiding this comment.
Done. Please check @jsafrane
k8s-ci-robot
added
size/S
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane, smileusd The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@jsafrane Thank you! |
This pr is to reduce the gap with the external-provisioner pv name generation at https://github.com/kubernetes-csi/external-provisioner/blob/0f8b1662e25f37675da1f82acb78cd820b359419/pkg/controller/controller.go#L626
There is a potential bug if user not use default prefix or length to generate the pv name. There is a unexpected error.
/kind bug
/sig storage