diff --git a/pkg/driver/hook/prestop.go b/pkg/driver/hook/prestop.go index b8f3d8d..6ba041e 100644 --- a/pkg/driver/hook/prestop.go +++ b/pkg/driver/hook/prestop.go @@ -86,25 +86,23 @@ func (h *hook) waitForVolumeAttachmentsCleanup(ctx context.Context) error { _, err := informer.AddEventHandler(cache.ResourceEventHandlerFuncs{ DeleteFunc: func(obj interface{}) { log.Warn().Msg("Received an object in DeleteFunc") - if err := h.volumeAttachmentEventHandler(ctx, obj); err != nil { + if err := h.volumeAttachmentEventHandler(ctx, obj, stopInformerFn); err != nil { log.Error(). Str("node_name", h.nodeName). Err(err). Msg("An error occurred while handling the VolumeAttachment event in DeleteFunc") return } - stopInformerFn() }, UpdateFunc: func(_, obj interface{}) { log.Warn().Msg("Received an object in UpdateFunc") - if err := h.volumeAttachmentEventHandler(ctx, obj); err != nil { + if err := h.volumeAttachmentEventHandler(ctx, obj, stopInformerFn); err != nil { log.Error(). Str("node_name", h.nodeName). Err(err). Msg("An error occurred while handling the VolumeAttachment event in UpdateFunc") return } - stopInformerFn() }, }) if err != nil { @@ -134,6 +132,8 @@ func (h *hook) waitForVolumeAttachmentsCleanup(ctx context.Context) error { for { select { case <-informerCh: + log.Info(). + Msg("Stopped informer to check event of VolumeAttachment resource") return nil case <-tick.C: log.Info(). @@ -141,13 +141,13 @@ func (h *hook) waitForVolumeAttachmentsCleanup(ctx context.Context) error { case <-ctx.Done(): log.Error(). Err(ctx.Err()). - Msg("Stopped waiting for VolumeAttachments, therefore some resources might still remain") + Msg("Stopped waiting for VolumeAttachments cleanup, therefore some resources might still remain") return nil } } } -func (h *hook) volumeAttachmentEventHandler(ctx context.Context, obj interface{}) error { +func (h *hook) volumeAttachmentEventHandler(ctx context.Context, obj interface{}, stopEventFn func()) error { va, ok := obj.(*storagev1.VolumeAttachment) if !ok { return errors.New("received an object that is not a VolumeAttachment") @@ -156,6 +156,7 @@ func (h *hook) volumeAttachmentEventHandler(ctx context.Context, obj interface{} if _, err := h.checkVolumeAttachmentsExist(ctx); err != nil { return err } + stopEventFn() } return nil } diff --git a/pkg/driver/hook/prestop_test.go b/pkg/driver/hook/prestop_test.go index d2be141..2c97db3 100644 --- a/pkg/driver/hook/prestop_test.go +++ b/pkg/driver/hook/prestop_test.go @@ -3,7 +3,9 @@ package hook import ( "context" "errors" + "sync/atomic" "testing" + "time" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" @@ -12,7 +14,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" ) @@ -23,9 +24,10 @@ func TestPreStop(t *testing.T) { opts []Option } type test struct { - name string - args args - wantErr bool + name string + args args + beforeFunc func(*hook) + wantErr bool } tests := []test{ @@ -35,21 +37,22 @@ func TestPreStop(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-01", - }, - Spec: v1.NodeSpec{}, - } - client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, node, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-01", + }, + Spec: v1.NodeSpec{}, + } + client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, node, nil + }) + }, }, { name: "Returns nil when drained node exists but no volume attachments are found", @@ -57,33 +60,34 @@ func TestPreStop(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-01", - }, - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - { - Key: v1.TaintNodeUnschedulable, - }, - }, + WithKubernetesClient(fake.NewSimpleClientset()), + }, + }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-01", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: v1.TaintNodeUnschedulable, }, - } - client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, node, nil - }) + }, + }, + } + client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, node, nil + }) - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{}, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), - }, + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{}, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) }, }, { @@ -92,25 +96,26 @@ func TestPreStop(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, k8serrors.NewNotFound(schema.GroupResource{ - Group: "", - Resource: "nodes", - }, "node-01") - }) - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{}, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, k8serrors.NewNotFound(schema.GroupResource{ + Group: "", + Resource: "nodes", + }, "node-01") + }) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{}, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, }, { name: "Returns error when node cannot be retrieved due to an internal error", @@ -118,15 +123,15 @@ func TestPreStop(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, errors.New("error") - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("error") + }) + }, wantErr: true, }, { @@ -135,30 +140,31 @@ func TestPreStop(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-01", - }, - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - { - Key: v1.TaintNodeUnschedulable, - }, - }, - }, - } - client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, node, nil - }) - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, errors.New("error") - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-01", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: v1.TaintNodeUnschedulable, + }, + }, + }, + } + client.Fake.PrependReactor("get", "nodes", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, node, nil + }) + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("error") + }) + }, wantErr: true, }, } @@ -170,7 +176,15 @@ func TestPreStop(t *testing.T) { tt.Fatal(err) } - err = h.PreStop(test.args.ctx) + ctx, cancel := context.WithCancel(test.args.ctx) + defer cancel() + + hook := h.(*hook) + if test.beforeFunc != nil { + test.beforeFunc(hook) + } + + err = h.PreStop(ctx) if test.wantErr { assert.NotNil(tt, err) } else { @@ -231,9 +245,10 @@ func TestWaitForVolumeAttachmentsCleanup(t *testing.T) { opts []Option } type test struct { - name string - args args - wantErr bool + name string + args args + beforeFunc func(*hook) + wantErr bool } tests := []test{ @@ -243,28 +258,28 @@ func TestWaitForVolumeAttachmentsCleanup(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-attachment-01", - }, - Spec: storagev1.VolumeAttachmentSpec{ - NodeName: "node-02", - }, - }, - }, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-02", + }, + }, + }, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, }, { name: "Returns nil when the context is canceled", @@ -276,44 +291,161 @@ func TestWaitForVolumeAttachmentsCleanup(t *testing.T) { }(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-attachment-01", - }, - Spec: storagev1.VolumeAttachmentSpec{ - NodeName: "node-01", - }, - }, - }, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-01", + }, + }, + }, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, }, + func() test { + ctx := context.Background() + volumeattachment := &storagev1.VolumeAttachment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-01", + }, + Status: storagev1.VolumeAttachmentStatus{ + Attached: true, + }, + } + var volumeAttachmentDeleted int64 + + return test{ + name: "Returns nil after volume attachment is cleaned up", + args: args{ + ctx: ctx, + opts: []Option{ + WithNodeName("node-01"), + WithKubernetesClient(fake.NewSimpleClientset()), + }, + }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + client.StorageV1().VolumeAttachments().Create(ctx, volumeattachment, metav1.CreateOptions{}) + + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + if atomic.LoadInt64(&volumeAttachmentDeleted) == 1 { + return true, &storagev1.VolumeAttachmentList{}, nil + } + return true, &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{*volumeattachment}, + }, nil + }) + + // To trigger the event after registering it to the informer. + go func() { + time.Sleep(time.Second) + + newObj := volumeattachment.DeepCopy() + newObj.Status.Attached = false + h.client.StorageV1().VolumeAttachments().Update(ctx, newObj, metav1.UpdateOptions{}) + time.Sleep(100 * time.Millisecond) + + // Since calling delete triggers the event handler, we change the value of volumeAttachmentDeleted before that. This allows us to dynamically modify the value. + atomic.StoreInt64(&volumeAttachmentDeleted, 1) + h.client.StorageV1().VolumeAttachments().Delete(ctx, newObj.GetName(), metav1.DeleteOptions{}) + time.Sleep(100 * time.Millisecond) + }() + }, + } + }(), + + func() test { + ctx := context.Background() + volumeattachments := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-01", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-02", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-01", + }, + }, + }, + } + var volumeAttachmentDeleted int64 + + return test{ + name: "Returns nil after successful deletion of multiple volume attachments", + args: args{ + ctx: ctx, + opts: []Option{ + WithNodeName("node-01"), + WithKubernetesClient(fake.NewSimpleClientset()), + }, + }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + for _, item := range volumeattachments.Items { + client.StorageV1().VolumeAttachments().Create(ctx, &item, metav1.CreateOptions{}) + } + + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + if atomic.LoadInt64(&volumeAttachmentDeleted) == 1 { + return true, &storagev1.VolumeAttachmentList{}, nil + } + return true, volumeattachments, nil + }) + + // To trigger the event after registering it to the informer. + go func() { + time.Sleep(time.Second) + + client.StorageV1().VolumeAttachments().Delete(ctx, "test-volume-attachment-01", metav1.DeleteOptions{}) + time.Sleep(100 * time.Millisecond) + + // Since calling delete triggers the event handler, we change the value of volumeAttachmentDeleted before that. This allows us to dynamically modify the value. + atomic.StoreInt64(&volumeAttachmentDeleted, 1) + client.StorageV1().VolumeAttachments().Delete(ctx, "test-volume-attachment-02", metav1.DeleteOptions{}) + time.Sleep(100 * time.Millisecond) + }() + }, + } + }(), { name: "Returns error when listing volume attachments fails in checkVolumeAttachmentsExist method", args: args{ ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, errors.New("error") - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("error") + }) + }, wantErr: true, }, } @@ -325,7 +457,15 @@ func TestWaitForVolumeAttachmentsCleanup(t *testing.T) { tt.Fatal(err) } - err = h.(*hook).waitForVolumeAttachmentsCleanup(test.args.ctx) + ctx, cancel := context.WithCancel(test.args.ctx) + defer cancel() + + hook := h.(*hook) + if test.beforeFunc != nil { + test.beforeFunc(hook) + } + + err = hook.waitForVolumeAttachmentsCleanup(ctx) if test.wantErr { assert.NotNil(tt, err) } else { @@ -337,14 +477,16 @@ func TestWaitForVolumeAttachmentsCleanup(t *testing.T) { func TestVolumeAttachmentEventHandler(t *testing.T) { type args struct { - ctx context.Context - obj interface{} - opts []Option + ctx context.Context + obj interface{} + stopEventFn func() + opts []Option } type test struct { - name string - args args - wantErr bool + name string + args args + beforeFunc func(*hook) + wantErr bool } tests := []test{ @@ -357,30 +499,31 @@ func TestVolumeAttachmentEventHandler(t *testing.T) { NodeName: "node-02", }, }, + stopEventFn: func() {}, opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-attachment-01", - }, - Spec: storagev1.VolumeAttachmentSpec{ - NodeName: "node-02", - }, - }, - }, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-02", + }, + }, + }, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, }, { name: "Returns nil when input object is VolumeAttachment but resource is not found (already deleted)", @@ -391,12 +534,10 @@ func TestVolumeAttachmentEventHandler(t *testing.T) { NodeName: "node-01", }, }, + stopEventFn: func() {}, opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, }, @@ -409,43 +550,42 @@ func TestVolumeAttachmentEventHandler(t *testing.T) { NodeName: "node-01", }, }, + stopEventFn: func() {}, opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-attachment-01", - }, - Spec: storagev1.VolumeAttachmentSpec{ - NodeName: "node-01", - }, - }, - }, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-01", + }, + }, + }, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, wantErr: true, }, { name: "Returns error when invalid object is provided", args: args{ - ctx: context.Background(), - obj: "invalid-object", + ctx: context.Background(), + obj: "invalid-object", + stopEventFn: func() {}, opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, wantErr: true, @@ -459,7 +599,15 @@ func TestVolumeAttachmentEventHandler(t *testing.T) { tt.Fatal(err) } - err = h.(*hook).volumeAttachmentEventHandler(test.args.ctx, test.args.obj) + ctx, cancel := context.WithCancel(test.args.ctx) + defer cancel() + + hook := h.(*hook) + if test.beforeFunc != nil { + test.beforeFunc(hook) + } + + err = hook.volumeAttachmentEventHandler(ctx, test.args.obj, test.args.stopEventFn) if test.wantErr { assert.NotNil(tt, err) } else { @@ -475,10 +623,11 @@ func TestCheckVolumeAttachmentsExist(t *testing.T) { opts []Option } type test struct { - name string - args args - want bool - wantErr bool + name string + args args + beforeFunc func(*hook) + want bool + wantErr bool } tests := []test{ @@ -488,10 +637,7 @@ func TestCheckVolumeAttachmentsExist(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, }, @@ -501,28 +647,28 @@ func TestCheckVolumeAttachmentsExist(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-attachment-01", - }, - Spec: storagev1.VolumeAttachmentSpec{ - NodeName: "node-02", - }, - }, - }, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-02", + }, + }, + }, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, }, { name: "Returns false and error when listing volume attachments fails", @@ -530,15 +676,16 @@ func TestCheckVolumeAttachmentsExist(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, errors.New("error") - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("error") + }) + }, wantErr: true, }, { @@ -547,28 +694,28 @@ func TestCheckVolumeAttachmentsExist(t *testing.T) { ctx: context.Background(), opts: []Option{ WithNodeName("node-01"), - WithKubernetesClient(func() kubernetes.Interface { - client := fake.NewSimpleClientset() - - list := &storagev1.VolumeAttachmentList{ - Items: []storagev1.VolumeAttachment{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-attachment-01", - }, - Spec: storagev1.VolumeAttachmentSpec{ - NodeName: "node-01", - }, - }, - }, - } - client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, list, nil - }) - return client - }()), + WithKubernetesClient(fake.NewSimpleClientset()), }, }, + beforeFunc: func(h *hook) { + client := h.client.(*fake.Clientset) + + list := &storagev1.VolumeAttachmentList{ + Items: []storagev1.VolumeAttachment{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-volume-attachment-01", + }, + Spec: storagev1.VolumeAttachmentSpec{ + NodeName: "node-01", + }, + }, + }, + } + client.Fake.PrependReactor("list", "volumeattachments", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, list, nil + }) + }, want: true, wantErr: true, }, @@ -581,7 +728,15 @@ func TestCheckVolumeAttachmentsExist(t *testing.T) { tt.Fatal(err) } - got, err := h.(*hook).checkVolumeAttachmentsExist(test.args.ctx) + ctx, cancel := context.WithCancel(test.args.ctx) + defer cancel() + + hook := h.(*hook) + if test.beforeFunc != nil { + test.beforeFunc(hook) + } + + got, err := hook.checkVolumeAttachmentsExist(ctx) if test.wantErr { assert.NotNil(tt, err) } else {