Merge pull request #7664 from shubham-pampattiwar/vol-policy-extension-impl

Extend Volume Policies feature to support more actions

pkg/backup/item_backupper.go

@@ -24,6 +24,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/vmware-tanzu/velero/internal/volumehelper"
+
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	corev1api "k8s.io/api/core/v1"
@@ -52,7 +54,6 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/podvolume"
 	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
 	csiutil "github.com/vmware-tanzu/velero/pkg/util/csi"
-	pdvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
 )
 
 const (
@@ -187,6 +188,16 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 		ib.trackSkippedPV(obj, groupResource, podVolumeApproach, fmt.Sprintf("opted out due to annotation in pod %s", podName), log)
 	}
 
+	// Instantiate volumepolicyhelper struct here
+	vh := &volumehelper.VolumeHelperImpl{
+		SnapshotVolumes: ib.backupRequest.Spec.SnapshotVolumes,
+		Logger:          logger,
+	}
+
+	if ib.backupRequest.ResPolicies != nil {
+		vh.VolumePolicy = ib.backupRequest.ResPolicies
+	}
+
 	if groupResource == kuberesource.Pods {
 		// pod needs to be initialized for the unstructured converter
 		pod = new(corev1api.Pod)
@@ -195,14 +206,13 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 			// nil it on error since it's not valid
 			pod = nil
 		} else {
-			// Get the list of volumes to back up using pod volume backup from the pod's annotations. Remove from this list
+			// Get the list of volumes to back up using pod volume backup from the pod's annotations or volume policy approach. Remove from this list
 			// any volumes that use a PVC that we've already backed up (this would be in a read-write-many scenario,
 			// where it's been backed up from another pod), since we don't need >1 backup per PVC.
-			includedVolumes, optedOutVolumes := pdvolumeutil.GetVolumesByPod(
-				pod,
-				boolptr.IsSetToTrue(ib.backupRequest.Spec.DefaultVolumesToFsBackup),
-				!ib.backupRequest.ResourceIncludesExcludes.ShouldInclude(kuberesource.PersistentVolumeClaims.String()),
-			)
+			includedVolumes, optedOutVolumes, err := vh.GetVolumesForFSBackup(pod, boolptr.IsSetToTrue(ib.backupRequest.Spec.DefaultVolumesToFsBackup), !ib.backupRequest.ResourceIncludesExcludes.ShouldInclude(kuberesource.PersistentVolumeClaims.String()), ib.kbClient)
+			if err != nil {
+				backupErrs = append(backupErrs, errors.WithStack(err))
+			}
 
 			for _, volume := range includedVolumes {
 				// track the volumes that are PVCs using the PVC snapshot tracker, so that when we backup PVCs/PVs
@@ -229,7 +239,7 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 	// the group version of the object.
 	versionPath := resourceVersion(obj)
 
-	updatedObj, additionalItemFiles, err := ib.executeActions(log, obj, groupResource, name, namespace, metadata, finalize)
+	updatedObj, additionalItemFiles, err := ib.executeActions(log, obj, groupResource, name, namespace, metadata, finalize, vh)
 	if err != nil {
 		backupErrs = append(backupErrs, err)
 
@@ -255,7 +265,7 @@ func (ib *itemBackupper) backupItemInternal(logger logrus.FieldLogger, obj runti
 			backupErrs = append(backupErrs, err)
 		}
 
-		if err := ib.takePVSnapshot(obj, log); err != nil {
+		if err := ib.takePVSnapshot(obj, log, vh); err != nil {
 			backupErrs = append(backupErrs, err)
 		}
 	}
@@ -351,6 +361,7 @@ func (ib *itemBackupper) executeActions(
 	name, namespace string,
 	metadata metav1.Object,
 	finalize bool,
+	vh *volumehelper.VolumeHelperImpl,
 ) (runtime.Unstructured, []FileForArchive, error) {
 	var itemFiles []FileForArchive
 	for _, action := range ib.backupRequest.ResolvedActions {
@@ -374,6 +385,19 @@ func (ib *itemBackupper) executeActions(
 			continue
 		}
 
+		if groupResource == kuberesource.PersistentVolumeClaims && actionName == csiBIAPluginName && vh.VolumePolicy != nil {
+			snapshotVolume, err := vh.ShouldPerformSnapshot(obj, kuberesource.PersistentVolumeClaims, ib.kbClient)
+			if err != nil {
+				return nil, itemFiles, errors.WithStack(err)
+			}
+
+			if !snapshotVolume {
+				log.Info(fmt.Sprintf("skipping csi volume snapshot for PVC %s as it does not fit the volume policy criteria specified by the user for snapshot action", namespace+"/"+name))
+				ib.trackSkippedPV(obj, kuberesource.PersistentVolumeClaims, volumeSnapshotApproach, "does not satisfy the criteria for volume policy based snapshot action", log)
+				continue
+			}
+		}
+
 		updatedItem, additionalItemIdentifiers, operationID, postOperationItems, err := action.Execute(obj, ib.backupRequest.Backup)
 		if err != nil {
 			return nil, itemFiles, errors.Wrapf(err, "error executing custom action (groupResource=%s, namespace=%s, name=%s)", groupResource.String(), namespace, name)
@@ -504,15 +528,9 @@ const (
 // takePVSnapshot triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided
 // backup has volume snapshots enabled and the PV is of a compatible type. Also records cloud
 // disk type and IOPS (if applicable) to be able to restore to current state later.
-func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.FieldLogger) error {
+func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.FieldLogger, vh *volumehelper.VolumeHelperImpl) error {
 	log.Info("Executing takePVSnapshot")
 
-	if boolptr.IsSetToFalse(ib.backupRequest.Spec.SnapshotVolumes) {
-		log.Info("Backup has volume snapshots disabled; skipping volume snapshot action.")
-		ib.trackSkippedPV(obj, kuberesource.PersistentVolumes, volumeSnapshotApproach, "backup has volume snapshots disabled", log)
-		return nil
-	}
-
 	pv := new(corev1api.PersistentVolume)
 	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pv); err != nil {
 		return errors.WithStack(err)
@@ -520,6 +538,26 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie
 
 	log = log.WithField("persistentVolume", pv.Name)
 
+	if vh.VolumePolicy != nil {
+		snapshotVolume, err := vh.ShouldPerformSnapshot(obj, kuberesource.PersistentVolumes, ib.kbClient)
+
+		if err != nil {
+			return err
+		}
+
+		if !snapshotVolume {
+			log.Info(fmt.Sprintf("skipping volume snapshot for PV %s as it does not fit the volume policy criteria specified by the user for snapshot action", pv.Name))
+			ib.trackSkippedPV(obj, kuberesource.PersistentVolumes, volumeSnapshotApproach, "does not satisfy the criteria for volume policy based snapshot action", log)
+			return nil
+		}
+	}
+
+	if boolptr.IsSetToFalse(ib.backupRequest.Spec.SnapshotVolumes) {
+		log.Info("Backup has volume snapshots disabled; skipping volume snapshot action.")
+		ib.trackSkippedPV(obj, kuberesource.PersistentVolumes, volumeSnapshotApproach, "backup has volume snapshots disabled", log)
+		return nil
+	}
+
 	// If this PV is claimed, see if we've already taken a (pod volume backup) snapshot of the contents
 	// of this PV. If so, don't take a snapshot.
 	if pv.Spec.ClaimRef != nil {

pkg/cmd/server/server.go

@@ -758,7 +758,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string
 
 	if _, ok := enabledRuntimeControllers[controller.Backup]; ok {
 		backupper, err := backup.NewKubernetesBackupper(
-			s.mgr.GetClient(),
+			s.crClient,
 			s.discoveryHelper,
 			client.NewDynamicFactory(s.dynamicClient),
 			podexec.NewPodCommandExecutor(s.kubeClientConfig, s.kubeClient.CoreV1().RESTClient()),

pkg/util/kube/pvc_pv.go

@@ -386,3 +386,21 @@ func GetPVForPVC(
 	}
 	return pv, nil
 }
+
+func GetPVCForPodVolume(vol *corev1api.Volume, pod *corev1api.Pod, crClient crclient.Client) (*corev1api.PersistentVolumeClaim, error) {
+	if vol.PersistentVolumeClaim == nil {
+		return nil, errors.Errorf("volume %s/%s has no PVC associated with it", pod.Namespace, vol.Name)
+	}
+	pvc := &corev1api.PersistentVolumeClaim{}
+	err := crClient.Get(
+		context.TODO(),
+		crclient.ObjectKey{Name: vol.PersistentVolumeClaim.ClaimName, Namespace: pod.Namespace},
+		pvc,
+	)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to get PVC %s for Volume %s/%s",
+			vol.PersistentVolumeClaim.ClaimName, pod.Namespace, vol.Name)
+	}
+
+	return pvc, nil
+}

pkg/util/kube/pvc_pv_test.go

@@ -1256,3 +1256,125 @@ func TestGetPVForPVC(t *testing.T) {
 		})
 	}
 }
+
+func TestGetPVCForPodVolume(t *testing.T) {
+	sampleVol := &corev1api.Volume{
+		Name: "sample-volume",
+		VolumeSource: corev1api.VolumeSource{
+			PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+				ClaimName: "sample-pvc",
+			},
+		},
+	}
+
+	sampleVol2 := &corev1api.Volume{
+		Name: "sample-volume",
+		VolumeSource: corev1api.VolumeSource{
+			PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+				ClaimName: "sample-pvc-1",
+			},
+		},
+	}
+
+	sampleVol3 := &corev1api.Volume{
+		Name:         "sample-volume",
+		VolumeSource: corev1api.VolumeSource{},
+	}
+
+	samplePod := &corev1api.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "sample-pod",
+			Namespace: "sample-ns",
+		},
+
+		Spec: corev1api.PodSpec{
+			Containers: []corev1api.Container{
+				{
+					Name:  "sample-container",
+					Image: "sample-image",
+					VolumeMounts: []corev1api.VolumeMount{
+						{
+							Name:      "sample-vm",
+							MountPath: "/etc/pod-info",
+						},
+					},
+				},
+			},
+			Volumes: []corev1api.Volume{
+				{
+					Name: "sample-volume",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	matchingPVC := &corev1api.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "sample-pvc",
+			Namespace: "sample-ns",
+		},
+		Spec: v1.PersistentVolumeClaimSpec{
+			AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
+			Resources: v1.VolumeResourceRequirements{
+				Requests: v1.ResourceList{},
+			},
+			StorageClassName: &csiStorageClass,
+			VolumeName:       "test-csi-7d28e566-ade7-4ed6-9e15-2e44d2fbcc08",
+		},
+		Status: v1.PersistentVolumeClaimStatus{
+			Phase:       v1.ClaimBound,
+			AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
+			Capacity:    v1.ResourceList{},
+		},
+	}
+
+	testCases := []struct {
+		name          string
+		vol           *corev1api.Volume
+		pod           *corev1api.Pod
+		expectedPVC   *corev1api.PersistentVolumeClaim
+		expectedError bool
+	}{
+		{
+			name:          "should find PVC for volume",
+			vol:           sampleVol,
+			pod:           samplePod,
+			expectedPVC:   matchingPVC,
+			expectedError: false,
+		},
+		{
+			name:          "should not find PVC for volume not found error case",
+			vol:           sampleVol2,
+			pod:           samplePod,
+			expectedPVC:   nil,
+			expectedError: true,
+		},
+		{
+			name:          "should not find PVC vol has no PVC, error case",
+			vol:           sampleVol3,
+			pod:           samplePod,
+			expectedPVC:   nil,
+			expectedError: true,
+		},
+	}
+
+	objs := []runtime.Object{matchingPVC}
+	fakeClient := velerotest.NewFakeControllerRuntimeClient(t, objs...)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualPVC, actualError := GetPVCForPodVolume(tc.vol, samplePod, fakeClient)
+			if tc.expectedError {
+				assert.NotNil(t, actualError, "Want error; Got nil error")
+				assert.Nilf(t, actualPVC, "Want PV: nil; Got PV: %q", actualPVC)
+				return
+			}
+			assert.Nilf(t, actualError, "Want: nil error; Got: %v", actualError)
+			assert.Equalf(t, actualPVC.Name, tc.expectedPVC.Name, "Want PVC with name %q; Got PVC with name %q", tc.expectedPVC.Name, actualPVC)
+		})
+	}
+}

