diff --git a/changelogs/unreleased/9166-claude b/changelogs/unreleased/9166-claude new file mode 100644 index 000000000..8e3cb6610 --- /dev/null +++ b/changelogs/unreleased/9166-claude @@ -0,0 +1 @@ +Add VolumePolicy support for PVC Phase conditions to allow skipping Pending PVCs \ No newline at end of file diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 0837e63e2..d484cabce 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -146,6 +146,9 @@ func (p *Policies) BuildPolicy(resPolicies *ResourcePolicies) error { if len(con.PVCLabels) > 0 { volP.conditions = append(volP.conditions, &pvcLabelsCondition{labels: con.PVCLabels}) } + if len(con.PVCPhase) > 0 { + volP.conditions = append(volP.conditions, &pvcPhaseCondition{phases: con.PVCPhase}) + } p.volumePolicies = append(p.volumePolicies, volP) } @@ -191,6 +194,9 @@ func (p *Policies) GetMatchAction(res any) (*Action, error) { if data.PVC != nil { volume.parsePVC(data.PVC) } + case data.PVC != nil: + // Handle PVC-only scenarios (e.g., unbound PVCs) + volume.parsePVC(data.PVC) default: return nil, errors.New("failed to convert object") } diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index b0d5f1fbd..06b1bea1c 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -983,6 +983,69 @@ volumePolicies: }, skip: false, }, + { + name: "PVC phase matching - Pending phase should skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcPhase: ["Pending"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-pending", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimPending, + }, + }, + skip: true, + }, + { + name: "PVC phase matching - Bound phase should not skip", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcPhase: ["Pending"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-bound", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, + }, + skip: false, + }, + { + name: "PVC phase matching - Multiple phases (Pending, Lost)", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcPhase: ["Pending", "Lost"] + action: + type: skip`, + vol: nil, + podVol: nil, + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-lost", + }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimLost, + }, + }, + skip: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -1059,32 +1122,53 @@ func TestParsePVC(t *testing.T) { name string pvc *corev1api.PersistentVolumeClaim expectedLabels map[string]string + expectedPhase string expectErr bool }{ { - name: "valid PVC with labels", + name: "valid PVC with labels and Pending phase", pvc: &corev1api.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"env": "prod"}, }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimPending, + }, }, expectedLabels: map[string]string{"env": "prod"}, + expectedPhase: "Pending", expectErr: false, }, { - name: "valid PVC with empty labels", + name: "valid PVC with Bound phase", pvc: &corev1api.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimBound, + }, }, expectedLabels: nil, + expectedPhase: "Bound", + expectErr: false, + }, + { + name: "valid PVC with Lost phase", + pvc: &corev1api.PersistentVolumeClaim{ + Status: corev1api.PersistentVolumeClaimStatus{ + Phase: corev1api.ClaimLost, + }, + }, + expectedLabels: nil, + expectedPhase: "Lost", expectErr: false, }, { name: "nil PVC pointer", pvc: (*corev1api.PersistentVolumeClaim)(nil), expectedLabels: nil, + expectedPhase: "", expectErr: false, }, } @@ -1095,6 +1179,66 @@ func TestParsePVC(t *testing.T) { s.parsePVC(tc.pvc) assert.Equal(t, tc.expectedLabels, s.pvcLabels) + assert.Equal(t, tc.expectedPhase, s.pvcPhase) + }) + } +} + +func TestPVCPhaseMatch(t *testing.T) { + tests := []struct { + name string + condition *pvcPhaseCondition + volume *structuredVolume + expectedMatch bool + }{ + { + name: "match Pending phase", + condition: &pvcPhaseCondition{phases: []string{"Pending"}}, + volume: &structuredVolume{pvcPhase: "Pending"}, + expectedMatch: true, + }, + { + name: "match multiple phases - Pending matches", + condition: &pvcPhaseCondition{phases: []string{"Pending", "Bound"}}, + volume: &structuredVolume{pvcPhase: "Pending"}, + expectedMatch: true, + }, + { + name: "match multiple phases - Bound matches", + condition: &pvcPhaseCondition{phases: []string{"Pending", "Bound"}}, + volume: &structuredVolume{pvcPhase: "Bound"}, + expectedMatch: true, + }, + { + name: "no match for different phase", + condition: &pvcPhaseCondition{phases: []string{"Pending"}}, + volume: &structuredVolume{pvcPhase: "Bound"}, + expectedMatch: false, + }, + { + name: "no match for empty phase", + condition: &pvcPhaseCondition{phases: []string{"Pending"}}, + volume: &structuredVolume{pvcPhase: ""}, + expectedMatch: false, + }, + { + name: "match with empty phases list (always match)", + condition: &pvcPhaseCondition{phases: []string{}}, + volume: &structuredVolume{pvcPhase: "Pending"}, + expectedMatch: true, + }, + { + name: "match with nil phases list (always match)", + condition: &pvcPhaseCondition{phases: nil}, + volume: &structuredVolume{pvcPhase: "Pending"}, + expectedMatch: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := tc.condition.match(tc.volume) + assert.Equal(t, tc.expectedMatch, result) }) } } diff --git a/internal/resourcepolicies/volume_resources.go b/internal/resourcepolicies/volume_resources.go index 545c1f3f6..4ad34f484 100644 --- a/internal/resourcepolicies/volume_resources.go +++ b/internal/resourcepolicies/volume_resources.go @@ -51,6 +51,7 @@ type structuredVolume struct { csi *csiVolumeSource volumeType SupportedVolume pvcLabels map[string]string + pvcPhase string } func (s *structuredVolume) parsePV(pv *corev1api.PersistentVolume) { @@ -70,8 +71,11 @@ func (s *structuredVolume) parsePV(pv *corev1api.PersistentVolume) { } func (s *structuredVolume) parsePVC(pvc *corev1api.PersistentVolumeClaim) { - if pvc != nil && len(pvc.GetLabels()) > 0 { - s.pvcLabels = pvc.Labels + if pvc != nil { + if len(pvc.GetLabels()) > 0 { + s.pvcLabels = pvc.Labels + } + s.pvcPhase = string(pvc.Status.Phase) } } @@ -110,6 +114,31 @@ func (c *pvcLabelsCondition) validate() error { return nil } +// pvcPhaseCondition defines a condition that matches if the PVC's phase matches any of the provided phases. +type pvcPhaseCondition struct { + phases []string +} + +func (c *pvcPhaseCondition) match(v *structuredVolume) bool { + // No phases specified: always match. + if len(c.phases) == 0 { + return true + } + if v.pvcPhase == "" { + return false + } + for _, phase := range c.phases { + if v.pvcPhase == phase { + return true + } + } + return false +} + +func (c *pvcPhaseCondition) validate() error { + return nil +} + type capacityCondition struct { capacity capacity } diff --git a/internal/resourcepolicies/volume_resources_validator.go b/internal/resourcepolicies/volume_resources_validator.go index 0c719b09e..b6031eec2 100644 --- a/internal/resourcepolicies/volume_resources_validator.go +++ b/internal/resourcepolicies/volume_resources_validator.go @@ -46,6 +46,7 @@ type volumeConditions struct { CSI *csiVolumeSource `yaml:"csi,omitempty"` VolumeTypes []SupportedVolume `yaml:"volumeTypes,omitempty"` PVCLabels map[string]string `yaml:"pvcLabels,omitempty"` + PVCPhase []string `yaml:"pvcPhase,omitempty"` } func (c *capacityCondition) validate() error { diff --git a/site/content/docs/main/resource-filtering.md b/site/content/docs/main/resource-filtering.md index 710564789..a9e65d157 100644 --- a/site/content/docs/main/resource-filtering.md +++ b/site/content/docs/main/resource-filtering.md @@ -278,6 +278,9 @@ The policies YAML config file would look like this: storageClass: - gp2 - standard + # pvc matches specific phase(s) + pvcPhase: + - Pending action: type: skip - conditions: @@ -370,6 +373,7 @@ Currently, Velero supports the volume attributes listed below: - "5Gi" which is not supported and will be failed in validating the configuration - storageClass: matching volumes those with specified `storageClass`, such as `gp2`, `ebs-sc` in eks - volume sources: matching volumes that used specified volume sources. Currently we support nfs or csi backend volume source +- pvcPhase: matching volumes based on the phase of their associated PVCs (Pending, Bound, Lost) Velero supported conditions and format listed below: - capacity @@ -462,9 +466,60 @@ Velero supported conditions and format listed below: type: skip ``` -#### VolumePolicies rules -- Velero already has lots of include or exclude filters. the volume policies are the final filters after others include or exclude filters in one backup processing workflow. So if use a defined similar filter like the opt-in approach to backup one pod volume but skip backup of the same pod volume in volume policies, as volume policies are the final filters that are applied, the volume will not be backed up. -- If volume policies conflict with themselves the first matched policy will be respected when many policies are defined. +- pvc Phase + + This condition filters volumes based on the phase of their associated PVCs. The condition is specified as a list of phases to match. The volume matches this condition if the PVC's phase matches any of the phases in the list. Supported phases are: `Pending`, `Bound`, and `Lost`. + ```yaml + pvcPhase: + - Pending + ``` + + Some examples: + - Skip Pending PVCs: Skip backup of volumes whose associated PVC is in `Pending` phase (useful for PVCs that haven't been bound to a PV yet). + ```yaml + volumePolicies: + - conditions: + pvcPhase: + - Pending + action: + type: skip + ``` + - Skip multiple phases: Skip backup of volumes whose associated PVC is either in `Pending` or `Lost` phase. + ```yaml + volumePolicies: + - conditions: + pvcPhase: + - Pending + - Lost + action: + type: skip + ``` + - Backup only Bound PVCs: Only backup volumes whose associated PVC is in `Bound` phase. + ```yaml + volumePolicies: + - conditions: + pvcPhase: + - Bound + action: + type: snapshot + ``` + - Combine with other conditions: You can combine PVC phase conditions with other conditions like storage class or labels. + ```yaml + volumePolicies: + - conditions: + pvcPhase: + - Pending + storageClass: + - gp2 + action: + type: skip + ``` + + + +### Resource policies rules +- Velero already has lots of include or exclude filters. the resource policies are the final filters after others include or exclude filters in one backup processing workflow. So if use a defined similar filter like the opt-in approach to backup one pod volume but skip backup of the same pod volume in resource policies, as resource policies are the final filters that are applied, the volume will not be backed up. +- If volume resource policies conflict with themselves the first matched policy will be respected when many policies are defined. #### VolumePolicy priority with existing filters * [Includes filters](#includes) and [Excludes filters](#excludes) have the highest priority. The filtered-out resources by them cannot reach to the VolumePolicy.