New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add generic DataSource feature gate #88636
Conversation
/sig storage |
/assign @msau42 @j-griffith |
@kubernetes/sig-storage-api-reviews |
/assign @xing-yang |
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
/label api-review |
/assign @thockin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are some details missing here, you'll at least need to address:
- What happens if this new "open it all up" feature is enabled but SnapshotDataSource is not?
- The godocs in types.go needs to be updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If others are "ok" with the level of testing that's fine; The doc additions however have to be added.
58136d1
to
d7c18b9
Compare
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggesting to fix up the docstring in types.go. I think you also need to update the copy in staging/src/k8s.io/api/core/v1/types.go
d7c18b9
to
d7a63fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally lgtm, just some minor nits on the comment.
Can you rebase and squash?
// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) | ||
// * An existing PVC (PersistentVolumeClaim) | ||
// * An existing custom resource/object that implements data population (Alpha) | ||
// In order to use VolumeSnapshot object types, the appropriate feature gate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to talk about the cloning case?
// If the provisioner does not support the specified data source, the volume will | ||
// This field can be used to specify either: | ||
// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) | ||
// * An existing PVC (PersistentVolumeClaim) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@j-griffith do you plan on updating alpha-beta designation for cloning in your PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see cloning is GA now.
d7a63fb
to
507cc02
Compare
I rebased, no squash yet. Let me do that now. |
05f880f
to
091978f
Compare
Had to push one more time to remove test cases where PVC cloning feature gate was disabled. It's squashed now. |
/lgtm |
/retest |
Allow any custom resource to be the data source of a PVC, if the AnyVolumeDataSource feature gate is enabled. This is an alpha feature.
091978f
to
e8b09d3
Compare
@msau42 I had to push another commit to add these 3 files generated by "make update": api/openapi-spec/swagger.json |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bswartz, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Allow any custom resource to be the data source of a PVC.
This is an alpha feature enabled by the GenericPVCDataSource feature gate.
Which issue(s) this PR fixes:
Fixes #88635
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: