Merge pull request #136173 from Phaow/add-vgs-tests

test: add vgs retain policy test

test/e2e/storage/framework/testpattern.go

@@ -338,6 +338,15 @@ var (
 		SnapshotDeletionPolicy: DeleteSnapshot,
 		VolType:                DynamicPV,
 	}
+
+	// VolumeGroupSnapshotRetain is TestPattern for "VolumeGroupSnapshot"
+	VolumeGroupSnapshotRetain = TestPattern{
+		Name:                   " (retain policy)",
+		SnapshotType:           VolumeGroupSnapshot,
+		SnapshotDeletionPolicy: RetainSnapshot,
+		VolType:                DynamicPV,
+	}
+
 	// PreprovisionedSnapshotDelete is TestPattern for "Pre-provisioned snapshot"
 	PreprovisionedSnapshotDelete = TestPattern{
 		Name:                   "Pre-provisioned Snapshot (delete policy)",

test/e2e/storage/framework/volume_group_snapshot_resource.go

@@ -25,8 +25,10 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/client-go/dynamic"
 	"k8s.io/kubernetes/test/e2e/framework"
 	"k8s.io/kubernetes/test/e2e/storage/utils"
 )
@@ -113,38 +115,14 @@ func CreateVolumeGroupSnapshot(ctx context.Context, sDriver VolumeGroupSnapshott
 	return gsclass, volumeGroupSnapshot, vgsc
 }
 
