Merge pull request #7060 from Lyndon-Li/release-1.12

[1.12] Issue 7027: backup exposer -- don't assume first volume as the backup volume

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

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

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