diff --git a/changelogs/unreleased/7944-blackpiglet b/changelogs/unreleased/7944-blackpiglet new file mode 100644 index 000000000..180bb05a7 --- /dev/null +++ b/changelogs/unreleased/7944-blackpiglet @@ -0,0 +1 @@ +Expose the VolumeHelper to third-party plugins. \ No newline at end of file diff --git a/changelogs/unreleased/7969-blackpiglet b/changelogs/unreleased/7969-blackpiglet new file mode 100644 index 000000000..180bb05a7 --- /dev/null +++ b/changelogs/unreleased/7969-blackpiglet @@ -0,0 +1 @@ +Expose the VolumeHelper to third-party plugins. \ No newline at end of file diff --git a/design/Implemented/Extend-VolumePolicies-to-support-more-actions.md b/design/Implemented/Extend-VolumePolicies-to-support-more-actions.md index 9bef03153..7997b6288 100644 --- a/design/Implemented/Extend-VolumePolicies-to-support-more-actions.md +++ b/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: diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index ebfe01a32..40f3fae24 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/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") } diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index f5643dac8..02d08254d 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/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 diff --git a/pkg/backup/actions/csi/pvc_action.go b/pkg/backup/actions/csi/pvc_action.go index 21e8371c9..a078a2308 100644 --- a/pkg/backup/actions/csi/pvc_action.go +++ b/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 diff --git a/pkg/backup/actions/csi/pvc_action_test.go b/pkg/backup/actions/csi/pvc_action_test.go index 3819e2950..790dde6e7 100644 --- a/pkg/backup/actions/csi/pvc_action_test.go +++ b/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 { diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 7794f6abc..af8ddd26e 100644 --- a/pkg/backup/item_backupper.go +++ b/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" diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 8ccd8db18..24f8e64b2 100644 --- a/pkg/controller/backup_controller.go +++ b/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 } diff --git a/pkg/plugin/utils/volumehelper/volume_policy_helper.go b/pkg/plugin/utils/volumehelper/volume_policy_helper.go new file mode 100644 index 000000000..0a7960faa --- /dev/null +++ b/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) +}