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

Commit 8271dbe

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #73 from mukhoakash/bug-fix
Fix: Bucket and BucketAccess object updation facing stale object and improved logging
2 parents 20e7fae + 9a139ce commit 8271dbe

File tree

2 files changed

+86
-29
lines changed

2 files changed

+86
-29
lines changed

Diff for: ‎pkg/bucket/bucket_controller.go

+57-14
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,12 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket)
8787
return nil
8888
}
8989

90-
if bucket.Spec.ExistingBucketID != "" {
91-
bucket.Status.BucketReady = true
92-
bucket.Status.BucketID = bucket.Spec.ExistingBucketID
90+
bucketReady := false
91+
var bucketID string
9392

93+
if bucket.Spec.ExistingBucketID != "" {
94+
bucketReady = true
95+
bucketID = bucket.Spec.ExistingBucketID
9496
} else {
9597
req := &cosi.DriverCreateBucketRequest{
9698
Parameters: bucket.Spec.Parameters,
@@ -100,7 +102,7 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket)
100102
rsp, err := b.provisionerClient.DriverCreateBucket(ctx, req)
101103
if err != nil {
102104
if status.Code(err) != codes.AlreadyExists {
103-
klog.ErrorS(err, "Failed to create bucket",
105+
klog.V(3).ErrorS(err, "Driver failed to create bucket",
104106
"bucket", bucket.ObjectMeta.Name)
105107
return errors.Wrap(err, "Failed to create bucket")
106108
}
@@ -109,14 +111,17 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket)
109111

110112
if rsp == nil {
111113
err = errors.New("DriverCreateBucket returned a nil response")
112-
klog.ErrorS(err, "Internal Error")
114+
klog.V(3).ErrorS(err, "Internal Error from driver",
115+
"bucket", bucket.ObjectMeta.Name)
113116
return err
114117
}
115118

116119
if rsp.BucketId != "" {
117-
bucket.Status.BucketID = rsp.BucketId
118-
bucket.Status.BucketReady = true
120+
bucketID = rsp.BucketId
121+
bucketReady = true
119122
} else {
123+
klog.V(3).ErrorS(err, "DriverCreateBucket returned an empty bucketID",
124+
"bucket", bucket.ObjectMeta.Name)
120125
err = errors.New("DriverCreateBucket returned an empty bucketID")
121126
return err
122127
}
@@ -126,29 +131,48 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket)
126131
ref := bucket.Spec.BucketClaim
127132
bucketClaim, err := b.bucketClaims(ref.Namespace).Get(ctx, ref.Name, metav1.GetOptions{})
128133
if err != nil {
134+
klog.V(3).ErrorS(err, "Failed to get bucketClaim",
135+
"bucketClaim", ref.Name,
136+
"bucket", bucket.ObjectMeta.Name)
129137
return err
130138
}
131139

132140
bucketClaim.Status.BucketReady = true
133-
if _, err = b.bucketClaims(bucketClaim.Namespace).Update(ctx, bucketClaim, metav1.UpdateOptions{}); err != nil {
141+
if _, err = b.bucketClaims(bucketClaim.Namespace).UpdateStatus(ctx, bucketClaim, metav1.UpdateOptions{}); err != nil {
142+
klog.V(3).ErrorS(err, "Failed to update bucketClaim",
143+
"bucketClaim", ref.Name,
144+
"bucket", bucket.ObjectMeta.Name)
134145
return err
135146
}
147+
148+
klog.V(5).Infof("Successfully updated status of bucketClaim: %s, bucket: %s", bucketClaim.ObjectMeta.Name, bucket.ObjectMeta.Name)
136149
}
137150
}
138151

139152
controllerutil.AddFinalizer(bucket, consts.BucketFinalizer)
140-
if _, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}); err != nil {
141-
klog.ErrorS(err, "Failed to update bucket finalizers", "bucket", bucket.ObjectMeta.Name)
153+
if bucket, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}); err != nil {
154+
klog.V(3).ErrorS(err, "Failed to update bucket finalizers", "bucket", bucket.ObjectMeta.Name)
142155
return errors.Wrap(err, "Failed to update bucket finalizers")
143156
}
144157

