admin/cozystack
1
0 Fork 0
mirror of https://github.com/cozystack/cozystack.git synced 2026-11-08 21:14:56 +09:00
Code Issues Packages Projects Releases Wiki Activity

fix(objectstorage-controller): requeue BucketClaim until backend Bucket is ready

Browse Source 
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 4437cccaf0)
This commit is contained in:
Aleksei Sviridkin authored and Myasnikov Daniil committed 2026-08-04 13:18:15 +05:00
1 parent ef72c63e6e
commit d9c6f579de
1 file changed
+145 -3
Download Patch File Download Diff File Expand all files Collapse all files
packages/system/objectstorage-controller/images/objectstorage/patches/92-bucketclaim-propagate-ready.diff
+145 -3
View File
@@ -1,11 +1,11 @@
diff --git a/controller/pkg/bucketclaim/bucketclaim.go b/controller/pkg/bucketclaim/bucketclaim.go
index 2b55c49..038cf28 100644
index 2b55c49..e554761 100644
--- a/controller/pkg/bucketclaim/bucketclaim.go
+++ b/controller/pkg/bucketclaim/bucketclaim.go
@@ -213,8 +213,23 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context,
return b.recordError(inputBucketClaim, v1.EventTypeWarning, v1alpha1.FailedCreateBucket, err)
}
+ // The Bucket for this claim may already exist from an earlier reconcile,
+ // in which case the sidecar may have provisioned the backend and flipped
+ // the Bucket to ready in the meantime. The controller does not watch
@@ -25,5 +25,147 @@ index 2b55c49..038cf28 100644
- bucketClaim.Status.BucketReady = false
+ bucketClaim.Status.BucketReady = createdBucket.Status.BucketReady
}
// Update status with retry logic for conflict errors
@@ -282,6 +297,18 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context,
return b.recordError(inputBucketClaim, v1.EventTypeWarning, v1alpha1.FailedCreateBucket, err)
}
+ // The backend Bucket is provisioned asynchronously by the sidecar, which
+ // flips Bucket.status.bucketReady on an object this controller does not
+ // watch. The periodic informer resync cannot re-drive convergence either:
+ // the generic controller drops no-op resync deltas (reflect.DeepEqual in
+ // internal/runtime/controller.go) and nothing mutates the BucketClaim after
+ // this point. So while the Bucket is not ready, return an error to force a
+ // rate-limited requeue; each retry re-reads the live Bucket above until its
+ // readiness has been propagated onto the claim.
+ if !statusBucketReady {
+ return fmt.Errorf("backend Bucket %q for BucketClaim %q not ready yet; requeuing", bucketName, bucketClaim.ObjectMeta.Name)
+ }
+
klog.V(3).Infof("Finished creating Bucket %v", bucketName)
return nil
}
diff --git a/controller/pkg/bucketclaim/bucketclaim_test.go b/controller/pkg/bucketclaim/bucketclaim_test.go
index fea6f16..2a8149c 100644
--- a/controller/pkg/bucketclaim/bucketclaim_test.go
+++ b/controller/pkg/bucketclaim/bucketclaim_test.go
@@ -379,6 +379,18 @@ func TestRetryOnConflictStatusUpdate(t *testing.T) {
t.Fatalf("Error occurred when creating BucketClaim: %v", err)
}
+ // Pre-create the backend Bucket already flipped to ready so readiness
+ // propagation lands true and Add completes. This test exercises the
+ // status-update conflict-retry path, not the not-ready requeue path
+ // (the latter is covered by TestProvisionRequeuesUntilBucketReady).
+ readyBucket := &v1alpha1.Bucket{
+ ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("bucket-%s", bucketClaim.UID)},
+ Status: v1alpha1.BucketStatus{BucketReady: true},
+ }
+ if _, err := client.ObjectstorageV1alpha1().Buckets().Create(ctx, readyBucket, metav1.CreateOptions{}); err != nil {
+ t.Fatalf("Error occurred when pre-creating ready Bucket: %v", err)
+ }
+
// Cleanup
defer util.DeleteObjects(ctx, client, *bucketClaim, *bucketclass)
@@ -436,8 +448,8 @@ func TestRetryOnConflictStatusUpdate(t *testing.T) {
t.Errorf("Expected BucketName %s, got %s", expectedBucketName, updatedClaim.Status.BucketName)
}
- if updatedClaim.Status.BucketReady != false {
- t.Errorf("Expected BucketReady to be false, got %v", updatedClaim.Status.BucketReady)
+ if updatedClaim.Status.BucketReady != true {
+ t.Errorf("Expected BucketReady to be true, got %v", updatedClaim.Status.BucketReady)
}
// Verify finalizer was added
@@ -445,3 +457,88 @@ func TestRetryOnConflictStatusUpdate(t *testing.T) {
t.Errorf("Expected finalizer to be added, but it was not found")
}
}
+
+// TestProvisionRequeuesUntilBucketReady pins the readiness-propagation contract.
+// The controller does not watch Bucket objects and the generic controller drops
+// no-op resync deltas, so the BucketClaim must be driven to ready by the
+// provision path itself: Add returns an error (forcing a rate-limited requeue)
+// while the backend Bucket is not ready, and returns nil with BucketReady=true
+// once the Bucket's readiness has propagated. Without the requeue the claim
+// would stay BucketReady=false forever and BucketAccess would never be granted.
+func TestProvisionRequeuesUntilBucketReady(t *testing.T) {
+ newListener := func(client *fakebucketclientset.Clientset) *BucketClaimListener {
+ l := NewBucketClaimListener()
+ l.InitializeKubeClient(fakekubeclientset.NewSimpleClientset())
+ l.InitializeBucketClient(client)
+ l.InitializeEventRecorder(record.NewFakeRecorder(3))
+ return l
+ }
+
+ t.Run("requeues while backend Bucket is not ready", func(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ client := fakebucketclientset.NewSimpleClientset()
+ if _, err := util.CreateBucketClass(ctx, client, &goldClass); err != nil {
+ t.Fatalf("Error occurred when creating BucketClass: %v", err)
+ }
+ bucketClaim, err := util.CreateBucketClaim(ctx, client, &bucketClaim1)
+ if err != nil {
+ t.Fatalf("Error occurred when creating BucketClaim: %v", err)
+ }
+
+ // The controller creates the Bucket with BucketReady=false and nothing
+ // flips it here, so Add must report an error to force a requeue.
+ if err := newListener(client).Add(ctx, bucketClaim); err == nil {
+ t.Fatal("Add should return an error to requeue while the backend Bucket is not ready")
+ }
+
+ updated, err := client.ObjectstorageV1alpha1().BucketClaims(bucketClaim.Namespace).Get(ctx, bucketClaim.Name, metav1.GetOptions{})
+ if err != nil {
+ t.Fatalf("Error occurred when reading BucketClaim: %v", err)
+ }
+ if updated.Status.BucketReady {
+ t.Errorf("Expected BucketReady to stay false while the backend Bucket is not ready")
+ }
+ // Status (name + finalizer) is still persisted before the requeue.
+ if updated.Status.BucketName != fmt.Sprintf("bucket-%s", bucketClaim.UID) {
+ t.Errorf("Expected BucketName to be set even while not ready, got %q", updated.Status.BucketName)
+ }
+ })
+
+ t.Run("converges once the backend Bucket is ready", func(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ client := fakebucketclientset.NewSimpleClientset()
+ if _, err := util.CreateBucketClass(ctx, client, &goldClass); err != nil {
+ t.Fatalf("Error occurred when creating BucketClass: %v", err)
+ }
+ bucketClaim, err := util.CreateBucketClaim(ctx, client, &bucketClaim1)
+ if err != nil {
+ t.Fatalf("Error occurred when creating BucketClaim: %v", err)
+ }
+
+ // Simulate the sidecar having already provisioned the backend and
+ // flipped the Bucket to ready before this reconcile.
+ readyBucket := &v1alpha1.Bucket{
+ ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("bucket-%s", bucketClaim.UID)},
+ Status: v1alpha1.BucketStatus{BucketReady: true},
+ }
+ if _, err := client.ObjectstorageV1alpha1().Buckets().Create(ctx, readyBucket, metav1.CreateOptions{}); err != nil {
+ t.Fatalf("Error occurred when pre-creating ready Bucket: %v", err)
+ }
+
+ if err := newListener(client).Add(ctx, bucketClaim); err != nil {
+ t.Fatalf("Add should succeed once the backend Bucket is ready: %v", err)
+ }
+
+ updated, err := client.ObjectstorageV1alpha1().BucketClaims(bucketClaim.Namespace).Get(ctx, bucketClaim.Name, metav1.GetOptions{})
+ if err != nil {
+ t.Fatalf("Error occurred when reading BucketClaim: %v", err)
+ }
+ if !updated.Status.BucketReady {
+ t.Errorf("Expected BucketReady to propagate true once the backend Bucket is ready")
+ }
+ })
+}