Skip to content

Commit

Permalink
Expose the VolumeHelper to third-party plugins.
Browse files Browse the repository at this point in the history
Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>
  • Loading branch information
blackpiglet committed Jul 3, 2024
1 parent 28d64c2 commit 2df78b3
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 19 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/7944-blackpiglet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Expose the VolumeHelper to third-party plugins.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
43 changes: 42 additions & 1 deletion internal/resourcepolicies/resource_policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())

Check warning on line 173 in internal/resourcepolicies/resource_policies.go

View check run for this annotation

Codecov / codecov/patch

internal/resourcepolicies/resource_policies.go#L160-L173

Added lines #L160 - L173 were not covered by tests
}
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())

Check warning on line 185 in internal/resourcepolicies/resource_policies.go

View check run for this annotation

Codecov / codecov/patch

internal/resourcepolicies/resource_policies.go#L175-L185

Added lines #L175 - L185 were not covered by tests
}
}

return resourcePolicies, nil

Check warning on line 189 in internal/resourcepolicies/resource_policies.go

View check run for this annotation

Codecov / codecov/patch

internal/resourcepolicies/resource_policies.go#L189

Added line #L189 was not covered by tests
}

func getResourcePoliciesFromConfig(cm *v1.ConfigMap) (*Policies, error) {
if cm == nil {
return nil, fmt.Errorf("could not parse config from nil configmap")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/resourcepolicies/resource_policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions pkg/backup/actions/csi/pvc_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Check warning on line 241 in pkg/backup/actions/csi/pvc_action.go

View check run for this annotation

Codecov / codecov/patch

pkg/backup/actions/csi/pvc_action.go#L241

Added line #L241 was not covered by tests
}
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

Check warning on line 246 in pkg/backup/actions/csi/pvc_action.go

View check run for this annotation

Codecov / codecov/patch

pkg/backup/actions/csi/pvc_action.go#L244-L246

Added lines #L244 - L246 were not covered by tests
}

vs, err := p.createVolumeSnapshot(pvc, backup)
if err != nil {
return nil, nil, "", nil, err
Expand Down
16 changes: 16 additions & 0 deletions pkg/backup/actions/csi/pvc_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions pkg/backup/item_backupper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
18 changes: 4 additions & 14 deletions pkg/controller/backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"context"
"fmt"
"os"
"strings"
"time"

snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1"
Expand Down Expand Up @@ -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())

Check warning on line 477 in pkg/controller/backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_controller.go#L477

Added line #L477 was not covered by tests
}
request.ResPolicies = resourcePolicies

return request
}
Expand Down
62 changes: 62 additions & 0 deletions pkg/plugin/utils/volumehelper/volume_policy_helper.go
Original file line number Diff line number Diff line change
@@ -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

Check warning on line 49 in pkg/plugin/utils/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

pkg/plugin/utils/volumehelper/volume_policy_helper.go#L49

Added line #L49 was not covered by tests
}

volumeHelperImpl := volumehelper.NewVolumeHelperImpl(
resourcePolicies,
backup.Spec.SnapshotVolumes,
logger,
crClient,
boolptr.IsSetToTrue(backup.Spec.DefaultVolumesToFsBackup),
true,
)

return volumeHelperImpl.ShouldPerformSnapshot(unstructured, groupResource)
}

0 comments on commit 2df78b3

Please sign in to comment.