mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-31 17:17:26 +09:00
fix(objectstorage-controller): converge BucketClaim readiness and speed up COSI provisioner failover (#3034)
## What this PR does Fixes two failure modes in the COSI control plane built from upstream container-object-storage-interface v0.2.2, both carried as image patches. **BucketClaim never converges to ready (#3032).** The controller does not watch Bucket objects, and the generic controller drops no-op resync deltas, so a BucketClaim whose backend Bucket flips to ready *after* the first reconcile stayed `bucketReady=false` indefinitely — BucketAccess was never granted and the consuming app (bucket, harbor) got no credentials. The existing re-read of the live Bucket only ran during a reconcile that nothing re-triggered. The controller now returns an error while the Bucket is not ready, forcing a rate-limited requeue that re-reads the Bucket until its readiness propagates, so the claim converges deterministically instead of racing the async sidecar. **Slow provisioner failover on pod recreate (part of #2429).** The controller and the per-driver sidecar run single-replica, so leader election adds no availability — only failover latency. The upstream 150s lease made a recreated provisioner pod stall reconciliation for up to ~150s. The lease is lowered to the standard Kubernetes values (15s/10s/2s), so a recreated pod resumes in ~15s. The stale unix-socket CrashLoop in #2429 was already resolved by the seaweedfs-cosi-driver v0.3.1 self-heal; the silent "Bucket ready with no backend bucket" finding on that issue is a separate driver bug and is left tracked there. Coverage: a unit test in the patched controller pins the requeue/propagation contract (fails without the requeue), and the bucket/harbor e2e suites now assert `bucketReady=true` on a tight bound instead of tolerating the lag, with scoped diagnostics on the failure path. Closes #3032 Refs #2429 ### Screenshots N/A — no UI changes. ### Release note ```release-note fix(objectstorage-controller): converge BucketClaim readiness to the backend Bucket and speed up single-replica COSI provisioner failover ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved object storage bucket readiness propagation so bucket claims now track the backend bucket’s actual `bucketReady` status and requeue when readiness isn’t yet observed. * Reduced and made readiness waits more reliable in end-to-end checks, with explicit match conditions and clearer debug output (including claim YAML, backend bucket list, and controller pod status). * Shortened leader-election timing to speed up recovery after pod restarts. * **Chores** * Updated automated PR labeling to map the object storage controller scope to the storage area label. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
5 files changed
+218
-11
No files matched your search
@@ -105,6 +105,7 @@ jobs:
|
||||
// area/storage
|
||||
'seaweedfs': 'area/storage',
|
||||
'seaweedfs-cosi-driver': 'area/storage',
|
||||
'objectstorage-controller': 'area/storage',
|
||||
'bucket': 'area/storage',
|
||||
'linstor': 'area/storage',
|
||||
'velero': 'area/storage',
|
||||
|
||||
@@ -21,11 +21,24 @@ EOF
|
||||
# Wait for the bucket to be ready
|
||||
kubectl -n tenant-test wait hr bucket-${name} --timeout=5m --for=condition=ready
|
||||
timeout 60 sh -ec "until kubectl -n tenant-test get bucketclaims.objectstorage.k8s.io bucket-${name} >/dev/null 2>&1; do sleep 2; done"
|
||||
kubectl -n tenant-test wait bucketclaims.objectstorage.k8s.io bucket-${name} --timeout=300s --for=jsonpath='{.status.bucketReady}'
|
||||
# The COSI controller requeues the BucketClaim until the backend Bucket's
|
||||
# readiness propagates, so this converges within tens of seconds. Assert
|
||||
# bucketReady=true explicitly: a bare jsonpath match also matches the literal
|
||||
# "false", so the unqualified form passed against an unready claim. The tight
|
||||
# bound makes a propagation-race regression fail fast instead of hiding.
|
||||
kubectl -n tenant-test wait bucketclaims.objectstorage.k8s.io bucket-${name} --timeout=120s --for=jsonpath='{.status.bucketReady}'=true || {
|
||||
echo "=== BucketClaim did not converge to bucketReady=true ==="
|
||||
kubectl -n tenant-test get bucketclaims.objectstorage.k8s.io bucket-${name} -o yaml 2>&1 || true
|
||||
echo "=== backend Buckets (cluster-scoped) ==="
|
||||
kubectl get buckets.objectstorage.k8s.io -o wide 2>&1 || true
|
||||
echo "=== objectstorage-controller ==="
|
||||
kubectl -n cozy-objectstorage-controller get pods 2>&1 || true
|
||||
false
|
||||
}
|
||||
timeout 60 sh -ec "until kubectl -n tenant-test get bucketaccesses.objectstorage.k8s.io bucket-${name}-admin >/dev/null 2>&1; do sleep 2; done"
|
||||
kubectl -n tenant-test wait bucketaccesses.objectstorage.k8s.io bucket-${name}-admin --timeout=300s --for=jsonpath='{.status.accessGranted}'
|
||||
kubectl -n tenant-test wait bucketaccesses.objectstorage.k8s.io bucket-${name}-admin --timeout=300s --for=jsonpath='{.status.accessGranted}'=true
|
||||
timeout 60 sh -ec "until kubectl -n tenant-test get bucketaccesses.objectstorage.k8s.io bucket-${name}-viewer >/dev/null 2>&1; do sleep 2; done"
|
||||
kubectl -n tenant-test wait bucketaccesses.objectstorage.k8s.io bucket-${name}-viewer --timeout=300s --for=jsonpath='{.status.accessGranted}'
|
||||
kubectl -n tenant-test wait bucketaccesses.objectstorage.k8s.io bucket-${name}-viewer --timeout=300s --for=jsonpath='{.status.accessGranted}'=true
|
||||
|
||||
# Get admin (readwrite) credentials
|
||||
kubectl -n tenant-test get secret bucket-${name}-admin -ojsonpath='{.data.BucketInfo}' | base64 -d > bucket-admin-credentials.json
|
||||
|
||||
@@ -43,13 +43,22 @@ EOF
|
||||
timeout 60 sh -ec "until kubectl -n tenant-test get hr $release >/dev/null 2>&1; do sleep 2; done"
|
||||
kubectl -n tenant-test wait hr $release --timeout=5m --for=condition=ready
|
||||
|
||||
# Wait for COSI to provision bucket. The driver creates the Bucket and grants
|
||||
# access quickly, but the central COSI controller's propagation of the Bucket's
|
||||
# readiness back onto the namespaced BucketClaim can lag several minutes on a
|
||||
# loaded runner, so allow the same 10m budget the dependent HelmRelease gets.
|
||||
# Wait for COSI to provision the bucket. The driver creates the backend Bucket
|
||||
# and grants access; the central COSI controller requeues the BucketClaim until
|
||||
# the Bucket's readiness has propagated, so it converges within tens of seconds.
|
||||
# A frozen claim (the propagation race this guards against) never converges, so
|
||||
# the tight bound surfaces that regression instead of masking it.
|
||||
timeout 60 sh -ec "until kubectl -n tenant-test get bucketclaims.objectstorage.k8s.io $release-registry >/dev/null 2>&1; do sleep 2; done"
|
||||
kubectl -n tenant-test wait bucketclaims.objectstorage.k8s.io $release-registry \
|
||||
--timeout=600s --for=jsonpath='{.status.bucketReady}'=true
|
||||
--timeout=120s --for=jsonpath='{.status.bucketReady}'=true || {
|
||||
echo "=== BucketClaim did not converge to bucketReady=true ==="
|
||||
kubectl -n tenant-test get bucketclaims.objectstorage.k8s.io $release-registry -o yaml 2>&1 || true
|
||||
echo "=== backend Buckets (cluster-scoped) ==="
|
||||
kubectl get buckets.objectstorage.k8s.io -o wide 2>&1 || true
|
||||
echo "=== objectstorage-controller ==="
|
||||
kubectl -n cozy-objectstorage-controller get pods 2>&1 || true
|
||||
false
|
||||
}
|
||||
timeout 60 sh -ec "until kubectl -n tenant-test get bucketaccesses.objectstorage.k8s.io $release-registry >/dev/null 2>&1; do sleep 2; done"
|
||||
kubectl -n tenant-test wait bucketaccesses.objectstorage.k8s.io $release-registry \
|
||||
--timeout=60s --for=jsonpath='{.status.accessGranted}'=true
|
||||
|
||||
+164
-3
@@ -1,11 +1,11 @@
|
||||
diff --git a/controller/pkg/bucketclaim/bucketclaim.go b/controller/pkg/bucketclaim/bucketclaim.go
|
||||
index 2b55c49..038cf28 100644
|
||||
index 2b55c49..9195195 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,166 @@ 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,22 @@ 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. Surface a Warning event so
|
||||
+ // an operator can see why the claim is still pending (the returned error is
|
||||
+ // logged at klog.V(3), suppressed at default verbosity); event aggregation
|
||||
+ // folds the per-requeue repetition into a single counted entry.
|
||||
+ if !statusBucketReady {
|
||||
+ err := fmt.Errorf("backend Bucket %q for BucketClaim %q not ready yet; requeuing", bucketName, bucketClaim.ObjectMeta.Name)
|
||||
+ return b.recordError(inputBucketClaim, v1.EventTypeWarning, v1alpha1.WaitingForBucket, err)
|
||||
+ }
|
||||
+
|
||||
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..837f7e5 100644
|
||||
--- a/controller/pkg/bucketclaim/bucketclaim_test.go
|
||||
+++ b/controller/pkg/bucketclaim/bucketclaim_test.go
|
||||
@@ -3,6 +3,7 @@ package bucketclaim
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
+ "strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
@@ -379,6 +380,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 +449,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 +458,95 @@ 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.
|
||||
+ err = newListener(client).Add(ctx, bucketClaim)
|
||||
+ if err == nil {
|
||||
+ t.Fatal("Add should return an error to requeue while the backend Bucket is not ready")
|
||||
+ }
|
||||
+ // Pin the stable substring the production requeue error carries. A
|
||||
+ // refactor that drops the requeue (returning nil or a different error)
|
||||
+ // would silently regress convergence; this keeps the contract loud.
|
||||
+ if !strings.Contains(err.Error(), "not ready yet") {
|
||||
+ t.Fatalf("Expected requeue error to contain %q, got: %v", "not ready yet", 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 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")
|
||||
+ }
|
||||
+ })
|
||||
+}
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
diff --git a/internal/runtime/controller.go b/internal/runtime/controller.go
|
||||
index 902116a..4cbe4c3 100644
|
||||
--- a/internal/runtime/controller.go
|
||||
+++ b/internal/runtime/controller.go
|
||||
@@ -171,10 +171,14 @@ func NewObjectStorageControllerWithClientset(identity string, leaderLockName str
|
||||
threadiness: threads,
|
||||
|
||||
ResyncPeriod: 30 * time.Second,
|
||||
- // leader election
|
||||
- LeaseDuration: 150 * time.Second,
|
||||
- RenewDeadline: 120 * time.Second,
|
||||
- RetryPeriod: 60 * time.Second,
|
||||
+ // Leader election. The COSI controller and the per-driver sidecar both
|
||||
+ // run single-replica, so leader election provides no HA — it only gates
|
||||
+ // how fast a freshly recreated pod takes over. Use the standard
|
||||
+ // Kubernetes leader-election timings instead of the upstream 150/120/60s
|
||||
+ // so a recreated pod resumes reconciliation in ~15s rather than ~150s.
|
||||
+ LeaseDuration: 15 * time.Second,
|
||||
+ RenewDeadline: 10 * time.Second,
|
||||
+ RetryPeriod: 2 * time.Second,
|
||||
|
||||
opMap: &sync.Map{},
|
||||
}, nil
|
||||
Reference in new issue
Block a user