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

Commit e62441a

Browse files
brahmaroutubrahmaroutu
committed
Adding logic to process BR and BAR deletes.
1 parent 20d2924 commit e62441a

File tree

5 files changed

+434
-10
lines changed

5 files changed

+434
-10
lines changed

‎pkg/bucketaccessrequest/bucketaccessrequest.go

+53-7
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ func (b *bucketAccessRequestListener) InitializeBucketClient(bc bucketclientset.
3333
b.bucketClient = bc
3434
}
3535

36+
// Add is in response to user adding a BucketAccessRequest. The call here will respond by creating a BucketAccess Object.
3637
func (b *bucketAccessRequestListener) Add(ctx context.Context, obj *v1alpha1.BucketAccessRequest) error {
3738
glog.V(1).Infof("Add called for BucketAccessRequest %s", obj.Name)
3839
bucketAccessRequest := obj
@@ -57,13 +58,24 @@ func (b *bucketAccessRequestListener) Add(ctx context.Context, obj *v1alpha1.Buc
5758
return nil
5859
}
5960

61+
// Update is called in response to a change to BucketAccessRequesst. At this point
62+
// BucketAccess cannot be changed once created as the Provisioner might have already acted upon the create BucketAccess and created the backend Bucket Credentials
63+
// Changes to Protocol, Provisioner, BucketInstanceName, BucketRequest cannot be allowed. Best is to delete and recreate a new BucketAccessRequest
64+
// Changes to ServiceAccount, PolicyActionsConfigMapData and Parameters should be considered in lieu with sidecar implementation
6065
func (b *bucketAccessRequestListener) Update(ctx context.Context, old, new *v1alpha1.BucketAccessRequest) error {
6166
glog.V(1).Infof("Update called for BucketAccessRequest %v", old.Name)
67+
if (old.ObjectMeta.DeletionTimestamp == nil) &&
68+
(new.ObjectMeta.DeletionTimestamp != nil) {
69+
// BucketAccessRequest is being deleted, check and remove finalizer once BA is deleted
70+
return b.removeBucketAccess(ctx, new)
71+
}
6272
return nil
6373
}
6474

65-
func (b *bucketAccessRequestListener) Delete(ctx context.Context, obj *v1alpha1.BucketAccessRequest) error {
66-
glog.V(1).Infof("Delete called for BucketAccessRequest %v", obj.Name)
75+
// Delete is in response to user deleting a BucketAccessRequest. The call here will respond by deleting a BucketAccess Object.
76+
func (b *bucketAccessRequestListener) Delete(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error {
77+
glog.V(1).Infof("Delete called for BucketAccessRequest %v", bucketAccessRequest.Name)
78+
6779
return nil
6880
}
6981

@@ -119,7 +131,7 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context,
119131
UID: sa.ObjectMeta.UID}
120132
// bucketaccess.Spec.MintedSecretName - set by the driver
121133
bucketaccess.Spec.PolicyActionsConfigMapData, err = util.ReadConfigData(b.kubeClient, bucketAccessClass.PolicyActionsConfigMap)
122-
if err != nil {
134+
if err != nil && err != util.ErrNilConfigMap {
123135
return err
124136
}
125137
// bucketaccess.Spec.Principal - set by the driver
@@ -131,6 +143,10 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context,
131143
return err
132144
}
133145

146+
if !util.CheckFinalizer(bucketAccessRequest, util.BARDeleteFinalizer) {
147+
bucketAccessRequest.ObjectMeta.Finalizers = append(bucketAccessRequest.ObjectMeta.Finalizers, util.BARDeleteFinalizer)
148+
}
149+
134150
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
135151
bucketAccessRequest.Spec.BucketAccessName = bucketaccess.Name
136152
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccessRequests(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{})
@@ -146,17 +162,47 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context,
146162
return nil
147163
}
148164

149-
func (b *bucketAccessRequestListener) FindBucketAccess(ctx context.Context, bar *v1alpha1.BucketAccessRequest) *v1alpha1.BucketAccess {
165+
func (b *bucketAccessRequestListener) removeBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error {
166+
bucketaccess := b.FindBucketAccess(ctx, bucketAccessRequest)
167+
if bucketaccess == nil {
168+
// bucketaccess for this BucketAccessRequest is not found
169+
return util.ErrBucketAccessDoesNotExist
170+
}
171+
172+
// time to delete the BucketAccess Object
173+
err := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().Delete(context.Background(), bucketaccess.Name, metav1.DeleteOptions{})
174+
if err != nil {
175+
return err
176+
}
177+
178+
// we can safely remove the finalizer
179+
return b.removeBARDeleteFinalizer(ctx, bucketAccessRequest)
180+
}
181+
182+
func (b *bucketAccessRequestListener) FindBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) *v1alpha1.BucketAccess {
150183
bucketAccessList, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().List(ctx, metav1.ListOptions{})
151184
if err != nil || len(bucketAccessList.Items) <= 0 {
152185
return nil
153186
}
154187
for _, bucketaccess := range bucketAccessList.Items {
155-
if bucketaccess.Spec.BucketAccessRequest.Name == bar.Name &&
156-
bucketaccess.Spec.BucketAccessRequest.Namespace == bar.Namespace &&
157-
bucketaccess.Spec.BucketAccessRequest.UID == bar.UID {
188+
if bucketaccess.Spec.BucketAccessRequest.Name == bucketAccessRequest.Name &&
189+
bucketaccess.Spec.BucketAccessRequest.Namespace == bucketAccessRequest.Namespace &&
190+
bucketaccess.Spec.BucketAccessRequest.UID == bucketAccessRequest.UID {
158191
return &bucketaccess
159192
}
160193
}
161194
return nil
162195
}
196+
197+
func (b *bucketAccessRequestListener) removeBARDeleteFinalizer(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error {
198+
newFinalizers := []string{}
199+
for _, finalizer := range bucketAccessRequest.ObjectMeta.Finalizers {
200+
if finalizer != util.BARDeleteFinalizer {
201+
newFinalizers = append(newFinalizers, finalizer)
202+
}
203+
}
204+
bucketAccessRequest.ObjectMeta.Finalizers = newFinalizers
205+
206+
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccessRequests(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{})
207+
return err
208+
}

‎pkg/bucketaccessrequest/bucketaccessrequest_test.go

+179-2
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,22 @@ func TestAddBAR(t *testing.T) {
119119

120120
// Test add with multipleBRs
121121
func TestAddWithMultipleBAR(t *testing.T) {
122-
runCreateBucketWithMultipleBA(t, "addWithMultipleBR")
122+
runCreateBucketWithMultipleBA(t, "addWithMultipleBAR")
123123
}
124124

125125
// Test add idempotency
126126
func TestAddBARIdempotency(t *testing.T) {
127-
runCreateBucketIdempotency(t, "addWithMultipleBR")
127+
runCreateBucketIdempotency(t, "addBARIdempotency")
128+
}
129+
130+
// Test delete BAR
131+
func TestDeleteBAR(t *testing.T) {
132+
runDeleteBucketAccessRequest(t, "deleteBAR")
133+
}
134+
135+
// Test delete BAR Idempotency
136+
func TestDeleteBARIdempotency(t *testing.T) {
137+
runDeleteBucketAccessRequestIdempotency(t, "deleteBARIdempotency")
128138
}
129139

130140
func runCreateBucketAccess(t *testing.T, name string) {
@@ -330,3 +340,170 @@ func runCreateBucketIdempotency(t *testing.T, name string) {
330340
t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items))
331341
}
332342
}
343+
344+
func runDeleteBucketAccessRequest(t *testing.T, name string) {
345+
ctx, cancel := context.WithCancel(context.Background())
346+
defer cancel()
347+
348+
client := bucketclientset.NewSimpleClientset()
349+
kubeClient := fake.NewSimpleClientset()
350+
351+
listener := NewListener()
352+
listener.InitializeKubeClient(kubeClient)
353+
listener.InitializeBucketClient(client)
354+
355+
_, err := kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Create(ctx, &sa1, metav1.CreateOptions{})
356+
if err != nil {
357+
t.Fatalf("Error occurred when creating ServiceAccount: %v", err)
358+
}
359+
defer kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Delete(ctx, sa1.Name, metav1.DeleteOptions{})
360+
361+
_, err = kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Create(ctx, &cosiConfigMap, metav1.CreateOptions{})
362+
if err != nil {
363+
t.Fatalf("Error occurred when creating ConfigMap: %v", err)
364+
}
365+
defer kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Delete(ctx, cosiConfigMap.Name, metav1.DeleteOptions{})
366+
367+
bucketaccessclass, err := util.CreateBucketAccessClass(ctx, client, &goldAccessClass)
368+
if err != nil {
369+
t.Fatalf("Error occurred when creating BucketAccessClass: %v", err)
370+
}
371+
372+
bucketrequest, err := util.CreateBucketRequest(ctx, client, &bucketRequest1)
373+
if err != nil {
374+
t.Fatalf("Error occurred when creating BucketRequest: %v", err)
375+
}
376+
377+
bucketaccessrequest, err := util.CreateBucketAccessRequest(ctx, client, &bucketAccessRequest1)
378+
if err != nil {
379+
t.Fatalf("Error occurred when creating BucketAccessRequest: %v", err)
380+
}
381+
382+
listener.Add(ctx, bucketaccessrequest)
383+
384+
bucketAccessList := util.GetBucketAccesses(ctx, client, 1)
385+
defer util.DeleteObjects(ctx, client, *bucketrequest, *bucketaccessrequest, *bucketaccessclass, bucketAccessList.Items)
386+
387+
if len(bucketAccessList.Items) != 1 {
388+
t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items))
389+
}
390+
bucketaccess := bucketAccessList.Items[0]
391+
392+
bucketaccessrequest2, err := client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest.Namespace).Get(ctx, bucketaccessrequest.Name, metav1.GetOptions{})
393+
if err != nil {
394+
t.Fatalf("Error occurred when updating BucketAccessRequest: %v", err)
395+
}
396+
397+
if !util.ValidateBucketAccess(bucketaccess, *bucketaccessrequest, *bucketaccessclass) {
398+
t.Fatalf("Failed to compare the resulting BucketAccess with the BucketAccessRequest %v and BucketAccessClass %v", bucketaccessrequest, bucketaccessclass)
399+
}
400+
401+
//peform delete and see if the bucketAccessRequest can be deleted
402+
err = client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest2.Namespace).Delete(ctx, bucketaccessrequest2.Name, metav1.DeleteOptions{})
403+
if err != nil {
404+
t.Fatalf("Error occurred when deleting BucketAccessRequest: %v", err)
405+
}
406+
407+
// force update for the finalizer
408+
old := bucketaccessrequest
409+
now := metav1.Now()
410+
bucketaccessrequest2.ObjectMeta.DeletionTimestamp = &now
411+
listener.Update(ctx, old, bucketaccessrequest2)
412+
413+
// there should not be a corresponding BucketAccess
414+
bucketAccessList = util.GetBucketAccesses(ctx, client, 0)
415+
if len(bucketAccessList.Items) > 0 {
416+
t.Fatalf("Expecting BucketAccess object be deleted but found %v", bucketAccessList.Items)
417+
}
418+
}
419+
420+
func runDeleteBucketAccessRequestIdempotency(t *testing.T, name string) {
421+
ctx, cancel := context.WithCancel(context.Background())
422+
defer cancel()
423+
424+
client := bucketclientset.NewSimpleClientset()
425+
kubeClient := fake.NewSimpleClientset()
426+
427+
listener := NewListener()
428+
listener.InitializeKubeClient(kubeClient)
429+
listener.InitializeBucketClient(client)
430+
431+
_, err := kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Create(ctx, &sa1, metav1.CreateOptions{})
432+
if err != nil {
433+
t.Fatalf("Error occurred when creating ServiceAccount: %v", err)
434+
}
435+
defer kubeClient.CoreV1().ServiceAccounts(sa1.Namespace).Delete(ctx, sa1.Name, metav1.DeleteOptions{})
436+
437+
_, err = kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Create(ctx, &cosiConfigMap, metav1.CreateOptions{})
438+
if err != nil {
439+
t.Fatalf("Error occurred when creating ConfigMap: %v", err)
440+
}
441+
defer kubeClient.CoreV1().ConfigMaps(cosiConfigMap.Namespace).Delete(ctx, cosiConfigMap.Name, metav1.DeleteOptions{})
442+
443+
bucketaccessclass, err := util.CreateBucketAccessClass(ctx, client, &goldAccessClass)
444+
if err != nil {
445+
t.Fatalf("Error occurred when creating BucketAccessClass: %v", err)
446+
}
447+
448+
bucketrequest, err := util.CreateBucketRequest(ctx, client, &bucketRequest1)
449+
if err != nil {
450+
t.Fatalf("Error occurred when creating BucketRequest: %v", err)
451+
}
452+
453+
bucketaccessrequest, err := util.CreateBucketAccessRequest(ctx, client, &bucketAccessRequest1)
454+
if err != nil {
455+
t.Fatalf("Error occurred when creating BucketAccessRequest: %v", err)
456+
}
457+
458+
listener.Add(ctx, bucketaccessrequest)
459+
460+
bucketAccessList := util.GetBucketAccesses(ctx, client, 1)
461+
defer util.DeleteObjects(ctx, client, *bucketrequest, *bucketaccessrequest, *bucketaccessclass, bucketAccessList.Items)
462+
463+
if len(bucketAccessList.Items) != 1 {
464+
t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items))
465+
}
466+
bucketaccess := bucketAccessList.Items[0]
467+
468+
bucketaccessrequest2, err := client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest.Namespace).Get(ctx, bucketaccessrequest.Name, metav1.GetOptions{})
469+
if err != nil {
470+
t.Fatalf("Error occurred when updating BucketAccessRequest: %v", err)
471+
}
472+
473+
if !util.ValidateBucketAccess(bucketaccess, *bucketaccessrequest, *bucketaccessclass) {
474+
t.Fatalf("Failed to compare the resulting BucketAccess with the BucketAccessRequest %v and BucketAccessClass %v", bucketaccessrequest, bucketaccessclass)
475+
}
476+
477+
//peform delete and see if the bucketAccessRequest can be deleted
478+
err = client.ObjectstorageV1alpha1().BucketAccessRequests(bucketaccessrequest2.Namespace).Delete(ctx, bucketaccessrequest2.Name, metav1.DeleteOptions{})
479+
if err != nil {
480+
t.Fatalf("Error occurred when deleting BucketAccessRequest: %v", err)
481+
}
482+
483+
// force update for the finalizer
484+
old := bucketaccessrequest
485+
now := metav1.Now()
486+
bucketaccessrequest2.ObjectMeta.DeletionTimestamp = &now
487+
listener.Update(ctx, old, bucketaccessrequest2)
488+
489+
//there should not be a corresponding BucketAccess
490+
bucketAccessList = util.GetBucketAccesses(ctx, client, 0)
491+
if len(bucketAccessList.Items) > 0 {
492+
t.Fatalf("Expecting BucketAccess object be deleted but found %v", bucketAccessList.Items)
493+
}
494+
495+
//Create a duplicate update
496+
listener.Update(ctx, old, bucketaccessrequest2)
497+
//there should not be a corresponding BucketAccess
498+
bucketAccessList = util.GetBucketAccesses(ctx, client, 0)
499+
if len(bucketAccessList.Items) > 0 {
500+
t.Fatalf("Expecting BucketAccess object be deleted but found %v", bucketAccessList.Items)
501+
}
502+
503+
// call the delete directly the second time
504+
listener.Delete(ctx, bucketaccessrequest)
505+
bucketAccessList = util.GetBucketAccesses(ctx, client, 0)
506+
if len(bucketAccessList.Items) != 0 {
507+
t.Fatalf("Expecting a single BucketAccess created but found %v", len(bucketAccessList.Items))
508+
}
509+
}

