OCPEDGE-2329: unit and e2e tests for the storageClass options feature

[Neilhamza]

this PR targets the following Jira tickets:
https://issues.redhat.com/browse/OCPEDGE-2329
https://issues.redhat.com/browse/OCPEDGE-2350

Summary

Add comprehensive test coverage for the StorageClassOptions feature and SSA migration across unit and e2e layers.

Unit tests

Webhook validation (api/v1alpha1/lvmcluster_test.go): 9 new envtest cases covering admission rejection of LVMS-owned parameter keys (topolvm.io/device-class, csi.storage.k8s.io/fstype), empty keys, invalid label keys/values, reserved app.kubernetes.io labels, owned-by.topolvm.io prefix labels, and acceptance of valid custom parameters and labels on both create and update.

StorageClass construction (internal/controllers/lvmcluster/resource/topolvm_storageclass_test.go): New file with tests for getTopolvmStorageClasses verifying defaults (Delete/WaitForFirstConsumer), custom options (Retain/Immediate), additional parameters with LVMS-owned key override protection, additional labels with managed label override protection, default SC annotation logic, SSA patch type and field ownership, and EnsureDeleted behavior (not found, exists, deletion in progress).

LVMCluster controller deletion gates (internal/controllers/lvmcluster/controller_test.go): New file testing activePVCsExistForClusterStorageClasses (no PVCs, matching PVC, non-matching PVC, list error) and retainPVsExistForCluster (no Retain policy, Retain with PVs, Retain without PVs, list error) using a field-filtering interceptor client since the fake client does not support MatchingFields natively.

VGManager retain policy (internal/controllers/vgmanager/controller_test.go): Tests for isRetainPolicy (Delete policy, Retain policy, SC not found, Get error safety default, nil ReclaimPolicy) and processDelete retain behavior (blocks deletion when user LVs exist, proceeds when only thin pool LV exists, proceeds when no LVs exist).

E2E tests

StorageClassOptions (test/e2e/storage_class_options_test.go): New file registered in the test suite, covering five areas against a live cluster (14 specs, 8 cluster cycles, ~5 min runtime):

• SC property verification (2 tests): empty options produce defaults (Delete/WaitForFirstConsumer), additionalLabels appear on SC alongside managed owned-by.topolvm.io labels
• Webhook validation (3 entries via DescribeTable): LVMS-owned parameter key rejection, reserved label key rejection, invalid label value rejection — no cluster creation needed
• XValidation immutability (4 tests sharing 1 cluster via Ordered+BeforeAll): CRD-level CEL rules rejecting day-2 changes to reclaimPolicy, volumeBindingMode, and additionalParameters, while allowing additionalLabels mutation
• Functional provisioning (4 tests):
  • Immediate binding: SC has volumeBindingMode=Immediate, PVC binds without consumer pod, mounts in a pod, writes and reads data back
  • WaitForFirstConsumer binding: PVC stays Pending until a consumer pod is created, then binds
  • Retain policy deletion lifecycle: Full gate sequence — PVC deletion leaves PV (Retain), LVMCluster deletion blocked by Retain PV gate (DeletionPending event), PV deletion triggers VGManager ManualCleanupRequired gate, LogicalVolume CR deletion via API unblocks final cluster deletion
  • Additional parameters: SC carries custom params alongside LVMS-owned keys, PVC provisions and pod mounts successfully
• SSA label pruning (1 test): removing an additionalLabel from the LVMCluster causes the operator to prune it from the StorageClass via server-side apply

Key design decisions:

• Cluster cleanup uses DeferCleanup in BeforeEach (not AfterEach) to ensure LIFO ordering: PVC/pod cleanups registered in It blocks run first, preventing cluster deletion from being blocked by active PVCs
• Retain policy test uses the TopoLVM LogicalVolume CR API for LV cleanup instead of exec-based lvremove, avoiding privileged pod access
• Immutability tests share a single cluster via Ordered+BeforeAll since rejected updates don't modify server state

[openshift-ci-robot]

@Neilhamza: This pull request references OCPEDGE-2329 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

this PR targets the following Jira tickets:
https://issues.redhat.com/browse/OCPEDGE-2329
https://issues.redhat.com/browse/OCPEDGE-2350

Summary

Add comprehensive test coverage for the StorageClassOptions feature and SSA migration across unit and e2e layers.

Unit tests

Webhook validation (api/v1alpha1/lvmcluster_test.go): 9 new envtest cases covering admission rejection of LVMS-owned parameter keys (topolvm.io/device-class, csi.storage.k8s.io/fstype), empty keys, invalid label keys/values, reserved app.kubernetes.io labels, owned-by.topolvm.io prefix labels, and acceptance of valid custom parameters and labels on both create and update.

StorageClass construction (internal/controllers/lvmcluster/resource/topolvm_storageclass_test.go): New file with tests for getTopolvmStorageClasses verifying defaults (Delete/WaitForFirstConsumer), custom options (Retain/Immediate), additional parameters with LVMS-owned key override protection, additional labels with managed label override protection, default SC annotation logic, SSA patch type and field ownership, and EnsureDeleted behavior (not found, exists, deletion in progress).

LVMCluster controller deletion gates (internal/controllers/lvmcluster/controller_test.go): New file testing activePVCsExistForClusterStorageClasses (no PVCs, matching PVC, non-matching PVC, list error) and retainPVsExistForCluster (no Retain policy, Retain with PVs, Retain without PVs, list error) using a field-filtering interceptor client since the fake client does not support MatchingFields natively.

VGManager retain policy (internal/controllers/vgmanager/controller_test.go): Tests for isRetainPolicy (Delete policy, Retain policy, SC not found, Get error safety default, nil ReclaimPolicy) and processDelete retain behavior (blocks deletion when user LVs exist, proceeds when only thin pool LV exists, proceeds when no LVs exist).

E2E tests

StorageClassOptions (test/e2e/storage_class_options_test.go): New file registered in the test suite, covering five areas against a live cluster:

• SC property verification: reclaimPolicy=Retain, volumeBindingMode=Immediate, empty options defaults, additionalParameters presence alongside LVMS-owned keys, additionalLabels presence alongside managed labels
• Webhook validation: LVMS-owned parameter key rejection, reserved label key rejection, invalid label value rejection
• XValidation immutability: CRD-level CEL rules rejecting day-2 changes to reclaimPolicy, volumeBindingMode, and additionalParameters, while allowing additionalLabels mutation
• Functional provisioning: PVC binding with Immediate mode (no consumer pod), PVC binding with custom additionalParameters
• SSA label pruning: removing an additionalLabel from the LVMCluster causes the operator to prune it from the StorageClass via server-side apply

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Neilhamza
Once this PR has been reviewed and has the lgtm label, please assign qjkee for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.73%. Comparing base (54fe522) to head (2267804).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2074      +/-   ##
==========================================
+ Coverage   49.39%   51.73%   +2.33%     
==========================================
  Files          52       52              
  Lines        3778     3901     +123     
==========================================
+ Hits         1866     2018     +152     
+ Misses       1759     1715      -44     
- Partials      153      168      +15     

see 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci-robot]

@Neilhamza: This pull request references OCPEDGE-2329 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

this PR targets the following Jira tickets:
https://issues.redhat.com/browse/OCPEDGE-2329
https://issues.redhat.com/browse/OCPEDGE-2350

Summary

Add comprehensive test coverage for the StorageClassOptions feature and SSA migration across unit and e2e layers.

Unit tests

Webhook validation (api/v1alpha1/lvmcluster_test.go): 9 new envtest cases covering admission rejection of LVMS-owned parameter keys (topolvm.io/device-class, csi.storage.k8s.io/fstype), empty keys, invalid label keys/values, reserved app.kubernetes.io labels, owned-by.topolvm.io prefix labels, and acceptance of valid custom parameters and labels on both create and update.

