-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[On Hold] Proposal to add generic pluginInputs field in backup/restore/schedule CRs #5981
Conversation
Signed-off-by: Anshul Ahuja <[email protected]>
Signed-off-by: Anshul Ahuja <[email protected]>
Signed-off-by: Anshul Ahuja <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #5981 +/- ##
==========================================
- Coverage 40.14% 39.96% -0.19%
==========================================
Files 249 256 +7
Lines 22029 23037 +1008
==========================================
+ Hits 8843 9206 +363
- Misses 12541 13139 +598
- Partials 645 692 +47 see 46 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
```yaml | ||
spec: | ||
pluginInputs: | ||
- name: velero.io/storageclass |
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.
Is the name
field used to represent the plugin's name?
If so, the name should be velero.io/change-storage-class
.
name: backup-1 | ||
spec: | ||
pluginInputs: | ||
- name: velero.io/csi |
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.
The plugin name should be velero.io/csi-volumesnapshot-backupper
.
I also wonder if the configuration is applied to many plugins, and we can only choose the plugin by name, then this gonna be a long list.
Could we consider adding some RE function here?
pluginInputs: | ||
- name: velero.io/csi | ||
- properties: | ||
- key: csi.cloud.disk.driver |
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 the format should be
- properties:
- csi.cloud.disk.driver: csi-diskdriver-snapclass
- csi.cloud.file.driver: csi-filedriver-snapclass
@blackpiglet thanks for going through this PR and providing feedback, currently this PR is on hold since we could not get agreement from community for implementing this. I'll address your comments in case in future we pick it again. |
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #(issue)
Please indicate you've done the following:
/kind changelog-not-required
as a comment on this pull request.site/content/docs/main
.