158+
klog.V(5).Infof("Successfully added finalizer to bucket: %s", bucket.ObjectMeta.Name)
159+
160+
// Setting the status here so that the updated object is used
161+
bucket.Status.BucketReady = bucketReady
162+
bucket.Status.BucketID = bucketID
163+
145164
// if this step fails, then controller will retry with backoff
146165
if _, err = b.buckets().UpdateStatus(ctx, bucket, metav1.UpdateOptions{}); err != nil {
147-
klog.ErrorS(err, "Failed to update bucket status",
166+
klog.V(3).ErrorS(err, "Failed to update bucket status",
148167
"bucket", bucket.ObjectMeta.Name)
149168
return errors.Wrap(err, "Failed to update bucket status")
150169
}
151170

171+
klog.V(3).InfoS("Add Bucket success",
172+
"bucket", bucket.ObjectMeta.Name,
173+
"bucketID", bucketID,
174+
"ns", bucket.ObjectMeta.Namespace)
175+
152176
return nil
153177
}
154178

@@ -174,12 +198,18 @@ func (b *BucketListener) Update(ctx context.Context, old, new *v1alpha1.Bucket)
174198
if strings.EqualFold(bucketAccess.Spec.BucketClaimName, bucketClaimName) {
175199
err = b.bucketAccesses(bucketClaimNs).Delete(ctx, bucketAccess.Name, metav1.DeleteOptions{})
176200
if err != nil {
201+
klog.V(3).ErrorS(err, "Error deleting BucketAccess",
202+
"name", bucketAccess.Name,
203+
"bucket", bucket.ObjectMeta.Name)
177204
return err
178205
}
179206
}
180207
}
181208

209+
klog.V(5).Infof("Successfully deleted dependent bucketAccess of bucket:%s", bucket.ObjectMeta.Name)
210+
182211
controllerutil.RemoveFinalizer(bucket, consts.BABucketFinalizer)
212+
klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BABucketFinalizer, bucket.ObjectMeta.Name)
183213
}
184214

185215
if controllerutil.ContainsFinalizer(bucket, consts.BucketFinalizer) {
@@ -190,6 +220,9 @@ func (b *BucketListener) Update(ctx context.Context, old, new *v1alpha1.Bucket)
190220
}
191221
}
192222

223+
klog.V(3).InfoS("Update Bucket success",
224+
"name", bucket.ObjectMeta.Name,
225+
"ns", bucket.ObjectMeta.Namespace)
193226
return nil
194227
}
195228

