feat: Add VolumePolicy support for PVC Phase conditions to allow skipping Pending PVCs

This commit implements VolumePolicy support for PVC Phase conditions, resolving
vmware-tanzu/velero#7233 where backups fail with ''PVC has no volume backing this claim''
for Pending PVCs.

Changes made:
- Extended VolumePolicy API to support PVC phase conditions
- Added pvcPhaseCondition struct with matching logic
- Modified getMatchAction() to evaluate policies for unbound PVCs before returning errors
- Added case to GetMatchAction() to handle PVC-only scenarios (nil PV)
- Added comprehensive unit tests for PVC phase parsing and matching

Users can now skip Pending PVCs through volume policy configuration:
  apiVersion: v1
  kind: ConfigMap
  metadata:
    name: volume-policy
    namespace: velero
  data:
    policy.yaml: |
      version: v1
      volumePolicies:
      - conditions:
          pvcPhase: [Pending]
        action:
          type: skip

chore: rename changelog file to match PR #9166

Renamed changelogs/unreleased/7233-claude to changelogs/unreleased/9166-claude
to match the opened PR at https://github.com/vmware-tanzu/velero/pull/9166

docs: Add PVC phase condition support to VolumePolicy documentation

- Added pvcPhase field to YAML template example
- Documented pvcPhase as a supported condition in the list
- Added comprehensive examples for using PVC phase conditions
- Included examples for Pending, Bound, and Lost phases
- Demonstrated combining PVC phase with other conditions

Co-Authored-By: Tiger Kaovilai <kaovilai@users.noreply.github.com>
This commit is contained in:
claude[bot]
2025-12-22 10:07:05 +07:00
committed by Tiger Kaovilai
co-authored by Tiger Kaovilai
parent 4b6708de2c
commit 80211d77e5
6 changed files with 243 additions and 7 deletions
@@ -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")
}
@@ -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)
})
}
}
+31 -2
View File
@@ -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
}
@@ -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 {