More granular bucket conditions (#185)

cmd/provider/main.go

@@ -100,7 +100,7 @@ func main() {
 		enableManagementPolicies   = app.Flag("enable-management-policies", "Enable support for Management Policies.").Default("false").Envar("ENABLE_MANAGEMENT_POLICIES").Bool()
 
 		autoPauseBucket       = app.Flag("auto-pause-bucket", "Enable auto pause of reconciliation of ready buckets").Default("false").Envar("AUTO_PAUSE_BUCKET").Bool()
-		minReplicas           = app.Flag("minimum-replicas", "Minimum number of replicas of a bucket").Default("2").Envar("MINIMUM_REPLICAS").Uint()
+		minReplicas           = app.Flag("minimum-replicas", "Minimum number of replicas of a bucket before it is considered synced").Default("2").Envar("MINIMUM_REPLICAS").Uint()
 		recreateMissingBucket = app.Flag("recreate-missing-bucket", "Recreates existing bucket if missing").Default("true").Envar("RECREATE_MISSING_BUCKET").Bool()
 
 		assumeRoleArn = app.Flag("assume-role-arn", "Assume role ARN to be used for STS authentication").Default("").Envar("ASSUME_ROLE_ARN").String()

e2e/tests/stable/provider-ceph/14-assert.yaml

@@ -51,8 +51,8 @@ status:
           status: "True"
           type: Ready
   conditions:
-  - reason: Unavailable
-    status: "False"
+  - reason: Available
+    status: "True"
     type: Ready
   - reason: ReconcileError
     status: "False"

internal/controller/bucket/helpers.go

@@ -2,7 +2,9 @@ package bucket
 
 import (
 	"context"
+	"fmt"
 	"math"
+	"slices"
 	"strings"
 
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
@@ -20,6 +22,8 @@ import (
 	"k8s.io/client-go/util/retry"
 )
 
+const errUnavailableBackends = "Bucket is unavailable on the following backends: %s"
+
 // isBucketPaused returns true if the bucket has the paused label set.
 func isBucketPaused(bucket *v1alpha1.Bucket) bool {
 	if val, ok := bucket.Labels[meta.AnnotationKeyReconciliationPaused]; ok && val == True {
@@ -142,14 +146,30 @@ func setBucketStatus(bucket *v1alpha1.Bucket, bucketBackends *bucketBackends, pr
 	bucket.Status.AtProvider.Backends = backends
 
 	ok := 0
-	for _, backend := range backends {
+	unavailableBackends := make([]string, 0)
+	for backendName, backend := range backends {
 		if backend.BucketCondition.Equal(xpv1.Available()) {
 			ok++
+
+			continue
 		}
+		unavailableBackends = append(unavailableBackends, backendName)
 	}
-	if ok > 0 && float64(ok) >= math.Min(float64(len(providerNames)), float64(minReplicas)) {
+	// The Bucket CR is considered Available if the bucket is available on any backend.
+	if ok > 0 {
 		bucket.Status.SetConditions(xpv1.Available())
 	}
+	// The Bucket CR is considered Synced (ReconcileSuccess) once the bucket is available
+	// on the lesser of all backends or minimum replicas.
+	if float64(ok) >= math.Min(float64(len(providerNames)), float64(minReplicas)) {
+		bucket.Status.SetConditions(xpv1.ReconcileSuccess())
+
+		return
+	}
+	// The Bucket CR cannot be considered Synced.
+	slices.Sort(unavailableBackends)
+	err := errors.New(fmt.Sprintf(errUnavailableBackends, strings.Join(unavailableBackends, ", ")))
+	bucket.Status.SetConditions(xpv1.ReconcileError(err))
 }
 
 type UpdateRequired int

internal/controller/bucket/update.go

@@ -128,6 +128,7 @@ func (c *external) updateOnAllBackends(ctx context.Context, bucket *v1alpha1.Buc
 	for backendName := range c.backendStore.GetActiveBackends(providerNames) {
 		if !c.backendStore.IsBackendActive(backendName) {
 			c.log.Info("Backend is marked inactive - bucket will not be updated on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName)
+			bb.setBucketCondition(bucket.Name, backendName, xpv1.Unavailable().WithMessage("Backend is marked inactive"))
 
 			continue
 		}
@@ -136,6 +137,7 @@ func (c *external) updateOnAllBackends(ctx context.Context, bucket *v1alpha1.Buc
 		if err != nil {
 			traces.SetAndRecordError(span, err)
 			c.log.Info("Failed to get client for backend - bucket cannot be updated on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName, "error", err.Error())
+			bb.setBucketCondition(bucket.Name, backendName, xpv1.Unavailable().WithMessage(err.Error()))
 
 			continue
 		}

internal/controller/bucket/update_test.go

@@ -2,6 +2,9 @@ package bucket
 
 import (
 	"context"
+	"fmt"
+	"slices"
+	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
@@ -172,7 +175,11 @@ func TestUpdate(t *testing.T) {
 
 					assert.True(t,
 						bucket.Status.Conditions[0].Equal(v1.Available()),
-						"unexpected bucket condition")
+						"unexpected bucket ready condition")
+
+					assert.True(t,
+						bucket.Status.Conditions[1].Equal(v1.ReconcileSuccess()),
+						"unexpected bucket synced condition")
 
 					assert.True(t,
 						bucket.Status.AtProvider.Backends["s3-backend-1"].BucketCondition.Equal(v1.Available()),
@@ -219,9 +226,22 @@ func TestUpdate(t *testing.T) {
 
 					assert.True(t,
 						bucket.Status.Conditions[0].Equal(v1.Unavailable()),
-						"unexpected bucket condition")
+						"unexpected bucket ready condition")
+
+					unavailableBackends := []string{"s3-backend-1", "s3-backend-2"}
+					slices.Sort(unavailableBackends)
+					assert.True(t,
+						bucket.Status.Conditions[1].Equal(v1.ReconcileError(errors.New(
+							fmt.Sprintf(errUnavailableBackends, strings.Join(unavailableBackends, ", "))))),
+						"unexpected bucket synced condition")
 
-					assert.True(t, (len(bucket.Status.AtProvider.Backends) == 0), "backends should not exist in status")
+					assert.True(t,
+						bucket.Status.AtProvider.Backends["s3-backend-1"].BucketCondition.Equal(v1.Unavailable().
+							WithMessage(errors.Wrap(errors.Wrap(someError, "failed to assume role"), "Failed to create s3 client via assume role").Error())), "unexpected bucket condition for s3-backend-1")
+
+					assert.True(t,
+						bucket.Status.AtProvider.Backends["s3-backend-2"].BucketCondition.Equal(v1.Unavailable().
+							WithMessage(errors.Wrap(errors.Wrap(someError, "failed to assume role"), "Failed to create s3 client via assume role").Error())), "unexpected bucket condition for s3-backend-2")
 				},
 			},
 		},
@@ -257,10 +277,16 @@ func TestUpdate(t *testing.T) {
 				specificDiff: func(t *testing.T, mg resource.Managed) {
 					t.Helper()
 					bucket, _ := mg.(*v1alpha1.Bucket)
-
 					assert.True(t,
 						bucket.Status.Conditions[0].Equal(v1.Unavailable()),
-						"unexpected bucket condition")
+						"unexpected bucket ready condition")
+
+					unavailableBackends := []string{"s3-backend-1", "s3-backend-2"}
+					slices.Sort(unavailableBackends)
+					assert.True(t,
+						bucket.Status.Conditions[1].Equal(v1.ReconcileError(errors.New(
+							fmt.Sprintf(errUnavailableBackends, strings.Join(unavailableBackends, ", "))))),
+						"unexpected bucket synced condition")
 
 					assert.True(t,
 						bucket.Status.AtProvider.Backends["s3-backend-1"].BucketCondition.Equal(v1.Unavailable().WithMessage(errors.Wrap(someError, "failed to perform head bucket").Error())),
@@ -314,7 +340,12 @@ func TestUpdate(t *testing.T) {
 					// buckets on backends are Available.
 					assert.True(t,
 						bucket.Status.Conditions[0].Equal(v1.Available()),
-						"unexpected bucket condition")
+						"unexpected bucket ready condition")
+
+					assert.True(t,
+						bucket.Status.Conditions[1].Equal(v1.ReconcileError(errors.New(
+							fmt.Sprintf(errUnavailableBackends, strings.Join([]string{"s3-backend-2"}, ", "))))),
+						"unexpected bucket synced condition")
 
 					assert.True(t,
 						bucket.Status.AtProvider.Backends["s3-backend-1"].BucketCondition.Equal(v1.Available()),
@@ -374,7 +405,11 @@ func TestUpdate(t *testing.T) {
 					bucket, _ := mg.(*v1alpha1.Bucket)
 					assert.True(t,
 						bucket.Status.Conditions[0].Equal(v1.Available()),
-						"unexpected bucket condition")
+						"unexpected bucket ready condition")
+
+					assert.True(t,
+						bucket.Status.Conditions[1].Equal(v1.ReconcileSuccess()),
+						"unexpected bucket synced condition")
 
 					assert.True(t,
 						bucket.Status.AtProvider.Backends["s3-backend-1"].BucketCondition.Equal(v1.Available()),
@@ -416,6 +451,7 @@ func TestUpdate(t *testing.T) {
 					s3clienthandler.WithBackendStore(tc.fields.backendStore),
 					s3clienthandler.WithKubeClient(cl)),
 				autoPauseBucket: tc.fields.autoPauseBucket,
+				minReplicas:     2,
 				log:             logging.NewNopLogger(),
 			}