Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

fix update method #86

Open
wants to merge 3 commits into
base: monorepo
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 68 additions & 42 deletions pkg/imported-sidecar/pkg/bucket/bucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -213,49 +219,14 @@ 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)
if err = b.handleDeletion(ctx, bucket); err != nil {
return err
}

_, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{})
} else {
// Trigger the Add logic to ensure that the Bucket is properly reconciled
err := b.Add(ctx, bucket)
if err != nil {
klog.V(3).ErrorS(err, "Error updating bucket after removing finalizers",
"bucket", bucket.ObjectMeta.Name)
return err
return b.recordError(bucket, v1.EventTypeWarning, events.FailedGrantAccess, err)
}
}

Expand All @@ -265,7 +236,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
Expand Down Expand Up @@ -297,6 +268,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
Expand Down Expand Up @@ -368,6 +345,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()
Expand Down
Copy link

@gauriKrishnan gauriKrishnan Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down