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
Make GetPluginConfig accessible from other packages. #6151
Make GetPluginConfig accessible from other packages. #6151
Conversation
8855347
to
54ad595
Compare
…n/framework/common/plugin_config.go/GetPluginConfig Fixes Expose GetPluginConfig vmware-tanzu#4865 Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> Signed-off-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>
54ad595
to
08f7f55
Compare
opts := metav1.ListOptions{ | ||
// velero.io/plugin-config: true | ||
// velero.io/pod-volume-restore: RestoreItemAction | ||
LabelSelector: fmt.Sprintf("velero.io/plugin-config,%s=%s", name, kind), |
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.
Not sure if this was a bug or not, but to get unit test to pass, I had to add =true
to velero.io/plugin-config
like here
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.
From velero source codes, also I couldn't see any significance of this variable. But plugins also use this label-selector, so it is better to retain this variable.
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.
@kaovilai Sorry for my misunderstanding, you needn't add =true
, In fact, velero.io/plugin-config
will pattern any values with the key. No matter whether the value is empty or is true. which means that if you're using velero.io/plugin-config
, the UT could still be passed.
But now if you change into velero.io/plugin-config=true
, It only could pattern those resources have velero.io/plugin-config=true
, which will make the instruction in the doc will not work
Codecov Report
@@ Coverage Diff @@
## main #6151 +/- ##
==========================================
+ Coverage 41.28% 41.36% +0.07%
==========================================
Files 251 252 +1
Lines 23399 23401 +2
==========================================
+ Hits 9661 9679 +18
+ Misses 12979 12965 -14
+ Partials 759 757 -2
... and 2 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. |
) | ||
|
||
func PluginConfigLabelSelector(kind PluginKind, name string) string { | ||
return fmt.Sprintf("velero.io/plugin-config=true,%s=%s", name, kind) |
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.
Regarding feature Changing PVC selected-node, plugin-config
should not set to true.
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.
@kaovilai could you please take a look here
…n/framework/common/plugin_config.go/GetPluginConfig
Fixes #4865 Expose GetPluginConfig
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
.