From ff5c0eac1f1b255c338f04ce4b84343cc36a34a7 Mon Sep 17 00:00:00 2001 From: Akash Mukhopadhyay Date: Sun, 18 Sep 2022 18:30:52 -0700 Subject: [PATCH 1/3] Fixing a bucketclaim object updation bug --- pkg/bucketclaim/bucketclaim.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index 9eb33c4..fb20d0f 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -99,7 +99,8 @@ func (b *bucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1. // ErrInvalidBucketClass - BucketClass does not exist [requeue'd with exponential backoff] // ErrBucketAlreadyExists - BucketClaim already processed // non-nil err - Internal error [requeue'd with exponential backoff] -func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { +func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, inputBucketClaim *v1alpha1.BucketClaim) error { + bucketClaim := inputBucketClaim.DeepCopy() if bucketClaim.Status.BucketReady { return util.ErrBucketAlreadyExists } From f5d937d1e682632800c1afd018850ffe66328076 Mon Sep 17 00:00:00 2001 From: Akash Mukhopadhyay Date: Mon, 19 Sep 2022 00:44:19 -0700 Subject: [PATCH 2/3] Fixing a couple of update bugs --- pkg/bucketclaim/bucketclaim.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index fb20d0f..7cb532d 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -86,7 +86,7 @@ func (b *bucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.Buc // Delete processes a bucket for which bucket request is deleted func (b *bucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { - klog.V(3).Infof("Delete BucketClaim %v", + klog.V(3).Infof("Delete BucketClaim", "name", bucketClaim.ObjectMeta.Name, "ns", bucketClaim.ObjectMeta.Namespace) @@ -172,8 +172,11 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClaim.Status.BucketReady = false } - _, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).UpdateStatus(ctx, bucketClaim, metav1.UpdateOptions{}) + // Fetching the updated bucketClaim again, so that the update + // operation doesn't happen on an outdated version of the bucketClaim. + bucketClaim, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).UpdateStatus(ctx, bucketClaim, metav1.UpdateOptions{}) if err != nil { + klog.ErrorS(err, "Failed to update status of BucketClaim", "name", bucketClaim.ObjectMeta.Name) return err } @@ -182,8 +185,10 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, controllerutil.AddFinalizer(bucketClaim, util.BucketClaimFinalizer) _, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).Update(ctx, bucketClaim, metav1.UpdateOptions{}) if err != nil { + klog.ErrorS(err, "Failed to update BucketClaim", "name", bucketClaim.ObjectMeta.Name) return err } + klog.Infof("Finished creating Bucket %v", bucketName) return nil } From 6fd93c5cf57e93cd8d3b2aff78b9e272988104c2 Mon Sep 17 00:00:00 2001 From: Akash Mukhopadhyay Date: Mon, 19 Sep 2022 11:22:23 -0700 Subject: [PATCH 3/3] Improving some logs --- pkg/bucketclaim/bucketclaim.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index 7cb532d..387fb50 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -39,7 +39,7 @@ func (b *bucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.Buc if err != nil { switch err { case util.ErrInvalidBucketClass: - klog.ErrorS(util.ErrInvalidBucketClass, + klog.V(3).ErrorS(util.ErrInvalidBucketClass, "bucketClaim", bucketClaim.ObjectMeta.Name, "ns", bucketClaim.ObjectMeta.Namespace, "bucketClassName", bucketClaim.Spec.BucketClassName) @@ -50,7 +50,7 @@ func (b *bucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.Buc ) return nil default: - klog.ErrorS(err, + klog.V(3).ErrorS(err, "name", bucketClaim.ObjectMeta.Name, "ns", bucketClaim.ObjectMeta.Namespace, "err", err) @@ -77,10 +77,19 @@ func (b *bucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.Buc bucketName := bucketClaim.Status.BucketName err := b.buckets().Delete(ctx, bucketName, metav1.DeleteOptions{}) if err != nil { + klog.V(3).ErrorS(err, "Error deleting bucket", + "bucket", bucketName, + "bucketClaim", bucketClaim.ObjectMeta.Name) return err } + + klog.V(5).Infof("Successfully deleted bucket: %s from bucketClaim: %s", bucketName, bucketClaim.ObjectMeta.Name) } } + + klog.V(3).InfoS("Update BucketClaim success", + "name", bucketClaim.ObjectMeta.Name, + "ns", bucketClaim.ObjectMeta.Namespace) return nil } @@ -112,7 +121,7 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) if err != nil { - klog.ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) + klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) return err } @@ -124,6 +133,9 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, _, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}) if err != nil { + klog.V(3).ErrorS(err, "Error updating existing bucket", + "bucket", bucket.ObjectMeta.Name, + "bucketClaim", bucketClaim.ObjectMeta.Name) return err } @@ -137,7 +149,7 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) if err != nil { - klog.ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) + klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) return util.ErrInvalidBucketClass } @@ -164,7 +176,9 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucket.Spec.Protocols = protocolCopy bucket, err = b.buckets().Create(ctx, bucket, metav1.CreateOptions{}) if err != nil && !errors.IsAlreadyExists(err) { - klog.ErrorS(err, "name", bucketName) + klog.V(3).ErrorS(err, "Error creationg bucket", + "bucket", bucketName, + "bucketClaim", bucketClaim.ObjectMeta.Name) return err } @@ -176,7 +190,7 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, // operation doesn't happen on an outdated version of the bucketClaim. bucketClaim, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).UpdateStatus(ctx, bucketClaim, metav1.UpdateOptions{}) if err != nil { - klog.ErrorS(err, "Failed to update status of BucketClaim", "name", bucketClaim.ObjectMeta.Name) + klog.V(3).ErrorS(err, "Failed to update status of BucketClaim", "name", bucketClaim.ObjectMeta.Name) return err } @@ -185,11 +199,11 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, controllerutil.AddFinalizer(bucketClaim, util.BucketClaimFinalizer) _, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).Update(ctx, bucketClaim, metav1.UpdateOptions{}) if err != nil { - klog.ErrorS(err, "Failed to update BucketClaim", "name", bucketClaim.ObjectMeta.Name) + klog.V(3).ErrorS(err, "Failed to add finalizer BucketClaim", "name", bucketClaim.ObjectMeta.Name) return err } - klog.Infof("Finished creating Bucket %v", bucketName) + klog.V(3).Infof("Finished creating Bucket %v", bucketName) return nil }