StorageClass construction (internal/controllers/lvmcluster/resource/topolvm_storageclass_test.go): New file with tests for getTopolvmStorageClasses verifying defaults (Delete/WaitForFirstConsumer), custom options (Retain/Immediate), additional parameters with LVMS-owned key override protection, additional labels with managed label override protection, default SC annotation logic, SSA patch type and field ownership, and EnsureDeleted behavior (not found, exists, deletion in progress).

LVMCluster controller deletion gates (internal/controllers/lvmcluster/controller_test.go): New file testing activePVCsExistForClusterStorageClasses (no PVCs, matching PVC, non-matching PVC, list error) and retainPVsExistForCluster (no Retain policy, Retain with PVs, Retain without PVs, list error) using a field-filtering interceptor client since the fake client does not support MatchingFields natively.

VGManager retain policy (internal/controllers/vgmanager/controller_test.go): Tests for isRetainPolicy (Delete policy, Retain policy, SC not found, Get error safety default, nil ReclaimPolicy) and processDelete retain behavior (blocks deletion when user LVs exist, proceeds when only thin pool LV exists, proceeds when no LVs exist).

E2E tests

StorageClassOptions (test/e2e/storage_class_options_test.go): New file registered in the test suite, covering five areas against a live cluster (14 specs, 8 cluster cycles, ~5 min runtime):

• SC property verification (2 tests): empty options produce defaults (Delete/WaitForFirstConsumer), additionalLabels appear on SC alongside managed owned-by.topolvm.io labels
• Webhook validation (3 entries via DescribeTable): LVMS-owned parameter key rejection, reserved label key rejection, invalid label value rejection — no cluster creation needed
• XValidation immutability (4 tests sharing 1 cluster via Ordered+BeforeAll): CRD-level CEL rules rejecting day-2 changes to reclaimPolicy, volumeBindingMode, and additionalParameters, while allowing additionalLabels mutation
• Functional provisioning (4 tests):
• Immediate binding: SC has volumeBindingMode=Immediate, PVC binds without consumer pod, mounts in a pod, writes and reads data back
• WaitForFirstConsumer binding: PVC stays Pending until a consumer pod is created, then binds
• Retain policy deletion lifecycle: Full gate sequence — PVC deletion leaves PV (Retain), LVMCluster deletion blocked by Retain PV gate (DeletionPending event), PV deletion triggers VGManager ManualCleanupRequired gate, LogicalVolume CR deletion via API unblocks final cluster deletion
• Additional parameters: SC carries custom params alongside LVMS-owned keys, PVC provisions and pod mounts successfully
• SSA label pruning (1 test): removing an additionalLabel from the LVMCluster causes the operator to prune it from the StorageClass via server-side apply

Key design decisions:

• Cluster cleanup uses DeferCleanup in BeforeEach (not AfterEach) to ensure LIFO ordering: PVC/pod cleanups registered in It blocks run first, preventing cluster deletion from being blocked by active PVCs
• Retain policy test uses the TopoLVM LogicalVolume CR API for LV cleanup instead of exec-based lvremove, avoiding privileged pod access
• Immutability tests share a single cluster via Ordered+BeforeAll since rejected updates don't modify server state

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[Neilhamza]

/test unit-test

[Neilhamza]

/test ci/prow/unit-test

[Neilhamza]

/test ci/prow/e2e-aws

[Neilhamza]

/test unit-test

[Neilhamza]

/test e2e-aws

[Neilhamza]

/test e2e-aws-hypershift

[openshift-ci]

@Neilhamza: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/unit-test	2267804	link	true	/test unit-test
ci/prow/e2e-aws-hypershift	2267804	link	true	/test e2e-aws-hypershift
ci/prow/e2e-aws	2267804	link	true	/test e2e-aws

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.