-// CleanupVGS deletes the VolumeGroupSnapshot and ensures the bound VolumeGroupSnapshotContent has Delete policy.
-// It waits for the VolumeGroupSnapshot, its owned VolumeSnapshots, VolumeSnapshotContents, and the bound
-// VolumeGroupSnapshotContent to be fully deleted to prevent resource leaks.
-func (r *VolumeGroupSnapshotResource) CleanupVGS(ctx context.Context, timeouts *framework.TimeoutContext) error {
-	if r.VGS == nil {
-		return nil
-	}
-
-	var cleanupErrs []error
-	dc := r.Config.Framework.DynamicClient
-	vgsNamespace := r.VGS.GetNamespace()
-	vgsName := r.VGS.GetName()
-	vgsUID := r.VGS.GetUID()
-	framework.Logf("deleting groupSnapshot %q/%q and ensuring content has Delete policy", vgsNamespace, vgsName)
-
-	vgs, err := dc.Resource(utils.VolumeGroupSnapshotGVR).Namespace(vgsNamespace).Get(ctx, vgsName, metav1.GetOptions{})
-	if err != nil {
-		if apierrors.IsNotFound(err) {
-			// Hope that the underlying snapshot contents and resources are gone already
-			return nil
-		}
-		return fmt.Errorf("failed to get VGS %q: %w", vgsName, err)
-	}
-	r.VGS = vgs
-
-	// Filter snapshots owned by this VGS
+// getVolumeSnapshotContentNames returns the set of VolumeSnapshotContent names bound to VolumeSnapshots owned by the given VGS
+func (r *VolumeGroupSnapshotResource) getVolumeSnapshotContentNames(ctx context.Context, dc dynamic.Interface, vgsNamespace string, vgsUID types.UID) (sets.Set[string], error) {
 	vss, err := dc.Resource(utils.SnapshotGVR).Namespace(vgsNamespace).List(ctx, metav1.ListOptions{})
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	contentNamesSet := sets.NewString()
+	contentNamesSet := sets.New[string]()
 	for _, vs := range vss.Items {
 		for _, owner := range vs.GetOwnerReferences() {
 			if owner.Kind == "VolumeGroupSnapshot" && owner.UID == vgsUID {
@@ -156,7 +134,113 @@ func (r *VolumeGroupSnapshotResource) CleanupVGS(ctx context.Context, timeouts *
 			}
 		}
 	}
+	return contentNamesSet, nil
+}
 
+// cleanupRetainedVolumeSnapshotContents forcibly deletes VolumeSnapshotContents with Retain policy
+func (r *VolumeGroupSnapshotResource) cleanupRetainedVolumeSnapshotContents(ctx context.Context, dc dynamic.Interface, contentNames sets.Set[string], timeouts *framework.TimeoutContext) {
+	// Update deletion policy and delete each VolumeSnapshotContent
+	for _, contentName := range contentNames.UnsortedList() {
+		// Patch deletion policy to Delete
+		patchData := []byte(`{"spec":{"deletionPolicy":"Delete"}}`)
+		_, err := dc.Resource(utils.SnapshotContentGVR).Patch(ctx, contentName, types.MergePatchType, patchData, metav1.PatchOptions{})
+		if err != nil {
+			framework.Failf("Failed to patch VolumeSnapshotContent %q deletion policy: %v", contentName, err)
+			return
+		}
+
+		// Delete VolumeSnapshotContent
+		err = dc.Resource(utils.SnapshotContentGVR).Delete(ctx, contentName, metav1.DeleteOptions{})
+		if err != nil {
+			framework.Failf("Failed to delete VolumeSnapshotContent %q: %v", contentName, err)
+			return
+		}
+	}
+
+	// Wait for all VolumeSnapshotContents to be deleted
+	framework.Logf("Waiting for VolumeSnapshotContents to be deleted")
+	for _, contentName := range contentNames.UnsortedList() {
+		if err := utils.WaitForGVRDeletion(ctx, dc, utils.SnapshotContentGVR, contentName, framework.Poll, timeouts.SnapshotDelete); err != nil {
+			framework.Failf("VolumeSnapshotContent %q was not fully deleted: %v", contentName, err)
+			return
+		}
+	}
+}
+
+// cleanupRetainedVGSContent forcibly deletes VolumeGroupSnapshotContent with Retain policy
+func (r *VolumeGroupSnapshotResource) cleanupRetainedVGSContent(ctx context.Context, dc dynamic.Interface, vgscName string, timeouts *framework.TimeoutContext) {
+	framework.Logf("Deleting VGSContent %q", vgscName)
+
+	// Patch deletion policy to Delete
+	patchData := []byte(`{"spec":{"deletionPolicy":"Delete"}}`)
+	_, err := dc.Resource(utils.VolumeGroupSnapshotContentGVR).Patch(ctx, vgscName, types.MergePatchType, patchData, metav1.PatchOptions{})
+	if err != nil {
+		framework.Failf("Failed to patch VGSContent %q deletion policy: %v", vgscName, err)
+		return
+	}
+
+	// Delete VGSContent
+	err = dc.Resource(utils.VolumeGroupSnapshotContentGVR).Delete(ctx, vgscName, metav1.DeleteOptions{})
+	if err != nil {
+		framework.Failf("Failed to delete VGSContent %q: %v", vgscName, err)
+		return
+	}
+
+	// Wait for VGSContent to be deleted
+	framework.Logf("Waiting for VGSContent %q to be deleted", vgscName)
+	err = utils.WaitForGVRDeletion(ctx, dc, utils.VolumeGroupSnapshotContentGVR, vgscName, framework.Poll, timeouts.SnapshotDelete)
+	if err != nil {
+		framework.Failf("VGSContent %q was not fully deleted: %v", vgscName, err)
+		return
+	}
+}
+
+// verifyVolumeSnapshotContentsDeletion verifies that all VolumeSnapshotContents have been deleted
+func (r *VolumeGroupSnapshotResource) verifyVolumeSnapshotContentsDeletion(ctx context.Context, dc dynamic.Interface, contentNames sets.Set[string], timeouts *framework.TimeoutContext) {
+	ginkgo.By("Verifying all VolumeSnapshotContents have been deleted")
+	var verifyErrs []error
+	for _, contentName := range contentNames.UnsortedList() {
+		err := utils.EnsureGVRDeletion(ctx, dc, utils.SnapshotContentGVR, contentName, framework.Poll, timeouts.SnapshotDelete, "")
+		if err != nil {
+			verifyErrs = append(verifyErrs, fmt.Errorf("VolumeSnapshotContent %q should be deleted: %w", contentName, err))
+		}
+	}
+	if len(verifyErrs) > 0 {
+		framework.ExpectNoError(utilerrors.NewAggregate(verifyErrs), "Failed to verify VolumeSnapshotContents deletion")
+	}
+}
+
+// CleanupVGS deletes the VolumeGroupSnapshot and ensures the bound VolumeGroupSnapshotContent has Delete policy.
+// It waits for the VolumeGroupSnapshot, its owned VolumeSnapshots, VolumeSnapshotContents, and the bound
+// VolumeGroupSnapshotContent to be fully deleted to prevent resource leaks.
+func (r *VolumeGroupSnapshotResource) CleanupVGS(ctx context.Context, timeouts *framework.TimeoutContext) error {
+	if r.VGS == nil {
+		return nil
+	}
+
+	dc := r.Config.Framework.DynamicClient
+	vgsNamespace := r.VGS.GetNamespace()
+	vgsName := r.VGS.GetName()
+	vgsUID := r.VGS.GetUID()
+	framework.Logf("deleting groupSnapshot %q/%q and ensuring content has Delete policy", vgsNamespace, vgsName)
+
+	// Get the VGS to ensure it exists
+	vgs, err := dc.Resource(utils.VolumeGroupSnapshotGVR).Namespace(vgsNamespace).Get(ctx, vgsName, metav1.GetOptions{})
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil
+		}
+		return fmt.Errorf("failed to get VGS %q: %w", vgsName, err)
+	}
+	r.VGS = vgs
+
+	// Get all VolumeSnapshotContent names owned by this VGS
+	contentNamesSet, err := r.getVolumeSnapshotContentNames(ctx, dc, vgsNamespace, vgsUID)
+	if err != nil {
+		return err
+	}
+
+	// Get the bound VolumeGroupSnapshotContent
 	groupSnapshotStatus := r.VGS.Object["status"].(map[string]interface{})
 	groupSnapshotContentName := groupSnapshotStatus["boundVolumeGroupSnapshotContentName"].(string)
 	framework.Logf("received groupSnapshotStatus %v", groupSnapshotStatus)
@@ -164,11 +248,12 @@ func (r *VolumeGroupSnapshotResource) CleanupVGS(ctx context.Context, timeouts *
 
 	boundVGSContent, err := dc.Resource(utils.VolumeGroupSnapshotContentGVR).Get(ctx, groupSnapshotContentName, metav1.GetOptions{})
 	if err != nil && !apierrors.IsNotFound(err) {
-		return fmt.Errorf("failed to get bound VGSContent %q: %w", boundVGSContent.GetName(), err)
+		return fmt.Errorf("failed to get bound VGSContent %q: %w", groupSnapshotContentName, err)
 	}
 
-	// Ensure deletion policy is set to Delete to prevent leaks
-	if boundVGSContent != nil {
+	// For Delete policy, ensure deletion policy is set to Delete to prevent leaks
+	// For Retain policy, we'll update it after verifying retention behavior
+	if boundVGSContent != nil && r.Pattern.SnapshotDeletionPolicy == DeleteSnapshot {
 		spec := boundVGSContent.Object["spec"].(map[string]interface{})
 		if spec["deletionPolicy"] != "Delete" {
 			spec["deletionPolicy"] = "Delete"
@@ -190,30 +275,43 @@ func (r *VolumeGroupSnapshotResource) CleanupVGS(ctx context.Context, timeouts *
 		return fmt.Errorf("failed waiting for VGS %q deletion: %w", vgsName, err)
 	}
 
-	// Wait for VolumeSnapshots owned by this group to be gone
-	if err := utils.WaitForOwnedResourcesDeleted(ctx, dc, utils.SnapshotGVR, vgsNamespace, vgsUID, framework.Poll, timeouts.SnapshotDelete); err != nil {
-		return fmt.Errorf("failed waiting for owned snapshots deletion of VGS %q: %w", vgsName, err)
-	}
-
-	// Wait for all VolumeSnapshotsContents owned by this group to be gone
-	for _, contentName := range contentNamesSet.List() {
-		if err := utils.WaitForGVRDeletion(ctx, dc, utils.SnapshotContentGVR, contentName, framework.Poll, timeouts.SnapshotDelete); err != nil {
-			cleanupErrs = append(cleanupErrs, fmt.Errorf("failed waiting for VSC %q deletion: %w", contentName, err))
+	// Wait for VolumeSnapshots owned by this VGS to be cascade-deleted
+	// This happens regardless of deletion policy because VGS owns VolumeSnapshots via owner references
+	if vgsUID != "" {
+		if err := utils.WaitForOwnedResourcesDeleted(ctx, dc, utils.SnapshotGVR, vgsNamespace, vgsUID, framework.Poll, timeouts.SnapshotDelete); err != nil {
+			return fmt.Errorf("failed waiting for owned snapshots deletion of VGS %q: %w", vgsName, err)
 		}
 	}
-	if len(cleanupErrs) > 0 {
-		return utilerrors.NewAggregate(cleanupErrs)
-	}
 
-	// Wait for VolumeGroupSnapshotContent deleted
+	// Verify deletion policy behavior for VolumeGroupSnapshotContent
 	if boundVGSContent != nil {
-		err = utils.WaitForGVRDeletion(ctx, dc, utils.VolumeGroupSnapshotContentGVR, boundVGSContent.GetName(), framework.Poll, timeouts.SnapshotDelete)
-		if err == nil {
+		vgscName := boundVGSContent.GetName()
+
+		switch r.Pattern.SnapshotDeletionPolicy {
+		case DeleteSnapshot:
+			ginkgo.By(fmt.Sprintf("Verifying VolumeGroupSnapshotContent %q has been deleted per Delete policy", vgscName))
+			err = utils.EnsureGVRDeletion(ctx, dc, utils.VolumeGroupSnapshotContentGVR, vgscName, framework.Poll, timeouts.SnapshotDelete, "")
+			framework.ExpectNoError(err, "VolumeGroupSnapshotContent should be deleted with Delete policy")
+			r.VGSContent = nil
+		case RetainSnapshot:
+			ginkgo.By(fmt.Sprintf("Verifying VolumeGroupSnapshotContent %q has been retained per Retain policy", vgscName))
+			err = utils.EnsureNoGVRDeletion(ctx, dc, utils.VolumeGroupSnapshotContentGVR, vgscName, framework.Poll, 20*framework.Poll, "")
+			framework.ExpectNoError(err, "VolumeGroupSnapshotContent should be retained with Retain policy")
+
+			// Force cleanup to prevent leaks
+			// Cleanup order:
+			// 1. VolumeSnapshotContents (update policy + delete + wait)
+			// 2. VGSContent (update policy + delete + wait)
+			framework.Logf("Retain policy verified, now forcing cleanup to prevent leaks")
+			r.cleanupRetainedVolumeSnapshotContents(ctx, dc, contentNamesSet, timeouts)
+			r.cleanupRetainedVGSContent(ctx, dc, vgscName, timeouts)
 			r.VGSContent = nil
-		} else {
-			return fmt.Errorf("failed waiting for VGSContent %q deletion: %w", vgsName, err)
 		}
 	}
+
+	// Verify all VolumeSnapshotContents have been deleted
+	r.verifyVolumeSnapshotContentsDeletion(ctx, dc, contentNamesSet, timeouts)
+
 	return nil
 }
 

test/e2e/storage/testsuites/volume_group_snapshottable.go

@@ -60,6 +60,7 @@ type VolumeGroupSnapshottableTestSuite struct {
 func InitVolumeGroupSnapshottableTestSuite() storageframework.TestSuite {
 	patterns := []storageframework.TestPattern{
 		storageframework.VolumeGroupSnapshotDelete,
+		storageframework.VolumeGroupSnapshotRetain,
 	}
 	return InitCustomGroupSnapshottableTestSuite(patterns)
 }