Ensure finalizer is removed when 409 Conflict returned on status update #2078
Conversation
k8s-ci-robot
added
cncf-cla: yes
MHBauer
added
the
needs-rebase
luksa
force-pushed
the
fix_finalizer_removal
branch
3 times, most recently
from
e04e5f0 to
1f5a5ae
Compare
jboyd01
removed
the
needs-rebase
|
|
||
| func (c *controller) updateServiceInstanceStatusWithRetries( | ||
| instance *v1beta1.ServiceInstance, | ||
| updateFunc func(*v1beta1.ServiceInstance)) (*v1beta1.ServiceInstance, error) { |
There was a problem hiding this comment.
I'd certainly benefit from a comment on this function indicating the updateFunc (if specified) applies additional updates to the instance which must be applied in the event this instance is out of date and an updated instance must be used.
LGTM though.
There was a problem hiding this comment.
Agreed, this really needs more explanation and perhaps a different name? If I understand this properly the function is applied only when retrying after an conflict, a more clear name would perhaps be resolveConflictFunc or something along those lines.
There was a problem hiding this comment.
I've added a comment & renamed the function to postConflictUpdateFunc.
|
|
||
| func (c *controller) updateServiceInstanceStatusWithRetries( | ||
| instance *v1beta1.ServiceInstance, | ||
| updateFunc func(*v1beta1.ServiceInstance)) (*v1beta1.ServiceInstance, error) { |
There was a problem hiding this comment.
Agreed, this really needs more explanation and perhaps a different name? If I understand this properly the function is applied only when retrying after an conflict, a more clear name would perhaps be resolveConflictFunc or something along those lines.
carolynvs
added
the
needs-rebase
luksa
force-pushed
the
fix_finalizer_removal
branch
from
e57ce63 to
addf226
Compare
…te (kubernetes-retired#2078) * Ensure finalizer is removed when 409 Conflict returned on status update * Improve naming & add documentation to updateServiceInstanceStatusWithRetries
Fixes #2066