Repository navigation
Conversation
| func allDriversRequests(ctx context.Context, c client.Client) []reconcile.Request { | ||
| driverList := csiv1.DriverList{} | ||
| if err := c.List(ctx, &driverList); err != nil { | ||
| return []reconcile.Request{} |
There was a problem hiding this comment.
log error or else it will be missed
There was a problem hiding this comment.
Done. The list error is now logged in driverRequests, which both map functions use.
lets keep the new behaviour |
|
@tillo can you please rebase the PR to run the CI |
The image set ConfigMaps are only read as part of a driver reconcile, and the controller does not watch them. Changing the data of a ConfigMap that the OperatorConfig or a Driver references as its image set therefore leaves the CSI plugin workloads on the previous images until the operator restarts or some unrelated event triggers a reconcile. Watch ConfigMaps and enqueue the drivers that reference the changed one as their image set. A change in the operator level image set, referenced from OperatorConfig.spec.driverSpecDefaults.imageSet, enqueues all the drivers, and a change in a per-driver image set (Driver.spec.imageSet) enqueues only the drivers in that namespace that reference it, looked up through a field index on the image set name. A failure to look up the drivers is logged, as the map function cannot return it. Signed-off-by: Martino Dell'Ambrogio <tillo@tillo.ch>
e3ebb69 to
535bfa5
Compare
|
@Madhu-1 rebased onto main and addressed the review comments. The delete behaviour stays as it is. The |
Hello, thanks for ceph-csi-operator. It runs the CSI layer of our Rook cluster well, and I wanted to share a fix for a gap I ran into while pinning a plugin image.
I opened #642 about it: an image set ConfigMap is only read as part of a driver reconcile, and the controller does not watch it. Editing its data therefore leaves the plugin workloads on the previous images until the operator is restarted.
Describe what this PR does
data. ConfigMaps have no generation, so the data has to be compared.OperatorConfig.spec.driverSpecDefaults.imageSet, in the operator namespace, applies to every driver, so all drivers are enqueued;Driver.spec.imageSetenqueues the drivers in that namespace that reference it.allDriversRequests, so the existingOperatorConfigwatch and the new watch share it.Is there anything that requires special attention
The change is backward compatible. There is no API, CRD, Helm chart, or RBAC change: the existing
configmapsrule already allows get/list/watch, and the image set ConfigMaps are in namespaces the cache covers (the operator namespace and the driver namespaces).One behaviour worth a look during review: deleting a ConfigMap now enqueues the drivers that reference it. Their reconcile returns the
Geterror while the ConfigMap is missing, so the drivers surface the misconfiguration through their status and logs instead of keeping the previous images silently. If you would rather keep delete events quiet, it is a one-line predicate change.Related issues
Fixes: #642
Some external context: Rook v1.21 stopped setting its own default CSI images (rook/rook#18420) and points users with custom images to this ConfigMap (rook/rook#18528). Without a watch, that edit only takes effect after an operator restart. In our case the plugin pods were crash-looping on x86-64-v2 nodes with the operator's default cephcsi image, and pinning an older one in the ConfigMap looked like it had done nothing.
Checklist:
guidelines in the developer
guide.
Request
notes
updated with breaking and/or notable changes for the next major release.