From bb4f9094fd92fc93d2feb36709fd2aed07273571 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Tue, 31 Oct 2023 12:08:48 +0800 Subject: [PATCH] issue 7027: backup exposer -- don't assume first volume as the backup volume Signed-off-by: Lyndon-Li --- changelogs/unreleased/7060-Lyndon-Li | 1 + pkg/exposer/csi_snapshot.go | 16 ++- pkg/exposer/csi_snapshot_test.go | 179 +++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/7060-Lyndon-Li diff --git a/changelogs/unreleased/7060-Lyndon-Li b/changelogs/unreleased/7060-Lyndon-Li new file mode 100644 index 0000000000..f2a3a9ae21 --- /dev/null +++ b/changelogs/unreleased/7060-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #7027, data mover backup exposer should not assume the first volume as the backup volume in backup pod \ No newline at end of file diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 85f511524c..8b501e6488 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -190,6 +190,7 @@ func (e *csiSnapshotExposer) GetExposed(ctx context.Context, ownerObject corev1. backupPodName := ownerObject.Name backupPVCName := ownerObject.Name + volumeName := string(ownerObject.UID) curLog := e.log.WithFields(logrus.Fields{ "owner": ownerObject.Name, @@ -218,7 +219,20 @@ func (e *csiSnapshotExposer) GetExposed(ctx context.Context, ownerObject corev1. curLog.WithField("backup pvc", backupPVCName).Info("Backup PVC is bound") - return &ExposeResult{ByPod: ExposeByPod{HostingPod: pod, VolumeName: pod.Spec.Volumes[0].Name}}, nil + i := 0 + for i = 0; i < len(pod.Spec.Volumes); i++ { + if pod.Spec.Volumes[i].Name == volumeName { + break + } + } + + if i == len(pod.Spec.Volumes) { + return nil, errors.Errorf("backup pod %s doesn't have the expected backup volume", pod.Name) + } + + curLog.WithField("pod", pod.Name).Infof("Backup volume is found in pod at index %v", i) + + return &ExposeResult{ByPod: ExposeByPod{HostingPod: pod, VolumeName: volumeName}}, nil } func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1.ObjectReference, vsName string, sourceNamespace string) { diff --git a/pkg/exposer/csi_snapshot_test.go b/pkg/exposer/csi_snapshot_test.go index 7ea6d5bf05..d57d78ed44 100644 --- a/pkg/exposer/csi_snapshot_test.go +++ b/pkg/exposer/csi_snapshot_test.go @@ -37,6 +37,8 @@ import ( velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerotest "github.com/vmware-tanzu/velero/pkg/test" "github.com/vmware-tanzu/velero/pkg/util/boolptr" + + clientFake "sigs.k8s.io/controller-runtime/pkg/client/fake" ) type reactor struct { @@ -384,3 +386,180 @@ func TestExpose(t *testing.T) { }) } } + +func TestGetExpose(t *testing.T) { + backup := &velerov1.Backup{ + TypeMeta: metav1.TypeMeta{ + APIVersion: velerov1.SchemeGroupVersion.String(), + Kind: "Backup", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1.DefaultNamespace, + Name: "fake-backup", + UID: "fake-uid", + }, + } + + backupPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: backup.Namespace, + Name: backup.Name, + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "fake-volume", + }, + { + Name: "fake-volume-2", + }, + { + Name: string(backup.UID), + }, + }, + }, + } + + backupPodWithoutVolume := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: backup.Namespace, + Name: backup.Name, + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "fake-volume-1", + }, + { + Name: "fake-volume-2", + }, + }, + }, + } + + backupPVC := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: backup.Namespace, + Name: backup.Name, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "fake-pv-name", + }, + } + + backupPV := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-pv-name", + }, + } + + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + + tests := []struct { + name string + kubeClientObj []runtime.Object + ownerBackup *velerov1.Backup + exposeWaitParam CSISnapshotExposeWaitParam + Timeout time.Duration + err string + expectedResult *ExposeResult + }{ + { + name: "backup pod is not found", + ownerBackup: backup, + exposeWaitParam: CSISnapshotExposeWaitParam{ + NodeName: "fake-node", + }, + }, + { + name: "wait pvc bound fail", + ownerBackup: backup, + exposeWaitParam: CSISnapshotExposeWaitParam{ + NodeName: "fake-node", + }, + kubeClientObj: []runtime.Object{ + backupPod, + }, + Timeout: time.Second, + err: "error to wait backup PVC bound, fake-backup: error to wait for rediness of PVC: error to get pvc velero/fake-backup: persistentvolumeclaims \"fake-backup\" not found", + }, + { + name: "backup volume not found in pod", + ownerBackup: backup, + exposeWaitParam: CSISnapshotExposeWaitParam{ + NodeName: "fake-node", + }, + kubeClientObj: []runtime.Object{ + backupPodWithoutVolume, + backupPVC, + backupPV, + }, + Timeout: time.Second, + err: "backup pod fake-backup doesn't have the expected backup volume", + }, + { + name: "succeed", + ownerBackup: backup, + exposeWaitParam: CSISnapshotExposeWaitParam{ + NodeName: "fake-node", + }, + kubeClientObj: []runtime.Object{ + backupPod, + backupPVC, + backupPV, + }, + Timeout: time.Second, + expectedResult: &ExposeResult{ + ByPod: ExposeByPod{ + HostingPod: backupPod, + VolumeName: string(backup.UID), + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...) + + fakeClientBuilder := clientFake.NewClientBuilder() + fakeClientBuilder = fakeClientBuilder.WithScheme(scheme) + + fakeClient := fakeClientBuilder.WithRuntimeObjects(test.kubeClientObj...).Build() + + exposer := csiSnapshotExposer{ + kubeClient: fakeKubeClient, + log: velerotest.NewLogger(), + } + + var ownerObject corev1.ObjectReference + if test.ownerBackup != nil { + ownerObject = corev1.ObjectReference{ + Kind: test.ownerBackup.Kind, + Namespace: test.ownerBackup.Namespace, + Name: test.ownerBackup.Name, + UID: test.ownerBackup.UID, + APIVersion: test.ownerBackup.APIVersion, + } + } + + test.exposeWaitParam.NodeClient = fakeClient + + result, err := exposer.GetExposed(context.Background(), ownerObject, test.Timeout, &test.exposeWaitParam) + if test.err == "" { + assert.NoError(t, err) + + if test.expectedResult == nil { + assert.Nil(t, result) + } else { + assert.NoError(t, err) + assert.Equal(t, test.expectedResult.ByPod.VolumeName, result.ByPod.VolumeName) + assert.Equal(t, test.expectedResult.ByPod.HostingPod.Name, result.ByPod.HostingPod.Name) + } + } else { + assert.EqualError(t, err, test.err) + } + }) + } +}