add UT for pkg/util

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>

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)
+}

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)
 		}
 	}
 }

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)
+			}
+		})
+	}
+}

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)
+			}
+		})
+	}
+}

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)
+			}
+		})
+	}
+}