From 230dcb22a572e121fe5e666b1b161201c56c11c4 Mon Sep 17 00:00:00 2001 From: nolancon Date: Tue, 21 Jan 2025 10:17:18 +0000 Subject: [PATCH] updates/simplifications with backend active status removed --- internal/controller/bucket/acl_test.go | 12 +-- .../bucket/bucket_validation_webhook.go | 4 +- internal/controller/bucket/consts.go | 5 +- internal/controller/bucket/create.go | 52 +++++------- internal/controller/bucket/create_test.go | 42 ++++++--- internal/controller/bucket/delete_test.go | 32 +++---- internal/controller/bucket/helpers.go | 2 +- .../bucket/lifecycleconfiguration_test.go | 30 +++---- .../bucket/objectlockconfiguration_test.go | 18 ++-- internal/controller/bucket/observe.go | 5 +- internal/controller/bucket/observe_test.go | 84 +++++++++--------- internal/controller/bucket/policy_test.go | 16 ++-- internal/controller/bucket/update.go | 59 ++++++------- internal/controller/bucket/update_test.go | 85 ++++++++++++------- .../bucket/versioningconfiguration_test.go | 28 +++--- .../s3clienthandler/s3clienthandler_test.go | 16 ++-- 16 files changed, 259 insertions(+), 231 deletions(-) diff --git a/internal/controller/bucket/acl_test.go b/internal/controller/bucket/acl_test.go index e472918d..45bcd07e 100644 --- a/internal/controller/bucket/acl_test.go +++ b/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 }(), diff --git a/internal/controller/bucket/bucket_validation_webhook.go b/internal/controller/bucket/bucket_validation_webhook.go index fd4d8a12..ccb68bfc 100644 --- a/internal/controller/bucket/bucket_validation_webhook.go +++ b/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) } diff --git a/internal/controller/bucket/consts.go b/internal/controller/bucket/consts.go index 3373fb86..04138ea9 100644 --- a/internal/controller/bucket/consts.go +++ b/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" diff --git a/internal/controller/bucket/create.go b/internal/controller/bucket/create.go index 841456cc..57a858a4 100644 --- a/internal/controller/bucket/create.go +++ b/internal/controller/bucket/create.go @@ -97,14 +97,10 @@ 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. @@ -112,32 +108,30 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext // 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.: "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 { diff --git a/internal/controller/bucket/create_test.go b/internal/controller/bucket/create_test.go index 011f0899..1cf888d0 100644 --- a/internal/controller/bucket/create_test.go +++ b/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 }(), diff --git a/internal/controller/bucket/delete_test.go b/internal/controller/bucket/delete_test.go index ff3446f6..62f01375 100644 --- a/internal/controller/bucket/delete_test.go +++ b/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 }(), diff --git a/internal/controller/bucket/helpers.go b/internal/controller/bucket/helpers.go index 838385a2..5bc70b16 100644 --- a/internal/controller/bucket/helpers.go +++ b/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 diff --git a/internal/controller/bucket/lifecycleconfiguration_test.go b/internal/controller/bucket/lifecycleconfiguration_test.go index f4e81d38..1d25d96a 100644 --- a/internal/controller/bucket/lifecycleconfiguration_test.go +++ b/internal/controller/bucket/lifecycleconfiguration_test.go @@ -82,7 +82,7 @@ func TestObserveBackend(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 }(), @@ -106,7 +106,7 @@ func TestObserveBackend(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 }(), @@ -141,7 +141,7 @@ func TestObserveBackend(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 }(), @@ -175,7 +175,7 @@ func TestObserveBackend(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 }(), @@ -213,7 +213,7 @@ func TestObserveBackend(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 }(), @@ -258,7 +258,7 @@ func TestObserveBackend(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 }(), @@ -309,7 +309,7 @@ func TestObserveBackend(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 }(), @@ -356,7 +356,7 @@ func TestObserveBackend(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 }(), @@ -409,7 +409,7 @@ func TestObserveBackend(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 }(), @@ -492,7 +492,7 @@ func TestHandle(t *testing.T) { backendStore: func() *backendstore.BackendStore { 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 }(), @@ -529,7 +529,7 @@ func TestHandle(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 }(), @@ -579,7 +579,7 @@ func TestHandle(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend(beName, &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend(beName, &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -620,7 +620,7 @@ func TestHandle(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 }(), @@ -672,7 +672,7 @@ func TestHandle(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 }(), @@ -734,7 +734,7 @@ func TestHandle(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 }(), diff --git a/internal/controller/bucket/objectlockconfiguration_test.go b/internal/controller/bucket/objectlockconfiguration_test.go index 4593ea8c..9f16674a 100644 --- a/internal/controller/bucket/objectlockconfiguration_test.go +++ b/internal/controller/bucket/objectlockconfiguration_test.go @@ -72,7 +72,7 @@ func TestObjectLockConfigObserveBackend(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 }(), @@ -112,7 +112,7 @@ func TestObjectLockConfigObserveBackend(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 }(), @@ -157,7 +157,7 @@ func TestObjectLockConfigObserveBackend(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 }(), @@ -238,7 +238,7 @@ func TestObjectLockConfigurationHandle(t *testing.T) { backendStore: func() *backendstore.BackendStore { fake := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend(beName, &fake, nil, true, apisv1alpha1.HealthStatusUnhealthy) + bs.AddOrUpdateBackend(beName, &fake, nil, apisv1alpha1.HealthStatusUnhealthy) return bs }(), @@ -274,7 +274,7 @@ func TestObjectLockConfigurationHandle(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 }(), @@ -300,7 +300,7 @@ func TestObjectLockConfigurationHandle(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 }(), @@ -341,7 +341,7 @@ func TestObjectLockConfigurationHandle(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 }(), @@ -389,7 +389,7 @@ func TestObjectLockConfigurationHandle(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 }(), @@ -447,7 +447,7 @@ func TestObjectLockConfigurationHandle(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 }(), diff --git a/internal/controller/bucket/observe.go b/internal/controller/bucket/observe.go index 4f54f74e..56f8aeda 100644 --- a/internal/controller/bucket/observe.go +++ b/internal/controller/bucket/observe.go @@ -58,10 +58,9 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex }, nil } - // If no Providers are specified in the Bucket Spec, the bucket is to be created on all backends. - providerNames := getBucketProvidersFilterDisabledLabel(bucket, c.backendStore.GetAllBackendNames(true)) + providerNames := getBucketProvidersFilterDisabledLabel(bucket, c.backendStore.GetAllBackendNames()) if len(providerNames) == 0 { - err := errors.New(errNoActiveS3Backends) + err := errors.New(errAllS3BackendsDisabled) traces.SetAndRecordError(span, err) return managed.ExternalObservation{}, err diff --git a/internal/controller/bucket/observe_test.go b/internal/controller/bucket/observe_test.go index a064e862..b22d9223 100644 --- a/internal/controller/bucket/observe_test.go +++ b/internal/controller/bucket/observe_test.go @@ -86,6 +86,42 @@ func TestObserveBasicErrors(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"}, + }, + Status: v1alpha1.BucketStatus{ + AtProvider: v1alpha1.BucketObservation{ + Backends: v1alpha1.Backends{ + "s3-backend-1": &v1alpha1.BackendInfo{ + BucketCondition: v1.Available(), + }, + }, + }, + ResourceStatus: v1.ResourceStatus{ + ConditionedStatus: v1.ConditionedStatus{ + Conditions: []v1.Condition{ + v1.Available(), + }, + }, + }, + }, + }, + }, + want: want{ + err: errors.New(errAllS3BackendsDisabled), + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -122,47 +158,11 @@ func TestObserve(t *testing.T) { args args want want }{ - "Bucket doesn't have any living backend": { - fields: fields{ - backendStore: func() *backendstore.BackendStore { - bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, nil, true, apisv1alpha1.HealthStatusHealthy) - - return bs - }(), - }, - args: args{ - mg: &v1alpha1.Bucket{ - Spec: v1alpha1.BucketSpec{ - ResourceSpec: v1.ResourceSpec{ - ProviderConfigReference: &v1.Reference{ - Name: "s3-backend-1", - }, - }, - }, - Status: v1alpha1.BucketStatus{ - AtProvider: v1alpha1.BucketObservation{ - Backends: v1alpha1.Backends{ - "s3-backend-1": &v1alpha1.BackendInfo{ - BucketCondition: v1.Available(), - }, - }, - }, - }, - }, - }, - want: want{ - o: managed.ExternalObservation{ - ResourceExists: true, - ResourceUpToDate: false, - }, - }, - }, "Bucket status is not available": { fields: fields{ backendStore: func() *backendstore.BackendStore { bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -205,7 +205,7 @@ func TestObserve(t *testing.T) { fields: fields{ backendStore: func() *backendstore.BackendStore { bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -255,7 +255,7 @@ func TestObserve(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 }(), @@ -309,7 +309,7 @@ func TestObserve(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 }(), @@ -363,7 +363,7 @@ func TestObserve(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 }(), @@ -417,7 +417,7 @@ func TestObserve(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 }(), diff --git a/internal/controller/bucket/policy_test.go b/internal/controller/bucket/policy_test.go index 3fbd3645..f94aac93 100644 --- a/internal/controller/bucket/policy_test.go +++ b/internal/controller/bucket/policy_test.go @@ -54,7 +54,7 @@ func TestPolicyObserveBackend(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 }(), @@ -81,7 +81,7 @@ func TestPolicyObserveBackend(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 }(), @@ -109,7 +109,7 @@ func TestPolicyObserveBackend(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 }(), @@ -141,7 +141,7 @@ func TestPolicyObserveBackend(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 }(), @@ -173,7 +173,7 @@ func TestPolicyObserveBackend(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 }(), @@ -201,7 +201,7 @@ func TestPolicyObserveBackend(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 }(), @@ -233,7 +233,7 @@ func TestPolicyObserveBackend(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 }(), @@ -261,7 +261,7 @@ func TestPolicyObserveBackend(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 }(), diff --git a/internal/controller/bucket/update.go b/internal/controller/bucket/update.go index dd2a97da..9d73858c 100644 --- a/internal/controller/bucket/update.go +++ b/internal/controller/bucket/update.go @@ -19,7 +19,6 @@ import ( "github.com/linode/provider-ceph/internal/consts" "github.com/linode/provider-ceph/internal/otel/traces" "github.com/linode/provider-ceph/internal/rgw" - "github.com/linode/provider-ceph/internal/utils" ) func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { @@ -48,14 +47,16 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalUpdate{}, err } - // allBackendNames is a list of the names of all backends from backend store which - // are Healthy. 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) + // allBackendNames is a list of the names of all backends from backend store. + allBackendNames := c.backendStore.GetAllBackendNames() + if len(allBackendNames) == 0 { + err := errors.New(errNoS3BackendsStored) + traces.SetAndRecordError(span, err) - // allBackendsToUpdateOn is a list of names of all backends on which this S3 bucket + return managed.ExternalUpdate{}, err + } + + // backendsToUpdateOnNames is a list of names of all backends on which this S3 bucket // is to be updated. This will either be: // 1. The list of bucket.Spec.Providers, if specified. // 2. Otherwise, the allBackendNames list. @@ -63,20 +64,16 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext // 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-a: "false"'). // This means that Provider Ceph should NOT update the bucket on this backend. - allBackendsToUpdateOn := getBucketProvidersFilterDisabledLabel(bucket, allBackendNames) - - // If none of the backends on which we wish to update the bucket are active then we - // return an error in order to requeue until backends become active. - activeBackendsToUpdateOn := c.backendStore.GetActiveBackends(allBackendsToUpdateOn) - if len(activeBackendsToUpdateOn) == 0 { - err := errors.New(errNoActiveS3Backends) + backendsToUpdateOnNames := getBucketProvidersFilterDisabledLabel(bucket, allBackendNames) + if len(backendsToUpdateOnNames) == 0 { + err := errors.New(errAllS3BackendsDisabled) traces.SetAndRecordError(span, err) return managed.ExternalUpdate{}, err } bucketBackends := newBucketBackends() - updateAllErr := c.updateOnAllBackends(ctx, bucket, bucketBackends, allBackendsToUpdateOn) + updateAllErr := c.updateOnAllBackends(ctx, bucket, bucketBackends, backendsToUpdateOnNames) if updateAllErr != nil { c.log.Info("Failed to update on all backends", consts.KeyBucketName, bucket.Name, "error", updateAllErr.Error()) traces.SetAndRecordError(span, updateAllErr) @@ -84,10 +81,9 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext // Whether buckets are updated successfully or not on backends, we need to update the // Bucket CR Status in all cases to represent the conditions of each individual bucket. - cls := c.backendStore.GetBackendS3Clients(allBackendsToUpdateOn) if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired { - setBucketStatus(bucketLatest, bucketBackends, allBackendsToUpdateOn, c.minReplicas) + setBucketStatus(bucketLatest, bucketBackends, backendsToUpdateOnNames, c.minReplicas) return NeedsStatusUpdate }); err != nil { @@ -106,21 +102,20 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext } // Auto pause the Bucket CR if required - ie if auto-pause has been enabled and the - // criteria is met before pausing a Bucket CR. Otherwise we check to see if there are - // backends that the bucket was not updated on and if so, we set the updateAllErr - // which will be returned at the end of this function, triggering a requeue. - if isPauseRequired(bucketLatest, allBackendsToUpdateOn, cls, bucketBackends, c.autoPauseBucket) { + // criteria is met before pausing a Bucket CR. + if isPauseRequired( + bucketLatest, + backendsToUpdateOnNames, + c.backendStore.GetBackendS3Clients(backendsToUpdateOnNames), + bucketBackends, + c.autoPauseBucket, + ) { c.log.Info("Auto pausing bucket", consts.KeyBucketName, bucket.Name) bucketLatest.Labels[meta.AnnotationKeyReconciliationPaused] = True - } else if updateAllErr == nil && len(activeBackendsToUpdateOn) != len(allBackendsToUpdateOn) { - updateAllErr = errors.New(errMissingS3Backend) - c.log.Info("Bucket was not updated on the following backends", consts.KeyBucketName, bucket.Name, "missing", utils.MissingStrings(allBackendsToUpdateOn, allBackendNames)) - traces.SetAndRecordError(span, updateAllErr) } // Apply the backend label to the Bucket CR for each backend that the bucket was - // intended to be updated on. This is to ensure the bucket will eventually be updated - // on these backends whenever they become active again. - setAllBackendLabels(bucketLatest, allBackendsToUpdateOn) + // intended to be updated on. + setAllBackendLabels(bucketLatest, backendsToUpdateOnNames) return NeedsObjectUpdate }) @@ -134,15 +129,15 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalUpdate{}, updateAllErr } -func (c *external) updateOnAllBackends(ctx context.Context, bucket *v1alpha1.Bucket, bb *bucketBackends, allBackendsToUpdateOn []string) error { +func (c *external) updateOnAllBackends(ctx context.Context, bucket *v1alpha1.Bucket, bb *bucketBackends, backendsToUpdateOnNames []string) error { ctx, span := otel.Tracer("").Start(ctx, "updateOnAllBackends") defer span.End() - defer setBucketStatus(bucket, bb, allBackendsToUpdateOn, c.minReplicas) + defer setBucketStatus(bucket, bb, backendsToUpdateOnNames, c.minReplicas) g := new(errgroup.Group) - for backendName := range c.backendStore.GetActiveBackends(allBackendsToUpdateOn) { + for _, backendName := range backendsToUpdateOnNames { // Attempt to get an S3 client for the backend. This will either be the default // S3 client created for each backend by the backend monitor or it will be a new // temporary S3 client created via the STS AssumeRole endpoint. The latter will diff --git a/internal/controller/bucket/update_test.go b/internal/controller/bucket/update_test.go index 3ba14232..e919f947 100644 --- a/internal/controller/bucket/update_test.go +++ b/internal/controller/bucket/update_test.go @@ -80,7 +80,7 @@ func TestUpdateBasicErrors(t *testing.T) { err: errors.New(errNoS3BackendsStored), }, }, - "No active backend": { + "No backend stored": { fields: fields{ backendStore: backendstore.NewBackendStore(), }, @@ -95,7 +95,28 @@ func TestUpdateBasicErrors(t *testing.T) { }, want: want{ o: managed.ExternalUpdate{}, - err: errors.New(errNoActiveS3Backends), + 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), }, }, } @@ -152,8 +173,8 @@ func TestUpdate(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -202,8 +223,8 @@ func TestUpdate(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 }(), @@ -256,8 +277,8 @@ func TestUpdate(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -314,8 +335,8 @@ func TestUpdate(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -368,7 +389,7 @@ func TestUpdate(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 }(), @@ -526,8 +547,8 @@ func TestUpdateLifecycleConfigSubResource(t *testing.T) { fake := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -577,8 +598,8 @@ func TestUpdateLifecycleConfigSubResource(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -643,8 +664,8 @@ func TestUpdateLifecycleConfigSubResource(t *testing.T) { fakeOK := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -697,7 +718,7 @@ func TestUpdateLifecycleConfigSubResource(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 }(), @@ -839,8 +860,8 @@ func TestUpdateVersioningConfigSubResource(t *testing.T) { fake := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -886,8 +907,8 @@ func TestUpdateVersioningConfigSubResource(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -949,8 +970,8 @@ func TestUpdateVersioningConfigSubResource(t *testing.T) { fakeOK := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -999,7 +1020,7 @@ func TestUpdateVersioningConfigSubResource(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 }(), @@ -1137,8 +1158,8 @@ func TestUpdateObjectLockConfigSubResource(t *testing.T) { fake := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -1186,8 +1207,8 @@ func TestUpdateObjectLockConfigSubResource(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -1250,8 +1271,8 @@ func TestUpdateObjectLockConfigSubResource(t *testing.T) { fakeOK := backendstorefakes.FakeS3Client{} bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, true, apisv1alpha1.HealthStatusHealthy) - bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", &fakeOK, nil, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-2", &fakeErr, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -1301,7 +1322,7 @@ func TestUpdateObjectLockConfigSubResource(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 }(), diff --git a/internal/controller/bucket/versioningconfiguration_test.go b/internal/controller/bucket/versioningconfiguration_test.go index 9959bdc9..947b4de7 100644 --- a/internal/controller/bucket/versioningconfiguration_test.go +++ b/internal/controller/bucket/versioningconfiguration_test.go @@ -77,7 +77,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -101,7 +101,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -132,7 +132,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -161,7 +161,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -193,7 +193,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -233,7 +233,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -273,7 +273,7 @@ func TestVersioningConfigObserveBackend(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 }(), @@ -347,7 +347,7 @@ func TestVersioningConfigurationHandle(t *testing.T) { backendStore: func() *backendstore.BackendStore { 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 }(), @@ -383,7 +383,7 @@ func TestVersioningConfigurationHandle(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 }(), @@ -418,7 +418,7 @@ func TestVersioningConfigurationHandle(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 }(), @@ -464,7 +464,7 @@ func TestVersioningConfigurationHandle(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend(beName, &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend(beName, &fake, nil, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -508,7 +508,7 @@ func TestVersioningConfigurationHandle(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 }(), @@ -547,7 +547,7 @@ func TestVersioningConfigurationHandle(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 }(), @@ -595,7 +595,7 @@ func TestVersioningConfigurationHandle(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 }(), diff --git a/internal/controller/s3clienthandler/s3clienthandler_test.go b/internal/controller/s3clienthandler/s3clienthandler_test.go index c4143781..0211f7af 100644 --- a/internal/controller/s3clienthandler/s3clienthandler_test.go +++ b/internal/controller/s3clienthandler/s3clienthandler_test.go @@ -92,7 +92,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -129,7 +129,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -168,7 +168,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -210,7 +210,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -252,7 +252,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -294,7 +294,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -336,7 +336,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(), @@ -391,7 +391,7 @@ func TestCreateAssumeRoleS3Client(t *testing.T) { } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, true, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend("s3-backend-1", nil, &fake, apisv1alpha1.HealthStatusHealthy) return bs }(),