fix race condition while creating buckets #33
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wlan0 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
approved
pkg/bucketrequest/bucketrequest.go
Outdated
|
|
||
| bucket, err := b.bucketClient.ObjectstorageV1alpha1().Buckets().Get(context.Background(), name, metav1.GetOptions{}) | ||
| if err != nil && !errors.IsNotFound(err) { |
There was a problem hiding this comment.
If the bucket is found, then err == nil and we'll continue to process. We would want to abort wouldn't we?
There was a problem hiding this comment.
I agree, FindBucket above returns util.ErrBucketAlreadyExists which makes the controller to discard this request.
pkg/bucketrequest/bucketrequest.go
Outdated
| name = name + "-" | ||
| } | ||
| name = name + string(bucketRequest.GetUID()) | ||
| name = "br-" + name |
There was a problem hiding this comment.
How does the prefix "br-" give us more determinism/uniqueness? The big change seems to be using bucketRequest.GetUID vs util.GetUUID. Am I missing something?
There was a problem hiding this comment.
I do feel "br-" is superfluous and changes the prefix intended.
There was a problem hiding this comment.
sure. I can remove this
wlan0
force-pushed
the
master
branch
2 times, most recently
from
8fa845c
to
d2f2201
Compare
k8s-ci-robot
added
size/L
|
/lgtm |
k8s-ci-robot
added
the
lgtm
By using the naming format
br-${br.spec.bucketprefix}-${br.uid}we get a deterministic name, but in such a way that if a br is created and deleted, and then another br is created by the same name - a new bucket is created the second time.