From 1829a9ebfe13d8f1f1bf9def81c2d780d83369ba Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Thu, 1 Aug 2024 17:40:10 +0200 Subject: [PATCH 1/3] Fix recreating BucketAccess with Update method Signed-off-by: Andrei Kvapil --- pkg/imported-sidecar/pkg/bucket/bucket_controller.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/imported-sidecar/pkg/bucket/bucket_controller.go b/pkg/imported-sidecar/pkg/bucket/bucket_controller.go index 9b0d672c..d865dfc3 100644 --- a/pkg/imported-sidecar/pkg/bucket/bucket_controller.go +++ b/pkg/imported-sidecar/pkg/bucket/bucket_controller.go @@ -297,6 +297,12 @@ func (b *BucketListener) Delete(ctx context.Context, inputBucket *v1alpha1.Bucke return err } } + } else { + // Trigger the Add logic to ensure that the BucketAccess is properly reconciled + err := bal.Add(ctx, bucketAccess) + if err != nil { + return bal.recordError(bucketAccess, v1.EventTypeWarning, events.FailedGrantAccess, err) + } } return nil From c8d8efbaa8dc0d4ead6b6037a3f4cc11e0e5ad5f Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Thu, 1 Aug 2024 17:41:41 +0200 Subject: [PATCH 2/3] Fix handle deletion from Update method Signed-off-by: Andrei Kvapil --- .../pkg/bucket/bucket_controller.go | 100 ++++++++++-------- .../bucketaccess/bucketaccess_controller.go | 7 ++ 2 files changed, 64 insertions(+), 43 deletions(-) diff --git a/pkg/imported-sidecar/pkg/bucket/bucket_controller.go b/pkg/imported-sidecar/pkg/bucket/bucket_controller.go index d865dfc3..04ee427d 100644 --- a/pkg/imported-sidecar/pkg/bucket/bucket_controller.go +++ b/pkg/imported-sidecar/pkg/bucket/bucket_controller.go @@ -69,6 +69,12 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket) var err error + if !bucket.GetDeletionTimestamp().IsZero() { + if err = b.handleDeletion(ctx, bucket); err != nil { + return err + } + } + klog.V(3).InfoS("Add Bucket", "name", bucket.ObjectMeta.Name) @@ -213,48 +219,7 @@ func (b *BucketListener) Update(ctx context.Context, old, new *v1alpha1.Bucket) var err error if !bucket.GetDeletionTimestamp().IsZero() { - if controllerutil.ContainsFinalizer(bucket, consts.BABucketFinalizer) { - bucketClaimNs := bucket.Spec.BucketClaim.Namespace - bucketClaimName := bucket.Spec.BucketClaim.Name - bucketAccessList, err := b.bucketAccesses(bucketClaimNs).List(ctx, metav1.ListOptions{}) - if err != nil { - klog.V(3).ErrorS(err, "Error fetching BucketAccessList", - "bucket", bucket.ObjectMeta.Name) - return err - } - - for _, bucketAccess := range bucketAccessList.Items { - if strings.EqualFold(bucketAccess.Spec.BucketClaimName, bucketClaimName) { - err = b.bucketAccesses(bucketClaimNs).Delete(ctx, bucketAccess.Name, metav1.DeleteOptions{}) - if err != nil { - klog.V(3).ErrorS(err, "Error deleting BucketAccess", - "name", bucketAccess.Name, - "bucket", bucket.ObjectMeta.Name) - return err - } - } - } - - klog.V(5).Infof("Successfully deleted dependent bucketAccess of bucket:%s", bucket.ObjectMeta.Name) - - controllerutil.RemoveFinalizer(bucket, consts.BABucketFinalizer) - klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BABucketFinalizer, bucket.ObjectMeta.Name) - } - - if controllerutil.ContainsFinalizer(bucket, consts.BucketFinalizer) { - err = b.deleteBucketOp(ctx, bucket) - if err != nil { - return b.recordError(bucket, v1.EventTypeWarning, events.FailedDeleteBucket, err) - } - - controllerutil.RemoveFinalizer(bucket, consts.BucketFinalizer) - klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BucketFinalizer, bucket.ObjectMeta.Name) - } - - _, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}) - if err != nil { - klog.V(3).ErrorS(err, "Error updating bucket after removing finalizers", - "bucket", bucket.ObjectMeta.Name) + if err = b.handleDeletion(ctx, bucket); err != nil { return err } } @@ -265,7 +230,7 @@ func (b *BucketListener) Update(ctx context.Context, old, new *v1alpha1.Bucket) return nil } -// Delete attemps to delete a bucket. This function must be idempotent +// Delete attempts to delete a bucket. This function must be idempotent // Delete function is called when the bucket was not able to add finalizers while creation. // Hence we will take care of removing the BucketClaim finalizer before deleting the Bucket object. // Return values @@ -374,6 +339,55 @@ func (b *BucketListener) deleteBucketOp(ctx context.Context, bucket *v1alpha1.Bu return nil } +func (b *BucketListener) handleDeletion(ctx context.Context, bucket *v1alpha1.Bucket) error { + var err error + + // Remove BABucketFinalizer and delete associated BucketAccess resources + if controllerutil.ContainsFinalizer(bucket, consts.BABucketFinalizer) { + bucketClaimNs := bucket.Spec.BucketClaim.Namespace + bucketClaimName := bucket.Spec.BucketClaim.Name + bucketAccessList, err := b.bucketAccesses(bucketClaimNs).List(ctx, metav1.ListOptions{}) + if err != nil { + klog.V(3).ErrorS(err, "Error fetching BucketAccessList", "bucket", bucket.ObjectMeta.Name) + return err + } + + for _, bucketAccess := range bucketAccessList.Items { + if strings.EqualFold(bucketAccess.Spec.BucketClaimName, bucketClaimName) { + err = b.bucketAccesses(bucketClaimNs).Delete(ctx, bucketAccess.Name, metav1.DeleteOptions{}) + if err != nil { + klog.V(3).ErrorS(err, "Error deleting BucketAccess", "name", bucketAccess.Name, "bucket", bucket.ObjectMeta.Name) + return err + } + } + } + + klog.V(5).Infof("Successfully deleted dependent bucketAccess of bucket:%s", bucket.ObjectMeta.Name) + + controllerutil.RemoveFinalizer(bucket, consts.BABucketFinalizer) + klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BABucketFinalizer, bucket.ObjectMeta.Name) + } + + // Remove BucketFinalizer and delete the bucket + if controllerutil.ContainsFinalizer(bucket, consts.BucketFinalizer) { + err = b.deleteBucketOp(ctx, bucket) + if err != nil { + return b.recordError(bucket, v1.EventTypeWarning, events.FailedDeleteBucket, err) + } + + controllerutil.RemoveFinalizer(bucket, consts.BucketFinalizer) + klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BucketFinalizer, bucket.ObjectMeta.Name) + + _, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}) + if err != nil { + klog.V(3).ErrorS(err, "Error updating bucket after removing finalizers", "bucket", bucket.ObjectMeta.Name) + return err + } + } + + return nil +} + func (b *BucketListener) buckets() bucketapi.BucketInterface { if b.bucketClient != nil { return b.bucketClient.ObjectstorageV1alpha1().Buckets() diff --git a/pkg/imported-sidecar/pkg/bucketaccess/bucketaccess_controller.go b/pkg/imported-sidecar/pkg/bucketaccess/bucketaccess_controller.go index c441b6b3..64f51771 100644 --- a/pkg/imported-sidecar/pkg/bucketaccess/bucketaccess_controller.go +++ b/pkg/imported-sidecar/pkg/bucketaccess/bucketaccess_controller.go @@ -69,6 +69,13 @@ func NewBucketAccessListener(driverName string, client cosi.ProvisionerClient) * func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1alpha1.BucketAccess) error { bucketAccess := inputBucketAccess.DeepCopy() + if !bucketAccess.GetDeletionTimestamp().IsZero() { + // If the bucketAccess has a deletion timestamp, handle it as a deletion + klog.V(3).InfoS("BucketAccess has deletion timestamp, handling deletion", + "name", bucketAccess.ObjectMeta.Name) + return bal.deleteBucketAccessOp(ctx, bucketAccess) + } + if bucketAccess.Status.AccessGranted && bucketAccess.Status.AccountID != "" { klog.V(3).InfoS("BucketAccess already exists", bucketAccess.ObjectMeta.Name) return nil From f34ef08240eae69ca238c69b93da5da57bdb9c52 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Thu, 1 Aug 2024 17:42:00 +0200 Subject: [PATCH 3/3] Fix recreating Bucket with Update method Signed-off-by: Andrei Kvapil --- pkg/imported-sidecar/pkg/bucket/bucket_controller.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/imported-sidecar/pkg/bucket/bucket_controller.go b/pkg/imported-sidecar/pkg/bucket/bucket_controller.go index 04ee427d..01b4e218 100644 --- a/pkg/imported-sidecar/pkg/bucket/bucket_controller.go +++ b/pkg/imported-sidecar/pkg/bucket/bucket_controller.go @@ -222,6 +222,12 @@ func (b *BucketListener) Update(ctx context.Context, old, new *v1alpha1.Bucket) if err = b.handleDeletion(ctx, bucket); err != nil { return err } + } else { + // Trigger the Add logic to ensure that the Bucket is properly reconciled + err := b.Add(ctx, bucket) + if err != nil { + return b.recordError(bucket, v1.EventTypeWarning, events.FailedGrantAccess, err) + } } klog.V(3).InfoS("Update Bucket success",