diff --git a/changelogs/unreleased/9532-Lyndon-Li‎ b/changelogs/unreleased/9532-Lyndon-Li‎ new file mode 100644 index 000000000..0d5094c22 --- /dev/null +++ b/changelogs/unreleased/9532-Lyndon-Li‎ @@ -0,0 +1 @@ +Fix issue #9343, include PV topology to data mover pod affinities \ No newline at end of file diff --git a/pkg/exposer/csi_snapshot_test.go b/pkg/exposer/csi_snapshot_test.go index b4dd92c3f..4ec1d6d9d 100644 --- a/pkg/exposer/csi_snapshot_test.go +++ b/pkg/exposer/csi_snapshot_test.go @@ -68,6 +68,12 @@ func TestExpose(t *testing.T) { var restoreSize int64 = 123456 + scObj := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-sc", + }, + } + snapshotClass := "fake-snapshot-class" vsObject := &snapshotv1api.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ @@ -199,6 +205,17 @@ func TestExpose(t *testing.T) { expectedAffinity *corev1api.Affinity expectedPVCAnnotation map[string]string }{ + { + name: "get volume topology fail", + ownerBackup: backup, + exposeParam: CSISnapshotExposeParam{ + SnapshotName: "fake-vs", + OperationTimeout: time.Millisecond, + ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", + }, + err: "error getting volume topology for PV , storage class fake-sc: error getting storage class fake-sc: storageclasses.storage.k8s.io \"fake-sc\" not found", + }, { name: "wait vs ready fail", ownerBackup: backup, @@ -206,6 +223,10 @@ func TestExpose(t *testing.T) { SnapshotName: "fake-vs", OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", + }, + kubeClientObj: []runtime.Object{ + scObj, }, err: "error wait volume snapshot ready: error to get VolumeSnapshot /fake-vs: volumesnapshots.snapshot.storage.k8s.io \"fake-vs\" not found", }, @@ -217,10 +238,14 @@ func TestExpose(t *testing.T) { SourceNamespace: "fake-ns", OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to get volume snapshot content: error getting volume snapshot content from API: volumesnapshotcontents.snapshot.storage.k8s.io \"fake-vsc\" not found", }, { @@ -231,6 +256,7 @@ func TestExpose(t *testing.T) { SourceNamespace: "fake-ns", OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -245,6 +271,9 @@ func TestExpose(t *testing.T) { }, }, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to delete volume snapshot: error to delete volume snapshot: fake-delete-error", }, { @@ -255,6 +284,7 @@ func TestExpose(t *testing.T) { SourceNamespace: "fake-ns", OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -269,6 +299,9 @@ func TestExpose(t *testing.T) { }, }, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to delete volume snapshot content: error to delete volume snapshot content: fake-delete-error", }, { @@ -279,6 +312,7 @@ func TestExpose(t *testing.T) { SourceNamespace: "fake-ns", OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -293,6 +327,9 @@ func TestExpose(t *testing.T) { }, }, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to create backup volume snapshot: fake-create-error", }, { @@ -303,6 +340,7 @@ func TestExpose(t *testing.T) { SourceNamespace: "fake-ns", OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -317,6 +355,9 @@ func TestExpose(t *testing.T) { }, }, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to create backup volume snapshot content: fake-create-error", }, { @@ -326,11 +367,15 @@ func TestExpose(t *testing.T) { SnapshotName: "fake-vs", SourceNamespace: "fake-ns", AccessMode: "fake-mode", + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, vscObj, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to create backup pvc: unsupported access mode fake-mode", }, { @@ -342,6 +387,7 @@ func TestExpose(t *testing.T) { OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, AccessMode: AccessModeFileSystem, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -356,6 +402,9 @@ func TestExpose(t *testing.T) { }, }, }, + kubeClientObj: []runtime.Object{ + scObj, + }, err: "error to create backup pvc: error to create pvc: fake-create-error", }, { @@ -367,6 +416,7 @@ func TestExpose(t *testing.T) { AccessMode: AccessModeFileSystem, OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -374,6 +424,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, kubeReactors: []reactor{ { @@ -395,6 +446,7 @@ func TestExpose(t *testing.T) { AccessMode: AccessModeFileSystem, OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -402,6 +454,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, }, { @@ -413,6 +466,7 @@ func TestExpose(t *testing.T) { AccessMode: AccessModeFileSystem, OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObject, @@ -420,6 +474,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, }, { @@ -432,6 +487,7 @@ func TestExpose(t *testing.T) { OperationTimeout: time.Millisecond, ExposeTimeout: time.Millisecond, VolumeSize: *resource.NewQuantity(567890, ""), + StorageClass: "fake-sc", }, snapshotClientObj: []runtime.Object{ vsObjectWithoutRestoreSize, @@ -439,6 +495,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedVolumeSize: resource.NewQuantity(567890, ""), }, @@ -465,6 +522,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedReadOnlyPVC: true, }, @@ -491,6 +549,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedReadOnlyPVC: true, expectedBackupPVCStorageClass: "fake-sc-read-only", @@ -517,6 +576,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedBackupPVCStorageClass: "fake-sc-read-only", }, @@ -551,6 +611,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedAffinity: &corev1api.Affinity{ NodeAffinity: &corev1api.NodeAffinity{ @@ -606,6 +667,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedBackupPVCStorageClass: "fake-sc-read-only", expectedAffinity: &corev1api.Affinity{ @@ -649,6 +711,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedBackupPVCStorageClass: "fake-sc-read-only", expectedAffinity: nil, @@ -677,6 +740,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, kubeReactors: []reactor{ { @@ -714,6 +778,7 @@ func TestExpose(t *testing.T) { }, kubeClientObj: []runtime.Object{ daemonSet, + scObj, }, expectedAffinity: nil, expectedPVCAnnotation: map[string]string{util.VSphereCNSFastCloneAnno: "true"}, @@ -744,6 +809,7 @@ func TestExpose(t *testing.T) { daemonSet, volumeAttachement1, volumeAttachement2, + scObj, }, expectedAffinity: &corev1api.Affinity{ NodeAffinity: &corev1api.NodeAffinity{ diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index c82040ed4..d5d2e2041 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -582,6 +582,10 @@ func GetPVAttachedNodes(ctx context.Context, pv string, storageClient storagev1. } func GetVolumeTopology(ctx context.Context, volumeClient corev1client.CoreV1Interface, storageClient storagev1.StorageV1Interface, pvName string, scName string) (*corev1api.NodeSelector, error) { + if pvName == "" || scName == "" { + return nil, errors.Errorf("invalid parameter, pv %s, sc %s", pvName, scName) + } + sc, err := storageClient.StorageClasses().Get(ctx, scName, metav1.GetOptions{}) if err != nil { return nil, errors.Wrapf(err, "error getting storage class %s", scName) diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index d94efa62e..63b8e1edd 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -1909,3 +1909,143 @@ func TestGetPVCAttachingNodeOS(t *testing.T) { }) } } + +func TestGetVolumeTopology(t *testing.T) { + pvWithoutNodeAffinity := &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-pv", + }, + } + + pvWithNodeAffinity := &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-pv", + }, + Spec: corev1api.PersistentVolumeSpec{ + NodeAffinity: &corev1api.VolumeNodeAffinity{ + Required: &corev1api.NodeSelector{ + NodeSelectorTerms: []corev1api.NodeSelectorTerm{ + { + MatchExpressions: []corev1api.NodeSelectorRequirement{ + { + Key: "fake-key", + }, + }, + }, + }, + }, + }, + }, + } + + scObjWithoutVolumeBind := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-storage-class", + }, + } + + volumeBindImmediate := storagev1api.VolumeBindingImmediate + scObjWithImeediateBind := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-storage-class", + }, + VolumeBindingMode: &volumeBindImmediate, + } + + volumeBindWffc := storagev1api.VolumeBindingWaitForFirstConsumer + scObjWithWffcBind := &storagev1api.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-storage-class", + }, + VolumeBindingMode: &volumeBindWffc, + } + + tests := []struct { + name string + pvName string + scName string + kubeClientObj []runtime.Object + expectedErr string + expected *corev1api.NodeSelector + }{ + { + name: "invalid pvName", + scName: "fake-storage-class", + expectedErr: "invalid parameter, pv , sc fake-storage-class", + }, + { + name: "invalid scName", + pvName: "fake-pv", + expectedErr: "invalid parameter, pv fake-pv, sc ", + }, + { + name: "no sc", + pvName: "fake-pv", + scName: "fake-storage-class", + expectedErr: "error getting storage class fake-storage-class: storageclasses.storage.k8s.io \"fake-storage-class\" not found", + }, + { + name: "sc without binding mode", + pvName: "fake-pv", + scName: "fake-storage-class", + kubeClientObj: []runtime.Object{scObjWithoutVolumeBind}, + }, + { + name: "sc without immediate binding mode", + pvName: "fake-pv", + scName: "fake-storage-class", + kubeClientObj: []runtime.Object{scObjWithImeediateBind}, + }, + { + name: "get pv fail", + pvName: "fake-pv", + scName: "fake-storage-class", + kubeClientObj: []runtime.Object{scObjWithWffcBind}, + expectedErr: "error getting PV fake-pv: persistentvolumes \"fake-pv\" not found", + }, + { + name: "pv with no affinity", + pvName: "fake-pv", + scName: "fake-storage-class", + kubeClientObj: []runtime.Object{ + scObjWithWffcBind, + pvWithoutNodeAffinity, + }, + }, + { + name: "pv with affinity", + pvName: "fake-pv", + scName: "fake-storage-class", + kubeClientObj: []runtime.Object{ + scObjWithWffcBind, + pvWithNodeAffinity, + }, + expected: &corev1api.NodeSelector{ + NodeSelectorTerms: []corev1api.NodeSelectorTerm{ + { + MatchExpressions: []corev1api.NodeSelectorRequirement{ + { + Key: "fake-key", + }, + }, + }, + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...) + + var kubeClient kubernetes.Interface = fakeKubeClient + + affinity, err := GetVolumeTopology(t.Context(), kubeClient.CoreV1(), kubeClient.StorageV1(), test.pvName, test.scName) + + if test.expectedErr != "" { + assert.EqualError(t, err, test.expectedErr) + } else { + assert.Equal(t, test.expected, affinity) + } + }) + } +}