Merge pull request #7944 from blackpiglet/7818_fix

Expose the VolumeHelper to third-party plugins.

changelogs/unreleased/7944-blackpiglet

@@ -0,0 +1 @@
+Expose the VolumeHelper to third-party plugins.

design/Implemented/Extend-VolumePolicies-to-support-more-actions.md

@@ -76,7 +76,7 @@ volumePolicies:
 - Update VolumePolicy action type validation to account for `fs-backup` and `snapshot` as valid VolumePolicy actions.
 - Modifications needed for `fs-backup` action:
   - Now based on the specification of volume policy on backup request we will decide whether to go via legacy pod annotations approach or the newer volume policy based fs-backup action approach.
-  - If there is a presence of volume policy(fs-backup/snapshot)  on the backup request that matches as an action for a volume we use the newer volume policy approach to get the list of the volumes for `fs-backup` action
+  - If there is a presence of volume policy(fs-backup/snapshot) on the backup request that matches as an action for a volume we use the newer volume policy approach to get the list of the volumes for `fs-backup` action
   - Else continue with the annotation based legacy approach workflow.
 
 - Modifications needed for `snapshot` action:

internal/resourcepolicies/resource_policies.go

@@ -16,11 +16,16 @@ limitations under the License.
 package resourcepolicies
 
 import (
+	"context"
 	"fmt"
 	"strings"
 
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 )
 
 type VolumeActionType string
@@ -148,7 +153,43 @@ func (p *Policies) Validate() error {
 	return nil
 }
 
-func GetResourcePoliciesFromConfig(cm *v1.ConfigMap) (*Policies, error) {
+func GetResourcePoliciesFromBackup(
+	backup velerov1api.Backup,
+	client crclient.Client,
+	logger logrus.FieldLogger,
+) (resourcePolicies *Policies, err error) {
+	if backup.Spec.ResourcePolicy != nil &&
+		strings.EqualFold(backup.Spec.ResourcePolicy.Kind, ConfigmapRefType) {
+		policiesConfigMap := &v1.ConfigMap{}
+		err = client.Get(
+			context.Background(),
+			crclient.ObjectKey{Namespace: backup.Namespace, Name: backup.Spec.ResourcePolicy.Name},
+			policiesConfigMap,
+		)
+		if err != nil {
+			logger.Errorf("Fail to get ResourcePolicies %s ConfigMap with error %s.",
+				backup.Namespace+"/"+backup.Spec.ResourcePolicy.Name, err.Error())
+			return nil, fmt.Errorf("fail to get ResourcePolicies %s ConfigMap with error %s",
+				backup.Namespace+"/"+backup.Spec.ResourcePolicy.Name, err.Error())
+		}
+		resourcePolicies, err = getResourcePoliciesFromConfig(policiesConfigMap)
+		if err != nil {
+			logger.Errorf("Fail to read ResourcePolicies from ConfigMap %s with error %s.",
+				backup.Namespace+"/"+backup.Name, err.Error())
+			return nil, fmt.Errorf("fail to read the ResourcePolicies from ConfigMap %s with error %s",
+				backup.Namespace+"/"+backup.Name, err.Error())
+		} else if err = resourcePolicies.Validate(); err != nil {
+			logger.Errorf("Fail to validate ResourcePolicies in ConfigMap %s with error %s.",
+				backup.Namespace+"/"+backup.Name, err.Error())
+			return nil, fmt.Errorf("fail to validate ResourcePolicies in ConfigMap %s with error %s",
+				backup.Namespace+"/"+backup.Name, err.Error())
+		}
+	}
+
+	return resourcePolicies, nil
+}
+
+func getResourcePoliciesFromConfig(cm *v1.ConfigMap) (*Policies, error) {
 	if cm == nil {
 		return nil, fmt.Errorf("could not parse config from nil configmap")
 	}

internal/resourcepolicies/resource_policies_test.go

@@ -231,7 +231,7 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {
 	}
 
 	// Call the function and check for errors
-	resPolicies, err := GetResourcePoliciesFromConfig(cm)
+	resPolicies, err := getResourcePoliciesFromConfig(cm)
 	assert.Nil(t, err)
 
 	// Check that the returned resourcePolicies object contains the expected data

pkg/backup/actions/csi/pvc_action.go

@@ -40,6 +40,7 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/kuberesource"
 	"github.com/vmware-tanzu/velero/pkg/label"
 	plugincommon "github.com/vmware-tanzu/velero/pkg/plugin/framework/common"
+	"github.com/vmware-tanzu/velero/pkg/plugin/utils/volumehelper"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	biav2 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v2"
 	uploaderUtil "github.com/vmware-tanzu/velero/pkg/uploader/util"
@@ -229,6 +230,22 @@ func (p *pvcBackupItemAction) Execute(
 		return item, nil, "", nil, nil
 	}
 
+	shouldSnapshot, err := volumehelper.ShouldPerformSnapshotWithBackup(
+		item,
+		kuberesource.PersistentVolumeClaims,
+		*backup,
+		p.crClient,
+		p.log,
+	)
+	if err != nil {
+		return nil, nil, "", nil, err
+	}
+	if !shouldSnapshot {
+		p.log.Debugf("CSI plugin skip snapshot for PVC %s according to the VolumeHelper setting.",
+			pvc.Namespace+"/"+pvc.Name)
+		return nil, nil, "", nil, err
+	}
+
 	vs, err := p.createVolumeSnapshot(pvc, backup)
 	if err != nil {
 		return nil, nil, "", nil, err

pkg/backup/actions/csi/pvc_action_test.go

@@ -61,6 +61,7 @@ func TestExecute(t *testing.T) {
 		expectedBackup     *velerov1api.Backup
 		expectedDataUpload *velerov2alpha1.DataUpload
 		expectedPVC        *corev1.PersistentVolumeClaim
+		resourcePolicy     *corev1.ConfigMap
 	}{
 		{
 			name:        "Skip PVC BIA when backup is in finalizing phase",
@@ -127,6 +128,16 @@ func TestExecute(t *testing.T) {
 					builder.WithLabels(velerov1api.BackupNameLabel, "test", velerov1api.VolumeSnapshotLabel, "")).
 				VolumeName("testPV").StorageClass("testSC").Phase(corev1.ClaimBound).Result(),
 		},
+		{
+			name:           "Test ResourcePolicy",
+			backup:         builder.ForBackup("velero", "test").ResourcePolicies("resourcePolicy").SnapshotVolumes(false).Result(),
+			resourcePolicy: builder.ForConfigMap("velero", "resourcePolicy").Data("policy", "{\"version\":\"v1\", \"volumePolicies\":[{\"conditions\":{\"csi\": {}},\"action\":{\"type\":\"snapshot\"}}]}").Result(),
+			pvc:            builder.ForPersistentVolumeClaim("velero", "testPVC").VolumeName("testPV").StorageClass("testSC").Phase(corev1.ClaimBound).Result(),
+			pv:             builder.ForPersistentVolume("testPV").CSI("hostpath", "testVolume").Result(),
+			sc:             builder.ForStorageClass("testSC").Provisioner("hostpath").Result(),
+			vsClass:        builder.ForVolumeSnapshotClass("tescVSClass").Driver("hostpath").ObjectMeta(builder.WithLabels(velerov1api.VolumeSnapshotClassSelectorLabel, "")).Result(),
+			expectedErr:    nil,
+		},
 	}
 
 	for _, tc := range tests {
@@ -147,6 +158,9 @@ func TestExecute(t *testing.T) {
 			if tc.vsClass != nil {
 				require.NoError(t, crClient.Create(context.Background(), tc.vsClass))
 			}
+			if tc.resourcePolicy != nil {
+				require.NoError(t, crClient.Create(context.Background(), tc.resourcePolicy))
+			}
 
 			pvcBIA := pvcBackupItemAction{
 				log:      logger,
@@ -190,6 +204,8 @@ func TestExecute(t *testing.T) {
 			resultUnstructed, _, _, _, err := pvcBIA.Execute(&unstructured.Unstructured{Object: pvcMap}, tc.backup)
 			if tc.expectedErr != nil {
 				require.Equal(t, err, tc.expectedErr)
+			} else {
+				require.NoError(t, err)
 			}
 
 			if tc.expectedDataUpload != nil {

pkg/backup/item_backupper.go

@@ -24,8 +24,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/vmware-tanzu/velero/internal/volumehelper"
-
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	corev1api "k8s.io/api/core/v1"
@@ -42,6 +40,7 @@ import (
 	"github.com/vmware-tanzu/velero/internal/hook"
 	"github.com/vmware-tanzu/velero/internal/resourcepolicies"
 	"github.com/vmware-tanzu/velero/internal/volume"
+	"github.com/vmware-tanzu/velero/internal/volumehelper"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/archive"
 	"github.com/vmware-tanzu/velero/pkg/client"

pkg/controller/backup_controller.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"strings"
 	"time"
 
 	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1"
@@ -473,20 +472,11 @@ func (b *backupReconciler) prepareBackupRequest(backup *velerov1api.Backup, logg
 		request.Status.ValidationErrors = append(request.Status.ValidationErrors, "encountered labelSelector as well as orLabelSelectors in backup spec, only one can be specified")
 	}
 
-	if request.Spec.ResourcePolicy != nil && strings.EqualFold(request.Spec.ResourcePolicy.Kind, resourcepolicies.ConfigmapRefType) {
-		policiesConfigmap := &corev1api.ConfigMap{}
-		err := b.kbClient.Get(context.Background(), kbclient.ObjectKey{Namespace: request.Namespace, Name: request.Spec.ResourcePolicy.Name}, policiesConfigmap)
-		if err != nil {
-			request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("failed to get resource policies %s/%s configmap with err %v", request.Namespace, request.Spec.ResourcePolicy.Name, err))
-		}
-		res, err := resourcepolicies.GetResourcePoliciesFromConfig(policiesConfigmap)
-		if err != nil {
-			request.Status.ValidationErrors = append(request.Status.ValidationErrors, errors.Wrapf(err, fmt.Sprintf("resource policies %s/%s", request.Namespace, request.Spec.ResourcePolicy.Name)).Error())
-		} else if err = res.Validate(); err != nil {
-			request.Status.ValidationErrors = append(request.Status.ValidationErrors, errors.Wrapf(err, fmt.Sprintf("resource policies %s/%s", request.Namespace, request.Spec.ResourcePolicy.Name)).Error())
-		}
-		request.ResPolicies = res
+	resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(*request.Backup, b.kbClient, logger)
+	if err != nil {
+		request.Status.ValidationErrors = append(request.Status.ValidationErrors, err.Error())
 	}
+	request.ResPolicies = resourcePolicies
 
 	return request
 }

pkg/plugin/utils/volumehelper/volume_policy_helper.go

@@ -0,0 +1,62 @@
+/*
+Copyright the Velero contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package volumehelper
+
+import (
+	"github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/vmware-tanzu/velero/internal/resourcepolicies"
+	"github.com/vmware-tanzu/velero/internal/volumehelper"
+	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/util/boolptr"
+)
+
+// ShouldPerformSnapshotWithBackup is used for third-party plugins.
+// It supports to check whether the PVC or PodVolume should be backed
+// up on demand. On the other hand, the volumeHelperImpl assume there
+// is a VolumeHelper instance initialized before calling the
+// ShouldPerformXXX functions.
+func ShouldPerformSnapshotWithBackup(
+	unstructured runtime.Unstructured,
+	groupResource schema.GroupResource,
+	backup velerov1api.Backup,
+	crClient crclient.Client,
+	logger logrus.FieldLogger,
+) (bool, error) {
+	resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(
+		backup,
+		crClient,
+		logger,
+	)
+	if err != nil {
+		return false, err
+	}
+
+	volumeHelperImpl := volumehelper.NewVolumeHelperImpl(
+		resourcePolicies,
+		backup.Spec.SnapshotVolumes,
+		logger,
+		crClient,
+		boolptr.IsSetToTrue(backup.Spec.DefaultVolumesToFsBackup),
+		true,
+	)
+
+	return volumeHelperImpl.ShouldPerformSnapshot(unstructured, groupResource)
+}