-
Notifications
You must be signed in to change notification settings - Fork 22
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
Updating of additonal config field of OBC #195
Comments
@jeffvance @guymguym @copejon any thoughts on above issue? |
|
@jeffvance the question is in the context bucket notification CRD. the plan is to referenced it from the OBC (via |
I guess that all bucket provisioning related fields of the OBC are not supposed to change. |
The code en-queues only the new obc. Maybe we can compare the obc retrieved from the GET call against the obc from the informer cache to determine which obc fields changed? |
IMO values in |
this is a nice enhancement regardless of bucket notifications. currently we just silently ignore updates to immutable fields, would be better to error on that. |
True. We do the same with the OB too (no OB watch even).
It is tricky to compare maps because you cannot expect the keys to be retrieved in the same order. Go's "reflect" pkg should work, eg: |
@jeffvance Cool. How will we able to move forward here? Will you able to push a PR ? |
Not likely. I do have some time to review lib PRs. Are you able to @thotz ? |
Sure I will give a try then |
@jeffvance : It looks a bit complex than I thought. If understand correctly in the library has obcInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: ctrl.enqueueOBC,
UpdateFunc: func(old, new interface{}) {
oldObc := old.(*v1alpha1.ObjectBucketClaim)
newObc := new.(*v1alpha1.ObjectBucketClaim)
if newObc.ResourceVersion == oldObc.ResourceVersion {
// periodic re-sync can be ignored
return
}
// if old and new both have deletionTimestamps we can also ignore the
// update since these events are occurring on an obc marked for deletion,
// eg. extra finalizers being added and deleted.
if newObc.ObjectMeta.DeletionTimestamp != nil && oldObc.ObjectMeta.DeletionTimestamp != nil {
return
}
// handle this update
ctrl.enqueueOBC(new)
},
DeleteFunc: func(obj interface{}) {
// Since a finalizer is added to the obc and thus the obc will remain
// visible, we do not need to handle delete events here. Instead, obc
// deletes are indicated by the deletionTimestamp being non-nil.
return
},
}) If I understand correctly |
The code in your comment above added the "namespace/name" of the new object to the work queue for update events. But the I think an approach would be to keep the delete logic where it is with no changes. If the timestamp is nil then we have either add or update. The What do you think? |
please see discussion here: rook/rook#6425 (comment) |
I am not sure whether bug or current design limitation. In rook quota for OBC is supported with help of
additionalConfig
string Map. When I try to update the field it is not working viakubectl edit
orkubectl apply
the request is not handled properly.I can see following logs from rook-operator
If I understand correctly current code follow in
lib-bucket-provisioner
the request reachessyncHandler
.Firsts it check for whether request delete/revokes, if not checks whether provisioning is needed via
shouldProvision
. TheshouldProvision
checks whetherOB
name is present onOBC
CRD. If exists it returns true. In case of updatingOBC
field this always exist and skips further part of code. Is this intended behavior?CCing @yuvalif
The text was updated successfully, but these errors were encountered: