Skip to content

Commit

Permalink
Extend Volume Policies feature to support more actions
Browse files Browse the repository at this point in the history
Signed-off-by: Shubham Pampattiwar <[email protected]>

fix volume policy action execution

Signed-off-by: Shubham Pampattiwar <[email protected]>

remove unused files

Signed-off-by: Shubham Pampattiwar <[email protected]>

add changelog file

Signed-off-by: Shubham Pampattiwar <[email protected]>

fix CI linter errors

fix linter errors

address pr review comments

Signed-off-by: Shubham Pampattiwar <[email protected]>

fix via make update cmd

Signed-off-by: Shubham Pampattiwar <[email protected]>
  • Loading branch information
shubham-pampattiwar committed Apr 20, 2024
1 parent 22b9465 commit f96d570
Show file tree
Hide file tree
Showing 10 changed files with 423 additions and 29 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/7664-shubham-pampattiwar
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implementation for Extending VolumePolicies to support more actions
9 changes: 7 additions & 2 deletions internal/resourcepolicies/resource_policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@ type VolumeActionType string

const (
// currently only support configmap type of resource config
ConfigmapRefType string = "configmap"
Skip VolumeActionType = "skip"
ConfigmapRefType string = "configmap"
// skip action implies the volume would be skipped from the backup operation
Skip VolumeActionType = "skip"
// fs-backup action implies that the volume would be backed up via file system copy method using the uploader(kopia/restic) configured by the user
FSBackup VolumeActionType = "fs-backup"
// snapshot action can have 3 different meaning based on velero configuration and backup spec - cloud provider based snapshots, local csi snapshots and datamover snapshots
Snapshot VolumeActionType = "snapshot"
)

// Action defined as one action for a specific way of backup
Expand Down
6 changes: 3 additions & 3 deletions internal/resourcepolicies/resource_policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestGetResourceMatchedAction(t *testing.T) {
},
},
{
Action: Action{Type: "volume-snapshot"},
Action: Action{Type: "snapshot"},
Conditions: map[string]interface{}{
"capacity": "10,100Gi",
"storageClass": []string{"gp2", "ebs-sc"},
Expand All @@ -147,7 +147,7 @@ func TestGetResourceMatchedAction(t *testing.T) {
},
},
{
Action: Action{Type: "file-system-backup"},
Action: Action{Type: "fs-backup"},
Conditions: map[string]interface{}{
"storageClass": []string{"gp2", "ebs-sc"},
"csi": interface{}(
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestGetResourceMatchedAction(t *testing.T) {
storageClass: "ebs-sc",
csi: &csiVolumeSource{Driver: "aws.efs.csi.driver"},
},
expectedAction: &Action{Type: "volume-snapshot"},
expectedAction: &Action{Type: "snapshot"},
},
{
name: "dismatch all policies",
Expand Down
6 changes: 5 additions & 1 deletion internal/resourcepolicies/volume_resources_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ func decodeStruct(r io.Reader, s interface{}) error {
// validate check action format
func (a *Action) validate() error {
// validate Type
if a.Type != Skip {
valid := false
if a.Type == Skip || a.Type == Snapshot || a.Type == FSBackup {
valid = true
}
if !valid {
return fmt.Errorf("invalid action type %s", a.Type)
}

Expand Down
77 changes: 76 additions & 1 deletion internal/resourcepolicies/volume_resources_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func TestValidate(t *testing.T) {
wantErr: true,
},
{
name: "supported formart volume policies",
name: "supported format volume policies",
res: &resourcePolicies{
Version: "v1",
VolumePolicies: []volumePolicy{
Expand All @@ -245,6 +245,81 @@ func TestValidate(t *testing.T) {
},
wantErr: false,
},
{
name: "supported format volume policies, action type snapshot",
res: &resourcePolicies{
Version: "v1",
VolumePolicies: []volumePolicy{
{
Action: Action{Type: "snapshot"},
Conditions: map[string]interface{}{
"capacity": "0,10Gi",
"storageClass": []string{"gp2", "ebs-sc"},
"csi": interface{}(
map[string]interface{}{
"driver": "aws.efs.csi.driver",
}),
"nfs": interface{}(
map[string]interface{}{
"server": "192.168.20.90",
"path": "/mnt/data/",
}),
},
},
},
},
wantErr: false,
},
{
name: "supported format volume policies, action type fs-backup",
res: &resourcePolicies{
Version: "v1",
VolumePolicies: []volumePolicy{
{
Action: Action{Type: "fs-backup"},
Conditions: map[string]interface{}{
"capacity": "0,10Gi",
"storageClass": []string{"gp2", "ebs-sc"},
"csi": interface{}(
map[string]interface{}{
"driver": "aws.efs.csi.driver",
}),
"nfs": interface{}(
map[string]interface{}{
"server": "192.168.20.90",
"path": "/mnt/data/",
}),
},
},
},
},
wantErr: false,
},
{
name: "supported format volume policies, action type fs-backup and snapshot",
res: &resourcePolicies{
Version: "v1",
VolumePolicies: []volumePolicy{
{
Action: Action{Type: Snapshot},
Conditions: map[string]interface{}{
"storageClass": []string{"gp2"},
},
},
{
Action: Action{Type: FSBackup},
Conditions: map[string]interface{}{
"nfs": interface{}(
map[string]interface{}{
"server": "192.168.20.90",
"path": "/mnt/data/",
}),
},
},
},
},
wantErr: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
214 changes: 214 additions & 0 deletions internal/volumehelper/volume_policy_helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
package volumehelper

import (
"fmt"
"strings"

"github.com/vmware-tanzu/velero/pkg/util/boolptr"
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"

"github.com/sirupsen/logrus"
corev1api "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/vmware-tanzu/velero/internal/resourcepolicies"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/client"
"github.com/vmware-tanzu/velero/pkg/kuberesource"
"github.com/vmware-tanzu/velero/pkg/util"
pdvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
)

type VolumeHelper interface {
GetVolumesForFSBackup(pod *corev1api.Pod) ([]string, []string, error)
ShouldPerformSnapshot(obj runtime.Unstructured, groupResource schema.GroupResource) (bool, error)
}

type VolumeHelperImpl struct {
Backup *velerov1api.Backup
VolumePolicy *resourcepolicies.Policies
SnapshotVolumes *bool
Logger logrus.FieldLogger
DynamicFactory client.DynamicFactory
}

func (v *VolumeHelperImpl) ShouldPerformSnapshot(obj runtime.Unstructured, groupResource schema.GroupResource) (bool, error) {

Check warning on line 37 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L37

Added line #L37 was not covered by tests
// check if volume policy exists and also check if the object(pv/pvc) fits a volume policy criteria and see if the associated action is snapshot
// if it is not snapshot then skip the code path for snapshotting the PV/PVC
pvc := new(corev1api.PersistentVolumeClaim)
pv := new(corev1api.PersistentVolume)
var err error

Check warning on line 42 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L40-L42

Added lines #L40 - L42 were not covered by tests

if groupResource == kuberesource.PersistentVolumeClaims {
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pvc); err != nil {
return false, err

Check warning on line 46 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L44-L46

Added lines #L44 - L46 were not covered by tests
}

pv, err = v.GetPVForPVC(pvc)
if err != nil {
return false, err

Check warning on line 51 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L49-L51

Added lines #L49 - L51 were not covered by tests
}

}

if groupResource == kuberesource.PersistentVolumes {
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pv); err != nil {
return false, err

Check warning on line 58 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L56-L58

Added lines #L56 - L58 were not covered by tests
}
}

if v.VolumePolicy != nil {
action, err := v.VolumePolicy.GetMatchAction(pv)
if err != nil {
return false, err

Check warning on line 65 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L62-L65

Added lines #L62 - L65 were not covered by tests
}

// Also account for SnapshotVolumes flag on backup CR
if action != nil && action.Type == resourcepolicies.Snapshot && boolptr.IsSetToTrue(v.SnapshotVolumes) {
v.Logger.Infof(fmt.Sprintf("performing snapshot action for pv %s as it satisfies the volume policy criteria and snapshotVolumes is set to true", pv.Name))
return true, nil

Check warning on line 71 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L69-L71

Added lines #L69 - L71 were not covered by tests
}
v.Logger.Infof(fmt.Sprintf("skipping snapshot action for pv %s possibly due to not satisfying the volume policy criteria or snapshotVolumes is not true", pv.Name))
return false, nil

Check warning on line 74 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L73-L74

Added lines #L73 - L74 were not covered by tests
}

return false, nil

Check warning on line 77 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L77

Added line #L77 was not covered by tests
}

func (v *VolumeHelperImpl) GetVolumesForFSBackup(pod *corev1api.Pod, defaultVolumesToFsBackup, backupExcludePVC bool) ([]string, []string, error) {
// Check if there is a fs-backup/snapshot volume policy specified by the user, if yes then use the volume policy approach to
// get the list volumes for fs-backup else go via the legacy annotation based approach
var includedVolumes, optedOutVolumes, volsToProcessByLegacyApproach = make([]string, 0), make([]string, 0), make([]string, 0)
var err error

if v.VolumePolicy != nil {
// Get the list of volumes to back up using pod volume backup for the given pod matching fs-backup volume policy action
v.Logger.Infof("Volume Policy specified by the user, using volume policy approach to segregate pod volumes for fs-backup")
includedVolumes, optedOutVolumes, volsToProcessByLegacyApproach, err = v.GetVolumesMatchingFSBackupAction(pod, v.VolumePolicy, backupExcludePVC)
if err != nil {
return includedVolumes, optedOutVolumes, err

Check warning on line 91 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L88-L91

Added lines #L88 - L91 were not covered by tests
}
}

// process legacy annotation based approach, this will be done for 2 scenarios:
// 1. volume policy not specified by the user
// 2. there are some volumes for which the volume policy approach did not get any supported matching actions
if v.VolumePolicy == nil || len(volsToProcessByLegacyApproach) > 0 {
// Get the list of volumes to back up using pod volume backup from the pod's annotations.
// We will also pass the list of volume that did not have any supported volume policy action matched in legacy approach so that
// those volumes get processed via legacy annotation based approach, this is a fallback option on annotation based legacy approach
v.Logger.Infof("fs-backup or snapshot Volume Policy not specified by the user, using legacy approach based on annotations")
includedVolumes, optedOutVolumes = pdvolumeutil.GetVolumesByPod(pod, defaultVolumesToFsBackup, backupExcludePVC, volsToProcessByLegacyApproach)
}
return includedVolumes, optedOutVolumes, nil
}

// GetVolumesMatchingFSBackupAction returns a list of volume names to backup for the provided pod having fs-backup volume policy action
func (v *VolumeHelperImpl) GetVolumesMatchingFSBackupAction(pod *corev1api.Pod, volumePolicies *resourcepolicies.Policies, backupExcludePVC bool) ([]string, []string, []string, error) {
FSBackupActionMatchingVols := []string{}
FSBackupNonActionMatchingVols := []string{}
NoActionMatchingVols := []string{}

Check warning on line 112 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L109-L112

Added lines #L109 - L112 were not covered by tests

volsToExclude := pdvolumeutil.GetVolumesToExclude(pod)
for _, vol := range pod.Spec.Volumes {
if !v.ShouldIncludeVolumeInBackup(vol, volsToExclude, backupExcludePVC) {
continue

Check warning on line 117 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L114-L117

Added lines #L114 - L117 were not covered by tests
}
// don't backup volumes that are included in the exclude list.
if util.Contains(volsToExclude, vol.Name) {
FSBackupNonActionMatchingVols = append(FSBackupNonActionMatchingVols, vol.Name)

Check warning on line 121 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L120-L121

Added lines #L120 - L121 were not covered by tests
}

if vol.PersistentVolumeClaim != nil {

Check warning on line 124 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L124

Added line #L124 was not covered by tests
// fetch the associated PVC first
pvc, err := kubeutil.GetPVCForPodVolume(vol, pod, v.DynamicFactory)
if err != nil {
return FSBackupActionMatchingVols, FSBackupNonActionMatchingVols, NoActionMatchingVols, err

Check warning on line 128 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L126-L128

Added lines #L126 - L128 were not covered by tests
}
// now fetch the PV and call GetMatchAction on it
pv, err := v.GetPVForPVC(pvc)
if err != nil {
return FSBackupActionMatchingVols, FSBackupNonActionMatchingVols, NoActionMatchingVols, err

Check warning on line 133 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L131-L133

Added lines #L131 - L133 were not covered by tests
}
// now get the action for pv
action, err := volumePolicies.GetMatchAction(pv)
if err != nil {
return FSBackupActionMatchingVols, FSBackupNonActionMatchingVols, NoActionMatchingVols, err

Check warning on line 138 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L136-L138

Added lines #L136 - L138 were not covered by tests
}

// Record volume list having at least some action so that they are not processed in legacy fallback option
if action == nil {
NoActionMatchingVols = append(NoActionMatchingVols, vol.Name)

Check warning on line 143 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L142-L143

Added lines #L142 - L143 were not covered by tests
}

// Now if the matched action is not nil and is `fs-backup` then add that Volume to the FSBackupActionMatchingVols
// else add that volume to FSBackupNonActionMatchingVols
// we already tracked the volume not matching any kind actions supported by volume policy in NoActionMatchingVols
// The NoActionMatchingVols list will be processed via legacy annotation based approach as a fallback option
if action != nil && action.Type == resourcepolicies.FSBackup {
FSBackupActionMatchingVols = append(FSBackupActionMatchingVols, vol.Name)
} else {
FSBackupNonActionMatchingVols = append(FSBackupNonActionMatchingVols, vol.Name)

Check warning on line 153 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L150-L153

Added lines #L150 - L153 were not covered by tests
}
}
}
return FSBackupActionMatchingVols, FSBackupNonActionMatchingVols, NoActionMatchingVols, nil

Check warning on line 157 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L157

Added line #L157 was not covered by tests
}

func (v *VolumeHelperImpl) ShouldIncludeVolumeInBackup(vol corev1api.Volume, volsToExclude []string, backupExcludePVC bool) bool {
includeVolumeInBackup := true

Check warning on line 161 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L160-L161

Added lines #L160 - L161 were not covered by tests
// 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 vol.HostPath != nil {
includeVolumeInBackup = false

Check warning on line 165 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L164-L165

Added lines #L164 - L165 were not covered by tests
}
// don't backup volumes mounting secrets. Secrets will be backed up separately.
if vol.Secret != nil {
includeVolumeInBackup = false

Check warning on line 169 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L168-L169

Added lines #L168 - L169 were not covered by tests
}
// don't backup volumes mounting ConfigMaps. ConfigMaps will be backed up separately.
if vol.ConfigMap != nil {
includeVolumeInBackup = false

Check warning on line 173 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L172-L173

Added lines #L172 - L173 were not covered by tests
}
// don't backup volumes mounted as projected volumes, all data in those come from kube state.
if vol.Projected != nil {
includeVolumeInBackup = false

Check warning on line 177 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L176-L177

Added lines #L176 - L177 were not covered by tests
}
// don't backup DownwardAPI volumes, all data in those come from kube state.
if vol.DownwardAPI != nil {
includeVolumeInBackup = false

Check warning on line 181 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L180-L181

Added lines #L180 - L181 were not covered by tests
}
if vol.PersistentVolumeClaim != nil && backupExcludePVC {
includeVolumeInBackup = false

Check warning on line 184 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L183-L184

Added lines #L183 - L184 were not covered by tests
}
// don't backup volumes that are included in the exclude list.
if util.Contains(volsToExclude, vol.Name) {
includeVolumeInBackup = false

Check warning on line 188 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L187-L188

Added lines #L187 - L188 were not covered by tests
}
// don't include volumes that mount the default service account token.
if strings.HasPrefix(vol.Name, "default-token") {
includeVolumeInBackup = false

Check warning on line 192 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L191-L192

Added lines #L191 - L192 were not covered by tests
}
return includeVolumeInBackup

Check warning on line 194 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L194

Added line #L194 was not covered by tests
}

func (v *VolumeHelperImpl) GetPVForPVC(pvc *corev1api.PersistentVolumeClaim) (*corev1api.PersistentVolume, error) {
pvResource := metav1.APIResource{Name: "persistentvolumes", Namespaced: false}
pvClient, err := v.DynamicFactory.ClientForGroupVersionResource(schema.GroupVersion{Group: "", Version: "v1"}, pvResource, "")
if err != nil {
return nil, err

Check warning on line 201 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L197-L201

Added lines #L197 - L201 were not covered by tests
}

pvObject, err := pvClient.Get(pvc.Spec.VolumeName, metav1.GetOptions{})
if err != nil {
return nil, err

Check warning on line 206 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L204-L206

Added lines #L204 - L206 were not covered by tests
}

pv := new(corev1api.PersistentVolume)
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(pvObject.UnstructuredContent(), pv); err != nil {
return nil, err

Check warning on line 211 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L209-L211

Added lines #L209 - L211 were not covered by tests
}
return pv, nil

Check warning on line 213 in internal/volumehelper/volume_policy_helper.go

View check run for this annotation

Codecov / codecov/patch

internal/volumehelper/volume_policy_helper.go#L213

Added line #L213 was not covered by tests
}
Loading

0 comments on commit f96d570

Please sign in to comment.