‎pkg/bucketrequest/bucketrequest.go

+40-1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ func (b *bucketRequestListener) Add(ctx context.Context, obj *v1alpha1.BucketReq
6262
// update processes any updates made to the bucket request
6363
func (b *bucketRequestListener) Update(ctx context.Context, old, new *v1alpha1.BucketRequest) error {
6464
glog.V(1).Infof("Update called for BucketRequest %v", old.Name)
65+
if (old.ObjectMeta.DeletionTimestamp == nil) &&
66+
(new.ObjectMeta.DeletionTimestamp != nil) {
67+
// BucketRequest is being deleted, check and remove finalizer once BA is deleted
68+
return b.removeBucket(ctx, new)
69+
}
6570
return nil
6671
}
6772

@@ -79,7 +84,6 @@ func (b *bucketRequestListener) Delete(ctx context.Context, obj *v1alpha1.Bucket
7984
func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error {
8085
// Most code here is identical to that found in controller.go of kube's controller...
8186
bucketClassName := b.GetBucketClass(bucketRequest)
82-
8387
// A previous doProvisionBucketRequest may just have finished while we were waiting for
8488
// the locks. Check that bucket (with deterministic name) hasn't been provisioned
8589
// yet.
@@ -121,6 +125,10 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont
121125
return err
122126
}
123127

128+
if !util.CheckFinalizer(bucketRequest, util.BRDeleteFinalizer) {
129+
bucketRequest.ObjectMeta.Finalizers = append(bucketRequest.ObjectMeta.Finalizers, util.BRDeleteFinalizer)
130+
}
131+
124132
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
125133
bucketRequest.Spec.BucketInstanceName = bucket.Name
126134
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketRequests(bucketRequest.Namespace).Update(ctx, bucketRequest, metav1.UpdateOptions{})
@@ -136,6 +144,24 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont
136144
return nil
137145
}
138146

147+
// When a BR is deleted before the finalizer is removed then the bucket corresponding to the BR should be deleted.
148+
func (b *bucketRequestListener) removeBucket(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error {
149+
bucket := b.FindBucket(ctx, bucketRequest)
150+
if bucket == nil {
151+
// bucket for this BucketRequest is not found
152+
return util.ErrBucketDoesNotExist
153+
}
154+
155+
// time to delete the Bucket Object
156+
err := b.bucketClient.ObjectstorageV1alpha1().Buckets().Delete(context.Background(), bucket.Name, metav1.DeleteOptions{})
157+
if err != nil {
158+
return err
159+
}
160+
161+
// we can safely remove the finalizer
162+
return b.removeBRDeleteFinalizer(ctx, bucketRequest)
163+
}
164+
139165
// GetBucketClass returns BucketClassName. If no bucket class was in the request it returns empty
140166
// TODO this methods can be more sophisticate to address bucketClass overrides using annotations just like SC.
141167
func (b *bucketRequestListener) GetBucketClass(bucketRequest *v1alpha1.BucketRequest) string {
@@ -166,6 +192,19 @@ func (b *bucketRequestListener) FindBucket(ctx context.Context, br *v1alpha1.Buc
166192
return nil
167193
}
168194

195+
func (b *bucketRequestListener) removeBRDeleteFinalizer(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error {
196+
newFinalizers := []string{}
197+
for _, finalizer := range bucketRequest.ObjectMeta.Finalizers {
198+
if finalizer != util.BRDeleteFinalizer {
199+
newFinalizers = append(newFinalizers, finalizer)
200+
}
201+
}
202+
bucketRequest.ObjectMeta.Finalizers = newFinalizers
203+
204+
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketRequests(bucketRequest.Namespace).Update(ctx, bucketRequest, metav1.UpdateOptions{})
205+
return err
206+
}
207+
169208
// cloneTheBucket clones a bucket to a different namespace when a BR is for brownfield.
170209
func (b *bucketRequestListener) cloneTheBucket(bucketRequest *v1alpha1.BucketRequest) error {
171210
glog.V(1).Infof("Clone called for Bucket %s", bucketRequest.Spec.BucketInstanceName)

0 commit comments

Comments
 (0)