RBD provisioner use provisioner name as identitiy by default instead of random string #267
Conversation
support `PROVISIONER_NAME` envionment variable. In production, provisioner should not use random identity on start. Because, provisioner Pod may crash or be recreated on different nodes, if a provisioner generates random identity string each time, then it will cannot manage old PVs it provisioned before. In this commit, I change the default behavior to use `provision.ProvisionerName` constant as default identity instead of random UUID string.
k8s-ci-robot
added
the
cncf-cla: yes
|
@cofyc: GitHub didn't allow me to request PR reviews from the following users: rootfs. Note that only kubernetes-incubator members can review this PR, and authors cannot review their own PRs. In response to this: 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/test-infra repository. |
|
Hmmm.... Does id have to be unique within the same type of provisioner? Say I have a statefulset of ceph rbd provisioners. If I use string for id of 'rbd', then multiple could handle the request, but maybe thats ok? If not, maybe using the statefulset's hostname is a better default? |
|
@kfox1111 id doesn't have to be unique, except in rare cases where only the provisioner that created a PV is capable of deleting it. By giving all provisioners serving the same class the same id (same prName), they will simply race to Delete PVs and everything should turn out fine. I'm okay with using prName as the default. On one hand, it just "makes sense" that the provisioner that created a PV should be responsible for deleting it, so the recommendation made when gluster/block and cephfs were created was to force the user to specify an ID. On the other, users expect provisioners to behave the same as the upstream mega-controller, i.e. if a PV has Delete it should be deleted, it doesn't matter by which provisioner instance. And maintaining state via an ID argument is painful, plus impossible with a statefulset. |
|
ok. sounds good to me. Does the cephfs one have the same issue currently or is it defaulting to hostname? |
|
Yes cephfs has the same issue, it is defaulting to random UUID atm. |
…, and support `PROVISIONER_NAME` envionment variable. See kubernetes-retired#267.
update to Go 1.12.4
What this PR does / why we need it:
In production, provisioner should not use random identity on start.
Because, provisioner Pod may crash or be recreated on different nodes, if a provisioner generates random identity string each time, then it will cannot delete old PVs it provisioned before. Example errors logs:
Most users deployed their RBD provisioner without specifying
-idflag. So I change the default behavior to use provisioner name as identity (like some other provisions,gluster/block, etc) instead of random string.Special notes for your reviewer:
In this PR, I also add
PROVISIONER_NAMEenvironment variable support, like many other provisioners do.