Skip to content

Commit

Permalink
Remove deprecate health check bucket (#101)
Browse files Browse the repository at this point in the history
  • Loading branch information
nolancon authored Nov 23, 2023
1 parent e30dc12 commit 403e0f0
Show file tree
Hide file tree
Showing 8 changed files with 5 additions and 122 deletions.
16 changes: 1 addition & 15 deletions apis/provider-ceph/v1alpha1/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,5 @@ limitations under the License.
package v1alpha1

const (
HealthCheckLabelKey = "provider-ceph.crossplane.io"
HealthCheckLabelVal = "health-check-bucket"
BackendLabelPrefix = "provider-ceph.backends."
BackendLabelPrefix = "provider-ceph.backends."
)

// Deprecation warning: This function exists for compatibility reasons,
// and would be removed soon.
func IsHealthCheckBucket(bucket *Bucket) bool {
if val, ok := bucket.GetLabels()[HealthCheckLabelKey]; ok {
if val == HealthCheckLabelVal {
return true
}
}

return false
}
75 changes: 0 additions & 75 deletions internal/controller/bucket/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,64 +483,6 @@ func TestObserve(t *testing.T) {
},
},
},
"Bucket check on external - OK, Health check bucket ignores auto pause": {
fields: fields{
backendStore: func() *backendstore.BackendStore {
fake := backendstorefakes.FakeS3Client{
HeadBucketStub: func(ctx context.Context, hbi *s3.HeadBucketInput, f ...func(*s3.Options)) (*s3.HeadBucketOutput, error) {
return &s3.HeadBucketOutput{}, nil
},
}

bs := backendstore.NewBackendStore()
bs.AddOrUpdateBackend("s3-backend-1", &fake, true, apisv1alpha1.HealthStatusHealthy)

return bs
}(),
autoPauseBucket: true,
},
args: args{
mg: &v1alpha1.Bucket{
ObjectMeta: metav1.ObjectMeta{
Finalizers: []string{inUseFinalizer},
Labels: map[string]string{
v1alpha1.HealthCheckLabelKey: v1alpha1.HealthCheckLabelVal,
},
},
Spec: v1alpha1.BucketSpec{
Providers: []string{"s3-backend-1"},
ResourceSpec: v1.ResourceSpec{
ProviderConfigReference: &v1.Reference{
Name: "s3-backend-1",
},
},
},
Status: v1alpha1.BucketStatus{
AtProvider: v1alpha1.BucketObservation{
Backends: v1alpha1.Backends{
"s3-backend-1": &v1alpha1.BackendInfo{
BucketStatus: v1alpha1.ReadyStatus,
},
},
},
ResourceStatus: v1.ResourceStatus{
ConditionedStatus: v1.ConditionedStatus{
Conditions: []v1.Condition{
v1.Available(),
},
},
},
},
},
},
want: want{
o: managed.ExternalObservation{
ResourceExists: true,
ResourceUpToDate: true,
ConnectionDetails: managed.ConnectionDetails{},
},
},
},
}
for name, tc := range cases {
tc := tc
Expand Down Expand Up @@ -821,23 +763,6 @@ func TestUpdate(t *testing.T) {
err: errors.New(errNotBucket),
},
},
"OK - Health check bucket": {
fields: fields{
backendStore: backendstore.NewBackendStore(),
},
args: args{
mg: &v1alpha1.Bucket{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
v1alpha1.HealthCheckLabelKey: v1alpha1.HealthCheckLabelVal,
},
},
},
},
want: want{
o: managed.ExternalUpdate{},
},
},
"Bucket is disabled": {
fields: fields{
backendStore: backendstore.NewBackendStore(),
Expand Down
8 changes: 0 additions & 8 deletions internal/controller/bucket/bucket_validation_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,6 @@ func (b *BucketValidator) ValidateDelete(ctx context.Context, obj runtime.Object
}

func (b *BucketValidator) validateCreateOrUpdate(ctx context.Context, bucket *v1alpha1.Bucket) error {
// Ignore validation for health check buckets as they do not
// behave as 'normal' buckets. For example, health check buckets
// need to be updated after their owning ProviderConfig has been deleted.
// This is to remove a finalizer and enable garbage collection.
if v1alpha1.IsHealthCheckBucket(bucket) {
return nil
}

if len(bucket.Spec.Providers) != 0 {
missingProviders := utils.MissingStrings(bucket.Spec.Providers, b.backendStore.GetAllActiveBackendNames())
if len(missingProviders) != 0 {
Expand Down
6 changes: 0 additions & 6 deletions internal/controller/bucket/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalCreation{}, errors.Wrap(err, errGetPC)
}

if v1alpha1.IsHealthCheckBucket(bucket) && pc.Spec.DisableHealthCheck {
c.log.Info("Health check is disabled on backend - health-check-bucket will not be created", "backend name", beName)

continue
}

errorsLeft++

beName := beName
Expand Down
6 changes: 0 additions & 6 deletions internal/controller/bucket/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error {
ctx, cancel := context.WithTimeout(ctx, c.operationTimeout)
defer cancel()

if v1alpha1.IsHealthCheckBucket(bucket) {
c.log.Info("Delete is NOOP for health check bucket as it is owned by, and garbage collected on deletion of its related providerconfig", "bucket", bucket.Name)

return nil
}

// There are two scenarios where the bucket status needs to be updated during a
// Delete invocation:
// 1. The caller attempts to delete the CR and an error occurs during the call to
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/bucket/observe.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
}, nil
}

if !v1alpha1.IsHealthCheckBucket(bucket) && (bucket.Spec.AutoPause || c.autoPauseBucket) && bucket.Labels[meta.AnnotationKeyReconciliationPaused] == "" {
if (bucket.Spec.AutoPause || c.autoPauseBucket) && bucket.Labels[meta.AnnotationKeyReconciliationPaused] == "" {
return managed.ExternalObservation{
ResourceExists: true,
ResourceUpToDate: false,
Expand All @@ -58,7 +58,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
break
}
}
if !available && !v1alpha1.IsHealthCheckBucket(bucket) {
if !available {
return managed.ExternalObservation{
ResourceExists: true,
ResourceUpToDate: false,
Expand Down
9 changes: 1 addition & 8 deletions internal/controller/bucket/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalUpdate{}, errors.New(errNotBucket)
}

if v1alpha1.IsHealthCheckBucket(bucket) {
c.log.Info("Update is NOOP for health check bucket - updates performed by health-check-controller", "bucket", bucket.Name)

return managed.ExternalUpdate{}, nil
}

ctx, cancel := context.WithTimeout(ctx, c.operationTimeout)
defer cancel()

Expand Down Expand Up @@ -74,8 +68,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
}
}

if !v1alpha1.IsHealthCheckBucket(bucket) &&
allBucketsReady &&
if allBucketsReady &&
(bucket.Spec.AutoPause || c.autoPauseBucket) &&
bucket.Labels[meta.AnnotationKeyReconciliationPaused] != "true" {
c.log.Info("Auto pausing bucket", "bucket_name", bucket.Name)
Expand Down
3 changes: 1 addition & 2 deletions internal/controller/providerconfig/healthcheck_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,7 @@ func (r *HealthCheckReconciler) unpauseBuckets(ctx context.Context, s3BackendNam
Factor: factor,
Jitter: jitter,
}, resource.IsAPIError, func() error {
if !v1alpha1.IsHealthCheckBucket(&buckets.Items[i]) &&
(r.autoPauseBucket || buckets.Items[i].Spec.AutoPause) &&
if (r.autoPauseBucket || buckets.Items[i].Spec.AutoPause) &&
buckets.Items[i].Labels[meta.AnnotationKeyReconciliationPaused] == "true" {
buckets.Items[i].Labels[meta.AnnotationKeyReconciliationPaused] = ""

Expand Down

0 comments on commit 403e0f0

Please sign in to comment.