updates/simplifications with backend active status removed

internal/controller/bucket/acl_test.go

@@ -62,7 +62,7 @@ func TestACLObserveBackend(t *testing.T) {
 					fake := backendstorefakes.FakeS3Client{}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusUnhealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusUnhealthy)
 
 					return bs
 				}(),
@@ -85,7 +85,7 @@ func TestACLObserveBackend(t *testing.T) {
 					fake := backendstorefakes.FakeS3Client{}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -113,7 +113,7 @@ func TestACLObserveBackend(t *testing.T) {
 					fake := backendstorefakes.FakeS3Client{}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -139,7 +139,7 @@ func TestACLObserveBackend(t *testing.T) {
 					fake := backendstorefakes.FakeS3Client{}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -167,7 +167,7 @@ func TestACLObserveBackend(t *testing.T) {
 					fake := backendstorefakes.FakeS3Client{}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -204,7 +204,7 @@ func TestACLObserveBackend(t *testing.T) {
 					fake := backendstorefakes.FakeS3Client{}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),

internal/controller/bucket/bucket_validation_webhook.go

@@ -73,7 +73,7 @@ func (b *BucketValidator) ValidateDelete(ctx context.Context, obj runtime.Object
 
 func (b *BucketValidator) validateCreateOrUpdate(ctx context.Context, bucket *v1alpha1.Bucket) error {
 	if len(bucket.Spec.Providers) != 0 {
-		missingProviders := utils.MissingStrings(bucket.Spec.Providers, b.backendStore.GetAllBackendNames(false))
+		missingProviders := utils.MissingStrings(bucket.Spec.Providers, b.backendStore.GetAllBackendNames())
 		if len(missingProviders) != 0 {
 			return errors.New(fmt.Sprintf("providers %v listed in bucket.Spec.Providers cannot be found", missingProviders))
 		}
@@ -89,7 +89,7 @@ func (b *BucketValidator) validateCreateOrUpdate(ctx context.Context, bucket *v1
 }
 
 func (b *BucketValidator) validateLifecycleConfiguration(ctx context.Context, bucket *v1alpha1.Bucket) error {
-	s3Client := b.backendStore.GetActiveBackends(b.backendStore.GetAllBackendNames(true)).GetFirst()
+	s3Client := b.backendStore.GetAllBackends().GetFirst()
 	if s3Client == nil {
 		return errors.New(errNoS3BackendsStored)
 	}

internal/controller/bucket/consts.go

@@ -12,9 +12,8 @@ const (
 	errUpdateBucketCR = "failed to update Bucket CR"
 
 	// Backend store error messages.
-	errNoS3BackendsStored = "no s3 backends stored in backendstore"
-	errNoActiveS3Backends = "no active s3 backends in backendstore"
-	errMissingS3Backend   = "one or more desired providers are inactive or unhealthy"
+	errNoS3BackendsStored    = "no s3 backends stored in backendstore"
+	errAllS3BackendsDisabled = "all s3 backends have been disabled for this bucket - please check cr labels"
 
 	// Subresource error messages.
 	errObserveSubresource = "failed to observe bucket subresource"

internal/controller/bucket/create.go

@@ -97,47 +97,41 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 		return managed.ExternalCreation{}, err
 	}
 
-	// allBackendNames is a list of the names of all backends. These backends can be
-	// active or inactive. A backend is marked as inactive in the backend store when
-	// its ProviderConfig object has been deleted. Inactive backends are included in
-	// this list so that we can attempt to recreate this bucket on those backends
-	// should they become active again.
-	allBackendNames := c.backendStore.GetAllBackendNames(false)
-
-	// allBackendsToCreateOn is a list of names of all backends on which this S3 bucket
+	// allBackendNames is a list of the names of all backends in the backend store.
+	allBackendNames := c.backendStore.GetAllBackendNames()
+
+	// backendsToCreateOnNames is a list of names of all backends on which this S3 bucket
 	// is to be created. This will either be:
 	// 1. The list of bucket.Spec.Providers, if specified.
 	// 2. Otherwise, the allBackendNames list.
 	// In either case, the list will exclude any backends which have been specified as
 	// disabled on the Bucket CR. A backend is specified as disabled for a given bucket
 	// if it has been given the backend label (eg 'provider-ceph.backends.<backend-name>: "false"').
 	// This means that Provider Ceph will NOT create the bucket on this backend.
-	allBackendsToCreateOn := getBucketProvidersFilterDisabledLabel(bucket, allBackendNames)
-
-	// If none of the backends on which we wish to create the bucket are active then we
-	// return an error in order to requeue until backends become active.
-	// Otherwise we do a quick sanity check to see if there are backends
-	// that the bucket will not be created on and log these backends.
-	activeBackendsToCreateOn := c.backendStore.GetActiveBackends(allBackendsToCreateOn)
-	if len(activeBackendsToCreateOn) == 0 {
-		err := errors.New(errNoActiveS3Backends)
+	backendsToCreateOnNames := getBucketProvidersFilterDisabledLabel(bucket, allBackendNames)
+	if len(backendsToCreateOnNames) == 0 {
+		err := errors.New(errAllS3BackendsDisabled)
 		traces.SetAndRecordError(span, err)
 
 		return managed.ExternalCreation{}, err
-	} else if len(activeBackendsToCreateOn) != len(allBackendsToCreateOn) {
-		c.log.Info("Bucket will not be created on the following S3 backends", consts.KeyBucketName, bucket.Name, "backends", utils.MissingStrings(allBackendsToCreateOn, allBackendNames))
-		traces.SetAndRecordError(span, errors.New(errMissingS3Backend))
+	}
+
+	// Quick sanity check to see if there are backends that the bucket will not be created
+	// on and log these backends.
+	if len(allBackendNames) != len(backendsToCreateOnNames) {
+		c.log.Info("Bucket will not be created on the following S3 backends", consts.KeyBucketName, bucket.Name, "backends", utils.MissingStrings(allBackendNames, backendsToCreateOnNames))
 	}
 
 	// This value shows a bucket on the given backend is already created.
 	// It is used to prevent go routines from sending duplicated messages to `readyChan`.
 	bucketAlreadyCreated := atomic.Bool{}
 	backendCount := 0
-	errChan := make(chan error, len(activeBackendsToCreateOn))
+	backendsToCreateOn := c.backendStore.GetBackends(backendsToCreateOnNames)
+	errChan := make(chan error, len(backendsToCreateOn))
 	readyChan := make(chan string)
 
-	// Now we're ready to start creating S3 buckets on our desired active backends.
-	for beName := range activeBackendsToCreateOn {
+	// Now we're ready to start creating S3 buckets on our desired backends.
+	for beName := range backendsToCreateOn {
 		originalBucket := bucket.DeepCopy()
 
 		// Attempt to get an S3 client for the backend. This will either be the default
@@ -199,10 +193,10 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 			// Although no backends were found for the bucket, we still apply the backend
 			// label to the Bucket CR for each backend that the bucket was intended to be
 			// created on. This is to ensure the bucket will eventually be created on these
-			// backends whenever they become active again.
-			setAllBackendLabels(bucketLatest, allBackendsToCreateOn)
+			// backends.
+			setAllBackendLabels(bucketLatest, backendsToCreateOnNames)
 			// Pause the Bucket CR because there is no backend for it to be created on.
-			// If a backend for which it was intended becomes active, the health-check
+			// If a backend for which it was intended becomes healthy, the health-check
 			// controller will un-pause the Bucket CR (identifying it by its backend label)
 			// and it will be re-reconciled.
 			bucketLatest.Labels[meta.AnnotationKeyReconciliationPaused] = True
@@ -219,10 +213,10 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 		return managed.ExternalCreation{}, nil
 	}
 
-	return c.waitForCreationAndUpdateBucketCR(ctx, bucket, allBackendsToCreateOn, readyChan, errChan, backendCount)
+	return c.waitForCreationAndUpdateBucketCR(ctx, bucket, backendsToCreateOnNames, readyChan, errChan, backendCount)
 }
 
-func (c *external) waitForCreationAndUpdateBucketCR(ctx context.Context, bucket *v1alpha1.Bucket, allBackendsToCreateOn []string, readyChan <-chan string, errChan <-chan error, backendCount int) (managed.ExternalCreation, error) {
+func (c *external) waitForCreationAndUpdateBucketCR(ctx context.Context, bucket *v1alpha1.Bucket, backendsToCreateOnNames []string, readyChan <-chan string, errChan <-chan error, backendCount int) (managed.ExternalCreation, error) {
 	ctx, span := otel.Tracer("").Start(ctx, "waitForCreationAndUpdateBucketCR")
 	defer span.End()
 
@@ -244,7 +238,7 @@ func (c *external) waitForCreationAndUpdateBucketCR(ctx context.Context, bucket
 			// 3. The Bucket CR Status Backends with a Ready condition for the backend the bucket
 			// was created on.
 			err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
-				setAllBackendLabels(bucketLatest, allBackendsToCreateOn)
+				setAllBackendLabels(bucketLatest, backendsToCreateOnNames)
 
 				return NeedsObjectUpdate
 			}, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {

internal/controller/bucket/create_test.go

@@ -64,7 +64,6 @@ func TestCreateBasicErrors(t *testing.T) {
 			fields: fields{
 				backendStore: func() *backendstore.BackendStore {
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-0", nil, nil, false, apisv1alpha1.HealthStatusUnknown)
 
 					return bs
 				}(),
@@ -80,7 +79,7 @@ func TestCreateBasicErrors(t *testing.T) {
 				},
 			},
 			want: want{
-				err: errors.New(errNoActiveS3Backends),
+				err: errors.New(errNoS3BackendsStored),
 			},
 		},
 		"S3 backend not referenced and none exist": {
@@ -98,6 +97,27 @@ func TestCreateBasicErrors(t *testing.T) {
 				err: errors.New(errNoS3BackendsStored),
 			},
 		},
+		"S3 backend exists but is disabled for bucket": {
+			fields: fields{
+				backendStore: func() *backendstore.BackendStore {
+					bs := backendstore.NewBackendStore()
+					bs.AddOrUpdateBackend("s3-backend-1", nil, nil, apisv1alpha1.HealthStatusHealthy)
+
+					return bs
+				}(),
+			},
+			args: args{
+				mg: &v1alpha1.Bucket{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: map[string]string{v1alpha1.BackendLabelPrefix + "s3-backend-1": "false"},
+						Name:   "test-bucket",
+					},
+				},
+			},
+			want: want{
+				err: errors.New(errAllS3BackendsDisabled),
+			},
+		},
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
@@ -168,7 +188,7 @@ func TestCreate(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -206,8 +226,8 @@ func TestCreate(t *testing.T) {
 					}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend("s3-backend-2", nil, &fake, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-2", nil, &fake, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -273,9 +293,9 @@ func TestCreate(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fakeClientError, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend("s3-backend-2", &fakeClientError, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend("s3-backend-3", &fakeClientOK, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fakeClientError, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-2", &fakeClientError, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-3", &fakeClientOK, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -333,9 +353,9 @@ func TestCreate(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend("s3-backend-1", &fakeClientError, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend("s3-backend-2", &fakeClientError, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend("s3-backend-3", &fakeClientError, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-1", &fakeClientError, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-2", &fakeClientError, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend("s3-backend-3", &fakeClientError, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),

internal/controller/bucket/delete_test.go

@@ -149,8 +149,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -225,8 +225,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -301,8 +301,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -364,8 +364,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -421,8 +421,8 @@ func TestDelete(t *testing.T) {
 					}
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, nil, &fake, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, nil, &fake, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, nil, &fake, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, nil, &fake, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -492,8 +492,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClientOK, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClientOK, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -579,8 +579,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClientOK, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClientOK, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),
@@ -671,8 +671,8 @@ func TestDelete(t *testing.T) {
 					)
 
 					bs := backendstore.NewBackendStore()
-					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, true, apisv1alpha1.HealthStatusHealthy)
-					bs.AddOrUpdateBackend(s3Backend2, fakeClientOK, nil, true, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend1, fakeClient, nil, apisv1alpha1.HealthStatusHealthy)
+					bs.AddOrUpdateBackend(s3Backend2, fakeClientOK, nil, apisv1alpha1.HealthStatusHealthy)
 
 					return bs
 				}(),

internal/controller/bucket/helpers.go

@@ -151,7 +151,7 @@ func getBucketProvidersFilterDisabledLabel(bucket *v1alpha1.Bucket, providerName
 
 	okProviders := []string{}
 	for i := range providers {
-		// Skip explicitly disableds
+		// Skip explicitly disabled backends
 		beLabel := utils.GetBackendLabel(providers[i])
 		if status, ok := bucket.Labels[beLabel]; ok && status != True {
 			continue