STOR-3005: Remove v1beta2 group snapshot API#283
Conversation
|
@jsafrane: This pull request references STOR-3005 which is a valid jira issue. DetailsIn response to this:
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. |
|
/hold |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe volume group snapshot CRD assets now comment out v1beta2 definitions, leaving v1 active, and the volumegroupsnapshotclasses webhook now advertises only v1. Changesv1beta2 CRD version removal
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/volumegroupsnapshotclasses.yaml (1)
21-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale rationale comment now contradicts the change.
This line explains why
v1beta1is disabled but asserts thatv1beta2is "served without conversion." With thev1beta2block commented out below, that statement is now misleading to future maintainers. Consider trimming thev1beta2reference here (it applies to all three CRD assets).📝 Suggested wording
-# Don't offer v1beta1 API in OCP, it needs a conversion webhook. v1 and v1beta2 are compatible and served without conversion. +# Don't offer v1beta1 API in OCP, it needs a conversion webhook. Only v1 is served (v1beta2 is disabled below).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/volumegroupsnapshotclasses.yaml` at line 21, The rationale comment above the CRD version blocks is stale and now misleading because it mentions v1beta2 as served without conversion even though that block is commented out. Update the comment in the volume snapshot class CRD assets to only explain why v1beta1 is disabled, and remove the v1beta2-specific wording so the note matches the current served versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/volumegroupsnapshotclasses.yaml`:
- Around line 180-263: The webhook configuration still lists the removed v1beta2
API for VolumeGroupSnapshotClass, so update the webhook rules to stop
advertising it. In assets/webhook_config.yaml, adjust the rule for
volumegroupsnapshotclasses so apiVersions only includes v1, matching the current
supported version and the removal of v1beta2 from
volumegroupsnapshotclasses.yaml.
---
Outside diff comments:
In `@assets/volumegroupsnapshotclasses.yaml`:
- Line 21: The rationale comment above the CRD version blocks is stale and now
misleading because it mentions v1beta2 as served without conversion even though
that block is commented out. Update the comment in the volume snapshot class CRD
assets to only explain why v1beta1 is disabled, and remove the v1beta2-specific
wording so the note matches the current served versions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e3b4f22c-c9ac-4dc2-8f81-98e68729ad09
📒 Files selected for processing (3)
assets/volumegroupsnapshotclasses.yamlassets/volumegroupsnapshotcontents.yamlassets/volumegroupsnapshots.yaml
|
@jsafrane: This PR was included in a payload test run from openshift/kubernetes#2715
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f6adc880-7b8f-11f1-9158-14ae2cd3eb2e-0 |
We don't want any beta APIs served for GA features.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@jsafrane: This PR was included in a payload test run from openshift/kubernetes#2715
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d9b91ad0-7b90-11f1-8f21-2bd69de242cc-0 |
|
@jsafrane: This PR was included in a payload test run from openshift/kubernetes#2715
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ffde5490-7baf-11f1-8c92-8b6201752e13-0 |
|
@rvagner78: This PR was included in a payload test run from openshift/kubernetes#2723
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/fc81f010-85a8-11f1-9c0f-837c9ad85463-0 |
|
@rvagner78: This PR was included in a payload test run from openshift/kubernetes#2723
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/e545e060-85b6-11f1-8987-0ab5c8304063-0 |
We don't want any beta APIs served for GA features.
Summary by CodeRabbit
v1beta2API version from volume group snapshot resources, leavingv1as the only active version.