[Backport release-1.5] fix(objectstorage-controller): converge BucketClaim readiness and speed up COSI provisioner failover - #3532
Conversation
…et is ready The dynamic-provisioning path re-reads the live Bucket after create and copies its readiness onto the BucketClaim, but only during a reconcile. Nothing re-drives that reconcile afterwards: the controller does not watch Buckets, and the generic controller drops no-op resync deltas (reflect.DeepEqual), so once the BucketClaim stops changing the listener is never re-invoked. The Bucket's false->true transition, performed asynchronously by the sidecar on a different object, is therefore reflected onto the claim only if the Bucket happened to be ready during that single reconcile -- a race the sidecar usually loses, leaving bucketReady frozen at false and BucketAccess never granted. Return an error while the backend Bucket is not ready so the work queue requeues the claim with exponential backoff; each retry re-reads the live Bucket until its readiness has been propagated, converging deterministically instead of depending on a resync that never fires. Add a unit test pinning the contract (requeue while not ready, propagate true once ready) and adjust the existing status-update conflict-retry test, which pinned the old never-requeue behavior. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> (cherry picked from commit 4437ccc)
The controller and the per-driver sidecar both run single-replica, so leader election provides no high availability -- it only governs how quickly a freshly recreated pod takes over. With the upstream 150s lease duration a recreated provisioner pod stalls reconciliation for up to 150s while it waits out the stale lease still held by the previous, now-deleted pod. Lower the lease timings to the standard Kubernetes values (15s/10s/2s) so a recreated pod resumes reconciliation in roughly 15s. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> (cherry picked from commit 250d923)
…ptly
The bucket suite waited on '{.status.bucketReady}' without an expected value;
a bare jsonpath match is satisfied by the literal "false", so the wait passed
against an unready claim. Both suites also allowed 300s/600s, long enough to
tolerate the propagation race rather than catch it, so they stayed green
whether or not the BucketClaim ever converged.
Assert bucketReady=true on a 120s bound. Convergence is now controller-driven
(the controller requeues the claim until the backend Bucket's readiness
propagates) and completes within tens of seconds; a claim frozen by the
propagation race never converges and now fails the wait fast instead of being
masked.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit 63d14e7)
The objectstorage-controller package ships the COSI control plane; its commits use the objectstorage-controller scope, which had no area mapping and fell back to area/uncategorized. Map it to area/storage alongside the other COSI/storage scopes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> (cherry picked from commit 4f5dcea)
… pods on bucket failure The two bucketaccesses waits used the bare jsonpath form, which also matches the literal "false"; require =true so a never-granted access fails the wait instead of passing against an ungranted object. Also print the objectstorage-controller pod status in the bucketReady failure block so controller-side issues are visible from the test log. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> (cherry picked from commit 0855ff2)
…ackend Bucket readiness While the backend Bucket is not ready the controller returns an error to force a requeue, but that error is only logged at klog.V(3) and is suppressed at default verbosity, so an operator cannot see why the claim is stuck. Record a WaitingForBucket Warning event on the BucketClaim on each requeue (event aggregation folds the repetition) and still return the error so the requeue happens as before. Pin the requeue contract in the test by asserting the error carries the stable "not ready yet" substring, not merely that it is non-nil. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> (cherry picked from commit 5991808)
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What this PR does
Hand-backport of #3034 to
release-1.5. Thebackportlabel was applied at merge time but no backport PR was ever opened, so the change never reached this branch — found by auditing the branch withcmd/backport-audit, which reported it asMISSING.Every candidate that went missing this way merged before #3155 landed, when
conflict_resolutionwas passed as a top-level input instead of nested underexperimental. The action ignored it and fell back tofail, which on a conflicting cherry-pick opens no PR and reports no failure, so these were dropped with no draft to find and no red check to notice.The six original commits are cherry-picked with
-x, unchanged and in order. No adaptation was needed: the cherry-pick was clean, and the net diff is identical to the change onmain.Verification
The two
.difffiles are image patches applied at build time against upstreamcontainer-object-storage-interface, so what matters is that this branch pins the same upstream source. It does — bothrelease-1.5andmainsetARG COMMIT_REF=v0.2.2inimages/objectstorage/Dockerfile, and the resulting patch set on this branch (91,92,93) is byte-identical tomain's. Patch91was already present here unchanged, so92applies over the same base it does onmain.The branch differs from
mainonly in its builder pins (golang:1.24versus1.26, unpinned base digests), none of which affect patch application.Screenshots
Not applicable — no UI change.
Release note