@@ -213,7 +246,7 @@ func (b *BucketListener) InitializeKubeClient(k kube.Interface) {
213246

214247
serverVersion, err := k.Discovery().ServerVersion()
215248
if err != nil {
216-
klog.ErrorS(err, "Cannot determine server version")
249+
klog.V(3).ErrorS(err, "Cannot determine server version")
217250
} else {
218251
b.kubeVersion = utilversion.MustParseSemantic(serverVersion.GitVersion)
219252
}
@@ -226,7 +259,7 @@ func (b *BucketListener) InitializeBucketClient(bc buckets.Interface) {
226259

227260
func (b *BucketListener) deleteBucketOp(ctx context.Context, bucket *v1alpha1.Bucket) error {
228261
if !strings.EqualFold(bucket.Spec.DriverName, b.driverName) {
229-
klog.V(5).InfoS("Skipping bucket for provisiner",
262+
klog.V(5).InfoS("Skipping bucket for provisioner",
230263
"bucket", bucket.ObjectMeta.Name,
231264
"driver", bucket.Spec.DriverName,
232265
)
@@ -242,26 +275,36 @@ func (b *BucketListener) deleteBucketOp(ctx context.Context, bucket *v1alpha1.Bu
242275

243276
if _, err := b.provisionerClient.DriverDeleteBucket(ctx, req); err != nil {
244277
if status.Code(err) != codes.NotFound {
245-
klog.ErrorS(err, "Failed to delete bucket",
278+
klog.V(3).ErrorS(err, "Failed to delete bucket",
246279
"bucket", bucket.ObjectMeta.Name,
247280
)
248281
return err
249282
}
250283
}
284+
285+
klog.V(5).Infof("Successfully deleted bucketID: %s from the object storage for bucket: %s", bucket.Status.BucketID, bucket.ObjectMeta.Name)
251286
}
252287

253288
if bucket.Spec.BucketClaim != nil {
254289
ref := bucket.Spec.BucketClaim
255290
bucketClaim, err := b.bucketClaims(ref.Namespace).Get(ctx, ref.Name, metav1.GetOptions{})
256291
if err != nil {
292+
klog.V(3).ErrorS(err, "Error fetching bucketClaim",
293+
"bucketClaim", ref.Name,
294+
"bucket", bucket.ObjectMeta.Name)
257295
return err
258296
}
259297

260298
if controllerutil.RemoveFinalizer(bucketClaim, consts.BCFinalizer) {
261299
if _, err := b.bucketClaims(bucketClaim.ObjectMeta.Namespace).Update(ctx, bucketClaim, metav1.UpdateOptions{}); err != nil {
300+
klog.V(3).ErrorS(err, "Error removing finalizer from bucketClaim",
301+
"bucketClaim", bucketClaim.ObjectMeta.Name,
302+
"bucket", bucket.ObjectMeta.Name)
262303
return err
263304
}
264305
}
306+
307+
klog.V(5).Infof("Successfully removed finalizer: %s from bucketClaim: %s for bucket: %s", consts.BCFinalizer, bucketClaim.ObjectMeta.Name, bucket.ObjectMeta.Name)
265308
}
266309

267310
return nil

Diff for: ‎pkg/bucketaccess/bucketaccess_controller.go

+29-15
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
107107
namespace := bucketAccess.ObjectMeta.Namespace
108108
bucketClaim, err := bal.bucketClaims(namespace).Get(ctx, bucketClaimName, metav1.GetOptions{})
109109
if err != nil {
110-
klog.ErrorS(err, "Failed to fetch bucketClaim", "bucketClaim", bucketClaimName)
110+
klog.V(3).ErrorS(err, "Failed to fetch bucketClaim", "bucketClaim", bucketClaimName)
111111
return errors.Wrap(err, "Failed to fetch bucketClaim")
112112
}
113113

114114
if bucketClaim.Status.BucketName == "" || bucketClaim.Status.BucketReady != true {
115115
err := errors.New("BucketName cannot be empty or BucketNotReady in bucketClaim")
116-
klog.ErrorS(err,
116+
klog.V(3).ErrorS(err,
117117
"Invalid arguments",
118118
"bucketClaim", bucketClaim.Name,
119119
"bucketAccess", bucketAccess.ObjectMeta.Name,
@@ -142,7 +142,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
142142

143143
bucket, err := bal.buckets().Get(ctx, bucketClaim.Status.BucketName, metav1.GetOptions{})
144144
if err != nil {
145-
klog.ErrorS(err, "Failed to fetch bucket", "bucket", bucketClaim.Status.BucketName)
145+
klog.V(3).ErrorS(err, "Failed to fetch bucket", "bucket", bucketClaim.Status.BucketName)
146146
return errors.Wrap(err, "Failed to fetch bucket")
147147
}
148148

@@ -163,7 +163,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
163163
rsp, err := bal.provisionerClient.DriverGrantBucketAccess(ctx, req)
164164
if err != nil {
165165
if status.Code(err) != codes.AlreadyExists {
166-
klog.ErrorS(err,
166+
klog.V(3).ErrorS(err,
167167
"Failed to grant access",
168168
"bucketAccess", bucketAccess.ObjectMeta.Name,
169169
"bucketClaim", bucketClaimName,
@@ -175,14 +175,14 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
175175

176176
if rsp.AccountId == "" {
177177
err = errors.New("AccountId not defined in DriverGrantBucketAccess")
178-
klog.ErrorS(err, "BucketAccess", bucketAccess.ObjectMeta.Name)
178+
klog.V(3).ErrorS(err, "BucketAccess", bucketAccess.ObjectMeta.Name)
179179
return errors.Wrap(err, fmt.Sprintf("BucketAccess %s", bucketAccess.ObjectMeta.Name))
180180
}
181181

182182
credentials := rsp.Credentials
183183
if len(credentials) != 1 {
184184
err = errors.New("Credentials returned in DriverGrantBucketAccessResponse should be of length 1")
185-
klog.ErrorS(err, "BucketAccess", bucketAccess.ObjectMeta.Name)
185+
klog.V(3).ErrorS(err, "BucketAccess", bucketAccess.ObjectMeta.Name)
186186
return errors.Wrap(err, fmt.Sprintf("BucketAccess %s", bucketAccess.ObjectMeta.Name))
187187
}
188188

@@ -230,7 +230,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
230230

231231
if _, err := bal.secrets(namespace).Get(ctx, secretCredName, metav1.GetOptions{}); err != nil {
232232
if !kubeerrors.IsNotFound(err) {
233-
klog.ErrorS(err,
233+
klog.V(3).ErrorS(err,
234234
"Failed to create secrets",
235235
"bucketAccess", bucketAccess.ObjectMeta.Name,
236236
"bucket", bucket.ObjectMeta.Name)
@@ -249,7 +249,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
249249
Type: corev1.SecretTypeOpaque,
250250
}, metav1.CreateOptions{}); err != nil {
251251
if !kubeerrors.IsAlreadyExists(err) {
252-
klog.ErrorS(err,
252+
klog.V(3).ErrorS(err,
253253
"Failed to create minted secret",
254254
"bucketAccess", bucketAccess.ObjectMeta.Name,
255255
"bucket", bucket.ObjectMeta.Name)
@@ -266,8 +266,9 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
266266
}
267267

268268
if controllerutil.AddFinalizer(bucketAccess, consts.BAFinalizer) {
269-
if _, err = bal.bucketAccesses(bucketAccess.ObjectMeta.Namespace).Update(ctx, bucketAccess, metav1.UpdateOptions{}); err != nil {
270-
klog.ErrorS(err, "Failed to update BucketAccess finalizer",
269+
bucketAccess, err = bal.bucketAccesses(bucketAccess.ObjectMeta.Namespace).Update(ctx, bucketAccess, metav1.UpdateOptions{})
270+
if err != nil {
271+
klog.V(3).ErrorS(err, "Failed to update BucketAccess finalizer",
271272
"bucketAccess", bucketAccess.ObjectMeta.Name,
272273
"bucket", bucket.ObjectMeta.Name)
273274
return errors.Wrap(err, fmt.Sprintf("Failed to update BucketAccess finalizer. BucketAccess: %s", bucketAccess.ObjectMeta.Name))
@@ -279,7 +280,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
279280

280281
// if this step fails, then controller will retry with backoff
281282
if _, err := bal.bucketAccesses(bucketAccess.ObjectMeta.Namespace).UpdateStatus(ctx, bucketAccess, metav1.UpdateOptions{}); err != nil {
282-
klog.ErrorS(err, "Failed to update BucketAccess Status",
283+
klog.V(3).ErrorS(err, "Failed to update BucketAccess Status",
283284
"bucketAccess", bucketAccess.ObjectMeta.Name,
284285
"bucket", bucket.ObjectMeta.Name)
285286
return errors.Wrap(err, fmt.Sprintf("Failed to update BucketAccess Status. BucketAccess: %s", bucketAccess.ObjectMeta.Name))
@@ -297,11 +298,15 @@ func (bal *BucketAccessListener) Update(ctx context.Context, old, new *v1alpha1.
297298
"name", old.ObjectMeta.Name)
298299

299300
bucketAccess := new.DeepCopy()
300-
err := bal.deleteBucketAccessOp(ctx, bucketAccess)
301-
if err != nil {
302-
return err
301+
if !bucketAccess.GetDeletionTimestamp().IsZero() {
302+
err := bal.deleteBucketAccessOp(ctx, bucketAccess)
303+
if err != nil {
304+
return err
305+
}
303306
}
304307

308+
klog.V(3).InfoS("Update BucketAccess success",
309+
"name", old.ObjectMeta.Name)
305310
return nil
306311
}
307312

@@ -328,15 +333,24 @@ func (bal *BucketAccessListener) deleteBucketAccessOp(ctx context.Context, bucke
328333
if controllerutil.RemoveFinalizer(secret, consts.SecretFinalizer) {
329334
_, err = bal.secrets(bucketAccess.ObjectMeta.Namespace).Update(ctx, secret, metav1.UpdateOptions{})
330335
if err != nil {
336+
klog.V(3).ErrorS(err, "Error removing finalizer from secret",
337+
"secret", secret.ObjectMeta.Name,
338+
"bucketAccess", bucketAccess.ObjectMeta.Name)
331339
return err
332340
}
341+
342+
klog.V(5).Infof("Successfully removed finalizer from secret: %s, bucketAccess: %s", secret.ObjectMeta.Name, bucketAccess.ObjectMeta.Name)
333343
}
334344

335345
if controllerutil.RemoveFinalizer(bucketAccess, consts.BAFinalizer) {
336346
_, err = bal.bucketAccesses(bucketAccess.ObjectMeta.Namespace).Update(ctx, bucketAccess, metav1.UpdateOptions{})
337347
if err != nil {
348+
klog.V(3).ErrorS(err, "Error removing finalizer from bucketAccess",
349+
"bucketAccess", bucketAccess.ObjectMeta.Name)
338350
return err
339351
}
352+
353+
klog.V(5).Infof("Successfully removed finalizer from bucketAccess: %s", bucketAccess.ObjectMeta.Name)
340354
}
341355

342356
return nil
@@ -383,7 +397,7 @@ func (bal *BucketAccessListener) InitializeKubeClient(k kube.Interface) {
383397

384398
serverVersion, err := k.Discovery().ServerVersion()
385399
if err != nil {
386-
klog.ErrorS(err, "Cannot determine server version")
400+
klog.V(3).ErrorS(err, "Cannot determine server version")
387401
} else {
388402
bal.kubeVersion = utilversion.MustParseSemantic(serverVersion.GitVersion)
389403
}

0 commit comments

Comments
 (0)