Merge pull request #9233 from Lyndon-Li/backup-pvc-to-different-node

backupPVC to different node

changelogs/unreleased/9233-Lyndon-Li

@@ -0,0 +1 @@
+Fix issue #9229, don't attach backupPVC to the source node

pkg/controller/data_upload_controller.go

@@ -916,6 +916,13 @@ func (r *DataUploadReconciler) setupExposeParam(du *velerov2alpha1api.DataUpload
 			return nil, errors.Wrapf(err, "failed to get PVC %s/%s", du.Spec.SourceNamespace, du.Spec.SourcePVC)
 		}
 
+		pv := &corev1api.PersistentVolume{}
+		if err := r.client.Get(context.Background(), types.NamespacedName{
+			Name: pvc.Spec.VolumeName,
+		}, pv); err != nil {
+			return nil, errors.Wrapf(err, "failed to get source PV %s", pvc.Spec.VolumeName)
+		}
+
 		nodeOS := kube.GetPVCAttachingNodeOS(pvc, r.kubeClient.CoreV1(), r.kubeClient.StorageV1(), log)
 
 		if err := kube.HasNodeWithOS(context.Background(), nodeOS, r.kubeClient.CoreV1()); err != nil {
@@ -963,6 +970,8 @@ func (r *DataUploadReconciler) setupExposeParam(du *velerov2alpha1api.DataUpload
 		return &exposer.CSISnapshotExposeParam{
 			SnapshotName:          du.Spec.CSISnapshot.VolumeSnapshot,
 			SourceNamespace:       du.Spec.SourceNamespace,
+			SourcePVCName:         pvc.Name,
+			SourcePVName:          pv.Name,
 			StorageClass:          du.Spec.CSISnapshot.StorageClass,
 			HostingPodLabels:      hostingPodLabels,
 			HostingPodAnnotations: hostingPodAnnotation,

pkg/exposer/csi_snapshot.go

@@ -35,6 +35,7 @@ import (
 
 	"github.com/vmware-tanzu/velero/pkg/nodeagent"
 	velerotypes "github.com/vmware-tanzu/velero/pkg/types"
+	"github.com/vmware-tanzu/velero/pkg/util"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
 	"github.com/vmware-tanzu/velero/pkg/util/csi"
 	"github.com/vmware-tanzu/velero/pkg/util/kube"
@@ -48,6 +49,12 @@ type CSISnapshotExposeParam struct {
 	// SourceNamespace is the original namespace of the volume that the snapshot is taken for
 	SourceNamespace string
 
+	// SourcePVCName is the original name of the PVC that the snapshot is taken for
+	SourcePVCName string
+
+	// SourcePVName is the name of PV for SourcePVC
+	SourcePVName string
+
 	// AccessMode defines the mode to access the snapshot
 	AccessMode string
 
@@ -189,6 +196,7 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1api.O
 	backupPVCReadOnly := false
 	spcNoRelabeling := false
 	backupPVCAnnotations := map[string]string{}
+	intoleratableNodes := []string{}
 	if value, exists := csiExposeParam.BackupPVCConfig[csiExposeParam.StorageClass]; exists {
 		if value.StorageClass != "" {
 			backupPVCStorageClass = value.StorageClass
@@ -206,6 +214,15 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1api.O
 		if len(value.Annotations) > 0 {
 			backupPVCAnnotations = value.Annotations
 		}
+
+		if _, found := backupPVCAnnotations[util.VSphereCNSFastCloneAnno]; found {
+			if n, err := kube.GetPVAttachedNodes(ctx, csiExposeParam.SourcePVName, e.kubeClient.StorageV1()); err != nil {
+				curLog.WithField("source PV", csiExposeParam.SourcePVName).WithError(err).Warnf("Failed to get attached node for source PV, ignore %s annotation", util.VSphereCNSFastCloneAnno)
+				delete(backupPVCAnnotations, util.VSphereCNSFastCloneAnno)
+			} else {
+				intoleratableNodes = n
+			}
+		}
 	}
 
 	backupPVC, err := e.createBackupPVC(ctx, ownerObject, backupVS.Name, backupPVCStorageClass, csiExposeParam.AccessMode, volumeSize, backupPVCReadOnly, backupPVCAnnotations)
@@ -236,6 +253,7 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1api.O
 		spcNoRelabeling,
 		csiExposeParam.NodeOS,
 		csiExposeParam.PriorityClassName,
+		intoleratableNodes,
 	)
 	if err != nil {
 		return errors.Wrap(err, "error to create backup pod")
@@ -564,6 +582,7 @@ func (e *csiSnapshotExposer) createBackupPod(
 	spcNoRelabeling bool,
 	nodeOS string,
 	priorityClassName string,
+	intoleratableNodes []string,
 ) (*corev1api.Pod, error) {
 	podName := ownerObject.Name
 
@@ -664,6 +683,18 @@ func (e *csiSnapshotExposer) createBackupPod(
 	}
 
 	var podAffinity *corev1api.Affinity
+	if len(intoleratableNodes) > 0 {
+		if affinity == nil {
+			affinity = &kube.LoadAffinity{}
+		}
+
+		affinity.NodeSelector.MatchExpressions = append(affinity.NodeSelector.MatchExpressions, metav1.LabelSelectorRequirement{
+			Key:      "kubernetes.io/hostname",
+			Values:   intoleratableNodes,
+			Operator: metav1.LabelSelectorOpNotIn,
+		})
+	}
+
 	if affinity != nil {
 		podAffinity = kube.ToSystemAffinity([]*kube.LoadAffinity{affinity})
 	}

pkg/exposer/csi_snapshot_priority_test.go

@@ -153,6 +153,7 @@ func TestCreateBackupPodWithPriorityClass(t *testing.T) {
 				false, // spcNoRelabeling
 				kube.NodeOSLinux,
 				tc.expectedPriorityClass,
+				nil,
 			)
 
 			require.NoError(t, err, tc.description)
@@ -237,6 +238,7 @@ func TestCreateBackupPodWithMissingConfigMap(t *testing.T) {
 		false, // spcNoRelabeling
 		kube.NodeOSLinux,
 		"", // empty priority class since config map is missing
+		nil,
 	)
 
 	// Should succeed even when config map is missing

pkg/exposer/csi_snapshot_test.go

@@ -39,8 +39,11 @@ import (
 	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 	velerotypes "github.com/vmware-tanzu/velero/pkg/types"
+	"github.com/vmware-tanzu/velero/pkg/util"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
 	"github.com/vmware-tanzu/velero/pkg/util/kube"
+
+	storagev1api "k8s.io/api/storage/v1"
 )
 
 type reactor struct {
@@ -156,6 +159,31 @@ func TestExpose(t *testing.T) {
 		},
 	}
 
+	pvName := "pv-1"
+	volumeAttachement1 := &storagev1api.VolumeAttachment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "va1",
+		},
+		Spec: storagev1api.VolumeAttachmentSpec{
+			Source: storagev1api.VolumeAttachmentSource{
+				PersistentVolumeName: &pvName,
+			},
+			NodeName: "node-1",
+		},
+	}
+
+	volumeAttachement2 := &storagev1api.VolumeAttachment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "va2",
+		},
+		Spec: storagev1api.VolumeAttachmentSpec{
+			Source: storagev1api.VolumeAttachmentSource{
+				PersistentVolumeName: &pvName,
+			},
+			NodeName: "node-2",
+		},
+	}
+
 	tests := []struct {
 		name                          string
 		snapshotClientObj             []runtime.Object
@@ -169,6 +197,7 @@ func TestExpose(t *testing.T) {
 		expectedReadOnlyPVC           bool
 		expectedBackupPVCStorageClass string
 		expectedAffinity              *corev1api.Affinity
+		expectedPVCAnnotation         map[string]string
 	}{
 		{
 			name:        "wait vs ready fail",
@@ -624,6 +653,117 @@ func TestExpose(t *testing.T) {
 			expectedBackupPVCStorageClass: "fake-sc-read-only",
 			expectedAffinity:              nil,
 		},
+		{
+			name:        "IntolerateSourceNode, get source node fail",
+			ownerBackup: backup,
+			exposeParam: CSISnapshotExposeParam{
+				SnapshotName:     "fake-vs",
+				SourceNamespace:  "fake-ns",
+				SourcePVName:     pvName,
+				StorageClass:     "fake-sc",
+				AccessMode:       AccessModeFileSystem,
+				OperationTimeout: time.Millisecond,
+				ExposeTimeout:    time.Millisecond,
+				BackupPVCConfig: map[string]velerotypes.BackupPVC{
+					"fake-sc": {
+						Annotations: map[string]string{util.VSphereCNSFastCloneAnno: "true"},
+					},
+				},
+				Affinity: nil,
+			},
+			snapshotClientObj: []runtime.Object{
+				vsObject,
+				vscObj,
+			},
+			kubeClientObj: []runtime.Object{
+				daemonSet,
+			},
+			kubeReactors: []reactor{
+				{
+					verb:     "list",
+					resource: "volumeattachments",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, errors.New("fake-create-error")
+					},
+				},
+			},
+			expectedAffinity:      nil,
+			expectedPVCAnnotation: nil,
+		},
+		{
+			name:        "IntolerateSourceNode, get empty source node",
+			ownerBackup: backup,
+			exposeParam: CSISnapshotExposeParam{
+				SnapshotName:     "fake-vs",
+				SourceNamespace:  "fake-ns",
+				SourcePVName:     pvName,
+				StorageClass:     "fake-sc",
+				AccessMode:       AccessModeFileSystem,
+				OperationTimeout: time.Millisecond,
+				ExposeTimeout:    time.Millisecond,
+				BackupPVCConfig: map[string]velerotypes.BackupPVC{
+					"fake-sc": {
+						Annotations: map[string]string{util.VSphereCNSFastCloneAnno: "true"},
+					},
+				},
+				Affinity: nil,
+			},
+			snapshotClientObj: []runtime.Object{
+				vsObject,
+				vscObj,
+			},
+			kubeClientObj: []runtime.Object{
+				daemonSet,
+			},
+			expectedAffinity:      nil,
+			expectedPVCAnnotation: map[string]string{util.VSphereCNSFastCloneAnno: "true"},
+		},
+		{
+			name:        "IntolerateSourceNode, get source nodes",
+			ownerBackup: backup,
+			exposeParam: CSISnapshotExposeParam{
+				SnapshotName:     "fake-vs",
+				SourceNamespace:  "fake-ns",
+				SourcePVName:     pvName,
+				StorageClass:     "fake-sc",
+				AccessMode:       AccessModeFileSystem,
+				OperationTimeout: time.Millisecond,
+				ExposeTimeout:    time.Millisecond,
+				BackupPVCConfig: map[string]velerotypes.BackupPVC{
+					"fake-sc": {
+						Annotations: map[string]string{util.VSphereCNSFastCloneAnno: "true"},
+					},
+				},
+				Affinity: nil,
+			},
+			snapshotClientObj: []runtime.Object{
+				vsObject,
+				vscObj,
+			},
+			kubeClientObj: []runtime.Object{
+				daemonSet,
+				volumeAttachement1,
+				volumeAttachement2,
+			},
+			expectedAffinity: &corev1api.Affinity{
+				NodeAffinity: &corev1api.NodeAffinity{
+					RequiredDuringSchedulingIgnoredDuringExecution: &corev1api.NodeSelector{
+						NodeSelectorTerms: []corev1api.NodeSelectorTerm{
+							{
+								MatchExpressions: []corev1api.NodeSelectorRequirement{
+									{
+										Key:      "kubernetes.io/hostname",
+										Operator: corev1api.NodeSelectorOpNotIn,
+										Values:   []string{"node-1", "node-2"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedPVCAnnotation: map[string]string{util.VSphereCNSFastCloneAnno: "true"},
+		},
 	}
 
 	for _, test := range tests {
@@ -705,6 +845,12 @@ func TestExpose(t *testing.T) {
 				if test.expectedAffinity != nil {
 					assert.Equal(t, test.expectedAffinity, backupPod.Spec.Affinity)
 				}
+
+				if test.expectedPVCAnnotation != nil {
+					assert.Equal(t, test.expectedPVCAnnotation, backupPVC.Annotations)
+				} else {
+					assert.Empty(t, backupPVC.Annotations)
+				}
 			} else {
 				assert.EqualError(t, err, test.err)
 			}

pkg/util/kube/pvc_pv.go

@@ -554,3 +554,19 @@ func GetPVAttachedNode(ctx context.Context, pv string, storageClient storagev1.S
 
 	return "", nil
 }
+
+func GetPVAttachedNodes(ctx context.Context, pv string, storageClient storagev1.StorageV1Interface) ([]string, error) {
+	vaList, err := storageClient.VolumeAttachments().List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, errors.Wrapf(err, "error listing volumeattachment")
+	}
+
+	nodes := []string{}
+	for _, va := range vaList.Items {
+		if va.Spec.Source.PersistentVolumeName != nil && *va.Spec.Source.PersistentVolumeName == pv {
+			nodes = append(nodes, va.Spec.NodeName)
+		}
+	}
+
+	return nodes, nil
+}

pkg/util/third_party.go

@@ -28,3 +28,7 @@ var ThirdPartyTolerations = []string{
 	"kubernetes.azure.com/scalesetpriority",
 	"CriticalAddonsOnly",
 }
+
+const (
+	VSphereCNSFastCloneAnno = "csi.vsphere.volume/fast-provisioning"
+)