diff --git a/internal/backendstore/backend.go b/internal/backendstore/backend.go index 75e37f2d..4c510c26 100644 --- a/internal/backendstore/backend.go +++ b/internal/backendstore/backend.go @@ -13,15 +13,13 @@ import ( type backend struct { s3Client S3Client stsClient STSClient - active bool health v1alpha1.HealthStatus } -func newBackend(s3Client S3Client, stsClient STSClient, active bool, health v1alpha1.HealthStatus) *backend { +func newBackend(s3Client S3Client, stsClient STSClient, health v1alpha1.HealthStatus) *backend { return &backend{ s3Client: s3Client, stsClient: stsClient, - active: active, health: health, } } diff --git a/internal/backendstore/backendstore.go b/internal/backendstore/backendstore.go index 9ce20d3c..f4ca6585 100644 --- a/internal/backendstore/backendstore.go +++ b/internal/backendstore/backendstore.go @@ -9,7 +9,7 @@ import ( // s3Backends is a map of S3 backend name (eg ceph cluster name) to backend. type s3Backends map[string]*backend -// BackendStore stores the active s3 backends. +// BackendStore stores the s3 backends. type BackendStore struct { s3Backends s3Backends mu sync.RWMutex @@ -77,24 +77,13 @@ func (b *BackendStore) GetBackendS3Clients(beNames []string) map[string]S3Client return clients } -func (b *BackendStore) IsBackendActive(backendName string) bool { +func (b *BackendStore) BackendExists(backendName string) bool { b.mu.RLock() defer b.mu.RUnlock() - if _, ok := b.s3Backends[backendName]; ok { - return b.s3Backends[backendName].active - } - - return false -} - -func (b *BackendStore) ToggleBackendActiveStatus(backendName string, active bool) { - b.mu.Lock() - defer b.mu.Unlock() + _, exists := b.s3Backends[backendName] - if _, ok := b.s3Backends[backendName]; ok { - b.s3Backends[backendName].active = active - } + return exists } func (b *BackendStore) GetBackendHealthStatus(backendName string) v1alpha1.HealthStatus { @@ -124,11 +113,11 @@ func (b *BackendStore) DeleteBackend(backendName string) { delete(b.s3Backends, backendName) } -func (b *BackendStore) AddOrUpdateBackend(backendName string, s3C S3Client, stsC STSClient, active bool, health v1alpha1.HealthStatus) { +func (b *BackendStore) AddOrUpdateBackend(backendName string, s3C S3Client, stsC STSClient, health v1alpha1.HealthStatus) { b.mu.Lock() defer b.mu.Unlock() - b.s3Backends[backendName] = newBackend(s3C, stsC, active, health) + b.s3Backends[backendName] = newBackend(s3C, stsC, health) } func (b *BackendStore) GetBackend(backendName string) *backend { @@ -155,7 +144,7 @@ func (b *BackendStore) GetAllBackends() s3Backends { return backends } -func (b *BackendStore) GetActiveBackends(beNames []string) s3Backends { +func (b *BackendStore) GetBackends(beNames []string) s3Backends { requestedBackends := map[string]bool{} for p := range beNames { requestedBackends[beNames[p]] = true @@ -167,7 +156,7 @@ func (b *BackendStore) GetActiveBackends(beNames []string) s3Backends { // Create a new s3Backends to hold a copy of the backends backends := make(s3Backends, 0) for k, v := range b.s3Backends { - if _, ok := requestedBackends[k]; !ok || !v.active || v.health == v1alpha1.HealthStatusUnhealthy { + if _, ok := requestedBackends[k]; !ok { continue } @@ -177,16 +166,12 @@ func (b *BackendStore) GetActiveBackends(beNames []string) s3Backends { return backends } -func (b *BackendStore) GetAllBackendNames(activeBackendsOnly bool) []string { +func (b *BackendStore) GetAllBackendNames() []string { b.mu.RLock() defer b.mu.RUnlock() backends := make([]string, 0) - for k, v := range b.s3Backends { - if activeBackendsOnly && !v.active || v.health == v1alpha1.HealthStatusUnhealthy { - continue - } - + for k := range b.s3Backends { backends = append(backends, k) } 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 a637c9e3..bd6810b9 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 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) - - // 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,29 @@ 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)) + errChan := make(chan error, len(backendsToCreateOnNames)) 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 backendsToCreateOnNames { originalBucket := bucket.DeepCopy() // Attempt to get an S3 client for the backend. This will either be the default @@ -199,10 +192,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 +212,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 +237,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/providerconfig/backendmonitor/backendmonitor_controller.go b/internal/controller/providerconfig/backendmonitor/backendmonitor_controller.go index d059595b..a59c11d9 100644 --- a/internal/controller/providerconfig/backendmonitor/backendmonitor_controller.go +++ b/internal/controller/providerconfig/backendmonitor/backendmonitor_controller.go @@ -19,6 +19,7 @@ package backendmonitor import ( "context" + "github.com/aws/aws-sdk-go-v2/aws" v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/errors" "go.opentelemetry.io/otel" @@ -28,17 +29,21 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "github.com/linode/provider-ceph/apis/provider-ceph/v1alpha1" apisv1alpha1 "github.com/linode/provider-ceph/apis/v1alpha1" + "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" ) const ( - errCreateS3Client = "failed create s3 client" - errCreateSTSClient = "failed create sts client" - errGetProviderConfig = "failed to get ProviderConfig" - errGetSecret = "failed to get Secret" + errCreateS3Client = "failed create s3 client" + errCreateSTSClient = "failed create sts client" + errGetProviderConfig = "failed to get ProviderConfig" + errGetSecret = "failed to get Secret" + errCleanup = "failed to perform cleanup" + errDeleteLCValidationBucket = "failed to delete lifecycle configuration validation bucket" ) func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -49,9 +54,16 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu providerConfig := &apisv1alpha1.ProviderConfig{} if err := c.kubeClient.Get(ctx, req.NamespacedName, providerConfig); err != nil { if kerrors.IsNotFound(err) { - c.log.Info("Marking s3 backend as inactive on backend store", "name", req.Name) - c.backendStore.ToggleBackendActiveStatus(req.Name, false) - c.backendStore.SetBackendHealthStatus(req.Name, apisv1alpha1.HealthStatusUnknown) + // ProviderConfig has been deleted, perform cleanup. + if err := c.cleanup(ctx, req); err != nil { + err = errors.Wrap(err, errCleanup) + traces.SetAndRecordError(span, err) + + return ctrl.Result{}, err + } + + c.log.Info("Removing s3 backend as from backend store", "name", req.Name) + c.backendStore.DeleteBackend(req.Name) // The ProviderConfig no longer exists so there is no need to requeue the reconcile key. return ctrl.Result{}, nil @@ -91,7 +103,7 @@ func (c *Controller) addOrUpdateBackend(ctx context.Context, pc *apisv1alpha1.Pr } readyCondition := pc.Status.GetCondition(v1.TypeReady) - c.backendStore.AddOrUpdateBackend(pc.Name, s3Client, stsClient, true, utils.MapConditionToHealthStatus(readyCondition)) + c.backendStore.AddOrUpdateBackend(pc.Name, s3Client, stsClient, utils.MapConditionToHealthStatus(readyCondition)) return nil } @@ -105,3 +117,21 @@ func (c *Controller) getProviderConfigSecret(ctx context.Context, secretNamespac return secret, nil } + +// cleanup deletes the lifecycle configuration validation bucket from the backend. +// This function is only called when a ProviderConfig has been deleted. +func (c *Controller) cleanup(ctx context.Context, req ctrl.Request) error { + backendClient := c.backendStore.GetBackendS3Client(req.Name) + if backendClient == nil { + c.log.Info("Backend client not found during validation bucket cleanup - aborting cleanup", consts.KeyBackendName, req.Name) + + return nil + } + + c.log.Info("Deleting lifecycle configuration validation bucket", consts.KeyBucketName, v1alpha1.LifecycleConfigValidationBucketName, consts.KeyBackendName, req.Name) + if err := rgw.DeleteBucket(ctx, backendClient, aws.String(v1alpha1.LifecycleConfigValidationBucketName), true); err != nil { + return errors.Wrap(err, errDeleteLCValidationBucket) + } + + return nil +} diff --git a/internal/controller/providerconfig/healthcheck/healthcheck_controller.go b/internal/controller/providerconfig/healthcheck/healthcheck_controller.go index 41039816..ca6ea2b9 100644 --- a/internal/controller/providerconfig/healthcheck/healthcheck_controller.go +++ b/internal/controller/providerconfig/healthcheck/healthcheck_controller.go @@ -21,7 +21,6 @@ import ( "net/http" "time" - "github.com/aws/aws-sdk-go-v2/aws" v1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/errors" "go.opentelemetry.io/otel" @@ -40,15 +39,12 @@ import ( apisv1alpha1 "github.com/linode/provider-ceph/apis/v1alpha1" "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" ) const ( - errHealthCheckCleanup = "failed to perform health check cleanup" - errDeleteLCValidationBucket = "failed to delete lifecycle configuration validation bucket" - errUpdateHealthStatus = "failed to update health status of provider config" - errFailedHealthCheckReq = "failed to forward health check request" + errUpdateHealthStatus = "failed to update health status of provider config" + errFailedHealthCheckReq = "failed to forward health check request" healthCheckSuffix = "-health-check" @@ -66,14 +62,8 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu providerConfig := &apisv1alpha1.ProviderConfig{} if err := c.kubeClientCached.Get(ctx, req.NamespacedName, providerConfig); err != nil { if kerrors.IsNotFound(err) { - // ProviderConfig has been deleted, perform cleanup. - if err := c.cleanup(ctx, req); err != nil { - err = errors.Wrap(err, errHealthCheckCleanup) - traces.SetAndRecordError(span, err) - - return ctrl.Result{}, err - } - + // ProviderConfig has been deleted so there is nothing to do and no need to requeue. + // The backend monitor controller will remove the backend from the backend store. return ctrl.Result{}, nil } @@ -83,8 +73,6 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if providerConfig.Spec.DisableHealthCheck { c.log.Debug("Health check is disabled for s3 backend", consts.KeyBackendName, providerConfig.Name) - c.backendStore.ToggleBackendActiveStatus(req.Name, true) - c.backendStore.SetBackendHealthStatus(req.Name, apisv1alpha1.HealthStatusUnknown) if providerConfig.Status.GetCondition(v1.TypeReady).Equal(v1alpha1.HealthCheckDisabled()) { return ctrl.Result{}, nil @@ -113,8 +101,6 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu health := utils.MapConditionToHealthStatus(providerConfig.Status.GetCondition(v1.TypeReady)) c.backendStore.SetBackendHealthStatus(req.Name, health) - c.backendStore.ToggleBackendActiveStatus(req.Name, health == apisv1alpha1.HealthStatusHealthy) - if providerConfig.Status.GetCondition(v1.TypeReady).Equal(conditionBeforeCheck) { return } @@ -156,24 +142,6 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu }, nil } -// cleanup deletes the lifecycle configuration validation bucket from the backend. -// This function is only called when a ProviderConfig has been deleted. -func (c *Controller) cleanup(ctx context.Context, req ctrl.Request) error { - backendClient := c.backendStore.GetBackendS3Client(req.Name) - if backendClient == nil { - c.log.Info("Backend client not found during validation bucket cleanup - aborting cleanup", consts.KeyBackendName, req.Name) - - return nil - } - - c.log.Info("Deleting lifecycle configuration validation bucket", consts.KeyBucketName, v1alpha1.LifecycleConfigValidationBucketName, consts.KeyBackendName, req.Name) - if err := rgw.DeleteBucket(ctx, backendClient, aws.String(v1alpha1.LifecycleConfigValidationBucketName), true); err != nil { - return errors.Wrap(err, errDeleteLCValidationBucket) - } - - return nil -} - // doHealthCheck performs a basic http request to the hostbase address. func (c *Controller) doHealthCheck(ctx context.Context, providerConfig *apisv1alpha1.ProviderConfig) error { ctx, span := otel.Tracer("").Start(ctx, "Controller.doHealthCheck") diff --git a/internal/controller/providerconfig/healthcheck/healthcheck_controller_test.go b/internal/controller/providerconfig/healthcheck/healthcheck_controller_test.go index 861aa9c6..d95666be 100644 --- a/internal/controller/providerconfig/healthcheck/healthcheck_controller_test.go +++ b/internal/controller/providerconfig/healthcheck/healthcheck_controller_test.go @@ -503,7 +503,7 @@ func TestReconcile(t *testing.T) { tc.fields.fakeS3Client(&fakeS3Client) } bs := backendstore.NewBackendStore() - bs.AddOrUpdateBackend(backendName, &fakeS3Client, nil, tc.fields.autopause, apisv1alpha1.HealthStatusHealthy) + bs.AddOrUpdateBackend(backendName, &fakeS3Client, nil, apisv1alpha1.HealthStatusHealthy) r := NewController( WithAutoPause(&tc.fields.autopause), 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 }(),