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

Commit 7b00219

Browse files
brahmaroutubrahmaroutu
committed
Adding logic to process BR and BAR deletes.
1 parent 89e8139 commit 7b00219

File tree

5 files changed

+439
-15
lines changed

5 files changed

+439
-15
lines changed

‎pkg/bucketaccessrequest/bucketaccessrequest.go

+58-13
Original file line numberDiff line numberDiff line change
@@ -34,34 +34,45 @@ func (b *bucketAccessRequestListener) InitializeBucketClient(bc bucketclientset.
3434
b.bucketClient = bc
3535
}
3636

37+
// Add is in response to user adding a BucketAccessRequest. The call here will respond by creating a BucketAccess Object.
3738
func (b *bucketAccessRequestListener) Add(ctx context.Context, obj *v1alpha1.BucketAccessRequest) error {
38-
glog.V(1).Infof("Add called for BucketAccessRequest %s", obj.Name)
39+
glog.V(1).Infof("Add called for BucketAccessRequest %v/%v", obj.Namespace, obj.Name)
3940
bucketAccessRequest := obj
4041

4142
err := b.provisionBucketAccess(ctx, bucketAccessRequest)
4243
if err != nil {
4344
// Provisioning is 100% finished / not in progress.
4445
switch err {
4546
case util.ErrBucketAccessAlreadyExists:
46-
glog.V(1).Infof("BucketAccess already exist for this BucketAccessRequest %v.", bucketAccessRequest.Name)
47+
glog.V(1).Infof("BucketAccess already exist for this BucketAccessRequest %v/%v.", bucketAccessRequest.Namespace, bucketAccessRequest.Name)
4748
err = nil
4849
default:
49-
glog.V(1).Infof("Error occurred processing BucketAccessRequest %v: %v", bucketAccessRequest.Name, err)
50+
glog.V(1).Infof("Error occurred processing BucketAccessRequest %v/%v: %v", bucketAccessRequest.Namespace, bucketAccessRequest.Name, err)
5051
}
5152
return err
5253
}
5354

54-
glog.V(1).Infof("BucketAccessRequest %v is successfully processed.", bucketAccessRequest.Name)
55+
glog.V(1).Infof("BucketAccessRequest %v/%v is successfully processed.", bucketAccessRequest.Namespace, bucketAccessRequest.Name)
5556
return nil
5657
}
5758

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

