Introduce v1beta2 VolumeGroupSnapshot API #1312
Conversation
This commit introduces the new VolumeGroupSnapshot v1beta2 API, updates the CRD and the generated code. Both v1beta1 and v1beta2 APIs are served, with v1beta1 being the stored version.
k8s-ci-robot
added
kind/api-change
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: leonardoce The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
| package v1beta2 | ||
|
|
||
| import ( | ||
| core_v1 "k8s.io/api/core/v1" |
There was a problem hiding this comment.
Yes, and in other places, the same package is referred to as v1.
We should decide on a name and stick with it.
In this PR, I believe it is better to keep this file as similar as the sibling in v1beta1 as possible.
I decided to update only the new files without touching the existing ones: this will make reviewing easier.
Shall we choose a name and handle it in a different PR?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
Do you need to update README to generate files? |
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| Copyright 2024 The Kubernetes Authors. | |||
| Copyright 2025 The Kubernetes Authors. | |||
There was a problem hiding this comment.
Why is this changed? Same comment in other places.
There was a problem hiding this comment.
I guess this happened because I regenerated these files.
We could avoid putting this changes into the patch if you think it makes easier to review it.
There was a problem hiding this comment.
Right. VolumeGroupSnapshot v1beta1 and VolumeSnapshot v1 related files should not be changed.
| - driver | ||
| type: object | ||
| served: true | ||
| storage: false |
There was a problem hiding this comment.
v1beta2 should have storage: true.
Same in other places.
There was a problem hiding this comment.
It should be definitely set to true when v1beta2 will become the "default" API, but in this moment we're internally not using it and we've no conversion webhook.
I was thinking to do it later, if you agree with me.
There was a problem hiding this comment.
Can you add that this will be changed in follow up PRs in the PR description?
| @@ -92,3 +92,77 @@ spec: | |||
| served: true | |||
| storage: true | |||
There was a problem hiding this comment.
v1beta1 should have storage: false.
There was a problem hiding this comment.
same as #1312 (comment)
| @@ -2,7 +2,7 @@ | |||
| // +build !ignore_autogenerated | |||
|
|
|||
| /* | |||
| Copyright 2024 The Kubernetes Authors. | |||
| Copyright 2025 The Kubernetes Authors. | |||
There was a problem hiding this comment.
This should not be updated.
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| Copyright 2024 The Kubernetes Authors. | |||
| Copyright 2025 The Kubernetes Authors. | |||
There was a problem hiding this comment.
This and other volumesnapshot v1 related files should not be updated.
What type of PR is this?
/kind api-change
What this PR does / why we need it:
This commit introduces the new VolumeGroupSnapshot v1beta2 API, updates the CRD, and the generated code.
Both v1beta1 and v1beta2 APIs are served, with v1beta1 being the stored version.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Important: this is only the first of a series of PR that will lead to all the changes needed to implement KEP 5013.
With this PR applied, the new version, v1beta2, will be served but not used.
v1beta1 is still the stored version, and no conversion is configured.
Does this PR introduce a user-facing change?: