openshift-velero-plugin
openshift-velero-plugin copied to clipboard
Add comprehensive test coverage for restore plugins
- Add test files for all restore.go files that were missing tests
- Update existing test files to improve coverage:
- horizontalpodautoscaler: Add Execute() tests for all scenarios
- pod: Add tests for AppliesTo(), PodHasVolumesToBackUp(), and PodHasRestoreHookAnnotations()
- route: Add Execute() tests for route host generation scenarios
- Add documentation comments explaining where Execute() functionality is tested for plugins that only test AppliesTo()
- Fix failing tests by adjusting to match implementation behavior
/cherry-pick oadp-1.5 oadp-1.4
@kaovilai: once the present PR merges, I will cherry-pick it on top of oadp-1.5 in a new PR and assign it to you.
In response to this:
/cherry-pick oadp-1.5 oadp-1.4
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.
/hold for https://github.com/openshift/openshift-velero-plugin/pull/315 and https://github.com/openshift/openshift-velero-plugin/pull/316 to merge first
/cherry-pick oadp-1.4
@kaovilai: once the present PR merges, I will cherry-pick it on top of oadp-1.4 in a new PR and assign it to you.
In response to this:
/cherry-pick oadp-1.4
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.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: kaovilai, shubham-pampattiwar, sseago
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [kaovilai,shubham-pampattiwar,sseago]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Issues go stale after 90d of inactivity.
Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.
If this issue is safe to close now please do so with /close.
/lifecycle stale
/test all
/unhold
@kaovilai: all tests passed!
Full PR test history. Your PR dashboard.
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.
Stale issues rot after 30d of inactivity.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.
If this issue is safe to close now please do so with /close.
/lifecycle rotten /remove-lifecycle stale
Walkthrough
This PR adds unit test coverage across 22 Velero restore plugins under velero-plugins/. Most tests validate the AppliesTo method to verify correct resource selectors, while several include Execute method tests covering transformation logic such as namespace mapping, registry swapping, annotation handling, and resource filtering.
Changes
| Cohort / File(s) | Summary |
|---|---|
Simple AppliesTo Tests velero-plugins/buildconfig/restore_test.go, velero-plugins/cronjob/restore_test.go, velero-plugins/daemonset/restore_test.go, velero-plugins/deployment/restore_test.go, velero-plugins/deploymentconfig/restore_test.go, velero-plugins/imagetag/restore_test.go, velero-plugins/replicaset/restore_test.go, velero-plugins/statefulset/restore_test.go |
Adds TestRestorePluginAppliesTo validating resource selector includes expected resource types. |
AppliesTo and Execute Tests velero-plugins/configmap/restore_test.go |
Tests AppliesTo for "configmaps" and Execute with table-driven cases covering annotation-based SkipRestore decisions (nil, empty, explicit true/false, invalid, unrelated annotations). |
Job and ReplicationController Ownership Logic velero-plugins/job/restore_test.go, velero-plugins/replicationcontroller/restore_test.go |
Tests AppliesTo and Execute with OwnerReference validation; skips restore when owned by CronJob or DeploymentConfig (unless paused). |
PersistentVolume and PVC Migration Tests velero-plugins/persistentvolume/restore_test.go, velero-plugins/pvc/restore_test.go |
Tests AppliesTo and Execute covering non-migration restores, snapshot PV skipping on stage-restore, and storageClassName handling based on migration annotations. |
Pod Volume and Hook Annotation Tests velero-plugins/pod/restore_test.go |
Tests AppliesTo, PodHasVolumesToBackUp, and PodHasRestoreHookAnnotations covering volume types and hook detection logic. |
RoleBindings Namespace Mapping velero-plugins/rolebindings/restore_test.go |
Adds exported helper functions SwapSubjectNamespaces, SwapUserNamesNamespaces, SwapGroupNamesNamespaces; refactors tests to unit-test transformation logic instead of end-to-end Execute path. |
Route Host Generation velero-plugins/route/restore_test.go |
Tests Execute with table-driven cases verifying host field removal when openshift.io/host.generated annotation is true. |
SCC Namespace Mapping velero-plugins/scc/restore_test.go |
Tests AppliesTo and Execute with namespace mapping for system:serviceaccount users in SecurityContextConstraints. |
Secret Originating Service Annotation velero-plugins/secret/restore_test.go |
Tests AppliesTo and Execute covering SkipRestore when originating-service annotation is present. |
Service External IP Handling velero-plugins/service/restore_test.go |
Tests AppliesTo and Execute verifying ExternalIPs are cleared for LoadBalancer service type but preserved for ClusterIP, NodePort, ExternalName. |
ServiceAccount Dockercfg Secret Removal velero-plugins/serviceaccount/restore_test.go |
Tests AppliesTo and Execute to verify dockercfg secrets are removed from both secrets and imagePullSecrets fields. |
HorizontalPodAutoscaler API Version Update velero-plugins/horizontalpodautoscaler/restore_test.go |
Tests Execute with table-driven cases updating scaleTargetRef.apiVersion to v1 for DeploymentConfig targets, leaving unchanged for other cases. |
ClusterRoleBindings Resource Selection velero-plugins/clusterrolebindings/restore_test.go |
Tests AppliesTo verifying ResourceSelector includes clusterrolebinding.authorization.openshift.io. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Rolebindings exported functions: Review new public helper functions (SwapSubjectNamespaces, SwapUserNamesNamespaces, SwapGroupNamesNamespaces) for correctness, namespace mapping logic, and service account handling.
- Configmap Execute tests: Validate table-driven test structure and annotation parsing logic for SkipRestore decisions.
- PersistentVolume and PVC migration logic: Verify stage-restore skipping conditions and storageClassName annotation handling.
- Service external IP logic: Confirm conditional clearing based on service type and proper field manipulation.
- Ownership and namespace mapping tests: Ensure OwnerReference comparisons and namespace mapping transformations are correct across job, replicationcontroller, scc, and rolebindings.
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
Comment @coderabbitai help to get the list of available commands and usage tips.