63-
func (b *bucketAccessRequestListener) Delete(ctx context.Context, obj *v1alpha1.BucketAccessRequest) error {
64-
glog.V(1).Infof("Delete called for BucketAccessRequest %v", obj.Name)
72+
// Delete is in response to user deleting a BucketAccessRequest. The call here will respond by deleting a BucketAccess Object.
73+
func (b *bucketAccessRequestListener) Delete(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error {
74+
glog.V(1).Infof("Delete called for BucketAccessRequest %v/%v", bucketAccessRequest.Namespace, bucketAccessRequest.Name)
75+
6576
return nil
6677
}
6778

@@ -137,10 +148,10 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context,
137148
}
138149
// bucketaccess.Spec.MintedSecretName - set by the driver
139150
bucketaccess.Spec.PolicyActionsConfigMapData, err = util.ReadConfigData(b.kubeClient, bucketAccessClass.PolicyActionsConfigMap)
140-
if err != nil {
151+
if err != nil && err != util.ErrNilConfigMap {
141152
return err
142153
}
143-
// bucketaccess.Spec.Principal - set by the driver
154+
// bucketaccess.Spec.Principal - set by the provisioner
144155
bucketaccess.Spec.Provisioner = bucketAccessClass.Provisioner
145156
bucketaccess.Spec.Parameters = util.CopySS(bucketAccessClass.Parameters)
146157

@@ -152,6 +163,10 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context,
152163
return err
153164
}
154165

166+
if !util.CheckFinalizer(bucketAccessRequest, util.BARDeleteFinalizer) {
167+
bucketAccessRequest.ObjectMeta.Finalizers = append(bucketAccessRequest.ObjectMeta.Finalizers, util.BARDeleteFinalizer)
168+
}
169+
155170
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
156171
bucketAccessRequest.Spec.BucketAccessName = bucketaccess.Name
157172
_, err := barClient(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{})
@@ -167,17 +182,47 @@ func (b *bucketAccessRequestListener) provisionBucketAccess(ctx context.Context,
167182
return nil
168183
}
169184

170-
func (b *bucketAccessRequestListener) FindBucketAccess(ctx context.Context, bar *v1alpha1.BucketAccessRequest) *v1alpha1.BucketAccess {
185+
func (b *bucketAccessRequestListener) removeBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error {
186+
bucketaccess := b.FindBucketAccess(ctx, bucketAccessRequest)
187+
if bucketaccess == nil {
188+
// bucketaccess for this BucketAccessRequest is not found
189+
return util.ErrBucketAccessDoesNotExist
190+
}
191+
192+
// time to delete the BucketAccess Object
193+
err := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().Delete(context.Background(), bucketaccess.Name, metav1.DeleteOptions{})
194+
if err != nil {
195+
return err
196+
}
197+
198+
// we can safely remove the finalizer
199+
return b.removeBARDeleteFinalizer(ctx, bucketAccessRequest)
200+
}
201+
202+
func (b *bucketAccessRequestListener) FindBucketAccess(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) *v1alpha1.BucketAccess {
171203
bucketAccessList, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccesses().List(ctx, metav1.ListOptions{})
172204
if err != nil || len(bucketAccessList.Items) <= 0 {
173205
return nil
174206
}
175207
for _, bucketaccess := range bucketAccessList.Items {
176-
if bucketaccess.Spec.BucketAccessRequest.Name == bar.Name &&
177-
bucketaccess.Spec.BucketAccessRequest.Namespace == bar.Namespace &&
178-
bucketaccess.Spec.BucketAccessRequest.UID == bar.UID {
208+
if bucketaccess.Spec.BucketAccessRequest.Name == bucketAccessRequest.Name &&
209+
bucketaccess.Spec.BucketAccessRequest.Namespace == bucketAccessRequest.Namespace &&
210+
bucketaccess.Spec.BucketAccessRequest.UID == bucketAccessRequest.UID {
179211
return &bucketaccess
180212
}
181213
}
182214
return nil
183215
}
216+
217+
func (b *bucketAccessRequestListener) removeBARDeleteFinalizer(ctx context.Context, bucketAccessRequest *v1alpha1.BucketAccessRequest) error {
218+
newFinalizers := []string{}
219+
for _, finalizer := range bucketAccessRequest.ObjectMeta.Finalizers {
220+
if finalizer != util.BARDeleteFinalizer {
221+
newFinalizers = append(newFinalizers, finalizer)
222+
}
223+
}
224+
bucketAccessRequest.ObjectMeta.Finalizers = newFinalizers
225+
226+
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketAccessRequests(bucketAccessRequest.Namespace).Update(ctx, bucketAccessRequest, metav1.UpdateOptions{})
227+
return err
228+
}

‎pkg/bucketaccessrequest/bucketaccessrequest_test.go

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

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

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

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

‎pkg/bucketrequest/bucketrequest.go

+40
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(3).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

@@ -128,6 +133,10 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont
128133
return err
129134
}
130135

136+
if !util.CheckFinalizer(bucketRequest, util.BRDeleteFinalizer) {
137+
bucketRequest.ObjectMeta.Finalizers = append(bucketRequest.ObjectMeta.Finalizers, util.BRDeleteFinalizer)
138+
}
139+
131140
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
132141
bucketRequest.Spec.BucketInstanceName = bucket.Name
133142
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketRequests(bucketRequest.Namespace).Update(ctx, bucketRequest, metav1.UpdateOptions{})
@@ -143,6 +152,24 @@ func (b *bucketRequestListener) provisionBucketRequestOperation(ctx context.Cont
143152
return nil
144153
}
145154

155+
// When a BR is deleted before the finalizer is removed then the bucket corresponding to the BR should be deleted.
156+
func (b *bucketRequestListener) removeBucket(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error {
157+
bucket := b.FindBucket(ctx, bucketRequest)
158+
if bucket == nil {
159+
// bucket for this BucketRequest is not found
160+
return util.ErrBucketDoesNotExist
161+
}
162+
163+
// time to delete the Bucket Object
164+
err := b.bucketClient.ObjectstorageV1alpha1().Buckets().Delete(context.Background(), bucket.Name, metav1.DeleteOptions{})
165+
if err != nil {
166+
return err
167+
}
168+
169+
// we can safely remove the finalizer
170+
return b.removeBRDeleteFinalizer(ctx, bucketRequest)
171+
}
172+
146173
// GetBucketClass returns BucketClassName. If no bucket class was in the request it returns empty
147174
// TODO this methods can be more sophisticate to address bucketClass overrides using annotations just like SC.
148175
func (b *bucketRequestListener) GetBucketClass(bucketRequest *v1alpha1.BucketRequest) string {
@@ -173,6 +200,19 @@ func (b *bucketRequestListener) FindBucket(ctx context.Context, br *v1alpha1.Buc
173200
return nil
174201
}
175202

203+
func (b *bucketRequestListener) removeBRDeleteFinalizer(ctx context.Context, bucketRequest *v1alpha1.BucketRequest) error {
204+
newFinalizers := []string{}
205+
for _, finalizer := range bucketRequest.ObjectMeta.Finalizers {
206+
if finalizer != util.BRDeleteFinalizer {
207+
newFinalizers = append(newFinalizers, finalizer)
208+
}
209+
}
210+
bucketRequest.ObjectMeta.Finalizers = newFinalizers
211+
212+
_, err := b.bucketClient.ObjectstorageV1alpha1().BucketRequests(bucketRequest.Namespace).Update(ctx, bucketRequest, metav1.UpdateOptions{})
213+
return err
214+
}
215+
176216
// cloneTheBucket clones a bucket to a different namespace when a BR is for brownfield.
177217
func (b *bucketRequestListener) cloneTheBucket(bucketRequest *v1alpha1.BucketRequest) error {
178218
glog.V(1).Infof("Clone called for Bucket %s", bucketRequest.Spec.BucketInstanceName)

0 commit comments

Comments
 (0)