From 217b1dd0662d9f49e8d15b0a7bd599457b1ce05a Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Wed, 7 Jun 2023 18:57:05 +0800 Subject: [PATCH] add UT for pkg/util Signed-off-by: Lyndon-Li --- pkg/test/test_logger.go | 16 ++ pkg/util/csi/volume_snapshot.go | 13 +- pkg/util/csi/volume_snapshot_test.go | 232 +++++++++++++++++++++++++++ pkg/util/kube/pvc_pv_test.go | 68 ++++++++ pkg/util/logging/kopia_log_test.go | 184 ++++++++++++++++++++- 5 files changed, 504 insertions(+), 9 deletions(-) diff --git a/pkg/test/test_logger.go b/pkg/test/test_logger.go index 05fc942cd..d8095a79d 100644 --- a/pkg/test/test_logger.go +++ b/pkg/test/test_logger.go @@ -34,3 +34,19 @@ func NewLoggerWithLevel(level logrus.Level) logrus.FieldLogger { logger.Level = level return logrus.NewEntry(logger) } + +type singleLogRecorder struct { + buffer *string +} + +func (s *singleLogRecorder) Write(p []byte) (n int, err error) { + *s.buffer = string(p[:]) + return len(p), nil +} + +func NewSingleLogger(buffer *string) logrus.FieldLogger { + logger := logrus.New() + logger.Out = &singleLogRecorder{buffer: buffer} + logger.Level = logrus.TraceLevel + return logrus.NewEntry(logger) +} diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go index c6d665e74..b4585e3ed 100644 --- a/pkg/util/csi/volume_snapshot.go +++ b/pkg/util/csi/volume_snapshot.go @@ -112,15 +112,12 @@ func RetainVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interfa // DeleteVolumeSnapshotContentIfAny deletes a VSC by name if it exists, and log an error when the deletion fails func DeleteVolumeSnapshotContentIfAny(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interface, vscName string, log logrus.FieldLogger) { - vsc, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vscName, metav1.GetOptions{}) + err := snapshotClient.VolumeSnapshotContents().Delete(ctx, vscName, metav1.DeleteOptions{}) if err != nil { - if !apierrors.IsNotFound(err) { - log.WithError(err).Warnf("Abort deleting VSC, it doesn't exist %s", vscName) - } - } else { - err = snapshotClient.VolumeSnapshotContents().Delete(ctx, vsc.Name, metav1.DeleteOptions{}) - if err != nil { - log.WithError(err).Warnf("Failed to delete volume snapshot content %s", vsc.Name) + if apierrors.IsNotFound(err) { + log.WithError(err).Debugf("Abort deleting VSC, it doesn't exist %s", vscName) + } else { + log.WithError(err).Errorf("Failed to delete volume snapshot content %s", vscName) } } } diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index 9c34f746a..37f202ef9 100644 --- a/pkg/util/csi/volume_snapshot_test.go +++ b/pkg/util/csi/volume_snapshot_test.go @@ -31,6 +31,8 @@ import ( clientTesting "k8s.io/client-go/testing" "github.com/vmware-tanzu/velero/pkg/util/boolptr" + + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) type reactor struct { @@ -282,6 +284,12 @@ func TestEnsureDeleteVS(t *testing.T) { }, err: "error to assure VolumeSnapshot is deleted, fake-vs: error to get VolumeSnapshot fake-vs: fake-get-error", }, + { + name: "success", + vsName: "fake-vs", + namespace: "fake-ns", + clientObj: []runtime.Object{vsObj}, + }, } for _, test := range tests { @@ -336,6 +344,11 @@ func TestEnsureDeleteVSC(t *testing.T) { }, err: "error to assure VolumeSnapshotContent is deleted, fake-vsc: error to get VolumeSnapshotContent fake-vsc: fake-get-error", }, + { + name: "success", + vscName: "fake-vsc", + clientObj: []runtime.Object{vscObj}, + }, } for _, test := range tests { @@ -355,3 +368,222 @@ func TestEnsureDeleteVSC(t *testing.T) { }) } } + +func TestDeleteVolumeSnapshotContentIfAny(t *testing.T) { + tests := []struct { + name string + clientObj []runtime.Object + reactors []reactor + vscName string + logMessage string + logLevel string + logError string + }{ + { + name: "vsc not exist", + vscName: "fake-vsc", + logMessage: "Abort deleting VSC, it doesn't exist fake-vsc", + logLevel: "level=debug", + }, + { + name: "deleete fail", + vscName: "fake-vsc", + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshotcontents", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-delete-error") + }, + }, + }, + logMessage: "Failed to delete volume snapshot content fake-vsc", + logLevel: "level=error", + logError: "error=fake-delete-error", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeSnapshotClient := snapshotFake.NewSimpleClientset(test.clientObj...) + + for _, reactor := range test.reactors { + fakeSnapshotClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc) + } + + logMessage := "" + + DeleteVolumeSnapshotContentIfAny(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vscName, velerotest.NewSingleLogger(&logMessage)) + + if len(test.logMessage) > 0 { + assert.Contains(t, logMessage, test.logMessage) + } + + if len(test.logLevel) > 0 { + assert.Contains(t, logMessage, test.logLevel) + } + + if len(test.logError) > 0 { + assert.Contains(t, logMessage, test.logError) + } + }) + } +} + +func TestDeleteVolumeSnapshotIfAny(t *testing.T) { + tests := []struct { + name string + clientObj []runtime.Object + reactors []reactor + vsName string + vsNamespace string + logMessage string + logLevel string + logError string + }{ + { + name: "vs not exist", + vsName: "fake-vs", + vsNamespace: "fake-ns", + logMessage: "Abort deleting volume snapshot, it doesn't exist fake-ns/fake-vs", + logLevel: "level=debug", + }, + { + name: "delete fail", + vsName: "fake-vs", + vsNamespace: "fake-ns", + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshots", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-delete-error") + }, + }, + }, + logMessage: "Failed to delete volume snapshot fake-ns/fake-vs", + logLevel: "level=error", + logError: "error=fake-delete-error", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeSnapshotClient := snapshotFake.NewSimpleClientset(test.clientObj...) + + for _, reactor := range test.reactors { + fakeSnapshotClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc) + } + + logMessage := "" + + DeleteVolumeSnapshotIfAny(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vsName, test.vsNamespace, velerotest.NewSingleLogger(&logMessage)) + + if len(test.logMessage) > 0 { + assert.Contains(t, logMessage, test.logMessage) + } + + if len(test.logLevel) > 0 { + assert.Contains(t, logMessage, test.logLevel) + } + + if len(test.logError) > 0 { + assert.Contains(t, logMessage, test.logError) + } + }) + } +} + +func TestRetainVSC(t *testing.T) { + vscObj := &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vsc", + }, + } + + tests := []struct { + name string + clientObj []runtime.Object + reactors []reactor + vsc *snapshotv1api.VolumeSnapshotContent + updated *snapshotv1api.VolumeSnapshotContent + err string + }{ + { + name: "already retained", + vsc: &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vsc", + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + }, + }, + }, + { + name: "path vsc fail", + vsc: &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vsc", + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1api.VolumeSnapshotContentDelete, + }, + }, + reactors: []reactor{ + { + verb: "patch", + resource: "volumesnapshotcontents", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-patch-error") + }, + }, + }, + err: "error patching VSC: fake-patch-error", + }, + { + name: "success", + vsc: &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vsc", + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1api.VolumeSnapshotContentDelete, + }, + }, + clientObj: []runtime.Object{vscObj}, + updated: &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-vsc", + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeSnapshotClient := snapshotFake.NewSimpleClientset(test.clientObj...) + + for _, reactor := range test.reactors { + fakeSnapshotClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc) + } + + returned, err := RetainVSC(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vsc) + + if len(test.err) == 0 { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, test.err) + } + + if test.updated != nil { + assert.Equal(t, *test.updated, *returned) + } else { + assert.Nil(t, returned) + } + }) + } +} diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index 4713e2e3a..9c71fed4a 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -30,6 +31,8 @@ import ( corev1api "k8s.io/api/core/v1" clientTesting "k8s.io/client-go/testing" + + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) type reactor struct { @@ -129,3 +132,68 @@ func TestWaitPVCBound(t *testing.T) { }) } } + +func TestDeletePVCIfAny(t *testing.T) { + tests := []struct { + name string + pvcName string + pvcNamespace string + kubeClientObj []runtime.Object + kubeReactors []reactor + logMessage string + logLevel string + logError string + }{ + { + name: "get fail", + pvcName: "fake-pvc", + pvcNamespace: "fake-namespace", + logMessage: "Abort deleting PVC, it doesn't exist, fake-namespace/fake-pvc", + logLevel: "level=debug", + }, + { + name: "delete fail", + pvcName: "fake-pvc", + pvcNamespace: "fake-namespace", + kubeReactors: []reactor{ + { + verb: "delete", + resource: "persistentvolumeclaims", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("fake-delete-error") + }, + }, + }, + logMessage: "Failed to delete pvc fake-namespace/fake-pvc", + logLevel: "level=error", + logError: "error=fake-delete-error", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...) + + for _, reactor := range test.kubeReactors { + fakeKubeClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc) + } + + var kubeClient kubernetes.Interface = fakeKubeClient + + logMessage := "" + DeletePVCIfAny(context.Background(), kubeClient.CoreV1(), test.pvcName, test.pvcNamespace, velerotest.NewSingleLogger(&logMessage)) + + if len(test.logMessage) > 0 { + assert.Contains(t, logMessage, test.logMessage) + } + + if len(test.logLevel) > 0 { + assert.Contains(t, logMessage, test.logLevel) + } + + if len(test.logError) > 0 { + assert.Contains(t, logMessage, test.logError) + } + }) + } +} diff --git a/pkg/util/logging/kopia_log_test.go b/pkg/util/logging/kopia_log_test.go index 1180988b8..848638c6d 100644 --- a/pkg/util/logging/kopia_log_test.go +++ b/pkg/util/logging/kopia_log_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap/zapcore" @@ -57,6 +58,12 @@ func TestEnabled(t *testing.T) { zapLevel: zapcore.InfoLevel, expected: true, }, + { + name: "check warn again warn", + level: logrus.WarnLevel, + zapLevel: zapcore.WarnLevel, + expected: true, + }, { name: "check info again error", level: logrus.ErrorLevel, @@ -69,12 +76,24 @@ func TestEnabled(t *testing.T) { zapLevel: zapcore.ErrorLevel, expected: true, }, + { + name: "check dppanic again panic", + level: logrus.PanicLevel, + zapLevel: zapcore.DPanicLevel, + expected: true, + }, { name: "check panic again error", level: logrus.ErrorLevel, zapLevel: zapcore.PanicLevel, expected: true, }, + { + name: "check fatal again fatal", + level: logrus.FatalLevel, + zapLevel: zapcore.FatalLevel, + expected: true, + }, } for _, tc := range testCases { @@ -89,7 +108,7 @@ func TestEnabled(t *testing.T) { } } -func TestWrite(t *testing.T) { +func TestLogrusFieldsForWrite(t *testing.T) { testCases := []struct { name string module string @@ -151,6 +170,65 @@ func TestWrite(t *testing.T) { "logger name": "logger-name-01", }, }, + { + name: "info with function name", + module: "module-05", + zapEntry: zapcore.Entry{ + Level: zapcore.InfoLevel, + Caller: zapcore.EntryCaller{ + Function: "function-name-01", + }, + }, + zapFields: nil, + expected: logrus.Fields{ + "logModule": "kopia/module-05", + "function": "function-name-01", + }, + }, + { + name: "info with undefined path", + module: "module-06", + zapEntry: zapcore.Entry{ + Level: zapcore.InfoLevel, + Caller: zapcore.EntryCaller{ + Defined: false, + }, + }, + zapFields: nil, + expected: logrus.Fields{ + "logModule": "kopia/module-06", + }, + }, + { + name: "info with defined path", + module: "module-06", + zapEntry: zapcore.Entry{ + Level: zapcore.InfoLevel, + Caller: zapcore.EntryCaller{ + Defined: true, + File: "file-name-01", + Line: 100, + }, + }, + zapFields: nil, + expected: logrus.Fields{ + "logModule": "kopia/module-06", + "path": "file-name-01:100", + }, + }, + { + name: "info with stack", + module: "module-07", + zapEntry: zapcore.Entry{ + Level: zapcore.InfoLevel, + Stack: "fake-stack", + }, + zapFields: nil, + expected: logrus.Fields{ + "logModule": "kopia/module-07", + "stack": "fake-stack", + }, + }, } for _, tc := range testCases { @@ -165,3 +243,107 @@ func TestWrite(t *testing.T) { }) } } + +func TestWrite(t *testing.T) { + testCases := []struct { + name string + ent zapcore.Entry + logMessage string + logLevel string + shouldPanic bool + }{ + { + name: "write debug", + ent: zapcore.Entry{ + Level: zapcore.DebugLevel, + Message: "fake-message", + }, + logMessage: "fake-message", + logLevel: "level=debug", + }, + { + name: "write info", + ent: zapcore.Entry{ + Level: zapcore.InfoLevel, + Message: "fake-message", + }, + logMessage: "fake-message", + logLevel: "level=info", + }, + { + name: "write warn", + ent: zapcore.Entry{ + Level: zapcore.WarnLevel, + Message: "fake-message", + }, + logMessage: "fake-message", + logLevel: "level=warn", + }, + { + name: "write error", + ent: zapcore.Entry{ + Level: zapcore.ErrorLevel, + Message: "fake-message", + }, + logMessage: "fake-message", + logLevel: "level=warn", + }, + { + name: "write DPanic", + ent: zapcore.Entry{ + Level: zapcore.DPanicLevel, + Message: "fake-message", + }, + logMessage: "fake-message", + logLevel: "level=panic", + shouldPanic: true, + }, + { + name: "write panic", + ent: zapcore.Entry{ + Level: zapcore.PanicLevel, + Message: "fake-message", + }, + logMessage: "fake-message", + logLevel: "level=panic", + shouldPanic: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + logMessage := "" + + log := kopiaLog{ + logger: test.NewSingleLogger(&logMessage), + } + + if tc.shouldPanic { + defer func() { + r := recover() + assert.NotNil(t, r) + + if len(tc.logMessage) > 0 { + assert.Contains(t, logMessage, tc.logMessage) + } + + if len(tc.logLevel) > 0 { + assert.Contains(t, logMessage, tc.logLevel) + } + }() + } + + err := log.Write(tc.ent, nil) + + assert.NoError(t, err) + + if len(tc.logMessage) > 0 { + assert.Contains(t, logMessage, tc.logMessage) + } + + if len(tc.logLevel) > 0 { + assert.Contains(t, logMessage, tc.logLevel) + } + }) + } +}