pkg/util/podvolume/pod_volume.go

@@ -30,7 +30,7 @@ import (
 )
 
 // GetVolumesByPod returns a list of volume names to backup for the provided pod.
-func GetVolumesByPod(pod *corev1api.Pod, defaultVolumesToFsBackup, backupExcludePVC bool) ([]string, []string) {
+func GetVolumesByPod(pod *corev1api.Pod, defaultVolumesToFsBackup, backupExcludePVC bool, volsToProcessByLegacyApproach []string) ([]string, []string) {
 	// tracks the volumes that have been explicitly opted out of backup via the annotation in the pod
 	optedOutVolumes := make([]string, 0)
 
@@ -38,9 +38,13 @@ func GetVolumesByPod(pod *corev1api.Pod, defaultVolumesToFsBackup, backupExclude
 		return GetVolumesToBackup(pod), optedOutVolumes
 	}
 
-	volsToExclude := getVolumesToExclude(pod)
+	volsToExclude := GetVolumesToExclude(pod)
 	podVolumes := []string{}
-	for _, pv := range pod.Spec.Volumes {
+	// Identify volume to process
+	// For normal case all the pod volume will be processed
+	// For case when volsToProcessByLegacyApproach is non-empty then only those volume will be processed
+	volsToProcess := GetVolumesToProcess(pod.Spec.Volumes, volsToProcessByLegacyApproach)
+	for _, pv := range volsToProcess {
 		// cannot backup hostpath volumes as they are not mounted into /var/lib/kubelet/pods
 		// and therefore not accessible to the node agent daemon set.
 		if pv.HostPath != nil {
@@ -96,7 +100,7 @@ func GetVolumesToBackup(obj metav1.Object) []string {
 	return strings.Split(backupsValue, ",")
 }
 
-func getVolumesToExclude(obj metav1.Object) []string {
+func GetVolumesToExclude(obj metav1.Object) []string {
 	annotations := obj.GetAnnotations()
 	if annotations == nil {
 		return nil
@@ -112,7 +116,7 @@ func IsPVCDefaultToFSBackup(pvcNamespace, pvcName string, crClient crclient.Clie
 	}
 
 	for index := range pods {
-		vols, _ := GetVolumesByPod(&pods[index], defaultVolumesToFsBackup, false)
+		vols, _ := GetVolumesByPod(&pods[index], defaultVolumesToFsBackup, false, []string{})
 		if len(vols) > 0 {
 			volName, err := getPodVolumeNameForPVC(pods[index], pvcName)
 			if err != nil {
@@ -160,3 +164,29 @@ func getPodsUsingPVC(
 
 	return podsUsingPVC, nil
 }
+
+func GetVolumesToProcess(volumes []corev1api.Volume, volsToProcessByLegacyApproach []string) []corev1api.Volume {
+	volsToProcess := make([]corev1api.Volume, 0)
+
+	// return empty list when no volumes associated with pod
+	if len(volumes) == 0 {
+		return volsToProcess
+	}
+
+	// legacy approach as a fallback option case
+	if len(volsToProcessByLegacyApproach) > 0 {
+		for _, vol := range volumes {
+			// don't process volumes that are already matched for supported action in volume policy approach
+			if !util.Contains(volsToProcessByLegacyApproach, vol.Name) {
+				continue
+			}
+
+			// add volume that is not processed in volume policy approach
+			volsToProcess = append(volsToProcess, vol)
+		}
+
+		return volsToProcess
+	}
+	// normal case return the list as in
+	return volumes
+}

pkg/util/podvolume/pod_volume_test.go

@@ -369,7 +369,7 @@ func TestGetVolumesByPod(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			actualIncluded, actualOptedOut := GetVolumesByPod(tc.pod, tc.defaultVolumesToFsBackup, tc.backupExcludePVC)
+			actualIncluded, actualOptedOut := GetVolumesByPod(tc.pod, tc.defaultVolumesToFsBackup, tc.backupExcludePVC, []string{})
 
 			sort.Strings(tc.expected.included)
 			sort.Strings(actualIncluded)
@@ -792,3 +792,97 @@ func TestGetPodsUsingPVC(t *testing.T) {
 		})
 	}
 }
+
+func TestGetVolumesToProcess(t *testing.T) {
+	testCases := []struct {
+		name                          string
+		volumes                       []corev1api.Volume
+		volsToProcessByLegacyApproach []string
+		expectedVolumes               []corev1api.Volume
+	}{
+		{
+			name: "pod has 2 volumes empty volsToProcessByLegacyApproach list return 2 volumes",
+			volumes: []corev1api.Volume{
+				{
+					Name: "sample-volume-1",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-1",
+						},
+					},
+				},
+				{
+					Name: "sample-volume-2",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-2",
+						},
+					},
+				},
+			},
+			volsToProcessByLegacyApproach: []string{},
+			expectedVolumes: []corev1api.Volume{
+				{
+					Name: "sample-volume-1",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-1",
+						},
+					},
+				},
+				{
+					Name: "sample-volume-2",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-2",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "pod has 2 volumes non-empty volsToProcessByLegacyApproach list returns 1 volumes",
+			volumes: []corev1api.Volume{
+				{
+					Name: "sample-volume-1",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-1",
+						},
+					},
+				},
+				{
+					Name: "sample-volume-2",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-2",
+						},
+					},
+				},
+			},
+			volsToProcessByLegacyApproach: []string{"sample-volume-2"},
+			expectedVolumes: []corev1api.Volume{
+				{
+					Name: "sample-volume-2",
+					VolumeSource: corev1api.VolumeSource{
+						PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{
+							ClaimName: "sample-pvc-2",
+						},
+					},
+				},
+			},
+		},
+		{
+			name:                          "empty case, return empty list",
+			volumes:                       []corev1api.Volume{},
+			volsToProcessByLegacyApproach: []string{},
+			expectedVolumes:               []corev1api.Volume{},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualVolumes := GetVolumesToProcess(tc.volumes, tc.volsToProcessByLegacyApproach)
+			assert.Equal(t, tc.expectedVolumes, actualVolumes, "Want Volumes List %v; Got Volumes List %v", tc.expectedVolumes, actualVolumes)
+		})
+	}
+}